Skip to content

Commit

Permalink
Using UnmarshalWithOpts in order to ignore undefined attributes (#426)
Browse files Browse the repository at this point in the history
Reference: #415
  • Loading branch information
bendbennett authored Jul 28, 2022
1 parent a5f970b commit efe6a2f
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 22 deletions.
3 changes: 3 additions & 0 deletions .changelog/426.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
internal/fwserver: Ensured `UpgradeResourceState` calls from Terraform 0.12 properly ignored attributes not defined in the schema
```
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
5 changes: 2 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
22 changes: 18 additions & 4 deletions internal/fwserver/server_upgraderesourcestate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
114 changes: 100 additions & 14 deletions internal/fwserver/server_upgraderesourcestate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -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) {
Expand All @@ -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<unknown>): 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,
},
},
},
Expand Down

0 comments on commit efe6a2f

Please sign in to comment.