Skip to content

Conversation

@austinvalle
Copy link
Member

Related Issue

Ref: hashicorp/terraform-provider-aws#44182

Description

There are some scenarios where an identity may be stored with all null values (a scenario that #1513 will prevent from occurring now), which should not be validated, since an identity with all null values is invalid.

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

No

@austinvalle austinvalle added this to the v2.38.1 milestone Sep 22, 2025
@austinvalle
Copy link
Member Author

austinvalle commented Sep 22, 2025

Ran CI with just the test additions (d8abe06) to show the reproduction of the error:
https://github.com/hashicorp/terraform-plugin-sdk/actions/runs/17916586034/job/50940282623?pr=1527

--- FAIL: TestApplyResourceChange (0.00s)
    --- FAIL: TestApplyResourceChange/update-resource-identity-with-empty-prior-identity-identity-may-change (0.01s)
        grpc_provider_test.go:9406: resp.NewState.MsgPack: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:changed test:initial]}
        grpc_provider_test.go:9410: expected: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:changed test:initial]}
        grpc_provider_test.go:9413:   &tfprotov5.ApplyResourceChangeResponse{
              	NewState: &{MsgPack: {0x82, 0xa2, 0x69, 0x64, ...}},
              	Private:  `{"schema_version":"1"}`,
            - 	Diagnostics: []*tfprotov5.Diagnostic{
            - 		&{
            - 			Severity: s"ERROR",
            - 			Summary:  "Unexpected Identity Change: During the update operation, the Ter"...,
            - 		},
            - 	},
            + 	Diagnostics:                 nil,
            - 	UnsafeToUseLegacyTypeSystem: false,
            + 	UnsafeToUseLegacyTypeSystem: true,
            - 	NewIdentity:                 nil,
            + 	NewIdentity: &tfprotov5.ResourceIdentityData{
            + 		IdentityData: &tfprotov5.DynamicValue{MsgPack: []uint8{0x82, 0xaf, 0x69, 0x64, ...}},
            + 	},
              }
            
--- FAIL: TestPlanResourceChange (0.00s)
    --- FAIL: TestPlanResourceChange/update-resource-identity-with-empty-prior-identity-identity-may-change (0.00s)
        grpc_provider_test.go:8046: resp.PlannedState.MsgPack: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:%!s(*cty.unknownType=&{}) test:initial]}
        grpc_provider_test.go:8050: expected: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:%!s(*cty.unknownType=&{}) test:initial]}
        grpc_provider_test.go:8053:   &tfprotov5.PlanResourceChangeResponse{
              	PlannedState:    &{MsgPack: {0x82, 0xa2, 0x69, 0x64, ...}},
              	RequiresReplace: {s`AttributeName("id")`},
              	PlannedPrivate:  `{"_new_extra_shim":{}}`,
            - 	Diagnostics: []*tfprotov5.Diagnostic{
            - 		&{
            - 			Severity: s"ERROR",
            - 			Summary:  "Unexpected Identity Change: During the planning operation, the T"...,
            - 		},
            - 	},
            + 	Diagnostics:                 nil,
              	UnsafeToUseLegacyTypeSystem: true,
              	Deferred:                    nil,
            - 	PlannedIdentity:             nil,
            + 	PlannedIdentity: &tfprotov5.ResourceIdentityData{
            + 		IdentityData: &tfprotov5.DynamicValue{MsgPack: []uint8{0x82, 0xaf, 0x69, 0x64, ...}},
            + 	},
              }
            
--- FAIL: TestReadResource (0.00s)
    --- FAIL: TestReadResource/update-resource-identity-with-empty-prior-identity-identity-may-change (0.00s)
        grpc_provider_test.go:6226: resp.NewState.MsgPack: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:initial test:hello]}
        grpc_provider_test.go:6230: expected: {{{{} map[id:{{{} %!s(cty.primitiveTypeKind=83)}} test:{{{} %!s(cty.primitiveTypeKind=83)}}]}} map[id:initial test:hello]}
        grpc_provider_test.go:6233:   &tfprotov5.ReadResourceResponse{
              	NewState: &{MsgPack: {0x82, 0xa2, 0x69, 0x64, ...}},
            - 	Diagnostics: []*tfprotov5.Diagnostic{
            - 		&{
            - 			Severity: s"ERROR",
            - 			Summary:  "Unexpected Identity Change: During the read operation, the Terra"...,
            - 		},
            - 	},
            + 	Diagnostics: nil,
              	Private:     nil,
              	Deferred:    nil,
            - 	NewIdentity: nil,
            + 	NewIdentity: &tfprotov5.ResourceIdentityData{
            + 		IdentityData: &tfprotov5.DynamicValue{MsgPack: []uint8{0x82, 0xaf, 0x69, 0x64, ...}},
            + 	},
              }
            
FAIL

@austinvalle austinvalle marked this pull request as ready for review September 22, 2025 13:23
@austinvalle austinvalle requested a review from a team as a code owner September 22, 2025 13:23
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

Running the acceptance tests added in hashicorp/terraform-provider-aws#44375

Before:

% make t K=s3 T=TestAccS3Object_Identity_ExistingResource_NoRefresh
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
make: Running acceptance tests on branch: 🌿 f-resource-identity-intercept-fix 🌿...
TF_ACC=1 go1.24.6 test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3Object_Identity_ExistingResource_NoRefresh'  -timeout 360m -vet=off
2025/09/22 09:41:40 Creating Terraform AWS Provider (SDKv2-style)...
2025/09/22 09:41:40 Initializing Terraform AWS Provider (SDKv2-style)...
=== RUN   TestAccS3Object_Identity_ExistingResource_NoRefresh
=== PAUSE TestAccS3Object_Identity_ExistingResource_NoRefresh
=== CONT  TestAccS3Object_Identity_ExistingResource_NoRefresh
    object_test.go:2128: Step 2/2 error: Error running apply: exit status 1

        Error: Unexpected Identity Change: During the update operation, the Terraform Provider unexpectedly returned a different identity than the previously stored one.

        This is always a problem with the provider and should be reported to the provider developer.

        Planned Identity: cty.ObjectVal(map[string]cty.Value{"account_id":cty.NullVal(cty.String), "bucket":cty.NullVal(cty.String), "key":cty.NullVal(cty.String), "region":cty.NullVal(cty.String)})

        New Identity: cty.ObjectVal(map[string]cty.Value{"account_id":cty.StringVal("727561393803"), "bucket":cty.StringVal("tf-acc-test-3353728079742213759"), "key":cty.StringVal("test-key"), "region":cty.StringVal("us-west-2")})

          with aws_s3_object.object,
          on terraform_plugin_test.tf line 16, in resource "aws_s3_object" "object":
          16: resource "aws_s3_object" "object" {

--- FAIL: TestAccS3Object_Identity_ExistingResource_NoRefresh (37.00s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-aws/internal/service/s3 43.809s

With this branch:

% make t K=s3 T=TestAccS3Object_Identity_ExistingResource_NoRefresh
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
make: Running acceptance tests on branch: 🌿 f-resource-identity-intercept-fix 🌿...
TF_ACC=1 go1.24.6 test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3Object_Identity_ExistingResource_NoRefresh'  -timeout 360m -vet=off
2025/09/22 09:45:18 Creating Terraform AWS Provider (SDKv2-style)...
2025/09/22 09:45:18 Initializing Terraform AWS Provider (SDKv2-style)...
=== RUN   TestAccS3Object_Identity_ExistingResource_NoRefresh
=== PAUSE TestAccS3Object_Identity_ExistingResource_NoRefresh
=== CONT  TestAccS3Object_Identity_ExistingResource_NoRefresh
--- PASS: TestAccS3Object_Identity_ExistingResource_NoRefresh (40.57s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/s3 47.523s

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants