Merge pull request #27321 from hashicorp/jbardin/path-marks-v0.14

v0.14 backport of #27318
hashicorp · Dec 17, 2020 · 56370ad · 56370ad
2 parents c8c0041 + d20119f
commit 56370ad
Show file tree

Hide file tree

Showing 5 changed files with 202 additions and 4 deletions.
diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go
@@ -6564,3 +6564,60 @@ resource "test_instance" "a" {
 		t.Fatal(diags.Err())
 	}
 }
+
+func TestContext2Plan_noSensitivityChange(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+variable "sensitive_var" {
+       default = "hello"
+       sensitive = true
+}
+
+resource "test_resource" "foo" {
+       value = var.sensitive_var
+       sensitive_value = var.sensitive_var
+}`,
+	})
+
+	p := testProvider("test")
+	p.ApplyResourceChangeFn = testApplyFn
+	p.PlanResourceChangeFn = testDiffFn
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: states.BuildState(func(s *states.SyncState) {
+			s.SetResourceInstanceCurrent(
+				addrs.Resource{
+					Mode: addrs.ManagedResourceMode,
+					Type: "test_resource",
+					Name: "foo",
+				}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+				&states.ResourceInstanceObjectSrc{
+					Status:    states.ObjectReady,
+					AttrsJSON: []byte(`{"id":"foo", "value":"hello", "sensitive_value":"hello", "network_interface":[]}`),
+					AttrSensitivePaths: []cty.PathValueMarks{
+						cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "value"}}, Marks: cty.NewValueMarks("sensitive")},
+						cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "sensitive_value"}}, Marks: cty.NewValueMarks("sensitive")},
+					},
+				},
+				addrs.AbsProviderConfig{
+					Provider: addrs.NewDefaultProvider("test"),
+					Module:   addrs.RootModule,
+				},
+			)
+		}),
+	})
+	plan, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+
+	for _, c := range plan.Changes.Resources {
+		if c.Action != plans.NoOp {
+			t.Fatalf("expected no changes, got %s for %q", c.Action, c.Addr)
+		}
+	}
+}
diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go
@@ -3,7 +3,6 @@ package terraform
 import (
 	"fmt"
 	"log"
-	"reflect"
 	"strings"
 
 	multierror "github.com/hashicorp/go-multierror"
@@ -121,7 +120,7 @@ func (n *EvalApply) Eval(ctx EvalContext) (interface{}, error) {
 	// persisted.
 	eqV := unmarkedBefore.Equals(unmarkedAfter)
 	eq := eqV.IsKnown() && eqV.True()
-	if change.Action == plans.Update && eq && !reflect.DeepEqual(beforePaths, afterPaths) {
+	if change.Action == plans.Update && eq && !marksEqual(beforePaths, afterPaths) {
 		// Copy the previous state, changing only the value
 		newState := &states.ResourceInstanceObject{
 			CreateBeforeDestroy: state.CreateBeforeDestroy,

diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go
@@ -3,7 +3,6 @@ package terraform
 import (
 	"fmt"
 	"log"
-	"reflect"
 	"strings"
 
 	"github.com/hashicorp/hcl/v2"
@@ -505,7 +504,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 
 	// If we plan to write or delete sensitive paths from state,
 	// this is an Update action
-	if action == plans.NoOp && !reflect.DeepEqual(priorPaths, unmarkedPaths) {
+	if action == plans.NoOp && !marksEqual(priorPaths, unmarkedPaths) {
 		action = plans.Update
 	}
 

diff --git a/terraform/marks.go b/terraform/marks.go
@@ -0,0 +1,39 @@
+package terraform
+
+import (
+	"fmt"
+	"sort"
+
+	"github.com/zclconf/go-cty/cty"
+)
+
+// marksEqual compares 2 unordered sets of PathValue marks for equality, with
+// the comparison using the cty.PathValueMarks.Equal method.
+func marksEqual(a, b []cty.PathValueMarks) bool {
+	if len(a) == 0 && len(b) == 0 {
+		return true
+	}
+
+	if len(a) != len(b) {
+		return false
+	}
+
+	less := func(s []cty.PathValueMarks) func(i, j int) bool {
+		return func(i, j int) bool {
+			// the sort only needs to be consistent, so use the GoString format
+			// to get a comparable value
+			return fmt.Sprintf("%#v", s[i]) < fmt.Sprintf("%#v", s[j])
+		}
+	}
+
+	sort.Slice(a, less(a))
+	sort.Slice(b, less(b))
+
+	for i := 0; i < len(a); i++ {
+		if !a[i].Equal(b[i]) {
+			return false
+		}
+	}
+
+	return true
+}
diff --git a/terraform/marks_test.go b/terraform/marks_test.go
@@ -0,0 +1,104 @@
+package terraform
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/zclconf/go-cty/cty"
+)
+
+func TestMarksEqual(t *testing.T) {
+	for i, tc := range []struct {
+		a, b  []cty.PathValueMarks
+		equal bool
+	}{
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			true,
+		},
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "A"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			false,
+		},
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "b"}}, Marks: cty.NewValueMarks("sensitive")},
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "c"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "b"}}, Marks: cty.NewValueMarks("sensitive")},
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "c"}}, Marks: cty.NewValueMarks("sensitive")},
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			true,
+		},
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{
+					Path:  cty.Path{cty.GetAttrStep{Name: "a"}, cty.GetAttrStep{Name: "b"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+				cty.PathValueMarks{
+					Path:  cty.Path{cty.GetAttrStep{Name: "a"}, cty.GetAttrStep{Name: "c"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+			},
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{
+					Path:  cty.Path{cty.GetAttrStep{Name: "a"}, cty.GetAttrStep{Name: "c"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+				cty.PathValueMarks{
+					Path:  cty.Path{cty.GetAttrStep{Name: "a"}, cty.GetAttrStep{Name: "b"}},
+					Marks: cty.NewValueMarks("sensitive"),
+				},
+			},
+			true,
+		},
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "b"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			false,
+		},
+		{
+			nil,
+			nil,
+			true,
+		},
+		{
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			nil,
+			false,
+		},
+		{
+			nil,
+			[]cty.PathValueMarks{
+				cty.PathValueMarks{Path: cty.Path{cty.GetAttrStep{Name: "a"}}, Marks: cty.NewValueMarks("sensitive")},
+			},
+			false,
+		},
+	} {
+		t.Run(fmt.Sprint(i), func(t *testing.T) {
+			if marksEqual(tc.a, tc.b) != tc.equal {
+				t.Fatalf("marksEqual(\n%#v,\n%#v,\n) != %t\n", tc.a, tc.b, tc.equal)
+			}
+		})
+	}
+}