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

@@ -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,