Fix plan modification error when updating tfe_registry_gpg_key

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

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

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")
+			}
+		})
+	}
+}

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