From 3dde9efc75cba762df8594606b9efb9beba6368e Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Thu, 24 Sep 2020 16:29:30 -0400 Subject: [PATCH 1/2] Support list diffs with sensitivity Adds support for specialized diffs with lists --- command/format/diff_test.go | 151 +++++++++++++++++++++++++++++------- plans/objchange/lcs.go | 7 +- plans/objchange/lcs_test.go | 17 ++++ 3 files changed, 147 insertions(+), 28 deletions(-) diff --git a/command/format/diff_test.go b/command/format/diff_test.go index 1d0a2fedb..26111a9d8 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -3631,28 +3631,44 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-123"), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), + cty.StringVal("!"), + }), }), AfterValMarks: []cty.PathValueMarks{ { Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, Marks: cty.NewValueMarks("sensitive"), - }}, + }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(1)}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, RequiredReplace: cty.NewPathSet(), Tainted: false, Schema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "id": {Type: cty.String, Optional: true, Computed: true}, - "ami": {Type: cty.String, Optional: true}, + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + "list_field": {Type: cty.List(cty.String), Optional: true}, }, }, ExpectedOutput: ` # test_instance.example will be created + resource "test_instance" "example" { - + ami = (sensitive) - + id = "i-02ae66f368e8518a9" + + ami = (sensitive) + + id = "i-02ae66f368e8518a9" + + list_field = [ + + "hello", + + (sensitive), + + "!", + ] } `, }, - "in-place update - before sensitive, primitive types": { + "in-place update - before sensitive": { Action: plans.Update, Mode: addrs.ManagedResourceMode, Before: cty.ObjectVal(map[string]cty.Value{ @@ -3660,12 +3676,22 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "ami": cty.StringVal("ami-BEFORE"), "special": cty.BoolVal(true), "some_number": cty.NumberIntVal(1), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), + cty.StringVal("!"), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-AFTER"), "special": cty.BoolVal(false), "some_number": cty.NumberIntVal(2), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), + cty.StringVal("."), + }), }), BeforeValMarks: []cty.PathValueMarks{ { @@ -3680,6 +3706,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Path: cty.Path{cty.GetAttrStep{Name: "some_number"}}, Marks: cty.NewValueMarks("sensitive"), }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(2)}}, + Marks: cty.NewValueMarks("sensitive"), + }, }, RequiredReplace: cty.NewPathSet(), Tainted: false, @@ -3687,6 +3717,7 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Attributes: map[string]*configschema.Attribute{ "id": {Type: cty.String, Optional: true, Computed: true}, "ami": {Type: cty.String, Optional: true}, + "list_field": {Type: cty.List(cty.String), Optional: true}, "special": {Type: cty.Bool, Optional: true}, "some_number": {Type: cty.Number, Optional: true}, }, @@ -3697,6 +3728,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { # after applying this change ~ ami = (sensitive) id = "i-02ae66f368e8518a9" + ~ list_field = [ + # (1 unchanged element hidden) + "friends", + - (sensitive), + + ".", + ] # Warning: this attribute value will no longer be marked as sensitive # after applying this change ~ some_number = (sensitive) @@ -3706,41 +3743,63 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { } `, }, - "in-place update - after sensitive, map type": { + "in-place update - after sensitive": { Action: plans.Update, Mode: addrs.ManagedResourceMode, Before: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "tags": cty.MapVal(map[string]cty.Value{ - "name": cty.StringVal("anna"), + "name": cty.StringVal("anna a"), + "address": cty.StringVal("123 Main St"), + }), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "tags": cty.MapVal(map[string]cty.Value{ - "name": cty.StringVal("bob"), + "name": cty.StringVal("anna b"), + "address": cty.StringVal("123 Main Ave"), + }), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("goodbye"), + cty.StringVal("friends"), }), }), AfterValMarks: []cty.PathValueMarks{ { - Path: cty.Path{cty.GetAttrStep{Name: "tags"}, cty.IndexStep{Key: cty.StringVal("name")}}, + Path: cty.Path{cty.GetAttrStep{Name: "tags"}, cty.IndexStep{Key: cty.StringVal("address")}}, Marks: cty.NewValueMarks("sensitive"), - }}, + }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(0)}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, RequiredReplace: cty.NewPathSet(), Tainted: false, Schema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "id": {Type: cty.String, Optional: true, Computed: true}, - "tags": {Type: cty.Map(cty.String), Optional: true}, + "id": {Type: cty.String, Optional: true, Computed: true}, + "tags": {Type: cty.Map(cty.String), Optional: true}, + "list_field": {Type: cty.List(cty.String), Optional: true}, }, }, ExpectedOutput: ` # test_instance.example will be updated in-place ~ resource "test_instance" "example" { - id = "i-02ae66f368e8518a9" - ~ tags = { + id = "i-02ae66f368e8518a9" + ~ list_field = [ + - "hello", + + (sensitive), + "friends", + ] + ~ tags = { # Warning: this attribute value will be marked as sensitive and will # not display in UI output after applying this change - ~ "name" = (sensitive) + ~ "address" = (sensitive) + ~ "name" = "anna a" -> "anna b" } } `, @@ -3751,33 +3810,57 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Before: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-BEFORE"), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), + }), }), After: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-AFTER"), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("goodbye"), + cty.StringVal("friends"), + }), }), BeforeValMarks: []cty.PathValueMarks{ { Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, Marks: cty.NewValueMarks("sensitive"), - }}, + }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(0)}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, AfterValMarks: []cty.PathValueMarks{ { Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, Marks: cty.NewValueMarks("sensitive"), - }}, + }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(0)}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, RequiredReplace: cty.NewPathSet(), Tainted: false, Schema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "id": {Type: cty.String, Optional: true, Computed: true}, - "ami": {Type: cty.String, Optional: true}, + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + "list_field": {Type: cty.List(cty.String), Optional: true}, }, }, ExpectedOutput: ` # test_instance.example will be updated in-place ~ resource "test_instance" "example" { - ~ ami = (sensitive) - id = "i-02ae66f368e8518a9" + ~ ami = (sensitive) + id = "i-02ae66f368e8518a9" + ~ list_field = [ + - (sensitive), + + (sensitive), + "friends", + ] } `, }, @@ -3787,25 +3870,39 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Before: cty.ObjectVal(map[string]cty.Value{ "id": cty.StringVal("i-02ae66f368e8518a9"), "ami": cty.StringVal("ami-BEFORE"), + "list_field": cty.ListVal([]cty.Value{ + cty.StringVal("hello"), + cty.StringVal("friends"), + }), }), After: cty.NullVal(cty.EmptyObject), BeforeValMarks: []cty.PathValueMarks{ { Path: cty.Path{cty.GetAttrStep{Name: "ami"}}, Marks: cty.NewValueMarks("sensitive"), - }}, + }, + { + Path: cty.Path{cty.GetAttrStep{Name: "list_field"}, cty.IndexStep{Key: cty.NumberIntVal(1)}}, + Marks: cty.NewValueMarks("sensitive"), + }, + }, RequiredReplace: cty.NewPathSet(), Tainted: false, Schema: &configschema.Block{ Attributes: map[string]*configschema.Attribute{ - "id": {Type: cty.String, Optional: true, Computed: true}, - "ami": {Type: cty.String, Optional: true}, + "id": {Type: cty.String, Optional: true, Computed: true}, + "ami": {Type: cty.String, Optional: true}, + "list_field": {Type: cty.List(cty.String), Optional: true}, }, }, ExpectedOutput: ` # test_instance.example will be destroyed - resource "test_instance" "example" { - - ami = (sensitive) -> null - - id = "i-02ae66f368e8518a9" -> null + - ami = (sensitive) -> null + - id = "i-02ae66f368e8518a9" -> null + - list_field = [ + - "hello", + - (sensitive), + ] -> null } `, }, diff --git a/plans/objchange/lcs.go b/plans/objchange/lcs.go index cbfefdddd..78ab64919 100644 --- a/plans/objchange/lcs.go +++ b/plans/objchange/lcs.go @@ -26,7 +26,12 @@ func LongestCommonSubsequence(xs, ys []cty.Value) []cty.Value { for y := 0; y < len(ys); y++ { for x := 0; x < len(xs); x++ { - eqV := xs[x].Equals(ys[y]) + unmarkedX, xMarks := xs[x].Unmark() + unmarkedY, yMarks := ys[y].Unmark() + eqV := unmarkedX.Equals(unmarkedY) + if len(xMarks) != len(yMarks) { + eqV = cty.False + } eq := false if eqV.IsKnown() && eqV.True() { eq = true diff --git a/plans/objchange/lcs_test.go b/plans/objchange/lcs_test.go index 9b57f4e9d..5b3e732ea 100644 --- a/plans/objchange/lcs_test.go +++ b/plans/objchange/lcs_test.go @@ -70,6 +70,23 @@ func TestLongestCommonSubsequence(t *testing.T) { []cty.Value{cty.UnknownVal(cty.Number)}, []cty.Value{}, }, + + // marked values + { + []cty.Value{cty.NumberIntVal(1).Mark("foo"), cty.NumberIntVal(2).Mark("foo"), cty.NumberIntVal(3)}, + []cty.Value{cty.NumberIntVal(1).Mark("foo"), cty.NumberIntVal(2).Mark("foo")}, + []cty.Value{cty.NumberIntVal(1).Mark("foo"), cty.NumberIntVal(2).Mark("foo")}, + }, + { + []cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2).Mark("foo"), cty.NumberIntVal(3)}, + []cty.Value{cty.NumberIntVal(2), cty.NumberIntVal(3)}, + []cty.Value{cty.NumberIntVal(3)}, + }, + { + []cty.Value{cty.NumberIntVal(1), cty.NumberIntVal(2).Mark("foo")}, + []cty.Value{cty.NumberIntVal(2)}, + []cty.Value{}, + }, } for _, test := range tests { From 0b3c21a3eb8e38972c4c8f6af3084ef80290e574 Mon Sep 17 00:00:00 2001 From: Pam Selle <204372+pselle@users.noreply.github.com> Date: Fri, 25 Sep 2020 13:33:44 -0400 Subject: [PATCH 2/2] Support lists of deeply marked values --- plans/objchange/lcs.go | 4 ++-- plans/objchange/lcs_test.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/plans/objchange/lcs.go b/plans/objchange/lcs.go index 78ab64919..0ab06f5bf 100644 --- a/plans/objchange/lcs.go +++ b/plans/objchange/lcs.go @@ -26,8 +26,8 @@ func LongestCommonSubsequence(xs, ys []cty.Value) []cty.Value { for y := 0; y < len(ys); y++ { for x := 0; x < len(xs); x++ { - unmarkedX, xMarks := xs[x].Unmark() - unmarkedY, yMarks := ys[y].Unmark() + unmarkedX, xMarks := xs[x].UnmarkDeep() + unmarkedY, yMarks := ys[y].UnmarkDeep() eqV := unmarkedX.Equals(unmarkedY) if len(xMarks) != len(yMarks) { eqV = cty.False diff --git a/plans/objchange/lcs_test.go b/plans/objchange/lcs_test.go index 5b3e732ea..e455adb14 100644 --- a/plans/objchange/lcs_test.go +++ b/plans/objchange/lcs_test.go @@ -87,6 +87,21 @@ func TestLongestCommonSubsequence(t *testing.T) { []cty.Value{cty.NumberIntVal(2)}, []cty.Value{}, }, + { + []cty.Value{ + cty.MapVal(map[string]cty.Value{"a": cty.StringVal("x").Mark("sensitive")}), + cty.MapVal(map[string]cty.Value{"b": cty.StringVal("y")}), + }, + []cty.Value{ + cty.MapVal(map[string]cty.Value{"a": cty.StringVal("x").Mark("sensitive")}), + cty.MapVal(map[string]cty.Value{"b": cty.StringVal("y")}), + cty.MapVal(map[string]cty.Value{"c": cty.StringVal("z")}), + }, + []cty.Value{ + cty.MapVal(map[string]cty.Value{"a": cty.StringVal("x").Mark("sensitive")}), + cty.MapVal(map[string]cty.Value{"b": cty.StringVal("y")}), + }, + }, } for _, test := range tests {