diff --git a/docs/resources/organization_custom_properties.md b/docs/resources/organization_custom_properties.md index f27d8af532..ee21ea4444 100644 --- a/docs/resources/organization_custom_properties.md +++ b/docs/resources/organization_custom_properties.md @@ -76,7 +76,7 @@ The following arguments are supported: - `description` - (Optional) The description of the custom property. -- `default_value` - (Optional) The default value of the custom property. +- `default_value` - (Optional) The default value of the custom property. Not supported for `multi_select` properties. - `allowed_values` - (Optional) List of allowed values for the custom property. Only applicable when `value_type` is `single_select` or `multi_select`. diff --git a/github/resource_github_organization_custom_properties.go b/github/resource_github_organization_custom_properties.go index 84916ab879..99094ea1af 100644 --- a/github/resource_github_organization_custom_properties.go +++ b/github/resource_github_organization_custom_properties.go @@ -2,8 +2,11 @@ package github import ( "context" + "strconv" "github.com/google/go-github/v89/github" + "github.com/hashicorp/go-cty/cty" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "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" @@ -11,12 +14,12 @@ import ( func resourceGithubOrganizationCustomProperties() *schema.Resource { return &schema.Resource{ - Create: resourceGithubCustomPropertiesCreate, - Read: resourceGithubCustomPropertiesRead, - Update: resourceGithubCustomPropertiesUpdate, - Delete: resourceGithubCustomPropertiesDelete, + CreateContext: resourceGithubCustomPropertiesCreate, + ReadContext: resourceGithubCustomPropertiesRead, + UpdateContext: resourceGithubCustomPropertiesUpdate, + DeleteContext: resourceGithubCustomPropertiesDelete, Importer: &schema.ResourceImporter{ - State: resourceGithubCustomPropertiesImport, + StateContext: resourceGithubCustomPropertiesImport, }, CustomizeDiff: customdiff.Sequence( @@ -44,7 +47,7 @@ func resourceGithubOrganizationCustomProperties() *schema.Resource { }, "default_value": { Type: schema.TypeString, - Description: "The default value of the custom property", + Description: "The default value of the custom property. Not supported for multi_select properties.", Optional: true, Computed: true, }, @@ -72,8 +75,7 @@ func resourceGithubOrganizationCustomProperties() *schema.Resource { } } -func resourceGithubCustomPropertiesCreate(d *schema.ResourceData, meta any) error { - ctx := context.Background() +func resourceGithubCustomPropertiesCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client ownerName := meta.(*Owner).name @@ -102,27 +104,59 @@ func resourceGithubCustomPropertiesCreate(d *schema.ResourceData, meta any) erro customProperty.ValuesEditableBy = &str } + diags := multiSelectDefaultValueWarning(valueType, defaultValue) + customProperty, _, err := client.Organizations.CreateOrUpdateCustomProperty(ctx, ownerName, d.Get("property_name").(string), customProperty) if err != nil { - return err + return append(diags, diag.FromErr(err)...) } d.SetId(*customProperty.PropertyName) - return resourceGithubCustomPropertiesRead(d, meta) + return append(diags, resourceGithubCustomPropertiesRead(ctx, d, meta)...) } -func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error { - ctx := context.Background() +// multiSelectDefaultValueWarning warns when a default value is configured for a +// multi_select property. GitHub returns those defaults as a list of strings, +// which cannot be represented by the string default_value attribute, so the +// configured value is not reflected in state and shows up as a change on every +// plan. +func multiSelectDefaultValueWarning(valueType github.PropertyValueType, defaultValue string) diag.Diagnostics { + if valueType != github.PropertyValueTypeMultiSelect || defaultValue == "" { + return nil + } + + return diag.Diagnostics{ + { + Severity: diag.Warning, + Summary: "default_value is not supported for multi_select properties", + Detail: "The default value of a multi_select property cannot be read back by this provider, so it is not stored in state and every plan will show a change for default_value. Remove default_value to avoid this.", + AttributePath: cty.GetAttrPath("default_value"), + }, + } +} + +func resourceGithubCustomPropertiesRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client ownerName := meta.(*Owner).name customProperty, _, err := client.Organizations.GetCustomProperty(ctx, ownerName, d.Get("property_name").(string)) if err != nil { - return err + return diag.FromErr(err) } - // TODO: Add support for other types of default values - defaultValue, _ := customProperty.DefaultValueString() + // multi_select is not supported: its default value is a []string, which + // cannot round-trip through the TypeString default_value attribute. + var defaultValue string + switch customProperty.ValueType { + case github.PropertyValueTypeTrueFalse: + if b, ok := customProperty.DefaultValueBool(); ok { + defaultValue = strconv.FormatBool(b) + } + default: + if s, ok := customProperty.DefaultValueString(); ok { + defaultValue = s + } + } d.SetId(*customProperty.PropertyName) _ = d.Set("allowed_values", customProperty.AllowedValues) @@ -136,26 +170,25 @@ func resourceGithubCustomPropertiesRead(d *schema.ResourceData, meta any) error return nil } -func resourceGithubCustomPropertiesUpdate(d *schema.ResourceData, meta any) error { - if err := resourceGithubCustomPropertiesCreate(d, meta); err != nil { - return err - } - return resourceGithubCustomPropertiesRead(d, meta) +func resourceGithubCustomPropertiesUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Create issues a PUT, which the API treats as an upsert, and reads the + // property back afterwards. + return resourceGithubCustomPropertiesCreate(ctx, d, meta) } -func resourceGithubCustomPropertiesDelete(d *schema.ResourceData, meta any) error { +func resourceGithubCustomPropertiesDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { client := meta.(*Owner).v3client ownerName := meta.(*Owner).name - _, err := client.Organizations.RemoveCustomProperty(context.Background(), ownerName, d.Get("property_name").(string)) + _, err := client.Organizations.RemoveCustomProperty(ctx, ownerName, d.Get("property_name").(string)) if err != nil { - return err + return diag.FromErr(err) } return nil } -func resourceGithubCustomPropertiesImport(d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { +func resourceGithubCustomPropertiesImport(_ context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { if err := d.Set("property_name", d.Id()); err != nil { return nil, err } diff --git a/github/resource_github_organization_custom_properties_test.go b/github/resource_github_organization_custom_properties_test.go index 1e7ee1024c..fe7ef423f9 100644 --- a/github/resource_github_organization_custom_properties_test.go +++ b/github/resource_github_organization_custom_properties_test.go @@ -5,10 +5,68 @@ import ( "regexp" "testing" + "github.com/google/go-github/v89/github" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" ) +func Test_multiSelectDefaultValueWarning(t *testing.T) { + t.Parallel() + + for _, d := range []struct { + testName string + valueType github.PropertyValueType + defaultValue string + expectWarn bool + }{ + { + testName: "multi_select_with_default_value", + valueType: github.PropertyValueTypeMultiSelect, + defaultValue: "Test", + expectWarn: true, + }, + { + testName: "multi_select_without_default_value", + valueType: github.PropertyValueTypeMultiSelect, + }, + { + testName: "single_select_with_default_value", + valueType: github.PropertyValueTypeSingleSelect, + defaultValue: "Test", + }, + { + testName: "true_false_with_default_value", + valueType: github.PropertyValueTypeTrueFalse, + defaultValue: "true", + }, + } { + t.Run(d.testName, func(t *testing.T) { + t.Parallel() + + got := multiSelectDefaultValueWarning(d.valueType, d.defaultValue) + + if !d.expectWarn { + if len(got) != 0 { + t.Fatalf("expected no diagnostics but got %v", got) + } + return + } + + if len(got) != 1 { + t.Fatalf("expected a single diagnostic but got %v", got) + } + if got[0].Severity != diag.Warning { + t.Errorf("expected a warning but got severity %v", got[0].Severity) + } + if got.HasError() { + t.Error("expected the diagnostics to not contain an error") + } + }) + } +} + func TestAccGithubOrganizationCustomProperties(t *testing.T) { t.Parallel() @@ -265,6 +323,40 @@ resource "github_organization_custom_properties" "test" { }) }) + t.Run("true_false property with default_value produces no drift", func(t *testing.T) { + t.Parallel() + + name := fmt.Sprintf("%s%s", testResourcePrefix, acctest.RandString(5)) + + config := fmt.Sprintf(` +resource "github_organization_custom_properties" "test" { + property_name = "%s" + value_type = "true_false" + required = false + description = "Test true_false default_value" + default_value = "true" +} +`, name) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessHasOrgs(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ConfigPlanChecks: resource.ConfigPlanChecks{ + // Read must round-trip the true_false default_value so the + // plan after refresh is empty (regression guard for + // perpetual drift). + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectEmptyPlan(), + }, + }, + }, + }, + }) + }) + t.Run("imports existing property with values_editable_by set via UI", func(t *testing.T) { t.Parallel() diff --git a/templates/resources/organization_custom_properties.md.tmpl b/templates/resources/organization_custom_properties.md.tmpl index a65ecca029..7171ab7166 100644 --- a/templates/resources/organization_custom_properties.md.tmpl +++ b/templates/resources/organization_custom_properties.md.tmpl @@ -40,7 +40,7 @@ The following arguments are supported: - `description` - (Optional) The description of the custom property. -- `default_value` - (Optional) The default value of the custom property. +- `default_value` - (Optional) The default value of the custom property. Not supported for `multi_select` properties. - `allowed_values` - (Optional) List of allowed values for the custom property. Only applicable when `value_type` is `single_select` or `multi_select`.