Merge pull request #43659 from marcinbelczewski/b-dont-set-region-on-null-state

fix: setting the region after resource has been removed from state due resource disappearance

Author: ewbankkit

.changelog/43659.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+provider: Fix failure to detect resources deleted outside of Terraform as missing for numerous resource types
+```

internal/provider/framework/region.go

@@ -332,6 +332,11 @@ func (r resourceSetRegionInStateInterceptor) read(ctx context.Context, opts inte
 
 	switch response, when := opts.response, opts.when; when {
 	case After:
+		// Will occur on a refresh when the resource does not exist in AWS and needs to be recreated, e.g. "_disappears" tests.
+		if response.State.Raw.IsNull() {
+			return diags
+		}
+
 		// Set region in state after R.
 		diags.Append(response.State.SetAttribute(ctx, path.Root(names.AttrRegion), c.Region(ctx))...)
 		if diags.HasError() {

internal/provider/framework/region_test.go

@@ -0,0 +1,98 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package framework
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/path"
+	"github.com/hashicorp/terraform-plugin-framework/resource"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+	"github.com/hashicorp/terraform-plugin-go/tftypes"
+	"github.com/hashicorp/terraform-provider-aws/internal/errs/fwdiag"
+	"github.com/hashicorp/terraform-provider-aws/internal/provider/framework/resourceattribute"
+	"github.com/hashicorp/terraform-provider-aws/names"
+)
+
+func TestResourceSetRegionInStateInterceptor_Read(t *testing.T) {
+	t.Parallel()
+
+	const name = "example"
+
+	region := os.Getenv("AWS_DEFAULT_REGION")
+	if region == "" {
+		t.Skip("AWS_DEFAULT_REGION env var must be set for ResourceSetRegionInStateInterceptor test.")
+	}
+
+	ctx := context.Background()
+	client := mockClient{region: region}
+	icpt := resourceSetRegionInStateInterceptor{}
+
+	s := schema.Schema{
+		Attributes: map[string]schema.Attribute{
+			names.AttrName:   schema.StringAttribute{Required: true},
+			names.AttrRegion: resourceattribute.Region(),
+		},
+	}
+
+	tests := map[string]struct {
+		startState tfsdk.State
+		expectSet  bool
+	}{
+		"when state is present then region is set": {
+			startState: stateFromSchema(ctx, s, map[string]string{"name": name}),
+			expectSet:  true,
+		},
+		"when state is null then it remains null": {
+			startState: tfsdk.State{
+				Raw:    tftypes.NewValue(s.Type().TerraformType(ctx), nil),
+				Schema: s,
+			},
+			expectSet: false,
+		},
+	}
+
+	for tn, tc := range tests {
+		t.Run(tn, func(t *testing.T) {
+			t.Parallel()
+
+			req := resource.ReadRequest{State: tc.startState}
+			resp := resource.ReadResponse{State: tc.startState}
+
+			if diags := icpt.read(ctx, interceptorOptions[resource.ReadRequest, resource.ReadResponse]{
+				c:        client,
+				request:  &req,
+				response: &resp,
+				when:     After,
+			}); diags.HasError() {
+				t.Fatalf("unexpected diags: %s", diags)
+			}
+
+			if tc.expectSet {
+				got := getStateAttributeValue(ctx, t, resp.State, path.Root("region"))
+				if got != region {
+					t.Errorf("expected region %q, got %q", region, got)
+				}
+			} else {
+				if !resp.State.Raw.IsNull() {
+					t.Errorf("expected State.Raw to stay null, got %#v", resp.State.Raw)
+				}
+			}
+		})
+	}
+}
+
+func getStateAttributeValue(ctx context.Context, t *testing.T, st tfsdk.State, p path.Path) string {
+	t.Helper()
+
+	var v types.String
+	if diags := st.GetAttribute(ctx, p, &v); diags.HasError() {
+		t.Fatalf("unexpected error getting State attribute %q: %s", p, fwdiag.DiagnosticsError(diags))
+	}
+	return v.ValueString()
+}

internal/provider/sdkv2/region.go

@@ -78,6 +78,11 @@ func setRegionInState() crudInterceptor {
 			// Set region in state after R.
 			switch why {
 			case Read:
+				// Will occur on a refresh when the resource does not exist in AWS and needs to be recreated, e.g. "_disappears" tests.
+				if d.Id() == "" {
+					return diags
+				}
+
 				if err := d.Set(names.AttrRegion, c.Region(ctx)); err != nil {
 					return sdkdiag.AppendErrorf(diags, "setting %s: %s", names.AttrRegion, err)
 				}

internal/service/appfabric/app_bundle_test.go

@@ -89,6 +89,11 @@ func testAccAppBundle_disappears(t *testing.T) {
 					acctest.CheckFrameworkResourceDisappears(ctx, acctest.Provider, tfappfabric.ResourceAppBundle, resourceName),
 				),
 				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+					},
+				},
 			},
 		},
 	})

internal/service/ec2/vpc_test.go

@@ -94,6 +94,11 @@ func TestAccVPC_disappears(t *testing.T) {
 					acctest.CheckResourceDisappears(ctx, acctest.Provider, tfec2.ResourceVPC(), resourceName),
 				),
 				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+					},
+				},
 			},
 		},
 	})

internal/service/s3control/access_point_test.go

@@ -14,6 +14,7 @@ import (
 	sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/statecheck"
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
@@ -92,6 +93,11 @@ func TestAccS3ControlAccessPoint_disappears(t *testing.T) {
 					acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3control.ResourceAccessPoint(), resourceName),
 				),
 				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+					},
+				},
 			},
 		},
 	})

internal/service/s3tables/table_test.go

@@ -129,6 +129,11 @@ func TestAccS3TablesTable_disappears(t *testing.T) {
 					acctest.CheckFrameworkResourceDisappears(ctx, acctest.Provider, tfs3tables.NewResourceTable, resourceName),
 				),
 				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionCreate),
+					},
+				},
 			},
 		},
 	})