diff --git a/CHANGELOG.md b/CHANGELOG.md index de8084a96..1eb3d7023 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # UNRELEASED BUG FIXES: -* Fixed default provider organization usage for `r/tfe_admin_organization_settings`, by @brandonc [1183](https://github.com/hashicorp/terraform-provider-tfe/pull/1183) +* `r/tfe_admin_organization_settings`: Fixed default provider organization usage, by @brandonc [1183](https://github.com/hashicorp/terraform-provider-tfe/pull/1183) +* `r/tfe_registry_gpg_key`: Fixed update plans when using default organization, by @brandonc [1190](https://github.com/hashicorp/terraform-provider-tfe/pull/1190) * `/r/tfe_workspace_settings`: Fix compatibility with older versions of Terraform Enterprise when using agent execution by @brandonc [1193](https://github.com/hashicorp/terraform-provider-tfe/pull/1193) # v0.51.0 diff --git a/internal/provider/provider_custom_diffs.go b/internal/provider/provider_custom_diffs.go index 6a6c6cc63..f9b65fc50 100644 --- a/internal/provider/provider_custom_diffs.go +++ b/internal/provider/provider_custom_diffs.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -28,18 +29,18 @@ func customizeDiffIfProviderDefaultOrganizationChanged(c context.Context, diff * return nil } -func modifyPlanForDefaultOrganizationChange(ctx context.Context, providerDefaultOrg string, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { - if req.State.Raw.IsNull() { +func modifyPlanForDefaultOrganizationChange(ctx context.Context, providerDefaultOrg string, state tfsdk.State, configAttributes, planAttributes AttrGettable, resp *resource.ModifyPlanResponse) { + if state.Raw.IsNull() { return } orgPath := path.Root("organization") - var configOrg, plannedOrg *string - resp.Diagnostics.Append(req.Config.GetAttribute(ctx, orgPath, &configOrg)...) - resp.Diagnostics.Append(req.Plan.GetAttribute(ctx, orgPath, &plannedOrg)...) + var configOrg, plannedOrg types.String + resp.Diagnostics.Append(configAttributes.GetAttribute(ctx, orgPath, &configOrg)...) + resp.Diagnostics.Append(planAttributes.GetAttribute(ctx, orgPath, &plannedOrg)...) - if configOrg == nil && plannedOrg != nil && providerDefaultOrg != *plannedOrg { + if configOrg.IsNull() && !plannedOrg.IsNull() && providerDefaultOrg != plannedOrg.ValueString() { // There is no organization configured on the resource, yet the provider org is different from // the planned organization value. We must conclude that the provider default organization changed. resp.Plan.SetAttribute(ctx, orgPath, types.StringValue(providerDefaultOrg)) diff --git a/internal/provider/provider_custom_diffs_test.go b/internal/provider/provider_custom_diffs_test.go new file mode 100644 index 000000000..30dfe764b --- /dev/null +++ b/internal/provider/provider_custom_diffs_test.go @@ -0,0 +1,115 @@ +package provider + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" + "github.com/hashicorp/terraform-plugin-go/tftypes" +) + +type getter struct { + val types.String +} + +func (g *getter) GetAttribute(_ context.Context, _ path.Path, target interface{}) diag.Diagnostics { + *(target.(*basetypes.StringValue)) = g.val + return diag.Diagnostics{} +} + +func TestModifyPlanForDefaultOrganizationChange(t *testing.T) { + // if configOrg.IsNull() && !plannedOrg.IsNull() && providerDefaultOrg != plannedOrg.ValueString() { + testCases := map[string]struct { + providerDefaultOrg string + planValue types.String + configValue types.String + expectedPlanValue string + expectedRequiresReplace bool + }{ + "No change in provider org": { + providerDefaultOrg: "foo", + planValue: types.StringValue("foo"), + configValue: types.StringNull(), + expectedPlanValue: "foo", + expectedRequiresReplace: false, + }, + "Change in provider org": { + providerDefaultOrg: "bar", + planValue: types.StringValue("foo"), + configValue: types.StringNull(), + expectedPlanValue: "bar", + expectedRequiresReplace: true, + }, + "Config org changed": { + providerDefaultOrg: "foo", + planValue: types.StringValue("bar"), + configValue: types.StringValue("bar"), + expectedPlanValue: "bar", + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + fakeState := tftypes.NewValue(tftypes.Object{}, make(map[string]tftypes.Value)) + + fakeSchema := schema.Schema{ + Attributes: map[string]schema.Attribute{ + "organization": schema.StringAttribute{ + Computed: true, + Optional: true, + Description: "Test organization", + }, + }, + } + + fakePlan := tftypes.NewValue( + tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "organization": tftypes.String, + }, + }, + map[string]tftypes.Value{ + "organization": tftypes.NewValue(tftypes.String, tc.planValue.ValueString()), + }, + ) + + fakeResponse := &resource.ModifyPlanResponse{ + Plan: tfsdk.Plan{Schema: fakeSchema, Raw: fakePlan}, + RequiresReplace: make(path.Paths, 0), + Diagnostics: diag.Diagnostics{}, + } + + c := context.TODO() + + modifyPlanForDefaultOrganizationChange( + c, + tc.providerDefaultOrg, + tfsdk.State{Raw: fakeState}, + &getter{val: tc.configValue}, + &getter{val: tc.planValue}, + fakeResponse, + ) + + orgPath := path.Root("organization") + var value types.String + fakeResponse.Plan.GetAttribute(c, orgPath, &value) + if fakeResponse.Diagnostics.HasError() { + t.Fatalf("Expected no errors, got %v", fakeResponse.Diagnostics) + } + + if value.ValueString() != tc.expectedPlanValue { + t.Fatalf("Expected plan value to be %q, got %q", tc.expectedPlanValue, value.ValueString()) + } + + if tc.expectedRequiresReplace && len(fakeResponse.RequiresReplace) == 0 { + t.Fatal("Expected RequiresReplace to be set, but it was not") + } + }) + } +} diff --git a/internal/provider/resource_tfe_registry_gpg_key.go b/internal/provider/resource_tfe_registry_gpg_key.go index 0a72ecbec..216a19903 100644 --- a/internal/provider/resource_tfe_registry_gpg_key.go +++ b/internal/provider/resource_tfe_registry_gpg_key.go @@ -39,7 +39,7 @@ func (r *resourceTFERegistryGPGKey) Metadata(ctx context.Context, req resource.M } func (r *resourceTFERegistryGPGKey) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { - modifyPlanForDefaultOrganizationChange(ctx, r.config.Organization, req, resp) + modifyPlanForDefaultOrganizationChange(ctx, r.config.Organization, req.State, req.Config, req.Plan, resp) } func (r *resourceTFERegistryGPGKey) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {