diff --git a/command/format/diff.go b/command/format/diff.go index abf42c6f9..ed1b44403 100644 --- a/command/format/diff.go +++ b/command/format/diff.go @@ -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}) diff --git a/command/format/diff_test.go b/command/format/diff_test.go index db2d0b7de..8c32f4593 100644 --- a/command/format/diff_test.go +++ b/command/format/diff_test.go @@ -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, + }, + }, +} diff --git a/configs/configschema/implied_type.go b/configs/configschema/implied_type.go index 1c8cbcd87..4d4a042c3 100644 --- a/configs/configschema/implied_type.go +++ b/configs/configschema/implied_type.go @@ -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") } diff --git a/configs/configschema/implied_type_test.go b/configs/configschema/implied_type_test.go index 70ad9fa56..7cd0f7309 100644 --- a/configs/configschema/implied_type_test.go +++ b/configs/configschema/implied_type_test.go @@ -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": {}, },