From b7ff04f1b69a17c201821b8e0a691df417bd2462 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 10 May 2019 17:56:23 -0400 Subject: [PATCH] remove extra attributes from state during upgrade Terraform core would previously ignore unexpected attributes found in the state, but since we now need to encode/decode the state according the schema, the attributes must match the schema. On any state upgrade, remove attributes no longer present in the schema from the state. The only change this requires from providers is that going forward removal of attribute is considered a schema change, and requires an increment of the SchemaVersion in order to trigger the removal of the attributes from state. --- helper/plugin/grpc_provider.go | 50 ++++++++++ helper/plugin/grpc_provider_test.go | 139 ++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go index f7914bc0c..c8065cac3 100644 --- a/helper/plugin/grpc_provider.go +++ b/helper/plugin/grpc_provider.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "strconv" "github.com/zclconf/go-cty/cty" @@ -293,6 +294,9 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto. return resp, nil } + // The provider isn't required to clean out removed fields + s.removeAttributes(jsonMap, blockForShimming.ImpliedType()) + // now we need to turn the state into the default json representation, so // that it can be re-decoded using the actual schema. val, err := schema.JSONMapToStateValue(jsonMap, blockForShimming) @@ -404,6 +408,52 @@ func (s *GRPCProviderServer) upgradeJSONState(version int, m map[string]interfac return m, nil } +// Remove any attributes no longer present in the schema, so that the json can +// be correctly decoded. +func (s *GRPCProviderServer) removeAttributes(v interface{}, ty cty.Type) { + // we're only concerned with finding maps that corespond to object + // attributes + switch v := v.(type) { + case []interface{}: + // If these aren't blocks the next call will be a noop + if ty.IsListType() || ty.IsSetType() { + eTy := ty.ElementType() + for _, eV := range v { + s.removeAttributes(eV, eTy) + } + } + return + case map[string]interface{}: + // map blocks aren't yet supported, but handle this just in case + if ty.IsMapType() { + eTy := ty.ElementType() + for _, eV := range v { + s.removeAttributes(eV, eTy) + } + return + } + + if !ty.IsObjectType() { + // This shouldn't happen, and will fail to decode further on, so + // there's no need to handle it here. + log.Printf("[WARN] unexpected type %#v for map in json state", ty) + return + } + + attrTypes := ty.AttributeTypes() + for attr, attrV := range v { + attrTy, ok := attrTypes[attr] + if !ok { + log.Printf("[DEBUG] attribute %q no longer present in schema", attr) + delete(v, attr) + continue + } + + s.removeAttributes(attrV, attrTy) + } + } +} + func (s *GRPCProviderServer) Stop(_ context.Context, _ *proto.Stop_Request) (*proto.Stop_Response, error) { resp := &proto.Stop_Response{} diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go index e70827e15..4a61f24fb 100644 --- a/helper/plugin/grpc_provider_test.go +++ b/helper/plugin/grpc_provider_test.go @@ -116,6 +116,145 @@ func TestUpgradeState_jsonState(t *testing.T) { } } +func TestUpgradeState_removedAttr(t *testing.T) { + r1 := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "two": { + Type: schema.TypeString, + Optional: true, + }, + }, + } + + r2 := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "multi": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "set": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "required": { + Type: schema.TypeString, + Required: true, + }, + }, + }, + }, + }, + }, + }, + }, + } + + r3 := &schema.Resource{ + Schema: map[string]*schema.Schema{ + "config_mode_attr": { + Type: schema.TypeList, + ConfigMode: schema.SchemaConfigModeAttr, + SkipCoreTypeCheck: true, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "foo": { + Type: schema.TypeString, + Optional: true, + }, + }, + }, + }, + }, + } + + p := &schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "r1": r1, + "r2": r2, + "r3": r3, + }, + } + + server := &GRPCProviderServer{ + provider: p, + } + + for _, tc := range []struct { + name string + raw string + expected cty.Value + }{ + { + name: "r1", + raw: `{"id":"bar","removed":"removed","two":"2"}`, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "two": cty.StringVal("2"), + }), + }, + { + name: "r2", + raw: `{"id":"bar","multi":[{"set":[{"required":"ok","removed":"removed"}]}]}`, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "multi": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "set": cty.SetVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "required": cty.StringVal("ok"), + }), + }), + }), + }), + }), + }, + { + name: "r3", + raw: `{"id":"bar","config_mode_attr":[{"foo":"ok","removed":"removed"}]}`, + expected: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("bar"), + "config_mode_attr": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "foo": cty.StringVal("ok"), + }), + }), + }), + }, + } { + t.Run(tc.name, func(t *testing.T) { + req := &proto.UpgradeResourceState_Request{ + TypeName: tc.name, + Version: 0, + RawState: &proto.RawState{ + Json: []byte(tc.raw), + }, + } + resp, err := server.UpgradeResourceState(nil, req) + if err != nil { + t.Fatal(err) + } + + if len(resp.Diagnostics) > 0 { + for _, d := range resp.Diagnostics { + t.Errorf("%#v", d) + } + t.Fatal("error") + } + val, err := msgpack.Unmarshal(resp.UpgradedState.Msgpack, p.ResourcesMap[tc.name].CoreConfigSchema().ImpliedType()) + if err != nil { + t.Fatal(err) + } + if !tc.expected.RawEquals(val) { + t.Fatalf("\nexpected: %#v\ngot: %#v\n", tc.expected, val) + } + }) + } + +} + func TestUpgradeState_flatmapState(t *testing.T) { r := &schema.Resource{ SchemaVersion: 4,