resilien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Merge pull request #27318 from hashicorp/jbardin/path-marks 

Correctly compare unordered sets of marks
This commit is contained in:
James Bardin 2020-12-17 13:49:50 -05:00 committed by GitHub
parent 428d404d92 1309b36b83
commit f5187aa869
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 202 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
terraform/context_plan_test.go
View File

@ -6668,3 +6668,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)
}
}
}

39
terraform/marks.go Normal file
View File

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

104
terraform/marks_test.go Normal file
View File

@ -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)
}
})
}
}

5
terraform/node_resource_abstract_instance.go
View File

@ -3,7 +3,6 @@ package terraform
import (
"fmt"
"log"
"reflect"
"strings"
multierror "github.com/hashicorp/go-multierror"
@ -920,7 +919,7 @@ func (n *NodeAbstractResourceInstance) plan(
// 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(unmarkedPaths, priorPaths) {
action = plans.Update
}
@ -1863,7 +1862,7 @@ func (n *NodeAbstractResourceInstance) apply(
// 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,