Merge pull request #21273 from hashicorp/jbardin/auto-remote-attributes

remove extra attributes from state during upgrade

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{}
 

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,