Mildwonkey/nested type format (#27793)

* command/format: check for sensitive NestedTypes

Eventually, the diff formatter will need to be updated to properly
handle NestedTypes, but for now we can let the existing function deal
with them as regular cty.Object-type attributes.

To avoid printing sensitive nested attributes, we will treat any
attribute with at least one sensitive nested attribute as an entirely
sensitive attribute.

* bugfix for Object ImpliedType()

ImpliedType() was returning too early when the given object had optional
attributes, therefore skipping the incredibly important step of
accounting for the nesting mode when returning said type.
This commit is contained in:
Kristin Laemmert 2021-02-18 08:48:52 -05:00 committed by GitHub
parent 1d0414a63a
commit 2b4e389839
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 261 additions and 163 deletions

View File

@ -328,6 +328,19 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
return true
}
// TODO: There will need to be an object-specific diff printer that handles
// things like individual attribute sensitivity and pathing into
// requiredReplace, but for now we will let writeAttrDiff handle attributes
// with NestedTypes like regular (object) attributes.
//
// To avoid printing any sensitive nested fields inside attributes (until
// the above is implemented) we will treat the entire attribute as
// sensitive.
var sensitive bool
if attrS.NestedType != nil && attrS.NestedType.ContainsSensitive() {
sensitive = true
}
p.buf.WriteString("\n")
p.writeSensitivityWarning(old, new, indent, action, false)
@ -341,7 +354,7 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
p.buf.WriteString(strings.Repeat(" ", nameLen-len(name)))
p.buf.WriteString(" = ")
if attrS.Sensitive {
if attrS.Sensitive || sensitive {
p.buf.WriteString("(sensitive value)")
} else {
switch {
@ -361,6 +374,21 @@ func (p *blockBodyDiffPrinter) writeAttrDiff(name string, attrS *configschema.At
return false
}
// TODO: writeNestedAttrDiff will be responsible for properly formatting
// Attributes with NestedTypes in the diff. This function will be called from
// writeAttrDiff when it recieves attribute with a NestedType. Right now, we are
// letting the existing formatter "just" print these attributes like regular,
// object-type attributes. Unlike the regular attribute printer, this function
// will need to descend into the NestedType to ensure that we are properly
// handling items such as:
// - nested sensitive fields
// - which nested field specifically requires replacement
//
// Examples of both can be seen in diff_test.go with FIXME comments.
func (p *blockBodyDiffPrinter) writeNestedAttrDiff(name string, attrS *configschema.Attribute, old, new cty.Value, nameLen, indent int, path cty.Path) bool {
panic("not implemented")
}
func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *configschema.NestedBlock, old, new cty.Value, blankBefore bool, indent int, path cty.Path) int {
skippedBlocks := 0
path = append(path, cty.GetAttrStep{Name: name})

View File

@ -317,19 +317,37 @@ new line
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.UnknownVal(cty.String),
"password": cty.StringVal("top-secret"),
"conn_info": cty.ObjectVal(map[string]cty.Value{
"user": cty.StringVal("not-secret"),
"password": cty.StringVal("top-secret"),
}),
}),
Schema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Computed: true},
"password": {Type: cty.String, Optional: true, Sensitive: true},
// FIXME: This is a temporary situation; once the NestedType
// specific printer is implemented this will need to be
// updated so that only the sensitive nested attribute is
// hidden.
"conn_info": {
NestedType: &configschema.Object{
Nesting: configschema.NestingSingle,
Attributes: map[string]*configschema.Attribute{
"user": {Type: cty.String, Optional: true},
"password": {Type: cty.String, Optional: true, Sensitive: true},
},
},
},
},
},
RequiredReplace: cty.NewPathSet(),
Tainted: false,
ExpectedOutput: ` # test_instance.example will be created
+ resource "test_instance" "example" {
+ id = (known after apply)
+ password = (sensitive value)
+ conn_info = (sensitive value)
+ id = (known after apply)
+ password = (sensitive value)
}
`,
},
@ -2209,6 +2227,12 @@ func TestResourceChange_nestedList(t *testing.T) {
"volume_type": cty.StringVal("gp2"),
}),
}),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
@ -2218,33 +2242,21 @@ func TestResourceChange_nestedList(t *testing.T) {
"volume_type": cty.StringVal("gp2"),
}),
}),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
}),
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},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
# (1 unchanged attribute hidden)
# (1 unchanged block hidden)
}
@ -2259,10 +2271,18 @@ func TestResourceChange_nestedList(t *testing.T) {
"root_block_device": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"volume_type": cty.String,
})),
"disks": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
})),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.NullVal(cty.String),
"size": cty.NullVal(cty.String),
})}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.NullVal(cty.String),
@ -2271,30 +2291,17 @@ func TestResourceChange_nestedList(t *testing.T) {
}),
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},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [] -> [
+ {
+ mount_point = null
+ size = null
},
]
id = "i-02ae66f368e8518a9"
+ root_block_device {}
}
@ -2309,10 +2316,20 @@ func TestResourceChange_nestedList(t *testing.T) {
"root_block_device": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"volume_type": cty.String,
})),
"disks": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
})),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.NullVal(cty.String),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
@ -2321,30 +2338,17 @@ func TestResourceChange_nestedList(t *testing.T) {
}),
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},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [] -> [
+ {
+ mount_point = "/var/diska"
+ size = null
},
]
id = "i-02ae66f368e8518a9"
+ root_block_device {
+ volume_type = "gp2"
@ -2358,6 +2362,12 @@ func TestResourceChange_nestedList(t *testing.T) {
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.NullVal(cty.String),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
@ -2368,6 +2378,12 @@ func TestResourceChange_nestedList(t *testing.T) {
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
@ -2381,6 +2397,15 @@ func TestResourceChange_nestedList(t *testing.T) {
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"ami": {Type: cty.String, Optional: true},
"disks": {
NestedType: &configschema.Object{
Attributes: map[string]*configschema.Attribute{
"mount_point": {Type: cty.String, Optional: true},
"size": {Type: cty.String, Optional: true},
},
Nesting: configschema.NestingList,
},
},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
@ -2404,8 +2429,14 @@ func TestResourceChange_nestedList(t *testing.T) {
},
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
~ size = null -> "50GB"
# (1 unchanged element hidden)
},
]
id = "i-02ae66f368e8518a9"
~ root_block_device {
+ new_field = "new_value"
@ -2414,12 +2445,18 @@ func TestResourceChange_nestedList(t *testing.T) {
}
`,
},
"force-new update (inside block)": {
"force-new update (inside blocks)": {
Action: plans.DeleteThenCreate,
Mode: addrs.ManagedResourceMode,
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
@ -2429,42 +2466,47 @@ func TestResourceChange_nestedList(t *testing.T) {
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("different"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(cty.Path{
cty.GetAttrStep{Name: "root_block_device"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
cty.GetAttrStep{Name: "volume_type"},
}),
RequiredReplace: cty.NewPathSet(
cty.Path{
cty.GetAttrStep{Name: "root_block_device"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
cty.GetAttrStep{Name: "volume_type"},
},
// FIXME: This is not currently used; when the diff printer is
// updated to fully handle NestedTypes this test should fail,
// and the expected output should look like this:
//
// ~ mount_point = "/var/diska" -> "/var/diskb" # forces replacement
cty.Path{
cty.GetAttrStep{Name: "disks"},
cty.IndexStep{Key: cty.NumberIntVal(0)},
cty.GetAttrStep{Name: "mount_point"},
},
),
Tainted: false,
Schema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"ami": {Type: cty.String, Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example must be replaced
-/+ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
~ {
~ mount_point = "/var/diska" -> "/var/diskb"
# (1 unchanged element hidden)
},
]
id = "i-02ae66f368e8518a9"
~ root_block_device {
~ volume_type = "gp2" -> "different" # forces replacement
@ -2478,6 +2520,12 @@ func TestResourceChange_nestedList(t *testing.T) {
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
@ -2487,40 +2535,34 @@ func TestResourceChange_nestedList(t *testing.T) {
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diskb"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("different"),
}),
}),
}),
RequiredReplace: cty.NewPathSet(cty.Path{
cty.GetAttrStep{Name: "root_block_device"},
}),
RequiredReplace: cty.NewPathSet(
cty.Path{cty.GetAttrStep{Name: "root_block_device"}},
cty.Path{cty.GetAttrStep{Name: "disks"}},
),
Tainted: false,
Schema: &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"ami": {Type: cty.String, Optional: true},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example must be replaced
-/+ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [ # forces replacement
~ {
~ mount_point = "/var/diska" -> "/var/diskb"
# (1 unchanged element hidden)
} # forces replacement,
]
id = "i-02ae66f368e8518a9"
~ root_block_device { # forces replacement
~ volume_type = "gp2" -> "different"
@ -2534,55 +2576,44 @@ func TestResourceChange_nestedList(t *testing.T) {
Before: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-BEFORE"),
"disks": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"mount_point": cty.StringVal("/var/diska"),
"size": cty.StringVal("50GB"),
}),
}),
"root_block_device": cty.ListVal([]cty.Value{
cty.ObjectVal(map[string]cty.Value{
"volume_type": cty.StringVal("gp2"),
"new_field": cty.StringVal("new_value"),
}),
}),
}),
After: cty.ObjectVal(map[string]cty.Value{
"id": cty.StringVal("i-02ae66f368e8518a9"),
"ami": cty.StringVal("ami-AFTER"),
"disks": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"mount_point": cty.String,
"size": cty.String,
})),
"root_block_device": cty.ListValEmpty(cty.Object(map[string]cty.Type{
"volume_type": cty.String,
"new_field": cty.String,
})),
}),
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},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
"new_field": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
},
Schema: testSchemaNestingList,
ExpectedOutput: ` # test_instance.example will be updated in-place
~ resource "test_instance" "example" {
~ ami = "ami-BEFORE" -> "ami-AFTER"
id = "i-02ae66f368e8518a9"
~ ami = "ami-BEFORE" -> "ami-AFTER"
~ disks = [
- {
- mount_point = "/var/diska"
- size = "50GB"
},
] -> []
id = "i-02ae66f368e8518a9"
- root_block_device {
- new_field = "new_value" -> null
- volume_type = "gp2" -> null
}
}
@ -4311,3 +4342,34 @@ func outputChange(name string, before, after cty.Value, sensitive bool) *plans.O
return changeSrc
}
// A basic test schema using NestingList for one attribute and one block
var testSchemaNestingList = &configschema.Block{
Attributes: map[string]*configschema.Attribute{
"id": {Type: cty.String, Optional: true, Computed: true},
"ami": {Type: cty.String, Optional: true},
"disks": {
NestedType: &configschema.Object{
Attributes: map[string]*configschema.Attribute{
"mount_point": {Type: cty.String, Optional: true},
"size": {Type: cty.String, Optional: true},
},
Nesting: configschema.NestingList,
},
},
},
BlockTypes: map[string]*configschema.NestedBlock{
"root_block_device": {
Block: configschema.Block{
Attributes: map[string]*configschema.Attribute{
"volume_type": {
Type: cty.String,
Optional: true,
Computed: true,
},
},
},
Nesting: configschema.NestingList,
},
},
}

View File

@ -62,19 +62,22 @@ func (o *Object) ImpliedType() cty.Type {
}
}
optAttrs := listOptionalAttrsFromObject(o)
if len(optAttrs) > 0 {
return cty.ObjectWithOptionalAttrs(attrTys, optAttrs)
}
var ret cty.Type
if len(optAttrs) > 0 {
ret = cty.ObjectWithOptionalAttrs(attrTys, optAttrs)
} else {
ret = cty.Object(attrTys)
}
switch o.Nesting {
case NestingSingle:
return cty.Object(attrTys)
return ret
case NestingList:
return cty.List(cty.Object(attrTys))
return cty.List(ret)
case NestingMap:
return cty.Map(cty.Object(attrTys))
return cty.Map(ret)
case NestingSet:
return cty.Set(cty.Object(attrTys))
return cty.Set(ret)
default: // Should never happen
panic("invalid Nesting")
}

View File

@ -134,6 +134,7 @@ func TestObjectImpliedType(t *testing.T) {
},
"attributes": {
&Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"optional": {
Type: cty.String,
@ -165,9 +166,11 @@ func TestObjectImpliedType(t *testing.T) {
},
"nested attributes": {
&Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"nested_type": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"optional": {
Type: cty.String,
@ -204,10 +207,10 @@ func TestObjectImpliedType(t *testing.T) {
&Object{
Nesting: NestingList,
Attributes: map[string]*Attribute{
"foo": {Type: cty.String},
"foo": {Type: cty.String, Optional: true},
},
},
cty.List(cty.Object(map[string]cty.Type{"foo": cty.String})),
cty.List(cty.ObjectWithOptionalAttrs(map[string]cty.Type{"foo": cty.String}, []string{"foo"})),
},
"NestingMap": {
&Object{
@ -281,6 +284,7 @@ func TestObjectContainsSensitive(t *testing.T) {
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"sensitive": {Sensitive: true},
},
@ -295,6 +299,7 @@ func TestObjectContainsSensitive(t *testing.T) {
Attributes: map[string]*Attribute{
"nested": {
NestedType: &Object{
Nesting: NestingSingle,
Attributes: map[string]*Attribute{
"public": {},
},