diff --git a/command/format/diff.go b/command/format/diff.go index e15d1590a..ad6886811 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -1373,10 +1373,15 @@ func ctyCollectionValues(val cty.Value) []cty.Value { return nil } + // Sets with marks mark the whole set as marked, + // so when we unmark it, apply those marks to all members + // of the set + val, marks := val.Unmark() + ret := make([]cty.Value, 0, val.LengthInt()) for it := val.ElementIterator(); it.Next(); { _, value := it.Element() - ret = append(ret, value) + ret = append(ret, value.WithMarks(marks)) } return ret } diff --git a/command/format/diff_test.go b/command/format/diff_test.go index 541770200..61159b266 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -3644,6 +3644,12 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { cty.StringVal("friends"), cty.StringVal("!"), }), + "nested_block": cty.ListVal([]cty.Value{ + cty.ObjectVal(map[string]cty.Value{ + "an_attr": cty.StringVal("secretval"), + "another": cty.StringVal("not secret"), + }), + }), }), AfterValMarks: []cty.PathValueMarks{ { @@ -3662,6 +3668,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { Path: cty.Path{cty.GetAttrStep{Name: "map_key"}, cty.IndexStep{Key: cty.StringVal("dinner")}}, Marks: cty.NewValueMarks("sensitive"), }, + { + // Nested blocks/sets will mark the whole set/block as sensitive + Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}}, + Marks: cty.NewValueMarks("sensitive"), + }, }, RequiredReplace: cty.NewPathSet(), Tainted: false, @@ -3673,6 +3684,17 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { "map_key": {Type: cty.Map(cty.Number), Optional: true}, "list_field": {Type: cty.List(cty.String), Optional: true}, }, + BlockTypes: map[string]*configschema.NestedBlock{ + "nested_block": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "an_attr": {Type: cty.String, Optional: true}, + "another": {Type: cty.String, Optional: true}, + }, + }, + Nesting: configschema.NestingList, + }, + }, }, ExpectedOutput: ` # test_instance.example will be created + resource "test_instance" "example" { @@ -3688,6 +3710,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) { + "dinner" = (sensitive) } + map_whole = (sensitive) + + + nested_block { + + an_attr = (sensitive) + + another = (sensitive) + } } `, }, diff --git a/states/instance_object.go b/states/instance_object.go index a9ebf4804..e92ffdc14 100644 --- a/states/instance_object.go +++ b/states/instance_object.go @@ -87,6 +87,11 @@ const ( // so the caller must not mutate the receiver any further once once this // method is called. func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*ResourceInstanceObjectSrc, error) { + // If it contains marks, remove these marks before traversing the + // structure with UnknownAsNull, and save the PathValueMarks + // so we can save them in state. + val, pvm := o.Value.UnmarkDeepWithPaths() + // Our state serialization can't represent unknown values, so we convert // them to nulls here. This is lossy, but nobody should be writing unknown // values here and expecting to get them out again later. @@ -96,15 +101,9 @@ func (o *ResourceInstanceObject) Encode(ty cty.Type, schemaVersion uint64) (*Res // for expression evaluation. The apply step should never produce unknown // values, but if it does it's the responsibility of the caller to detect // and raise an error about that. - val := cty.UnknownAsNull(o.Value) + val = cty.UnknownAsNull(val) - // If it contains marks, save these in state - unmarked := val - var pvm []cty.PathValueMarks - if val.ContainsMarked() { - unmarked, pvm = val.UnmarkDeepWithPaths() - } - src, err := ctyjson.Marshal(unmarked, ty) + src, err := ctyjson.Marshal(val, ty) if err != nil { return nil, err } diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 49be3be1c..5818e1ea8 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -279,11 +279,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { panic(fmt.Sprintf("PlanResourceChange of %s produced nil value", absAddr.String())) } - // Add the marks back to the planned new value - if len(unmarkedPaths) > 0 { - plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths) - } - // We allow the planned new value to disagree with configuration _values_ // here, since that allows the provider to do special logic like a // DiffSuppressFunc, but we still require that the provider produces @@ -302,7 +297,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.Err() } - if errs := objchange.AssertPlanValid(schema, priorVal, configValIgnored, plannedNewVal); len(errs) > 0 { + if errs := objchange.AssertPlanValid(schema, unmarkedPriorVal, unmarkedConfigVal, plannedNewVal); len(errs) > 0 { if resp.LegacyTypeSystem { // The shimming of the old type system in the legacy SDK is not precise // enough to pass this consistency check, so we'll give it a pass here, @@ -333,6 +328,13 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } } + // Add the marks back to the planned new value -- this must happen after ignore changes + // have been processed + unmarkedPlannedNewVal := plannedNewVal + if len(unmarkedPaths) > 0 { + plannedNewVal = plannedNewVal.MarkWithPaths(unmarkedPaths) + } + // The provider produces a list of paths to attributes whose changes mean // that we must replace rather than update an existing remote object. // However, we only need to do that if the identified attributes _have_ @@ -395,7 +397,6 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { // Unmark for this test for equality. If only sensitivity has changed, // this does not require an Update or Replace - unmarkedPlannedNewVal, _ := plannedNewVal.UnmarkDeep() eqV := unmarkedPlannedNewVal.Equals(unmarkedPriorVal) eq := eqV.IsKnown() && eqV.True()