From 8330b7295b51d417a984c3e7aa3f195ce5d3adb0 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 11 Jan 2023 09:04:26 +0100 Subject: [PATCH] Structured plan renderer: Add support for map blocks and sensitive blocks. (#32491) * change -> diff, value -> change * also update readme# * pause * Update internal/command/jsonformat/computed/diff.go Co-authored-by: Alisdair McDiarmid * add interface assertions for diff renderers * Add support for different kinds of blocks, and for sensitive blocks Co-authored-by: Alisdair McDiarmid --- .../jsonformat/computed/renderers/block.go | 48 +++- .../jsonformat/computed/renderers/blocks.go | 75 +++++ .../computed/renderers/renderer_test.go | 267 ++++++++++++++---- .../computed/renderers/sensitive_block.go | 51 ++++ .../jsonformat/computed/renderers/testing.go | 60 +++- internal/command/jsonformat/differ/block.go | 76 +++-- .../command/jsonformat/differ/change_test.go | 126 ++++----- internal/command/jsonformat/differ/map.go | 6 +- .../command/jsonformat/differ/sensitive.go | 12 +- 9 files changed, 538 insertions(+), 183 deletions(-) create mode 100644 internal/command/jsonformat/computed/renderers/blocks.go create mode 100644 internal/command/jsonformat/computed/renderers/sensitive_block.go diff --git a/internal/command/jsonformat/computed/renderers/block.go b/internal/command/jsonformat/computed/renderers/block.go index 993e0a884d..c6b6c6608e 100644 --- a/internal/command/jsonformat/computed/renderers/block.go +++ b/internal/command/jsonformat/computed/renderers/block.go @@ -28,7 +28,7 @@ func importantAttribute(attr string) bool { return false } -func Block(attributes map[string]computed.Diff, blocks map[string][]computed.Diff) computed.DiffRenderer { +func Block(attributes map[string]computed.Diff, blocks Blocks) computed.DiffRenderer { return &blockRenderer{ attributes: attributes, blocks: blocks, @@ -39,7 +39,7 @@ type blockRenderer struct { NoWarningsRenderer attributes map[string]computed.Diff - blocks map[string][]computed.Diff + blocks Blocks } func (renderer blockRenderer) RenderHuman(diff computed.Diff, indent int, opts computed.RenderHumanOpts) string { @@ -87,31 +87,51 @@ func (renderer blockRenderer) RenderHuman(diff computed.Diff, indent int, opts c buf.WriteString(fmt.Sprintf("%s%s %s\n", formatIndent(indent+1), format.DiffActionSymbol(plans.NoOp), unchanged("attribute", unchangedAttributes))) } - var blockKeys []string - for key := range renderer.blocks { - blockKeys = append(blockKeys, key) - } - sort.Strings(blockKeys) - + blockKeys := renderer.blocks.GetAllKeys() for _, key := range blockKeys { - blocks := renderer.blocks[key] foundChangedBlock := false - for _, block := range blocks { - if block.Action == plans.NoOp && !opts.ShowUnchangedChildren { + renderBlock := func(diff computed.Diff, mapKey string) { + if diff.Action == plans.NoOp && !opts.ShowUnchangedChildren { unchangedBlocks++ - continue + return } if !foundChangedBlock && len(renderer.attributes) > 0 { + // We always want to put an extra new line between the + // attributes and blocks, and between groups of blocks. buf.WriteString("\n") foundChangedBlock = true } - for _, warning := range block.WarningsHuman(indent + 1) { + for _, warning := range diff.WarningsHuman(indent + 1) { buf.WriteString(fmt.Sprintf("%s%s\n", formatIndent(indent+1), warning)) } - buf.WriteString(fmt.Sprintf("%s%s %s %s\n", formatIndent(indent+1), format.DiffActionSymbol(block.Action), ensureValidAttributeName(key), block.RenderHuman(indent+1, opts))) + buf.WriteString(fmt.Sprintf("%s%s %s%s %s\n", formatIndent(indent+1), format.DiffActionSymbol(diff.Action), ensureValidAttributeName(key), mapKey, diff.RenderHuman(indent+1, opts))) + + } + + switch { + case renderer.blocks.IsSingleBlock(key): + renderBlock(renderer.blocks.SingleBlocks[key], "") + case renderer.blocks.IsMapBlock(key): + var keys []string + for key := range renderer.blocks.MapBlocks[key] { + keys = append(keys, key) + } + sort.Strings(keys) + + for _, innerKey := range keys { + renderBlock(renderer.blocks.MapBlocks[key][innerKey], fmt.Sprintf(" %q", innerKey)) + } + case renderer.blocks.IsSetBlock(key): + for _, block := range renderer.blocks.SetBlocks[key] { + renderBlock(block, "") + } + case renderer.blocks.IsListBlock(key): + for _, block := range renderer.blocks.ListBlocks[key] { + renderBlock(block, "") + } } } diff --git a/internal/command/jsonformat/computed/renderers/blocks.go b/internal/command/jsonformat/computed/renderers/blocks.go new file mode 100644 index 0000000000..b9992b79dc --- /dev/null +++ b/internal/command/jsonformat/computed/renderers/blocks.go @@ -0,0 +1,75 @@ +package renderers + +import ( + "sort" + + "github.com/hashicorp/terraform/internal/command/jsonformat/computed" +) + +// Blocks is a helper struct for collating the different kinds of blocks in a +// simple way for rendering. +type Blocks struct { + SingleBlocks map[string]computed.Diff + ListBlocks map[string][]computed.Diff + SetBlocks map[string][]computed.Diff + MapBlocks map[string]map[string]computed.Diff +} + +func (blocks *Blocks) GetAllKeys() []string { + var keys []string + for key := range blocks.SingleBlocks { + keys = append(keys, key) + } + for key := range blocks.ListBlocks { + keys = append(keys, key) + } + for key := range blocks.SetBlocks { + keys = append(keys, key) + } + for key := range blocks.MapBlocks { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func (blocks *Blocks) IsSingleBlock(key string) bool { + _, ok := blocks.SingleBlocks[key] + return ok +} + +func (blocks *Blocks) IsListBlock(key string) bool { + _, ok := blocks.ListBlocks[key] + return ok +} + +func (blocks *Blocks) IsMapBlock(key string) bool { + _, ok := blocks.MapBlocks[key] + return ok +} + +func (blocks *Blocks) IsSetBlock(key string) bool { + _, ok := blocks.SetBlocks[key] + return ok +} + +func (blocks *Blocks) AddSingleBlock(key string, diff computed.Diff) { + blocks.SingleBlocks[key] = diff +} + +func (blocks *Blocks) AddListBlock(key string, diff computed.Diff) { + blocks.ListBlocks[key] = append(blocks.ListBlocks[key], diff) +} + +func (blocks *Blocks) AddSetBlock(key string, diff computed.Diff) { + blocks.SetBlocks[key] = append(blocks.SetBlocks[key], diff) +} + +func (blocks *Blocks) AddMapBlock(key string, entry string, diff computed.Diff) { + m := blocks.MapBlocks[key] + if m == nil { + m = make(map[string]computed.Diff) + } + m[entry] = diff + blocks.MapBlocks[key] = m +} diff --git a/internal/command/jsonformat/computed/renderers/renderer_test.go b/internal/command/jsonformat/computed/renderers/renderer_test.go index 2884d18056..16aefaa99b 100644 --- a/internal/command/jsonformat/computed/renderers/renderer_test.go +++ b/internal/command/jsonformat/computed/renderers/renderer_test.go @@ -1236,7 +1236,7 @@ func TestRenderers_Human(t *testing.T) { }, "create_empty_block": { diff: computed.Diff{ - Renderer: Block(nil, nil), + Renderer: Block(nil, Blocks{}), Action: plans.Create, }, expected: ` @@ -1254,26 +1254,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(nil, true, cty.Bool), Action: plans.Create, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "one", cty.String), Action: plans.Create, }, - }, nil), + }, Blocks{}), Action: plans.Create, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "two", cty.String), Action: plans.Create, }, - }, nil), + }, Blocks{}), Action: plans.Create, }, }, @@ -1305,26 +1303,26 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(nil, true, cty.Bool), Action: plans.Create, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { + Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "one", cty.String), Action: plans.Create, }, - }, nil), + }, Blocks{}), Action: plans.Create, }, - }, - "nested_block_two": { - { + "nested_block_two": { + Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "two", cty.String), Action: plans.Create, }, - }, nil), + }, Blocks{}), Action: plans.Create, }, }, @@ -1356,26 +1354,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(false, true, cty.Bool), Action: plans.Update, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "one", cty.String), Action: plans.NoOp, }, - }, nil), + }, Blocks{}), Action: plans.NoOp, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive(nil, "two", cty.String), Action: plans.Create, }, - }, nil), + }, Blocks{}), Action: plans.Create, }, }, @@ -1404,26 +1400,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(true, nil, cty.Bool), Action: plans.Delete, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("one", nil, cty.String), Action: plans.Delete, }, - }, nil), + }, Blocks{}), Action: plans.Delete, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("two", nil, cty.String), Action: plans.Delete, }, - }, nil), + }, Blocks{}), Action: plans.Delete, }, }, @@ -1455,26 +1449,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(true, nil, cty.Bool), Action: plans.Delete, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("one", nil, cty.String), Action: plans.Delete, }, - }, nil), + }, Blocks{}), Action: plans.Delete, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("two", nil, cty.String), Action: plans.Delete, }, - }, nil), + }, Blocks{}), Action: plans.Delete, }, }, @@ -1494,10 +1486,173 @@ func TestRenderers_Human(t *testing.T) { - string = "two" -> null } }`, + }, + "list_block_update": { + diff: computed.Diff{ + Renderer: Block( + nil, + Blocks{ + ListBlocks: map[string][]computed.Diff{ + "list_blocks": { + { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, 2.0, cty.Number), + Action: plans.Update, + }, + "string": { + Renderer: Primitive(nil, "new", cty.String), + Action: plans.Create, + }, + }, Blocks{}), + Action: plans.Update, + }, + { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, nil, cty.Number), + Action: plans.Delete, + }, + "string": { + Renderer: Primitive("old", "new", cty.String), + Action: plans.Update, + }, + }, Blocks{}), + Action: plans.Update, + }, + }, + }, + }), + }, + expected: ` +{ + ~ list_blocks { + ~ number = 1 -> 2 + + string = "new" + } + ~ list_blocks { + - number = 1 -> null + ~ string = "old" -> "new" + } + }`, + }, + "set_block_update": { + diff: computed.Diff{ + Renderer: Block( + nil, + Blocks{ + SetBlocks: map[string][]computed.Diff{ + "set_blocks": { + { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, 2.0, cty.Number), + Action: plans.Update, + }, + "string": { + Renderer: Primitive(nil, "new", cty.String), + Action: plans.Create, + }, + }, Blocks{}), + Action: plans.Update, + }, + { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, nil, cty.Number), + Action: plans.Delete, + }, + "string": { + Renderer: Primitive("old", "new", cty.String), + Action: plans.Update, + }, + }, Blocks{}), + Action: plans.Update, + }, + }, + }, + }), + }, + expected: ` +{ + ~ set_blocks { + ~ number = 1 -> 2 + + string = "new" + } + ~ set_blocks { + - number = 1 -> null + ~ string = "old" -> "new" + } + }`, + }, + "map_block_update": { + diff: computed.Diff{ + Renderer: Block( + nil, + Blocks{ + MapBlocks: map[string]map[string]computed.Diff{ + "list_blocks": { + "key_one": { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, 2.0, cty.Number), + Action: plans.Update, + }, + "string": { + Renderer: Primitive(nil, "new", cty.String), + Action: plans.Create, + }, + }, Blocks{}), + Action: plans.Update, + }, + "key:two": { + Renderer: Block(map[string]computed.Diff{ + "number": { + Renderer: Primitive(1.0, nil, cty.Number), + Action: plans.Delete, + }, + "string": { + Renderer: Primitive("old", "new", cty.String), + Action: plans.Update, + }, + }, Blocks{}), + Action: plans.Update, + }, + }, + }, + }), + }, + expected: ` +{ + ~ list_blocks "key:two" { + - number = 1 -> null + ~ string = "old" -> "new" + } + ~ list_blocks "key_one" { + ~ number = 1 -> 2 + + string = "new" + } + } +`, + }, + "sensitive_block": { + diff: computed.Diff{ + Renderer: SensitiveBlock(computed.Diff{ + Renderer: Block(nil, Blocks{}), + Action: plans.NoOp, + }, true, true), + Action: plans.Update, + }, + expected: ` +{ + # At least one attribute in this block is (or was) sensitive, + # so its contents will not be displayed. + } +`, }, "delete_empty_block": { diff: computed.Diff{ - Renderer: Block(nil, nil), + Renderer: Block(nil, Blocks{}), Action: plans.Delete, }, expected: ` @@ -1519,26 +1674,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(3.0, 4.0, cty.Number), Action: plans.Update, }, - }, map[string][]computed.Diff{ - "nested_block:one": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block:one": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("one", "four", cty.String), Action: plans.Update, }, - }, nil), + }, Blocks{}), Action: plans.Update, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("two", "three", cty.String), Action: plans.Update, }, - }, nil), + }, Blocks{}), Action: plans.Update, }, }, @@ -1571,26 +1724,24 @@ func TestRenderers_Human(t *testing.T) { Renderer: Primitive(false, false, cty.Bool), Action: plans.NoOp, }, - }, map[string][]computed.Diff{ - "nested_block": { - { + }, Blocks{ + SingleBlocks: map[string]computed.Diff{ + "nested_block": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("one", "one", cty.String), Action: plans.NoOp, }, - }, nil), + }, Blocks{}), Action: plans.NoOp, }, - }, - "nested_block_two": { - { + "nested_block_two": { Renderer: Block(map[string]computed.Diff{ "string": { Renderer: Primitive("two", "two", cty.String), Action: plans.NoOp, }, - }, nil), + }, Blocks{}), Action: plans.NoOp, }, }, diff --git a/internal/command/jsonformat/computed/renderers/sensitive_block.go b/internal/command/jsonformat/computed/renderers/sensitive_block.go new file mode 100644 index 0000000000..3edad888ac --- /dev/null +++ b/internal/command/jsonformat/computed/renderers/sensitive_block.go @@ -0,0 +1,51 @@ +package renderers + +import ( + "fmt" + + "github.com/hashicorp/terraform/internal/command/format" + "github.com/hashicorp/terraform/internal/command/jsonformat/computed" + "github.com/hashicorp/terraform/internal/plans" +) + +func SensitiveBlock(diff computed.Diff, beforeSensitive, afterSensitive bool) computed.DiffRenderer { + return &sensitiveBlockRenderer{ + inner: diff, + beforeSensitive: beforeSensitive, + afterSensitive: afterSensitive, + } +} + +type sensitiveBlockRenderer struct { + inner computed.Diff + + afterSensitive bool + beforeSensitive bool +} + +func (renderer sensitiveBlockRenderer) RenderHuman(diff computed.Diff, indent int, opts computed.RenderHumanOpts) string { + cachedLinePrefix := fmt.Sprintf("%s%s ", formatIndent(indent), format.DiffActionSymbol(plans.NoOp)) + return fmt.Sprintf("{%s\n%s # At least one attribute in this block is (or was) sensitive,\n%s # so its contents will not be displayed.\n%s}", + forcesReplacement(diff.Replace), cachedLinePrefix, cachedLinePrefix, cachedLinePrefix) +} + +func (renderer sensitiveBlockRenderer) WarningsHuman(diff computed.Diff, indent int) []string { + if (renderer.beforeSensitive == renderer.afterSensitive) || renderer.inner.Action == plans.Create || renderer.inner.Action == plans.Delete { + // Only display warnings for sensitive values if they are changing from + // being sensitive or to being sensitive and if they are not being + // destroyed or created. + return []string{} + } + + var warning string + if renderer.beforeSensitive { + warning = fmt.Sprintf(" # [yellow]Warning[reset]: this block will no longer be marked as sensitive\n%s # after applying this change.", formatIndent(indent)) + } else { + warning = fmt.Sprintf(" # [yellow]Warning[reset]: this block will be marked as sensitive and will not\n%s # display in UI output after applying this change.", formatIndent(indent)) + } + + if renderer.inner.Action == plans.NoOp { + return []string{fmt.Sprintf("%s The value is unchanged.", warning)} + } + return []string{warning} +} diff --git a/internal/command/jsonformat/computed/renderers/testing.go b/internal/command/jsonformat/computed/renderers/testing.go index 60aaf16218..4adbcc2ee7 100644 --- a/internal/command/jsonformat/computed/renderers/testing.go +++ b/internal/command/jsonformat/computed/renderers/testing.go @@ -180,7 +180,14 @@ func validateSliceType(t *testing.T, actual []computed.Diff, expected []Validate } } -func ValidateBlock(attributes map[string]ValidateDiffFunction, blocks map[string][]ValidateDiffFunction, action plans.Action, replace bool) ValidateDiffFunction { +func ValidateBlock( + attributes map[string]ValidateDiffFunction, + singleBlocks map[string]ValidateDiffFunction, + listBlocks map[string][]ValidateDiffFunction, + mapBlocks map[string]map[string]ValidateDiffFunction, + setBlocks map[string][]ValidateDiffFunction, + action plans.Action, + replace bool) ValidateDiffFunction { return func(t *testing.T, diff computed.Diff) { validateDiff(t, diff, action, replace) @@ -191,7 +198,10 @@ func ValidateBlock(attributes map[string]ValidateDiffFunction, blocks map[string } validateKeys(t, block.attributes, attributes) - validateKeys(t, block.blocks, blocks) + validateKeys(t, block.blocks.SingleBlocks, singleBlocks) + validateKeys(t, block.blocks.ListBlocks, listBlocks) + validateKeys(t, block.blocks.MapBlocks, mapBlocks) + validateKeys(t, block.blocks.SetBlocks, setBlocks) for key, expected := range attributes { if actual, ok := block.attributes[key]; ok { @@ -199,14 +209,50 @@ func ValidateBlock(attributes map[string]ValidateDiffFunction, blocks map[string } } - for key, expected := range blocks { - if actual, ok := block.blocks[key]; ok { + for key, expected := range singleBlocks { + expected(t, block.blocks.SingleBlocks[key]) + } + + for key, expected := range listBlocks { + if actual, ok := block.blocks.ListBlocks[key]; ok { if len(actual) != len(expected) { t.Errorf("expected %d blocks within %s but found %d elements", len(expected), key, len(actual)) + } + for ix := range expected { + expected[ix](t, actual[ix]) + } + } + } - for ix := range expected { - expected[ix](t, actual[ix]) - } + for key, expected := range setBlocks { + if actual, ok := block.blocks.SetBlocks[key]; ok { + if len(actual) != len(expected) { + t.Errorf("expected %d blocks within %s but found %d elements", len(expected), key, len(actual)) + } + for ix := range expected { + expected[ix](t, actual[ix]) + } + } + } + + for key, expected := range setBlocks { + if actual, ok := block.blocks.SetBlocks[key]; ok { + if len(actual) != len(expected) { + t.Errorf("expected %d blocks within %s but found %d elements", len(expected), key, len(actual)) + } + for ix := range expected { + expected[ix](t, actual[ix]) + } + } + } + + for key, expected := range mapBlocks { + if actual, ok := block.blocks.MapBlocks[key]; ok { + if len(actual) != len(expected) { + t.Errorf("expected %d blocks within %s but found %d elements", len(expected), key, len(actual)) + } + for dKey := range expected { + expected[dKey](t, actual[dKey]) } } } diff --git a/internal/command/jsonformat/differ/block.go b/internal/command/jsonformat/differ/block.go index 593315b432..585947f356 100644 --- a/internal/command/jsonformat/differ/block.go +++ b/internal/command/jsonformat/differ/block.go @@ -12,8 +12,8 @@ func (change Change) ComputeDiffForBlock(block *jsonprovider.Block) computed.Dif return sensitive } - if computed, ok := change.checkForUnknownBlock(block); ok { - return computed + if unknown, ok := change.checkForUnknownBlock(block); ok { + return unknown } current := change.getDefaultActionForIteration() @@ -33,33 +33,59 @@ func (change Change) ComputeDiffForBlock(block *jsonprovider.Block) computed.Dif current = compareActions(current, childChange.Action) } - blocks := make(map[string][]computed.Diff) + blocks := renderers.Blocks{ + SingleBlocks: make(map[string]computed.Diff), + ListBlocks: make(map[string][]computed.Diff), + SetBlocks: make(map[string][]computed.Diff), + MapBlocks: make(map[string]map[string]computed.Diff), + } + for key, blockType := range block.BlockTypes { childValue := blockValue.getChild(key) - childChanges, next := childValue.computeDiffsForBlockType(blockType) - if next == plans.NoOp && childValue.Before == nil && childValue.After == nil { - // Don't record nil values at all in blocks. - continue + + switch NestingMode(blockType.NestingMode) { + case nestingModeSet: + diffs, action := childValue.computeBlockDiffsAsSet(blockType.Block) + if action == plans.NoOp && childValue.Before == childValue.After { + // Don't record nil values in blocks. + continue + } + for _, diff := range diffs { + blocks.AddSetBlock(key, diff) + } + current = compareActions(current, action) + case nestingModeList: + diffs, action := childValue.computeBlockDiffsAsList(blockType.Block) + if action == plans.NoOp && childValue.Before == childValue.After { + // Don't record nil values in blocks. + continue + } + for _, diff := range diffs { + blocks.AddListBlock(key, diff) + } + current = compareActions(current, action) + case nestingModeMap: + diffs, action := childValue.computeBlockDiffsAsMap(blockType.Block) + if action == plans.NoOp && childValue.Before == childValue.After { + // Don't record nil values in blocks. + continue + } + for dKey, diff := range diffs { + blocks.AddMapBlock(key, dKey, diff) + } + current = compareActions(current, action) + case nestingModeSingle, nestingModeGroup: + diff := childValue.ComputeDiffForBlock(blockType.Block) + if diff.Action == plans.NoOp && childValue.Before == childValue.After { + // Don't record nil values in blocks. + continue + } + blocks.AddSingleBlock(key, diff) + current = compareActions(current, diff.Action) + default: + panic("unrecognized nesting mode: " + blockType.NestingMode) } - blocks[key] = childChanges - current = compareActions(current, next) } return computed.NewDiff(renderers.Block(attributes, blocks), current, change.replacePath()) } - -func (change Change) computeDiffsForBlockType(blockType *jsonprovider.BlockType) ([]computed.Diff, plans.Action) { - switch NestingMode(blockType.NestingMode) { - case nestingModeSet: - return change.computeBlockDiffsAsSet(blockType.Block) - case nestingModeList: - return change.computeBlockDiffsAsList(blockType.Block) - case nestingModeMap: - return change.computeBlockDiffsAsMap(blockType.Block) - case nestingModeSingle, nestingModeGroup: - diff := change.ComputeDiffForBlock(blockType.Block) - return []computed.Diff{diff}, diff.Action - default: - panic("unrecognized nesting mode: " + blockType.NestingMode) - } -} diff --git a/internal/command/jsonformat/differ/change_test.go b/internal/command/jsonformat/differ/change_test.go index d3ecc26fde..9038d919c0 100644 --- a/internal/command/jsonformat/differ/change_test.go +++ b/internal/command/jsonformat/differ/change_test.go @@ -912,12 +912,12 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Update, false), + }, nil, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ - renderers.ValidateBlock(nil, nil, plans.Delete, false), + renderers.ValidateBlock(nil, nil, nil, nil, nil, plans.Delete, false), renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), }, }, "update_attribute": { @@ -936,14 +936,14 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive("old", "new", plans.Update, false), - }, nil, plans.Update, false), + }, nil, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Delete, false), + }, nil, nil, nil, nil, plans.Delete, false), renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), }, }, "delete_attribute": { @@ -960,12 +960,12 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Update, false), + }, nil, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Delete, false), - renderers.ValidateBlock(nil, nil, plans.Create, false), + }, nil, nil, nil, nil, plans.Delete, false), + renderers.ValidateBlock(nil, nil, nil, nil, nil, plans.Create, false), }, }, "create_block": { @@ -989,22 +989,18 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, }, }, - validate: renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Create, false), - }, - }, plans.Update, false), + validate: renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ - renderers.ValidateBlock(nil, nil, plans.Delete, false), - renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Create, false), - }, - }, plans.Create, false), + renderers.ValidateBlock(nil, nil, nil, nil, nil, plans.Delete, false), + renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, nil, nil, nil, plans.Create, false), }, }, "update_block": { @@ -1032,28 +1028,22 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, }, }, - validate: renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive("old", "new", plans.Update, false), - }, nil, plans.Update, false), - }, - }, plans.Update, false), + validate: renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive("old", "new", plans.Update, false), + }, nil, nil, nil, nil, plans.Update, false), + }, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ - renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Delete, false), - }, - }, plans.Delete, false), - renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), - }, nil, plans.Create, false), - }, - }, plans.Create, false), + renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), + }, nil, nil, nil, nil, plans.Delete, false), + }, nil, nil, nil, plans.Delete, false), + renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive(nil, "new", plans.Create, false), + }, nil, nil, nil, nil, plans.Create, false), + }, nil, nil, nil, plans.Create, false), }, }, "delete_block": { @@ -1077,22 +1067,18 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, }, }, - validate: renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Delete, false), - }, - }, plans.Update, false), + validate: renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), + }, nil, nil, nil, nil, plans.Delete, false), + }, nil, nil, nil, plans.Update, false), validateSet: []renderers.ValidateDiffFunction{ - renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_one": { - renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ - "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), - }, nil, plans.Delete, false), - }, - }, plans.Delete, false), - renderers.ValidateBlock(nil, nil, plans.Create, false), + renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_one": renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{ + "attribute_one": renderers.ValidatePrimitive("old", nil, plans.Delete, false), + }, nil, nil, nil, nil, plans.Delete, false), + }, nil, nil, nil, plans.Delete, false), + renderers.ValidateBlock(nil, nil, nil, nil, nil, plans.Create, false), }, }, } @@ -1119,11 +1105,9 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, } - validate := renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ - "block_type": { - tc.validate, - }, - }, plans.Update, false) + validate := renderers.ValidateBlock(nil, map[string]renderers.ValidateDiffFunction{ + "block_type": tc.validate, + }, nil, nil, nil, plans.Update, false) validate(t, input.ComputeDiffForBlock(block)) }) t.Run("map", func(t *testing.T) { @@ -1149,11 +1133,11 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, } - validate := renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ + validate := renderers.ValidateBlock(nil, nil, nil, map[string]map[string]renderers.ValidateDiffFunction{ "block_type": { - tc.validate, + "one": tc.validate, }, - }, plans.Update, false) + }, nil, plans.Update, false) validate(t, input.ComputeDiffForBlock(block)) }) t.Run("list", func(t *testing.T) { @@ -1179,11 +1163,11 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, } - validate := renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ + validate := renderers.ValidateBlock(nil, nil, map[string][]renderers.ValidateDiffFunction{ "block_type": { tc.validate, }, - }, plans.Update, false) + }, nil, nil, plans.Update, false) validate(t, input.ComputeDiffForBlock(block)) }) t.Run("set", func(t *testing.T) { @@ -1209,7 +1193,7 @@ func TestValue_BlockAttributesAndNestedBlocks(t *testing.T) { }, } - validate := renderers.ValidateBlock(nil, map[string][]renderers.ValidateDiffFunction{ + validate := renderers.ValidateBlock(nil, nil, nil, nil, map[string][]renderers.ValidateDiffFunction{ "block_type": func() []renderers.ValidateDiffFunction { if tc.validateSet != nil { return tc.validateSet diff --git a/internal/command/jsonformat/differ/map.go b/internal/command/jsonformat/differ/map.go index 7d9412a4e8..da647dfe9d 100644 --- a/internal/command/jsonformat/differ/map.go +++ b/internal/command/jsonformat/differ/map.go @@ -36,12 +36,12 @@ func (change Change) computeAttributeDiffAsNestedMap(attributes map[string]*json return computed.NewDiff(renderers.Map(elements), current, change.replacePath()) } -func (change Change) computeBlockDiffsAsMap(block *jsonprovider.Block) ([]computed.Diff, plans.Action) { +func (change Change) computeBlockDiffsAsMap(block *jsonprovider.Block) (map[string]computed.Diff, plans.Action) { current := change.getDefaultActionForIteration() - var elements []computed.Diff + elements := make(map[string]computed.Diff) change.processMap(func(key string, value Change) { element := value.ComputeDiffForBlock(block) - elements = append(elements, element) + elements[key] = element current = compareActions(current, element.Action) }) return elements, current diff --git a/internal/command/jsonformat/differ/sensitive.go b/internal/command/jsonformat/differ/sensitive.go index 623a7c8cea..3834560854 100644 --- a/internal/command/jsonformat/differ/sensitive.go +++ b/internal/command/jsonformat/differ/sensitive.go @@ -10,25 +10,27 @@ import ( "github.com/hashicorp/terraform/internal/command/jsonprovider" ) +type CreateSensitiveRenderer func(computed.Diff, bool, bool) computed.DiffRenderer + func (change Change) checkForSensitiveType(ctype cty.Type) (computed.Diff, bool) { - return change.checkForSensitive(func(value Change) computed.Diff { + return change.checkForSensitive(renderers.Sensitive, func(value Change) computed.Diff { return value.computeDiffForType(ctype) }) } func (change Change) checkForSensitiveNestedAttribute(attribute *jsonprovider.NestedType) (computed.Diff, bool) { - return change.checkForSensitive(func(value Change) computed.Diff { + return change.checkForSensitive(renderers.Sensitive, func(value Change) computed.Diff { return value.computeDiffForNestedAttribute(attribute) }) } func (change Change) checkForSensitiveBlock(block *jsonprovider.Block) (computed.Diff, bool) { - return change.checkForSensitive(func(value Change) computed.Diff { + return change.checkForSensitive(renderers.SensitiveBlock, func(value Change) computed.Diff { return value.ComputeDiffForBlock(block) }) } -func (change Change) checkForSensitive(computedDiff func(value Change) computed.Diff) (computed.Diff, bool) { +func (change Change) checkForSensitive(create CreateSensitiveRenderer, computedDiff func(value Change) computed.Diff) (computed.Diff, bool) { beforeSensitive := change.isBeforeSensitive() afterSensitive := change.isAfterSensitive() @@ -56,7 +58,7 @@ func (change Change) checkForSensitive(computedDiff func(value Change) computed. inner := computedDiff(value) - return computed.NewDiff(renderers.Sensitive(inner, beforeSensitive, afterSensitive), inner.Action, change.replacePath()), true + return computed.NewDiff(create(inner, beforeSensitive, afterSensitive), inner.Action, change.replacePath()), true } func (change Change) isBeforeSensitive() bool {