fix: improve github_organization_custom_properties resource

github/resource_github_organization_custom_properties.go

@@ -2,39 +2,39 @@ package github
 
 import (
 	"context"
+	"fmt"
+	"log"
+	"net/http"
 
 	"github.com/google/go-github/v85/github"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
 func resourceGithubOrganizationCustomProperties() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceGithubCustomPropertiesCreate,
-		Read:   resourceGithubCustomPropertiesRead,
-		Update: resourceGithubCustomPropertiesUpdate,
-		Delete: resourceGithubCustomPropertiesDelete,
+		Description:   "Creates and manages a custom property for a GitHub Organization.",
+		CreateContext: resourceGithubCustomPropertiesCreate,
+		ReadContext:   resourceGithubCustomPropertiesRead,
+		UpdateContext: resourceGithubCustomPropertiesUpdate,
+		DeleteContext: resourceGithubCustomPropertiesDelete,
 		Importer: &schema.ResourceImporter{
-			State: resourceGithubCustomPropertiesImport,
+			StateContext: resourceGithubCustomPropertiesImport,
 		},
 
-		CustomizeDiff: customdiff.Sequence(
-			customdiff.ComputedIf("slug", func(_ context.Context, d *schema.ResourceDiff, meta any) bool {
-				return d.HasChange("name")
-			}),
-		),
-
 		Schema: map[string]*schema.Schema{
 			"property_name": {
 				Type:        schema.TypeString,
 				Required:    true,
+				ForceNew:    true,
 				Description: "The name of the custom property",
 			},
 			"value_type": {
 				Type:             schema.TypeString,
-				Optional:         true,
-				Description:      "The type of the custom property",
+				Required:         true,
+				ForceNew:         true,
+				Description:      "The type of the custom property. Can be one of: 'string', 'single_select', 'multi_select', 'true_false', or 'url'.",
 				ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice([]string{string(github.PropertyValueTypeString), string(github.PropertyValueTypeSingleSelect), string(github.PropertyValueTypeMultiSelect), string(github.PropertyValueTypeTrueFalse), string(github.PropertyValueTypeURL)}, false)),
 			},
 			"required": {
@@ -72,55 +72,51 @@ func resourceGithubOrganizationCustomProperties() *schema.Resource {
 	}
 }
 
-func resourceGithubCustomPropertiesCreate(d *schema.ResourceData, meta any) error {
-	ctx := context.Background()
-	client := meta.(*Owner).v3client
-	ownerName := meta.(*Owner).name
-
+// buildCustomProperty constructs a github.CustomProperty from the resource data.
+func buildCustomProperty(d *schema.ResourceData) (*github.CustomProperty, error) {
 	propertyName := d.Get("property_name").(string)
 	valueType := github.PropertyValueType(d.Get("value_type").(string))
 	required := d.Get("required").(bool)
-	defaultValue := d.Get("default_value").(string)
 	description := d.Get("description").(string)
-	allowedValues := d.Get("allowed_values").([]any)
-	var allowedValuesString []string
-	for _, v := range allowedValues {
-		allowedValuesString = append(allowedValuesString, v.(string))
-	}
 
 	customProperty := &github.CustomProperty{
-		PropertyName:  &propertyName,
-		ValueType:     valueType,
-		Required:      &required,
-		DefaultValue:  &defaultValue,
-		Description:   &description,
-		AllowedValues: allowedValuesString,
+		PropertyName: &propertyName,
+		ValueType:    valueType,
+		Required:     &required,
+		Description:  &description,
 	}
 
-	if val, ok := d.GetOk("values_editable_by"); ok {
-		str := val.(string)
-		customProperty.ValuesEditableBy = &str
+	// Set default value if provided
+	if v, ok := d.GetOk("default_value"); ok {
+		defaultValue := v.(string)
+		customProperty.DefaultValue = &defaultValue
 	}
 
-	customProperty, _, err := client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, d.Get("property_name").(string), customProperty)
-	if err != nil {
-		return err
+	// Set allowed values if provided (only valid for select types)
+	if v, ok := d.GetOk("allowed_values"); ok {
+		allowedValues := expandStringList(v.([]any))
+		if valueType == github.PropertyValueTypeSingleSelect || valueType == github.PropertyValueTypeMultiSelect {
+			customProperty.AllowedValues = allowedValues
+		} else {
+			return nil, fmt.Errorf("allowed_values can only be set for single_select or multi_select value types")
+		}
 	}
 
-	d.SetId(*customProperty.PropertyName)
-	return resourceGithubCustomPropertiesRead(d, meta)
-}
-
-func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error {
-	ctx := context.Background()
-	client := meta.(*Owner).v3client
-	ownerName := meta.(*Owner).name
+	// Validate that allowed_values is provided for select types
+	if (valueType == github.PropertyValueTypeSingleSelect || valueType == github.PropertyValueTypeMultiSelect) && len(customProperty.AllowedValues) == 0 {
+		return nil, fmt.Errorf("allowed_values is required for %s value type", valueType)
+	}
 
-	customProperty, _, err := client.Organizations.GetCustomProperty(ctx, ownerName, d.Get("property_name").(string))
-	if err != nil {
-		return err
+	if val, ok := d.GetOk("values_editable_by"); ok {
+		str := val.(string)
+		customProperty.ValuesEditableBy = &str
 	}
 
+	return customProperty, nil
+}
+
+// setCustomPropertyState sets all resource data fields from a CustomProperty API response.
+func setCustomPropertyState(d *schema.ResourceData, customProperty *github.CustomProperty) {
 	// TODO: Add support for other types of default values
 	defaultValue, _ := customProperty.DefaultValueString()
 
@@ -132,30 +128,102 @@ func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error
 	_ = d.Set("required", customProperty.Required)
 	_ = d.Set("value_type", string(customProperty.ValueType))
 	_ = d.Set("values_editable_by", customProperty.ValuesEditableBy)
+}
+
+func resourceGithubCustomPropertiesCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
+	}
+
+	client := meta.(*Owner).v3client
+	ownerName := meta.(*Owner).name
+
+	customProperty, err := buildCustomProperty(d)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	propertyName := d.Get("property_name").(string)
+	customProperty, _, err = client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, propertyName, customProperty)
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("error creating organization custom property %s: %w", propertyName, err))
+	}
+
+	setCustomPropertyState(d, customProperty)
 
 	return nil
 }
 
-func resourceGithubCustomPropertiesUpdate(d *schema.ResourceData, meta any) error {
-	if err := resourceGithubCustomPropertiesCreate(d, meta); err != nil {
-		return err
+func resourceGithubCustomPropertiesRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
 	}
-	return resourceGithubCustomPropertiesRead(d, meta)
+
+	client := meta.(*Owner).v3client
+	ownerName := meta.(*Owner).name
+
+	propertyName := d.Id()
+	if pn, ok := d.GetOk("property_name"); ok {
+		propertyName = pn.(string)
+	}
+
+	customProperty, resp, err := client.Organizations.GetCustomProperty(ctx, ownerName, propertyName)
+	if err != nil {
+		if resp != nil && resp.StatusCode == http.StatusNotFound {
+			log.Printf("[WARN] Removing organization custom property %s from state because it no longer exists in GitHub", propertyName)
+			d.SetId("")
+			return nil
+		}
+		return diag.FromErr(fmt.Errorf("error reading organization custom property %s: %w", propertyName, err))
+	}
+
+	setCustomPropertyState(d, customProperty)
+
+	return nil
 }
 
-func resourceGithubCustomPropertiesDelete(d *schema.ResourceData, meta any) error {
+func resourceGithubCustomPropertiesUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
+	}
+
 	client := meta.(*Owner).v3client
 	ownerName := meta.(*Owner).name
 
-	_, err := client.Organizations.RemoveCustomProperty(context.Background(), ownerName, d.Get("property_name").(string))
+	customProperty, err := buildCustomProperty(d)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+
+	propertyName := d.Get("property_name").(string)
+	customProperty, _, err = client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, propertyName, customProperty)
+	if err != nil {
+		return diag.FromErr(fmt.Errorf("error updating organization custom property %s: %w", propertyName, err))
+	}
+
+	setCustomPropertyState(d, customProperty)
+
+	return nil
+}
+
+func resourceGithubCustomPropertiesDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics {
+	if err := checkOrganization(meta); err != nil {
+		return diag.FromErr(err)
+	}
+
+	client := meta.(*Owner).v3client
+	ownerName := meta.(*Owner).name
+	propertyName := d.Get("property_name").(string)
+
+	_, err := client.Organizations.RemoveCustomProperty(ctx, ownerName, propertyName)
 	if err != nil {
-		return err
+		return diag.FromErr(fmt.Errorf("error deleting organization custom property %s: %w", propertyName, err))
 	}
 
 	return nil
 }
 
-func resourceGithubCustomPropertiesImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+func resourceGithubCustomPropertiesImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
 	if err := d.Set("property_name", d.Id()); err != nil {
 		return nil, err
 	}

github/resource_github_organization_custom_properties_test.go

@@ -30,6 +30,45 @@ func TestAccGithubOrganizationCustomPropertiesValidation(t *testing.T) {
 			},
 		})
 	})
+
+	t.Run("rejects allowed_values for string type", func(t *testing.T) {
+		config := `
+		resource "github_organization_custom_properties" "test" {
+			property_name  = "TestInvalidAllowedValues"
+			value_type     = "string"
+			allowed_values = ["a", "b"]
+		}`
+
+		resource.Test(t, resource.TestCase{
+			PreCheck:          func() { skipUnlessHasOrgs(t) },
+			ProviderFactories: providerFactories,
+			Steps: []resource.TestStep{
+				{
+					Config:      config,
+					ExpectError: regexp.MustCompile("allowed_values can only be set for single_select or multi_select"),
+				},
+			},
+		})
+	})
+
+	t.Run("requires allowed_values for single_select type", func(t *testing.T) {
+		config := `
+		resource "github_organization_custom_properties" "test" {
+			property_name = "TestMissingAllowedValues"
+			value_type    = "single_select"
+		}`
+
+		resource.Test(t, resource.TestCase{
+			PreCheck:          func() { skipUnlessHasOrgs(t) },
+			ProviderFactories: providerFactories,
+			Steps: []resource.TestStep{
+				{
+					Config:      config,
+					ExpectError: regexp.MustCompile("allowed_values is required for single_select"),
+				},
+			},
+		})
+	})
 }
 
 func TestAccGithubOrganizationCustomProperties(t *testing.T) {

website/docs/r/organization_custom_properties.html.markdown

@@ -65,21 +65,34 @@ resource "github_organization_custom_properties" "archived" {
 }
 ```
 
+~> **Note:** This resource requires the provider to be configured with an organization owner. Individual user accounts are not supported.
+
+## Argument Reference
+
+```hcl
+resource "github_organization_custom_properties" "docs_link" {
+  property_name = "docs_link"
+  value_type    = "url"
+  required      = false
+  description   = "Link to the documentation for this repository"
+}
+```
+
 ## Argument Reference
 
 The following arguments are supported:
 
-* `property_name` - (Required) The name of the custom property.
+* `property_name` - (Required) The name of the custom property. Changing this will force the resource to be recreated.
 
-* `value_type` - (Optional) The type of the custom property. Can be one of `string`, `single_select`, `multi_select`, or `true_false`. Defaults to `string`.
+* `value_type` - (Required) The type of the custom property. Can be one of `string`, `single_select`, `multi_select`, `true_false`, or `url`. Changing this will force the resource to be recreated.
 
 * `required` - (Optional) Whether the custom property is required. Defaults to `false`.
 
 * `description` - (Optional) The description of the custom property.
 
 * `default_value` - (Optional) The default value of the custom property.
 
-* `allowed_values` - (Optional) List of allowed values for the custom property. Only applicable when `value_type` is `single_select` or `multi_select`.
+* `allowed_values` - (Optional) List of allowed values for the custom property. Required when `value_type` is `single_select` or `multi_select`, and must not be set for other value types.
 
 * `values_editable_by` - (Optional) Who can edit the values of the custom property. Can be one of `org_actors` or `org_and_repo_actors`. When set to `org_actors` (the default), only organization owners can edit the property values on repositories. When set to `org_and_repo_actors`, both organization owners and repository administrators with the custom properties permission can edit the values.