fix: improve github_organization_custom_properties resource

- Remove broken CustomizeDiff referencing non-existent 'slug' field
- Make 'property_name' ForceNew (renaming creates a new property, orphans old)
- Make 'value_type' Required and ForceNew (can't create without type, changing type requires recreation)
- Add allowed_values validation: required for select types, rejected for others
- Add checkOrganization guard to all CRUD functions
- Add graceful 404 handling in Read (removes from state instead of erroring)
- Improve error messages with context
- Fix Update double-read (Create already calls Read at the end)
- Add resource Description
- Use GetOk for optional fields to avoid sending zero values

Resolves #2806

Author: mkushakov

github/resource_github_organization_custom_properties.go

@@ -2,39 +2,38 @@ package github
 
 import (
 	"context"
+	"fmt"
+	"log"
+	"net/http"
 
 	"github.com/google/go-github/v84/github"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 	"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.",
+		Create:      resourceGithubCustomPropertiesCreate,
+		Read:        resourceGithubCustomPropertiesRead,
+		Update:      resourceGithubCustomPropertiesUpdate,
+		Delete:      resourceGithubCustomPropertiesDelete,
 		Importer: &schema.ResourceImporter{
 			State: 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": {
@@ -73,52 +72,83 @@ func resourceGithubOrganizationCustomProperties() *schema.Resource {
 }
 
 func resourceGithubCustomPropertiesCreate(d *schema.ResourceData, meta any) error {
+	if err := checkOrganization(meta); err != nil {
+		return err
+	}
+
 	ctx := context.Background()
 	client := meta.(*Owner).v3client
 	ownerName := meta.(*Owner).name
 
 	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,
+	}
+
+	// Set default value if provided
+	if v, ok := d.GetOk("default_value"); ok {
+		defaultValue := v.(string)
+		customProperty.DefaultValue = &defaultValue
+	}
+
+	// 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 fmt.Errorf("allowed_values can only be set for single_select or multi_select value types")
+		}
+	}
+
+	// Validate that allowed_values is provided for select types
+	if (valueType == github.PropertyValueTypeSingleSelect || valueType == github.PropertyValueTypeMultiSelect) && len(customProperty.AllowedValues) == 0 {
+		return fmt.Errorf("allowed_values is required for %s value type", valueType)
 	}
 
 	if val, ok := d.GetOk("values_editable_by"); ok {
 		str := val.(string)
 		customProperty.ValuesEditableBy = &str
 	}
 
-	customProperty, _, err := client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, d.Get("property_name").(string), customProperty)
+	customProperty, _, err := client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, propertyName, customProperty)
 	if err != nil {
-		return err
+		return fmt.Errorf("error creating organization custom property %s: %w", propertyName, err)
 	}
 
 	d.SetId(*customProperty.PropertyName)
 	return resourceGithubCustomPropertiesRead(d, meta)
 }
 
 func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error {
+	if err := checkOrganization(meta); err != nil {
+		return err
+	}
+
 	ctx := context.Background()
 	client := meta.(*Owner).v3client
 	ownerName := meta.(*Owner).name
 
-	customProperty, _, err := client.Organizations.GetCustomProperty(ctx, ownerName, d.Get("property_name").(string))
+	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 {
-		return err
+		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 fmt.Errorf("error reading organization custom property %s: %w", propertyName, err)
 	}
 
 	// TODO: Add support for other types of default values
@@ -137,19 +167,22 @@ func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error
 }
 
 func resourceGithubCustomPropertiesUpdate(d *schema.ResourceData, meta any) error {
-	if err := resourceGithubCustomPropertiesCreate(d, meta); err != nil {
-		return err
-	}
-	return resourceGithubCustomPropertiesRead(d, meta)
+	// Create uses the same upsert API, and already calls Read at the end
+	return resourceGithubCustomPropertiesCreate(d, meta)
 }
 
 func resourceGithubCustomPropertiesDelete(d *schema.ResourceData, meta any) error {
+	if err := checkOrganization(meta); err != nil {
+		return err
+	}
+
 	client := meta.(*Owner).v3client
 	ownerName := meta.(*Owner).name
+	propertyName := d.Get("property_name").(string)
 
-	_, err := client.Organizations.RemoveCustomProperty(context.Background(), ownerName, d.Get("property_name").(string))
+	_, err := client.Organizations.RemoveCustomProperty(context.Background(), ownerName, propertyName)
 	if err != nil {
-		return err
+		return fmt.Errorf("error deleting organization custom property %s: %w", propertyName, err)
 	}
 
 	return nil

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) {