diff --git a/.changelog/426.txt b/.changelog/426.txt new file mode 100644 index 000000000..cbf4996a8 --- /dev/null +++ b/.changelog/426.txt @@ -0,0 +1,3 @@ +```release-note:bug +internal/fwserver: Ensured `UpgradeResourceState` calls from Terraform 0.12 properly ignored attributes not defined in the schema +``` diff --git a/go.mod b/go.mod index d468b1d89..065ff1d25 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.17 require ( github.com/google/go-cmp v0.5.8 - github.com/hashicorp/terraform-plugin-go v0.12.0 + github.com/hashicorp/terraform-plugin-go v0.13.0 github.com/hashicorp/terraform-plugin-log v0.7.0 ) diff --git a/go.sum b/go.sum index c945c7e9b..bb27976c3 100644 --- a/go.sum +++ b/go.sum @@ -64,9 +64,8 @@ github.com/hashicorp/go-plugin v1.4.4/go.mod h1:viDMjcLJuDui6pXb8U4HVfb8AamCWhHG github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8= github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= github.com/hashicorp/go-version v1.2.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= -github.com/hashicorp/terraform-plugin-go v0.12.0 h1:6wW9mT1dSs0Xq4LR6HXj1heQ5ovr5GxXNJwkErZzpJw= -github.com/hashicorp/terraform-plugin-go v0.12.0/go.mod h1:kwhmaWHNDvT1B3QiSJdAtrB/D4RaKSY/v3r2BuoWK4M= -github.com/hashicorp/terraform-plugin-log v0.6.0/go.mod h1:p4R1jWBXRTvL4odmEkFfDdhUjHf9zcs/BCoNHAc7IK4= +github.com/hashicorp/terraform-plugin-go v0.13.0 h1:Zm+o91HUOcTLotaEu3X2jV/6wNi6f09gkZwGg/MDvCk= +github.com/hashicorp/terraform-plugin-go v0.13.0/go.mod h1:NYGFEM9GeRdSl52txue3RcBDFt2tufaqS22iURP8Bxs= github.com/hashicorp/terraform-plugin-log v0.7.0 h1:SDxJUyT8TwN4l5b5/VkiTIaQgY6R+Y2BQ0sRZftGKQs= github.com/hashicorp/terraform-plugin-log v0.7.0/go.mod h1:p4R1jWBXRTvL4odmEkFfDdhUjHf9zcs/BCoNHAc7IK4= github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c h1:D8aRO6+mTqHfLsK/BC3j5OAoogv1WLRWzY1AaTo3rBg= diff --git a/internal/fwserver/server_upgraderesourcestate.go b/internal/fwserver/server_upgraderesourcestate.go index 93905b6ea..504757263 100644 --- a/internal/fwserver/server_upgraderesourcestate.go +++ b/internal/fwserver/server_upgraderesourcestate.go @@ -4,10 +4,12 @@ import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/logging" "github.com/hashicorp/terraform-plugin-framework/tfsdk" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" ) // UpgradeResourceStateRequest is the framework server request for the @@ -42,6 +44,15 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS return } + // Define options to be used when unmarshalling raw state. + // IgnoreUndefinedAttributes will silently skip over fields in the JSON + // that do not have a matching entry in the schema. + unmarshalOpts := tfprotov6.UnmarshalOpts{ + ValueFromJSONOpts: tftypes.ValueFromJSONOpts{ + IgnoreUndefinedAttributes: true, + }, + } + // Terraform CLI can call UpgradeResourceState even if the stored state // version matches the current schema. Presumably this is to account for // the previous terraform-plugin-sdk implementation, which handled some @@ -52,17 +63,20 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS // detail for provider developers. Instead, the framework will attempt to // roundtrip the prior RawState to a State matching the current Schema. // - // TODO: To prevent provider developers from accidentially implementing + // TODO: To prevent provider developers from accidentally implementing // ResourceWithUpgradeState with a version matching the current schema // version which would never get called, the framework can introduce a // unit test helper. // Reference: https://github.com/hashicorp/terraform-plugin-framework/issues/113 + // + // UnmarshalWithOpts allows optionally ignoring instances in which elements being + // do not have a corresponding attribute within the schema. if req.Version == req.ResourceSchema.Version { logging.FrameworkTrace(ctx, "UpgradeResourceState request version matches current Schema version, using framework defined passthrough implementation") resourceSchemaType := req.ResourceSchema.TerraformType(ctx) - rawStateValue, err := req.RawState.Unmarshal(resourceSchemaType) + rawStateValue, err := req.RawState.UnmarshalWithOpts(resourceSchemaType, unmarshalOpts) if err != nil { resp.Diagnostics.AddError( @@ -138,7 +152,7 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS priorSchemaType := resourceStateUpgrader.PriorSchema.TerraformType(ctx) - rawStateValue, err := req.RawState.Unmarshal(priorSchemaType) + rawStateValue, err := req.RawState.UnmarshalWithOpts(priorSchemaType, unmarshalOpts) if err != nil { resp.Diagnostics.AddError( diff --git a/internal/fwserver/server_upgraderesourcestate_test.go b/internal/fwserver/server_upgraderesourcestate_test.go index e8ed00716..796099862 100644 --- a/internal/fwserver/server_upgraderesourcestate_test.go +++ b/internal/fwserver/server_upgraderesourcestate_test.go @@ -7,14 +7,15 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" + "github.com/hashicorp/terraform-plugin-go/tftypes" + "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" "github.com/hashicorp/terraform-plugin-framework/internal/testing/testprovider" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" - "github.com/hashicorp/terraform-plugin-go/tftypes" ) func TestServerUpgradeResourceState(t *testing.T) { @@ -465,6 +466,91 @@ func TestServerUpgradeResourceState(t *testing.T) { }, }, }, + "PriorSchema-and-State-json-mismatch": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.UpgradeResourceStateRequest{ + RawState: testNewRawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": true, + "nonexistent_attribute": "value", + }), + ResourceSchema: schema, + ResourceType: &testprovider.ResourceType{ + GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) { + return schema, nil + }, + NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) { + return &testprovider.ResourceWithUpgradeState{ + Resource: &testprovider.Resource{}, + UpgradeStateMethod: func(ctx context.Context) map[int64]tfsdk.ResourceStateUpgrader { + return map[int64]tfsdk.ResourceStateUpgrader{ + 0: { + PriorSchema: &tfsdk.Schema{ + Attributes: map[string]tfsdk.Attribute{ + "id": { + Type: types.StringType, + Computed: true, + }, + "optional_attribute": { + Type: types.BoolType, + Optional: true, + }, + "required_attribute": { + Type: types.BoolType, + Required: true, + }, + }, + }, + StateUpgrader: func(ctx context.Context, req tfsdk.UpgradeResourceStateRequest, resp *tfsdk.UpgradeResourceStateResponse) { + var priorStateData struct { + Id string `tfsdk:"id"` + OptionalAttribute *bool `tfsdk:"optional_attribute"` + RequiredAttribute bool `tfsdk:"required_attribute"` + } + + resp.Diagnostics.Append(req.State.Get(ctx, &priorStateData)...) + + if resp.Diagnostics.HasError() { + return + } + + upgradedStateData := struct { + Id string `tfsdk:"id"` + OptionalAttribute *string `tfsdk:"optional_attribute"` + RequiredAttribute string `tfsdk:"required_attribute"` + }{ + Id: priorStateData.Id, + RequiredAttribute: fmt.Sprintf("%t", priorStateData.RequiredAttribute), + } + + if priorStateData.OptionalAttribute != nil { + v := fmt.Sprintf("%t", *priorStateData.OptionalAttribute) + upgradedStateData.OptionalAttribute = &v + } + + resp.Diagnostics.Append(resp.State.Set(ctx, upgradedStateData)...) + }, + }, + } + }, + }, nil + }, + }, + Version: 0, + }, + expectedResponse: &fwserver.UpgradeResourceStateResponse{ + UpgradedState: &tfsdk.State{ + Raw: tftypes.NewValue(schemaType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "test-id-value"), + "optional_attribute": tftypes.NewValue(tftypes.String, nil), + "required_attribute": tftypes.NewValue(tftypes.String, "true"), + }), + Schema: schema, + }, + }, + }, "UpgradedState-missing": { server: &fwserver.Server{ Provider: &testprovider.Provider{}, @@ -599,9 +685,11 @@ func TestServerUpgradeResourceState(t *testing.T) { Provider: &testprovider.Provider{}, }, request: &fwserver.UpgradeResourceStateRequest{ - RawState: &tfprotov6.RawState{ - JSON: []byte(`{"nonexistent_attribute":"value"}`), - }, + RawState: testNewRawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": "true", + "nonexistent_attribute": "value", + }), ResourceSchema: schema, ResourceType: &testprovider.ResourceType{ GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) { @@ -615,15 +703,13 @@ func TestServerUpgradeResourceState(t *testing.T) { Version: 1, // Must match current tfsdk.Schema version to trigger framework implementation }, expectedResponse: &fwserver.UpgradeResourceStateResponse{ - Diagnostics: diag.Diagnostics{ - diag.NewErrorDiagnostic( - "Unable to Read Previously Saved State for UpgradeResourceState", - "There was an error reading the saved resource state using the current resource schema.\n\n"+ - "If this resource state was last refreshed with Terraform CLI 0.11 and earlier, it must be refreshed or applied with an older provider version first. "+ - "If you manually modified the resource state, you will need to manually modify it to match the current resource schema. "+ - "Otherwise, please report this to the provider developer:\n\n"+ - "ElementKeyValue(tftypes.String): unsupported attribute \"nonexistent_attribute\"", - ), + UpgradedState: &tfsdk.State{ + Raw: tftypes.NewValue(schemaType, map[string]tftypes.Value{ + "id": tftypes.NewValue(tftypes.String, "test-id-value"), + "optional_attribute": tftypes.NewValue(tftypes.String, nil), + "required_attribute": tftypes.NewValue(tftypes.String, "true"), + }), + Schema: schema, }, }, },