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

Merge pull request #26379 from hashicorp/pselle/sensitive-nested-block-support 

Support sensitivity within nested blocks
This commit is contained in:
Pam Selle 2020-10-02 17:28:15 -04:00 committed by GitHub
parent 593cf7b4d5 111aadd0f0
commit 69b03ebf42
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 305 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
command/format/diff.go
View File

@ -338,7 +338,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
p.buf.WriteString("\n")
p.writeSensitivityWarning(old, new, indent, action)
p.writeSensitivityWarning(old, new, indent, action, false)
p.buf.WriteString(strings.Repeat(" ", indent))
p.writeActionSymbol(action)
@ -377,6 +377,14 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config
return skippedBlocks
}
// If either the old or the new value is marked,
// Display a special diff because it is irrelevant
// to list all obfuscated attributes as (sensitive)
if old.IsMarked() || new.IsMarked() {
p.writeSensitiveNestedBlockDiff(name, old, new, indent, blankBefore)
return 0
}
// Where old/new are collections representing a nesting mode other than
// NestingSingle, we assume the collection value can never be unknown
// since we always produce the container for the nested objects, even if
@ -581,6 +589,46 @@ func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *config
return skippedBlocks
}
func (p *blockBodyDiffPrinter) writeSensitiveNestedBlockDiff(name string, old, new cty.Value, indent int, blankBefore bool) {
unmarkedOld, _ := old.Unmark()
unmarkedNew, _ := new.Unmark()
eqV := unmarkedNew.Equals(unmarkedOld)
var action plans.Action
switch {
case old.IsNull():
action = plans.Create
case new.IsNull():
action = plans.Delete
case !new.IsWhollyKnown() || !old.IsWhollyKnown():
// "old" should actually always be known due to our contract
// that old values must never be unknown, but we'll allow it
// anyway to be robust.
action = plans.Update
case !eqV.IsKnown() || !eqV.True():
action = plans.Update
}
if blankBefore {
p.buf.WriteRune('\n')
}
// New line before warning printing
p.buf.WriteRune('\n')
p.writeSensitivityWarning(old, new, indent, action, true)
p.buf.WriteString(strings.Repeat(" ", indent))
p.writeActionSymbol(action)
fmt.Fprintf(p.buf, "%s {", name)
p.buf.WriteRune('\n')
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("# At least one attribute in this block is (or was) sensitive,\n")
p.buf.WriteString(strings.Repeat(" ", indent+4))
p.buf.WriteString("# so its contents will not be displayed.")
p.buf.WriteRune('\n')
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.buf.WriteString("}")
return
}
func (p *blockBodyDiffPrinter) writeNestedBlockDiff(name string, label *string, blockS *configschema.Block, action plans.Action, old, new cty.Value, indent int, path cty.Path) bool {
if action == plans.NoOp && p.concise {
return true
@ -1129,7 +1177,7 @@ func (p *blockBodyDiffPrinter) writeValueDiff(old, new cty.Value, indent int, pa
oldV := old.Index(kV)
newV := new.Index(kV)
p.writeSensitivityWarning(oldV, newV, indent+2, action)
p.writeSensitivityWarning(oldV, newV, indent+2, action, false)
p.buf.WriteString(strings.Repeat(" ", indent+2))
p.writeActionSymbol(action)
@ -1307,15 +1355,21 @@ func (p *blockBodyDiffPrinter) writeActionSymbol(action plans.Action) {
}
}
func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, indent int, action plans.Action) {
func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, indent int, action plans.Action, isBlock bool) {
// Dont' show this warning for create or delete
if action == plans.Create || action == plans.Delete {
return
}
// Customize the warning based on if it is an attribute or block
diffType := "attribute value"
if isBlock {
diffType = "block"
}
if new.IsMarked() && !old.IsMarked() {
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# [yellow]Warning:[reset] this attribute value will be marked as sensitive and will\n"))
p.buf.WriteString(p.color.Color(fmt.Sprintf("# [yellow]Warning:[reset] this %s will be marked as sensitive and will\n", diffType)))
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# not display in UI output after applying this change\n"))
}
@ -1323,7 +1377,7 @@ func (p *blockBodyDiffPrinter) writeSensitivityWarning(old, new cty.Value, inden
// Note if changing this attribute will change its sensitivity
if old.IsMarked() && !new.IsMarked() {
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# [yellow]Warning:[reset] this attribute value will no longer be marked as sensitive\n"))
p.buf.WriteString(p.color.Color(fmt.Sprintf("# [yellow]Warning:[reset] this %s will no longer be marked as sensitive\n", diffType)))
p.buf.WriteString(strings.Repeat(" ", indent))
p.buf.WriteString(p.color.Color("# after applying this change\n"))
}

207
command/format/diff_test.go
View File

@ -3644,6 +3644,18 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
cty.StringVal("friends"),
cty.StringVal("!"),
}),
"nested_block_list": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
"another": cty.StringVal("not secret"),
}),
}),
"nested_block_set": 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 +3674,15 @@ 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_list"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
@ -3673,6 +3694,26 @@ 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_list": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
"another": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingList,
},
"nested_block_set": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
"another": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSet,
},
},
},
ExpectedOutput: ` # test_instance.example will be created
+ resource "test_instance" "example" {
@ -3688,6 +3729,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
+ "dinner" = (sensitive)
}
+ map_whole = (sensitive)
+ nested_block_list {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
+ nested_block_set {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`,
},
@ -3712,6 +3763,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secretval"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
@ -3731,6 +3792,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("cereal"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}),
}),
}),
BeforeValMarks: []cty.PathValueMarks{
{
@ -3757,6 +3828,14 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
@ -3770,6 +3849,24 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(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},
},
},
Nesting: configschema.NestingList,
},
"nested_block_set": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSet,
},
},
},
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
@ -3798,6 +3895,20 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# Warning: this attribute value will no longer be marked as sensitive
# after applying this change
~ special = (sensitive)
# Warning: this block will no longer be marked as sensitive
# after applying this change
~ nested_block {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
# Warning: this block will no longer be marked as sensitive
# after applying this change
~ nested_block_set {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`,
},
@ -3818,6 +3929,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block_single": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("original"),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
@ -3833,6 +3947,9 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("cereal"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block_single": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("changed"),
}),
}),
AfterValMarks: []cty.PathValueMarks{
{
@ -3851,6 +3968,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_single"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
@ -3861,6 +3982,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block_single": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSingle,
},
},
},
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
@ -3879,6 +4010,13 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# Warning: this attribute value will be marked as sensitive and will
# not display in UI output after applying this change
~ map_whole = (sensitive)
# Warning: this block will be marked as sensitive and will
# not display in UI output after applying this change
~ nested_block_single {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`,
},
@ -3900,6 +4038,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block_map": cty.MapVal(map[string]cty.Value{
"foo": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("original"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
@ -3916,6 +4059,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("cereal"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block_map": cty.MapVal(map[string]cty.Value{
"foo": cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.UnknownVal(cty.String),
}),
}),
}),
BeforeValMarks: []cty.PathValueMarks{
{
@ -3934,6 +4082,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_map"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
AfterValMarks: []cty.PathValueMarks{
{
@ -3952,6 +4104,10 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_map"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
@ -3963,6 +4119,16 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block_map": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingMap,
},
},
},
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
@ -3978,6 +4144,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
# (1 unchanged element hidden)
}
~ map_whole = (sensitive)
~ nested_block_map {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`,
},
@ -3999,6 +4170,18 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"breakfast": cty.StringVal("pizza"),
"dinner": cty.StringVal("pizza"),
}),
"nested_block": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secret"),
"another": cty.StringVal("not secret"),
}),
}),
"nested_block_set": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"an_attr": cty.StringVal("secret"),
"another": cty.StringVal("not secret"),
}),
}),
}),
After: cty.NullVal(cty.EmptyObject),
BeforeValMarks: []cty.PathValueMarks{
@ -4018,6 +4201,14 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
Path: cty.Path{cty.GetAttrStep{Name: "map_whole"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block"}},
Marks: cty.NewValueMarks("sensitive"),
},
{
Path: cty.Path{cty.GetAttrStep{Name: "nested_block_set"}},
Marks: cty.NewValueMarks("sensitive"),
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
@ -4029,6 +4220,17 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
"map_key": {Type: cty.Map(cty.Number), Optional: true},
"map_whole": {Type: cty.Map(cty.String), Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"nested_block_set": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"an_attr": {Type: cty.String, Optional: true},
"another": {Type: cty.String, Optional: true},
},
},
Nesting: configschema.NestingSet,
},
},
},
ExpectedOutput: ` # test_instance.example will be destroyed
- resource "test_instance" "example" {
@ -4043,6 +4245,11 @@ func TestResourceChange_sensitiveVariable(t *testing.T) {
- "dinner" = (sensitive)
} -> null
- map_whole = (sensitive) -> null
- nested_block_set {
# At least one attribute in this block is (or was) sensitive,
# so its contents will not be displayed.
}
}
`,
},

5
plans/objchange/compatible.go
View File

@ -73,8 +73,9 @@ func assertObjectCompatible(schema *configschema.Block, planned, actual cty.Valu
}
}
for name, blockS := range schema.BlockTypes {
plannedV := planned.GetAttr(name)
actualV := actual.GetAttr(name)
// Unmark values before testing compatibility
plannedV, _ := planned.GetAttr(name).UnmarkDeep()
actualV, _ := actual.GetAttr(name).UnmarkDeep()
// As a special case, if there were any blocks whose leaf attributes
// are all unknown then we assume (possibly incorrectly) that the

15
states/instance_object.go
View File

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

11
terraform/context_apply_test.go
View File

@ -11939,8 +11939,17 @@ variable "sensitive_var" {
sensitive = true
}
variable "sensitive_id" {
default = "secret id"
sensitive = true
}
resource "test_resource" "foo" {
value = var.sensitive_var
network_interface {
network_interface_id = var.sensitive_id
}
}`,
})
@ -11982,7 +11991,7 @@ resource "test_resource" "foo" {
t.Fatalf("apply errors: %s", diags.Err())
}
// Now change the variable value
// Now change the variable value for sensitive_var
ctx = testContext2(t, &ContextOpts{
Config: m,
Providers: map[addrs.Provider]providers.Factory{

11
terraform/context_test.go
View File

@ -636,6 +636,17 @@ func testProviderSchema(name string) *ProviderSchema {
Optional: true,
},
},
BlockTypes: map[string]*configschema.NestedBlock{
"network_interface": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"network_interface_id": {Type: cty.String, Optional: true},
"device_index": {Type: cty.Number, Optional: true},
},
},
Nesting: configschema.NestingSet,
},
},
},
name + "_ami_list": {
Attributes: map[string]*configschema.Attribute{

15
terraform/eval_diff.go
View File

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