Merge pull request #19726 from hashicorp/jbardin/destroy-plan

Don't fail decoding null values from plan changes

plans/changes_src.go

@@ -166,13 +166,21 @@ type ChangeSrc struct {
 // to call the corresponding Decode method of that struct rather than working
 // directly with its embedded Change.
 func (cs *ChangeSrc) Decode(ty cty.Type) (*Change, error) {
-	before, err := cs.Before.Decode(ty)
-	if err != nil {
-		return nil, fmt.Errorf("error decoding 'before' value: %s", err)
+	var err error
+	before := cty.NullVal(ty)
+	after := cty.NullVal(ty)
+
+	if len(cs.Before) > 0 {
+		before, err = cs.Before.Decode(ty)
+		if err != nil {
+			return nil, fmt.Errorf("error decoding 'before' value: %s", err)
+		}
 	}
-	after, err := cs.After.Decode(ty)
-	if err != nil {
-		return nil, fmt.Errorf("error decoding 'after' value: %s", err)
+	if len(cs.After) > 0 {
+		after, err = cs.After.Decode(ty)
+		if err != nil {
+			return nil, fmt.Errorf("error decoding 'after' value: %s", err)
+		}
 	}
 	return &Change{
 		Action: cs.Action,

plans/dynamic_value.go

@@ -86,7 +86,7 @@ func (v DynamicValue) ImpliedType() (cty.Type, error) {
 
 // Copy produces a copy of the receiver with a distinct backing array.
 func (v DynamicValue) Copy() DynamicValue {
-	if len(v) == 0 {
+	if v == nil {
 		return nil
 	}
 

plans/planfile/tfplan_test.go

@@ -26,7 +26,7 @@ func TestTFPlanRoundTrip(t *testing.T) {
 					Addr: addrs.OutputValue{Name: "bar"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.Create,
-						After:  mustNewDynamicValueStr("bar value"),
+						After:  mustDynamicOutputValue("bar value"),
 					},
 					Sensitive: false,
 				},
@@ -34,8 +34,8 @@ func TestTFPlanRoundTrip(t *testing.T) {
 					Addr: addrs.OutputValue{Name: "baz"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.NoOp,
-						Before: mustNewDynamicValueStr("baz value"),
-						After:  mustNewDynamicValueStr("baz value"),
+						Before: mustDynamicOutputValue("baz value"),
+						After:  mustDynamicOutputValue("baz value"),
 					},
 					Sensitive: false,
 				},
@@ -43,8 +43,8 @@ func TestTFPlanRoundTrip(t *testing.T) {
 					Addr: addrs.OutputValue{Name: "secret"}.Absolute(addrs.RootModuleInstance),
 					ChangeSrc: plans.ChangeSrc{
 						Action: plans.Update,
-						Before: mustNewDynamicValueStr("old secret value"),
-						After:  mustNewDynamicValueStr("new secret value"),
+						Before: mustDynamicOutputValue("old secret value"),
+						After:  mustDynamicOutputValue("new secret value"),
 					},
 					Sensitive: true,
 				},
@@ -143,6 +143,14 @@ func TestTFPlanRoundTrip(t *testing.T) {
 	}
 }
 
+func mustDynamicOutputValue(val string) plans.DynamicValue {
+	ret, err := plans.NewDynamicValue(cty.StringVal(val), cty.DynamicPseudoType)
+	if err != nil {
+		panic(err)
+	}
+	return ret
+}
+
 func mustNewDynamicValue(val cty.Value, ty cty.Type) plans.DynamicValue {
 	ret, err := plans.NewDynamicValue(val, ty)
 	if err != nil {
@@ -159,3 +167,96 @@ func mustNewDynamicValueStr(val string) plans.DynamicValue {
 	}
 	return ret
 }
+
+// TestTFPlanRoundTripDestroy ensures that encoding and decoding null values for
+// destroy doesn't leave us with any nil values.
+func TestTFPlanRoundTripDestroy(t *testing.T) {
+	objTy := cty.Object(map[string]cty.Type{
+		"id": cty.String,
+	})
+
+	plan := &plans.Plan{
+		Changes: &plans.Changes{
+			Outputs: []*plans.OutputChangeSrc{
+				{
+					Addr: addrs.OutputValue{Name: "bar"}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: plans.Delete,
+						Before: mustDynamicOutputValue("output"),
+						After:  mustNewDynamicValue(cty.NullVal(cty.String), cty.String),
+					},
+				},
+			},
+			Resources: []*plans.ResourceInstanceChangeSrc{
+				{
+					Addr: addrs.Resource{
+						Mode: addrs.ManagedResourceMode,
+						Type: "test_thing",
+						Name: "woot",
+					}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance),
+					ProviderAddr: addrs.ProviderConfig{
+						Type: "test",
+					}.Absolute(addrs.RootModuleInstance),
+					ChangeSrc: plans.ChangeSrc{
+						Action: plans.Delete,
+						Before: mustNewDynamicValue(cty.ObjectVal(map[string]cty.Value{
+							"id": cty.StringVal("foo-bar-baz"),
+						}), objTy),
+						After: mustNewDynamicValue(cty.NullVal(objTy), objTy),
+					},
+				},
+			},
+		},
+		TargetAddrs: []addrs.Targetable{
+			addrs.Resource{
+				Mode: addrs.ManagedResourceMode,
+				Type: "test_thing",
+				Name: "woot",
+			}.Absolute(addrs.RootModuleInstance),
+		},
+		Backend: plans.Backend{
+			Type: "local",
+			Config: mustNewDynamicValue(
+				cty.ObjectVal(map[string]cty.Value{
+					"foo": cty.StringVal("bar"),
+				}),
+				cty.Object(map[string]cty.Type{
+					"foo": cty.String,
+				}),
+			),
+			Workspace: "default",
+		},
+	}
+
+	var buf bytes.Buffer
+	err := writeTfplan(plan, &buf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	newPlan, err := readTfplan(&buf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, rics := range newPlan.Changes.Resources {
+		ric, err := rics.Decode(objTy)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if ric.After == cty.NilVal {
+			t.Fatalf("unexpected nil After value: %#v\n", ric)
+		}
+	}
+	for _, ocs := range newPlan.Changes.Outputs {
+		oc, err := ocs.Decode()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if oc.After == cty.NilVal {
+			t.Fatalf("unexpected nil After value: %#v\n", ocs)
+		}
+	}
+}