Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using UnmarshalWithOpts in order to ignore undefined attributes #426

Merged
merged 4 commits into from Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .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
```
2 changes: 1 addition & 1 deletion go.mod
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
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
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
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