From 166a9a91b150cfd61e57723ddf466f9a39923ca4 Mon Sep 17 00:00:00 2001 From: Daniel Schmidt Date: Wed, 12 Feb 2025 10:47:36 +0100 Subject: [PATCH] implement resource identity upgrade on refresh --- .../builtin/providers/terraform/provider.go | 4 + .../providers/terraform/resource_data.go | 5 + internal/plugin/grpc_provider.go | 44 ++++ internal/plugin6/grpc_provider.go | 44 ++++ internal/provider-simple-v6/provider.go | 9 + internal/provider-simple/provider.go | 9 + internal/providers/mock.go | 34 +++ internal/providers/provider.go | 26 ++ internal/providers/testing/provider_mock.go | 48 ++++ internal/refactoring/mock_provider.go | 4 + .../internal/stackeval/stubs/errored.go | 16 ++ .../internal/stackeval/stubs/offline.go | 13 + .../internal/stackeval/stubs/unknown.go | 6 + internal/states/instance_object_src.go | 4 +- internal/terraform/context_refresh_test.go | 223 ++++++++++++------ internal/terraform/node_resource_abstract.go | 61 ++++- .../node_resource_abstract_instance.go | 13 + 17 files changed, 487 insertions(+), 76 deletions(-) diff --git a/internal/builtin/providers/terraform/provider.go b/internal/builtin/providers/terraform/provider.go index ef189f5cbf4d..f4fdb187a7ad 100644 --- a/internal/builtin/providers/terraform/provider.go +++ b/internal/builtin/providers/terraform/provider.go @@ -155,6 +155,10 @@ func (p *Provider) UpgradeResourceState(req providers.UpgradeResourceStateReques return upgradeDataStoreResourceState(req) } +func (p *Provider) UpgradeResourceIdentity(req providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse { + return upgradeDataStoreResourceIdentity(req) +} + // ReadResource refreshes a resource and returns its current state. func (p *Provider) ReadResource(req providers.ReadResourceRequest) providers.ReadResourceResponse { return readDataStoreResourceState(req) diff --git a/internal/builtin/providers/terraform/resource_data.go b/internal/builtin/providers/terraform/resource_data.go index 34900091364e..312a5b80839d 100644 --- a/internal/builtin/providers/terraform/resource_data.go +++ b/internal/builtin/providers/terraform/resource_data.go @@ -55,6 +55,11 @@ func upgradeDataStoreResourceState(req providers.UpgradeResourceStateRequest) (r return resp } +func upgradeDataStoreResourceIdentity(req providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + // TODO: Implement this + panic("not implemented") +} + func readDataStoreResourceState(req providers.ReadResourceRequest) (resp providers.ReadResourceResponse) { resp.NewState = req.PriorState return resp diff --git a/internal/plugin/grpc_provider.go b/internal/plugin/grpc_provider.go index 38f03fdea0d9..497ddab3c7d5 100644 --- a/internal/plugin/grpc_provider.go +++ b/internal/plugin/grpc_provider.go @@ -382,6 +382,50 @@ func (p *GRPCProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ return resp } +func (p *GRPCProvider) UpgradeResourceIdentity(r providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + logger.Trace("GRPCProvider: UpgradeResourceIdentity") + + schema := p.GetResourceIdentitySchemas() + if schema.Diagnostics.HasErrors() { + resp.Diagnostics = schema.Diagnostics + return resp + } + + resSchema, ok := schema.IdentityTypes[r.TypeName] + if !ok { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type %q", r.TypeName)) + return resp + } + + protoReq := &proto.UpgradeResourceIdentity_Request{ + TypeName: r.TypeName, + Version: int64(r.Version), + RawIdentity: r.RawIdentityJSON, + } + + protoResp, err := p.client.UpgradeResourceIdentity(p.ctx, protoReq) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err)) + return resp + } + resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + + ty := resSchema.Attributes.ImpliedType() + resp.UpgradedIdentity = cty.NullVal(ty) + if protoResp.UpgradedIdentity == nil { + return resp + } + + identity, err := decodeDynamicValue(protoResp.UpgradedIdentity.IdentityData, ty) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } + resp.UpgradedIdentity = identity + + return resp +} + func (p *GRPCProvider) ConfigureProvider(r providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { logger.Trace("GRPCProvider: ConfigureProvider") diff --git a/internal/plugin6/grpc_provider.go b/internal/plugin6/grpc_provider.go index cf4ea4dc673a..3579c03acd68 100644 --- a/internal/plugin6/grpc_provider.go +++ b/internal/plugin6/grpc_provider.go @@ -375,6 +375,50 @@ func (p *GRPCProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ return resp } +func (p *GRPCProvider) UpgradeResourceIdentity(r providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + logger.Trace("GRPCProvider.v6: UpgradeResourceIdentity") + + schema := p.GetResourceIdentitySchemas() + if schema.Diagnostics.HasErrors() { + resp.Diagnostics = schema.Diagnostics + return resp + } + + resSchema, ok := schema.IdentityTypes[r.TypeName] + if !ok { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("unknown resource type %q", r.TypeName)) + return resp + } + + protoReq := &proto6.UpgradeResourceIdentity_Request{ + TypeName: r.TypeName, + Version: int64(r.Version), + RawIdentity: r.RawIdentityJSON, + } + + protoResp, err := p.client.UpgradeResourceIdentity(p.ctx, protoReq) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err)) + return resp + } + resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics)) + + ty := resSchema.Attributes.ImpliedType() + resp.UpgradedIdentity = cty.NullVal(ty) + if protoResp.UpgradedIdentity == nil { + return resp + } + + identity, err := decodeDynamicValue(protoResp.UpgradedIdentity.IdentityData, ty) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } + resp.UpgradedIdentity = identity + + return resp +} + func (p *GRPCProvider) ConfigureProvider(r providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { logger.Trace("GRPCProvider.v6: ConfigureProvider") diff --git a/internal/provider-simple-v6/provider.go b/internal/provider-simple-v6/provider.go index 1055111f57b3..4626d3ba7115 100644 --- a/internal/provider-simple-v6/provider.go +++ b/internal/provider-simple-v6/provider.go @@ -105,6 +105,15 @@ func (p simple) UpgradeResourceState(req providers.UpgradeResourceStateRequest) return resp } +func (p simple) UpgradeResourceIdentity(req providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + schema := p.GetResourceIdentitySchemas().IdentityTypes[req.TypeName].Attributes + ty := schema.ImpliedType() + val, err := ctyjson.Unmarshal(req.RawIdentityJSON, ty) + resp.Diagnostics = resp.Diagnostics.Append(err) + resp.UpgradedIdentity = val + return resp +} + func (s simple) ConfigureProvider(providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { return resp } diff --git a/internal/provider-simple/provider.go b/internal/provider-simple/provider.go index 0f17c2d398f1..68bcefe01e1a 100644 --- a/internal/provider-simple/provider.go +++ b/internal/provider-simple/provider.go @@ -82,6 +82,15 @@ func (p simple) UpgradeResourceState(req providers.UpgradeResourceStateRequest) return resp } +func (p simple) UpgradeResourceIdentity(req providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + schema := p.GetResourceIdentitySchemas().IdentityTypes[req.TypeName].Attributes + ty := schema.ImpliedType() + val, err := ctyjson.Unmarshal(req.RawIdentityJSON, ty) + resp.Diagnostics = resp.Diagnostics.Append(err) + resp.UpgradedIdentity = val + return resp +} + func (s simple) ConfigureProvider(providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { return resp } diff --git a/internal/providers/mock.go b/internal/providers/mock.go index 30ee923b6d33..b8c4dfa9b0c4 100644 --- a/internal/providers/mock.go +++ b/internal/providers/mock.go @@ -136,6 +136,40 @@ func (m *Mock) UpgradeResourceState(request UpgradeResourceStateRequest) (respon return response } +func (m *Mock) UpgradeResourceIdentity(request UpgradeResourceIdentityRequest) (response UpgradeResourceIdentityResponse) { + // We can't do this from a mocked provider, so we just return whatever identity + // is in the request back unchanged. + + schema := m.GetResourceIdentitySchemas() + response.Diagnostics = response.Diagnostics.Append(schema.Diagnostics) + if schema.Diagnostics.HasErrors() { + // We couldn't retrieve the schema for some reason, so the mock + // provider can't really function. + return response + } + + resource, exists := schema.IdentityTypes[request.TypeName] + if !exists { + // This means something has gone wrong much earlier, we should have + // failed a validation somewhere if a resource type doesn't exist. + panic(fmt.Errorf("failed to retrieve identity schema for resource %s", request.TypeName)) + } + + schemaType := resource.Attributes.ImpliedType() + value, err := ctyjson.Unmarshal(request.RawIdentityJSON, schemaType) + + if err != nil { + // Generally, we shouldn't get an error here. The mocked providers are + // only used in tests, and we can't use different versions of providers + // within/between tests so the types should always match up. As such, + // we're not gonna return a super detailed error here. + response.Diagnostics = response.Diagnostics.Append(err) + return response + } + response.UpgradedIdentity = value + return response +} + func (m *Mock) ConfigureProvider(request ConfigureProviderRequest) (response ConfigureProviderResponse) { // Do nothing here, we don't have anything to configure within the mocked // providers. We don't want to call the original providers from here as diff --git a/internal/providers/provider.go b/internal/providers/provider.go index 68c5de89c49c..c58926789189 100644 --- a/internal/providers/provider.go +++ b/internal/providers/provider.go @@ -43,6 +43,12 @@ type Interface interface { // result is used for any further processing. UpgradeResourceState(UpgradeResourceStateRequest) UpgradeResourceStateResponse + // UpgradeResourceIdentity is called when the state loader encounters an + // instance identity whose schema version is less than the one reported by + // the currently-used version of the corresponding provider, and the upgraded + // result is used for any further processing. + UpgradeResourceIdentity(UpgradeResourceIdentityRequest) UpgradeResourceIdentityResponse + // Configure configures and initialized the provider. ConfigureProvider(ConfigureProviderRequest) ConfigureProviderResponse @@ -275,6 +281,26 @@ type UpgradeResourceStateResponse struct { Diagnostics tfdiags.Diagnostics } +type UpgradeResourceIdentityRequest struct { + // TypeName is the name of the resource type being upgraded + TypeName string + + // Version is version of the schema that created the current identity. + Version int64 + + // RawIdentityJSON contains the identity that needs to be + // upgraded to match the current schema version. + RawIdentityJSON []byte +} + +type UpgradeResourceIdentityResponse struct { + // UpgradedState is the newly upgraded resource identity. + UpgradedIdentity cty.Value + + // Diagnostics contains any warnings or errors from the method call. + Diagnostics tfdiags.Diagnostics +} + type ConfigureProviderRequest struct { // Terraform version is the version string from the running instance of // terraform. Providers can use TerraformVersion to verify compatibility, diff --git a/internal/providers/testing/provider_mock.go b/internal/providers/testing/provider_mock.go index 927d48789eef..ac43e5842a95 100644 --- a/internal/providers/testing/provider_mock.go +++ b/internal/providers/testing/provider_mock.go @@ -58,6 +58,12 @@ type MockProvider struct { UpgradeResourceStateRequest providers.UpgradeResourceStateRequest UpgradeResourceStateFn func(providers.UpgradeResourceStateRequest) providers.UpgradeResourceStateResponse + UpgradeResourceIdentityCalled bool + UpgradeResourceIdentityTypeName string + UpgradeResourceIdentityResponse *providers.UpgradeResourceIdentityResponse + UpgradeResourceIdentityRequest providers.UpgradeResourceIdentityRequest + UpgradeResourceIdentityFn func(providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse + ConfigureProviderCalled bool ConfigureProviderResponse *providers.ConfigureProviderResponse ConfigureProviderRequest providers.ConfigureProviderRequest @@ -150,6 +156,10 @@ func (p *MockProvider) GetResourceIdentitySchemas() providers.GetResourceIdentit defer p.Unlock() p.GetResourceIdentitySchemasCalled = true + return p.getResourceIdentitySchemas() +} + +func (p *MockProvider) getResourceIdentitySchemas() providers.GetResourceIdentitySchemasResponse { if p.GetResourceIdentitySchemasResponse != nil { return *p.GetResourceIdentitySchemasResponse } @@ -323,6 +333,44 @@ func (p *MockProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ return resp } +func (p *MockProvider) UpgradeResourceIdentity(r providers.UpgradeResourceIdentityRequest) (resp providers.UpgradeResourceIdentityResponse) { + p.Lock() + defer p.Unlock() + + if !p.ConfigureProviderCalled { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("Configure not called before UpgradeResourceIdentity %q", r.TypeName)) + return resp + } + p.UpgradeResourceIdentityCalled = true + p.UpgradeResourceIdentityRequest = r + + if p.UpgradeResourceIdentityFn != nil { + return p.UpgradeResourceIdentityFn(r) + } + + if p.UpgradeResourceIdentityResponse != nil { + return *p.UpgradeResourceIdentityResponse + } + + identitySchema, ok := p.getResourceIdentitySchemas().IdentityTypes[r.TypeName] + if !ok { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("no schema found for %q", r.TypeName)) + return resp + } + + identityType := identitySchema.Attributes.ImpliedType() + + v, err := ctyjson.Unmarshal(r.RawIdentityJSON, identityType) + + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + return resp + } + resp.UpgradedIdentity = v + + return resp +} + func (p *MockProvider) ConfigureProvider(r providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { p.Lock() defer p.Unlock() diff --git a/internal/refactoring/mock_provider.go b/internal/refactoring/mock_provider.go index c6cfd16a403e..42506b6a6c23 100644 --- a/internal/refactoring/mock_provider.go +++ b/internal/refactoring/mock_provider.go @@ -48,6 +48,10 @@ func (provider *mockProvider) UpgradeResourceState(providers.UpgradeResourceStat panic("not implemented in mock") } +func (provider *mockProvider) UpgradeResourceIdentity(providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse { + panic("not implemented in mock") +} + func (provider *mockProvider) ConfigureProvider(providers.ConfigureProviderRequest) providers.ConfigureProviderResponse { panic("not implemented in mock") } diff --git a/internal/stacks/stackruntime/internal/stackeval/stubs/errored.go b/internal/stacks/stackruntime/internal/stackeval/stubs/errored.go index 48a3180e14b6..b03addf24328 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stubs/errored.go +++ b/internal/stacks/stackruntime/internal/stackeval/stubs/errored.go @@ -184,6 +184,22 @@ func (p *erroredProvider) UpgradeResourceState(req providers.UpgradeResourceStat } } +func (p *erroredProvider) UpgradeResourceIdentity(req providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse { + // Ideally we'd just skip this altogether and echo back what the caller + // provided, but the request is in a different serialization format than + // the response and so only the real provider can deal with this one. + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.AttributeValue( + tfdiags.Error, + "Provider configuration is invalid", + "Cannot decode the prior state for this resource instance because its provider configuration is invalid.", + nil, // nil attribute path means the overall configuration block + )) + return providers.UpgradeResourceIdentityResponse{ + Diagnostics: diags, + } +} + // ValidateDataResourceConfig implements providers.Interface. func (p *erroredProvider) ValidateDataResourceConfig(req providers.ValidateDataResourceConfigRequest) providers.ValidateDataResourceConfigResponse { // We'll just optimistically assume the configuration is valid, so that diff --git a/internal/stacks/stackruntime/internal/stackeval/stubs/offline.go b/internal/stacks/stackruntime/internal/stackeval/stubs/offline.go index 780acbb00735..e594586398d8 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stubs/offline.go +++ b/internal/stacks/stackruntime/internal/stackeval/stubs/offline.go @@ -103,6 +103,19 @@ func (o *offlineProvider) UpgradeResourceState(request providers.UpgradeResource } } +func (o *offlineProvider) UpgradeResourceIdentity(request providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse { + var diags tfdiags.Diagnostics + diags = diags.Append(tfdiags.AttributeValue( + tfdiags.Error, + "Called UpgradeResourceIdentity on an unconfigured provider", + "Cannot upgrade the state of this resource because this provider is not configured. This is a bug in Terraform - please report it.", + nil, // nil attribute path means the overall configuration block + )) + return providers.UpgradeResourceIdentityResponse{ + Diagnostics: diags, + } +} + func (o *offlineProvider) ConfigureProvider(request providers.ConfigureProviderRequest) providers.ConfigureProviderResponse { var diags tfdiags.Diagnostics diags = diags.Append(tfdiags.AttributeValue( diff --git a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go index adf178158ff3..6d8ada14f764 100644 --- a/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go +++ b/internal/stacks/stackruntime/internal/stackeval/stubs/unknown.go @@ -74,6 +74,12 @@ func (u *unknownProvider) UpgradeResourceState(request providers.UpgradeResource return u.unconfiguredClient.UpgradeResourceState(request) } +func (u *unknownProvider) UpgradeResourceIdentity(request providers.UpgradeResourceIdentityRequest) providers.UpgradeResourceIdentityResponse { + // This is offline functionality, so we can hand it off to the unconfigured + // client. + return u.unconfiguredClient.UpgradeResourceIdentity(request) +} + func (u *unknownProvider) ConfigureProvider(request providers.ConfigureProviderRequest) providers.ConfigureProviderResponse { // This shouldn't be called, we don't configure an unknown provider within // stacks and Terraform Core shouldn't call this method. diff --git a/internal/states/instance_object_src.go b/internal/states/instance_object_src.go index f2c577b9efaf..95c16a76473f 100644 --- a/internal/states/instance_object_src.go +++ b/internal/states/instance_object_src.go @@ -75,7 +75,7 @@ type ResourceInstanceObjectSrc struct { } // DecodeWithIdentity unmarshals the raw representation of the object attributes -// and identity schema. +// and identity schema. We expect the caller to make sure upgrades of the resource identity happen beforehand. func (os *ResourceInstanceObjectSrc) DecodeWithIdentity(ty cty.Type, identityTy cty.Type, identitySchemaVersion uint64) (*ResourceInstanceObject, error) { // TODO: On call-side this should lead to an upgrade call (plus we need the old schema as well) // We might have no identity data at all @@ -94,7 +94,7 @@ func (os *ResourceInstanceObjectSrc) DecodeWithIdentity(ty cty.Type, identityTy rio.Identity, err = ctyjson.Unmarshal(os.IdentitySchemaJSON, identityTy) if err != nil { - return nil, fmt.Errorf("failed to decode identity schema: %e", err) + return nil, fmt.Errorf("failed to decode identity schema: %s. This is most likely a bug in the Provider, providers must not change the identity schema without updating the identity schema version", err.Error()) } return rio, nil diff --git a/internal/terraform/context_refresh_test.go b/internal/terraform/context_refresh_test.go index ec92019f5c89..ebc14dd9c317 100644 --- a/internal/terraform/context_refresh_test.go +++ b/internal/terraform/context_refresh_test.go @@ -4,6 +4,7 @@ package terraform import ( + "fmt" "reflect" "sort" "strings" @@ -20,6 +21,7 @@ import ( "github.com/hashicorp/terraform/internal/providers" testing_provider "github.com/hashicorp/terraform/internal/providers/testing" "github.com/hashicorp/terraform/internal/states" + "github.com/hashicorp/terraform/internal/tfdiags" ) func TestContext2Refresh(t *testing.T) { @@ -1695,15 +1697,15 @@ resource "test_resource" "foo" { // TODO: Move to plan tests // TODO: Double check if we need specific refresh tests as well func TestContext2Refresh_resource_identity_adds_missing(t *testing.T) { - for name, tc := range map[string]struct { - StoredIdentitySchemaVersion uint64 - StoredIdentityJSON []byte - IdentitySchema providers.IdentitySchema - IdentityType cty.Type - IdentityData cty.Value - ExpectedIdentity cty.Value - ExpectedError error + StoredIdentitySchemaVersion uint64 + StoredIdentityJSON []byte + IdentitySchema providers.IdentitySchema + IdentityData cty.Value + ExpectedIdentity cty.Value + ExpectedError error + ExpectUpgradeResourceIdentityCalled bool + UpgradeResourceIdentityResponse providers.UpgradeResourceIdentityResponse }{ "no previous identity": { IdentitySchema: providers.IdentitySchema{ @@ -1715,61 +1717,122 @@ func TestContext2Refresh_resource_identity_adds_missing(t *testing.T) { }, }, }, - IdentityType: cty.Object(map[string]cty.Type{ - "id": cty.String, + IdentityData: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, + "identity version mismatch": { + StoredIdentitySchemaVersion: 1, + StoredIdentityJSON: []byte(`{"id": "foo"}`), + IdentitySchema: providers.IdentitySchema{ + Version: 0, + Attributes: configschema.IdentityAttributes{ + "id": { + Type: cty.String, + RequiredForImport: true, + }, + }, + }, + IdentityData: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + ExpectedError: fmt.Errorf("identity schema version mismatch: got 1, want 0"), + }, + "identity type mismatch": { + StoredIdentitySchemaVersion: 0, + StoredIdentityJSON: []byte(`{"arn": "foo"}`), + IdentitySchema: providers.IdentitySchema{ + Version: 0, + Attributes: configschema.IdentityAttributes{ + "id": { + Type: cty.String, + RequiredForImport: true, + }, + }, + }, + IdentityData: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), }), + ExpectedError: fmt.Errorf("failed to decode identity schema: unsupported attribute \"arn\". This is most likely a bug in the Provider, providers must not change the identity schema without updating the identity schema version"), + }, + "identity upgrade succeeds": { + StoredIdentitySchemaVersion: 1, + StoredIdentityJSON: []byte(`{"arn": "foo"}`), + IdentitySchema: providers.IdentitySchema{ + Version: 2, + Attributes: configschema.IdentityAttributes{ + "id": { + Type: cty.String, + RequiredForImport: true, + }, + }, + }, IdentityData: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("foo"), }), + UpgradeResourceIdentityResponse: providers.UpgradeResourceIdentityResponse{ + UpgradedIdentity: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + }, ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("foo"), }), + ExpectUpgradeResourceIdentityCalled: true, + }, + "identity upgrade failed": { + StoredIdentitySchemaVersion: 1, + StoredIdentityJSON: []byte(`{"id": "foo"}`), + IdentitySchema: providers.IdentitySchema{ + Version: 2, + Attributes: configschema.IdentityAttributes{ + "arn": { + Type: cty.String, + RequiredForImport: true, + }, + }, + }, + IdentityData: cty.ObjectVal(map[string]cty.Value{ + "arn": cty.StringVal("arn:foo"), + }), + UpgradeResourceIdentityResponse: providers.UpgradeResourceIdentityResponse{ + UpgradedIdentity: cty.NilVal, + Diagnostics: tfdiags.Diagnostics{ + tfdiags.Sourceless(tfdiags.Error, "failed to upgrade resource identity", "provider was unable to do so"), + }, + }, + ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ + "arn": cty.StringVal("arn:foo"), + }), + ExpectUpgradeResourceIdentityCalled: true, + ExpectedError: fmt.Errorf("failed to upgrade resource identity: provider was unable to do so"), + }, + "identity sent to provider differs from returned one": { + StoredIdentitySchemaVersion: 0, + StoredIdentityJSON: []byte(`{"id": "foo"}`), + IdentitySchema: providers.IdentitySchema{ + Version: 0, + Attributes: configschema.IdentityAttributes{ + "id": { + Type: cty.String, + RequiredForImport: true, + }, + }, + }, + IdentityData: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + }), + ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("foo"), + }), + ExpectedError: fmt.Errorf("Provider produced different identity: Provider \"registry.terraform.io/hashicorp/aws\" planned an different identity for aws_instance.web during refresh. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker."), }, - // "identity version mismatch": { - // StoredIdentitySchemaVersion: 1, - // StoredIdentityJSON: []byte(`{"id": "foo"}`), - // IdentitySchema: providers.IdentitySchema{ - // Version: 0, - // Attributes: configschema.IdentityAttributes{ - // "id": { - // Type: cty.String, - // RequiredForImport: true, - // }, - // }, - // }, - // IdentityType: cty.Object(map[string]cty.Type{ - // "id": cty.String, - // }), - // IdentityData: cty.ObjectVal(map[string]cty.Value{ - // "id": cty.StringVal("foo"), - // }), - // ExpectedError: fmt.Errorf("identity schema version mismatch: stored 1, expected 0"), - // }, - // "identity type mismatch": { - // StoredIdentitySchemaVersion: 0, - // StoredIdentityJSON: []byte(`{"arn": "foo"}`), - // IdentitySchema: providers.IdentitySchema{ - // Version: 0, - // Attributes: configschema.IdentityAttributes{ - // "id": { - // Type: cty.String, - // RequiredForImport: true, - // }, - // }, - // }, - // IdentityType: cty.Object(map[string]cty.Type{ - // "id": cty.String, - // }), - // IdentityData: cty.ObjectVal(map[string]cty.Value{ - // "id": cty.StringVal("foo"), - // }), - // ExpectedIdentity: cty.ObjectVal(map[string]cty.Value{ - // "id": cty.StringVal("foo"), - // }), - // ExpectedError: fmt.Errorf("identity schema mismatch, could not decode"), - // }, - // "identity upgrade": {}, - // "identity recorded, no identity sent": {}, } { t.Run(name, func(t *testing.T) { p := testProvider("aws") @@ -1812,9 +1875,24 @@ func TestContext2Refresh_resource_identity_adds_missing(t *testing.T) { Identity: tc.IdentityData, } + p.UpgradeResourceIdentityResponse = &tc.UpgradeResourceIdentityResponse + s, diags := ctx.Plan(m, state, &PlanOpts{Mode: plans.RefreshOnlyMode}) - if diags.HasErrors() { - t.Fatal(diags.Err()) + + // TODO: maybe move to comparing diagnostics instead + if tc.ExpectedError != nil { + if !diags.HasErrors() { + t.Fatal("expected error, got none") + } + if diags.Err().Error() != tc.ExpectedError.Error() { + t.Fatalf("unexpected error\nwant: %v\ngot: %v", tc.ExpectedError, diags.Err()) + } + + return + } else { + if diags.HasErrors() { + t.Fatal(diags.Err()) + } } if !p.ReadResourceCalled { @@ -1825,30 +1903,29 @@ func TestContext2Refresh_resource_identity_adds_missing(t *testing.T) { t.Fatal("GetResourceIdentitySchemas should be called") } + if tc.ExpectUpgradeResourceIdentityCalled && !p.UpgradeResourceIdentityCalled { + t.Fatal("UpgradeResourceIdentity should be called") + } + mod := s.PriorState.RootModule() - fromState, err := mod.Resources["aws_instance.web"].Instances[addrs.NoKey].Current.DecodeWithIdentity(ty, tc.IdentityType, uint64(tc.IdentitySchema.Version)) + fromState, err := mod.Resources["aws_instance.web"].Instances[addrs.NoKey].Current.DecodeWithIdentity(ty, tc.IdentitySchema.Attributes.ImpliedType(), uint64(tc.IdentitySchema.Version)) if err != nil { t.Fatal(err) } - if tc.ExpectedError != nil { - if err != tc.ExpectedError { - t.Fatalf("unexpected error\nwant: %v\ngot: %v", tc.ExpectedError, err) - } - } else { - newState, err := schema.CoerceValue(fromState.Value) - if err != nil { - t.Fatal(err) - } + newState, err := schema.CoerceValue(fromState.Value) + if err != nil { + t.Fatal(err) + } - if !cmp.Equal(readState, newState, valueComparer) { - t.Fatal(cmp.Diff(readState, newState, valueComparer, equateEmpty)) - } + if !cmp.Equal(readState, newState, valueComparer) { + t.Fatal(cmp.Diff(readState, newState, valueComparer, equateEmpty)) + } - if tc.ExpectedIdentity.Equals(fromState.Identity).False() { - t.Fatalf("wrong identity\nwant: %s\ngot: %s", tc.ExpectedIdentity.GoString(), fromState.Identity.GoString()) - } + if tc.ExpectedIdentity.Equals(fromState.Identity).False() { + t.Fatalf("wrong identity\nwant: %s\ngot: %s", tc.ExpectedIdentity.GoString(), fromState.Identity.GoString()) } + }) } } diff --git a/internal/terraform/node_resource_abstract.go b/internal/terraform/node_resource_abstract.go index ae0403267860..2991257e3b7b 100644 --- a/internal/terraform/node_resource_abstract.go +++ b/internal/terraform/node_resource_abstract.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform/internal/configs/configschema" "github.com/hashicorp/terraform/internal/dag" "github.com/hashicorp/terraform/internal/lang/langrefs" + "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -505,7 +506,65 @@ func (n *NodeAbstractResource) readResourceInstanceState(ctx EvalContext, addr a return nil, diags } - obj, err := src.Decode(schema.ImpliedType()) + providerIdentitySchema, err := getResourceIdentitySchemas(ctx, n.ResolvedProvider) + if err != nil { + return nil, diags.Append(err) // TODO: Wrap error + } + identitySchema, currentIdentityVersion := providerIdentitySchema.IdentitySchemaForResourceAddr(addr.Resource.ContainingResource()) + + // We need to upgrade the identity schema as well, if necessary. + if src.IdentitySchemaVersion < currentIdentityVersion { + providerType := addr.Resource.Resource.ImpliedProvider() + req := providers.UpgradeResourceIdentityRequest{ + TypeName: addr.Resource.Resource.Type, + + // TODO: The internal schema version representations are all using + // uint64 instead of int64, but unsigned integers aren't friendly + // to all protobuf target languages so in practice we use int64 + // on the wire. In future we will change all of our internal + // representations to int64 too. + Version: int64(src.SchemaVersion), + RawIdentityJSON: src.IdentitySchemaJSON, + } + + resp := provider.UpgradeResourceIdentity(req) + diags = diags.Append(resp.Diagnostics) + if diags.HasErrors() { + return nil, diags + } + + if !resp.UpgradedIdentity.IsWhollyKnown() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid resource identity upgrade", + fmt.Sprintf("The %s provider upgraded the identity for %s from a previous version, but produced an invalid result: The returned state contains unknown values.", providerType, addr), + )) + return nil, diags + } + + newIdentity := resp.UpgradedIdentity + if errs := newIdentity.Type().TestConformance(identitySchema.ImpliedType()); len(errs) > 0 { + for _, err := range errs { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid resource identity upgrade", + fmt.Sprintf("The %s provider upgraded the identity for %s from a previous version, but produced an invalid result: %s.", providerType, addr, tfdiags.FormatError(err)), + )) + } + return nil, diags + } + + // As we upgraded the identity we don't need to read it from state anymore + // we can just decode the src without the identity and add it in afterwards + obj, err := src.Decode(schema.ImpliedType()) + if err != nil { + diags = diags.Append(err) + } + obj.Identity = newIdentity + return obj, diags + } + + obj, err := src.DecodeWithIdentity(schema.ImpliedType(), identitySchema.ImpliedType(), currentIdentityVersion) if err != nil { diags = diags.Append(err) } diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index ec7d67c674bb..52d4f6b027fd 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -644,6 +644,7 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state // to the provider so we'll just return whatever was in state. resp = providers.ReadResourceResponse{ NewState: priorVal, + Identity: state.Identity, } } else { resp = provider.ReadResource(providers.ReadResourceRequest{ @@ -726,6 +727,18 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state return state, deferred, diags } + // Identities can not change on read + if state.Identity.Equals(resp.Identity).False() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Provider produced different identity", + fmt.Sprintf( + "Provider %q planned an different identity for %s during refresh. \n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.", + n.ResolvedProvider.Provider, absAddr, + ), + )) + } + newState := objchange.NormalizeObjectFromLegacySDK(resp.NewState, schema) if !newState.RawEquals(resp.NewState) { // We had to fix up this object in some way, and we still need to