helper/schema: Remove timeouts during ImportResourceState (#1146)

Reference: #1145
Reference: hashicorp/terraform#32463

Terraform 1.3.8 and later now correctly handles null values for single nested blocks. This means Terraform will now report differences between a null block and known block with null values. This SDK only supported single nested blocks via its timeouts functionality.

This change is a very targeted removal of any potential `timeouts` block values in a resource state from the `ImportResourceState` RPC. Since configuration is not available during that RPC, it is never valid to return any data beyond null for that block. This will prevent unexpected differences on the first plan after import, where Terraform will report the block removal for configurations which do not contain the block.

New unit test failure prior to code updates:

```
--- FAIL: TestImportResourceState_Timeouts_Removed (0.00s)
    /Users/bflad/src/github.com/hashicorp/terraform-plugin-sdk/helper/schema/grpc_provider_test.go:1159: unexpected difference:   cty.Value(
        - 	{
        - 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        - 		v:  map[string]any{"id": string("test"), "string_attribute": nil, "timeouts": nil},
        - 	},
        + 	{
        + 		ty: cty.Type{typeImpl: cty.typeObject{AttrTypes: map[string]cty.Type{...}}},
        + 		v: map[string]any{
        + 			"id":               string("test"),
        + 			"string_attribute": nil,
        + 			"timeouts":         map[string]any{"create": nil, "read": nil},
        + 		},
        + 	},
          )
```

.changes/unreleased/BUG FIXES-20230213-150146.yaml

@@ -0,0 +1,6 @@
+kind: BUG FIXES
+body: 'helper/schema: Prevented unexpected difference for timeouts on first plan after
+  import'
+time: 2023-02-13T15:01:46.441029-05:00
+custom:
+  Issue: "1146"

helper/schema/grpc_provider.go

@@ -1110,6 +1110,22 @@ func (s *GRPCProviderServer) ImportResourceState(ctx context.Context, req *tfpro
 		// Normalize the value and fill in any missing blocks.
 		newStateVal = objchange.NormalizeObjectFromLegacySDK(newStateVal, schemaBlock)
 
+		// Ensure any timeouts block is null in the imported state. There is no
+		// configuration to read from during import, so it is never valid to
+		// return a known value for the block.
+		//
+		// This is done without modifying HCL2ValueFromFlatmap or
+		// NormalizeObjectFromLegacySDK to prevent other unexpected changes.
+		//
+		// Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/1145
+		newStateType := newStateVal.Type()
+
+		if newStateVal != cty.NilVal && !newStateVal.IsNull() && newStateType.IsObjectType() && newStateType.HasAttribute(TimeoutsConfigKey) {
+			newStateValueMap := newStateVal.AsValueMap()
+			newStateValueMap[TimeoutsConfigKey] = cty.NullVal(newStateType.AttributeType(TimeoutsConfigKey))
+			newStateVal = cty.ObjectVal(newStateValueMap)
+		}
+
 		newStateMP, err := msgpack.Marshal(newStateVal, schemaBlock.ImpliedType())
 		if err != nil {
 			resp.Diagnostics = convert.AppendProtoDiag(ctx, resp.Diagnostics, err)

helper/schema/grpc_provider_test.go

@@ -1006,6 +1006,160 @@ func TestApplyResourceChange_bigint(t *testing.T) {
 	}
 }
 
+// Timeouts should never be present in imported resources.
+// Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/1145
+func TestImportResourceState_Timeouts_None(t *testing.T) {
+	t.Parallel()
+
+	resourceDefinition := &Resource{
+		Importer: &ResourceImporter{
+			StateContext: ImportStatePassthroughContext,
+		},
+		Schema: map[string]*Schema{
+			"string_attribute": {
+				Type:     TypeString,
+				Optional: true,
+			},
+		},
+	}
+	resourceTypeName := "test"
+
+	server := NewGRPCProviderServer(&Provider{
+		ResourcesMap: map[string]*Resource{
+			resourceTypeName: resourceDefinition,
+		},
+	})
+
+	schema := resourceDefinition.CoreConfigSchema()
+
+	// Import shim state should not require all attributes.
+	stateVal, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{
+		"id": cty.StringVal("test"),
+	}))
+
+	if err != nil {
+		t.Fatalf("unable to coerce state value: %s", err)
+	}
+
+	testReq := &tfprotov5.ImportResourceStateRequest{
+		ID:       "test",
+		TypeName: resourceTypeName,
+	}
+
+	resp, err := server.ImportResourceState(context.Background(), testReq)
+
+	if err != nil {
+		t.Fatalf("unexpected error during ImportResourceState: %s", err)
+	}
+
+	if resp == nil {
+		t.Fatal("expected ImportResourceState response")
+	}
+
+	if len(resp.Diagnostics) > 0 {
+		var diagnostics []string
+
+		for _, diagnostic := range resp.Diagnostics {
+			diagnostics = append(diagnostics, fmt.Sprintf("%s: %s: %s", diagnostic.Severity, diagnostic.Summary, diagnostic.Detail))
+		}
+
+		t.Fatalf("unexpected ImportResourceState diagnostics: %s", strings.Join(diagnostics, " | "))
+	}
+
+	if len(resp.ImportedResources) != 1 {
+		t.Fatalf("expected 1 ImportedResource, got: %#v", resp.ImportedResources)
+	}
+
+	gotStateVal, err := msgpack.Unmarshal(resp.ImportedResources[0].State.MsgPack, schema.ImpliedType())
+
+	if err != nil {
+		t.Fatalf("unexpected error during MessagePack unmarshal: %s", err)
+	}
+
+	if diff := cmp.Diff(stateVal, gotStateVal, valueComparer); diff != "" {
+		t.Errorf("unexpected difference: %s", diff)
+	}
+}
+
+// Timeouts should never be present in imported resources.
+// Reference: https://github.com/hashicorp/terraform-plugin-sdk/issues/1145
+func TestImportResourceState_Timeouts_Removed(t *testing.T) {
+	t.Parallel()
+
+	resourceDefinition := &Resource{
+		Importer: &ResourceImporter{
+			StateContext: ImportStatePassthroughContext,
+		},
+		Schema: map[string]*Schema{
+			"string_attribute": {
+				Type:     TypeString,
+				Optional: true,
+			},
+		},
+		Timeouts: &ResourceTimeout{
+			Create: DefaultTimeout(10 * time.Minute),
+			Read:   DefaultTimeout(10 * time.Minute),
+		},
+	}
+	resourceTypeName := "test"
+
+	server := NewGRPCProviderServer(&Provider{
+		ResourcesMap: map[string]*Resource{
+			resourceTypeName: resourceDefinition,
+		},
+	})
+
+	schema := resourceDefinition.CoreConfigSchema()
+
+	// Import shim state should not require all attributes.
+	stateVal, err := schema.CoerceValue(cty.ObjectVal(map[string]cty.Value{
+		"id": cty.StringVal("test"),
+	}))
+
+	if err != nil {
+		t.Fatalf("unable to coerce state value: %s", err)
+	}
+
+	testReq := &tfprotov5.ImportResourceStateRequest{
+		ID:       "test",
+		TypeName: resourceTypeName,
+	}
+
+	resp, err := server.ImportResourceState(context.Background(), testReq)
+
+	if err != nil {
+		t.Fatalf("unexpected error during ImportResourceState: %s", err)
+	}
+
+	if resp == nil {
+		t.Fatal("expected ImportResourceState response")
+	}
+
+	if len(resp.Diagnostics) > 0 {
+		var diagnostics []string
+
+		for _, diagnostic := range resp.Diagnostics {
+			diagnostics = append(diagnostics, fmt.Sprintf("%s: %s: %s", diagnostic.Severity, diagnostic.Summary, diagnostic.Detail))
+		}
+
+		t.Fatalf("unexpected ImportResourceState diagnostics: %s", strings.Join(diagnostics, " | "))
+	}
+
+	if len(resp.ImportedResources) != 1 {
+		t.Fatalf("expected 1 ImportedResource, got: %#v", resp.ImportedResources)
+	}
+
+	gotStateVal, err := msgpack.Unmarshal(resp.ImportedResources[0].State.MsgPack, schema.ImpliedType())
+
+	if err != nil {
+		t.Fatalf("unexpected error during MessagePack unmarshal: %s", err)
+	}
+
+	if diff := cmp.Diff(stateVal, gotStateVal, valueComparer); diff != "" {
+		t.Errorf("unexpected difference: %s", diff)
+	}
+}
+
 func TestReadDataSource(t *testing.T) {
 	t.Parallel()