Skip to content

Commit 3ea88aa

Browse files
The Terraform TeamLiam Cervante
The Terraform Team
and
Liam Cervante
authored
Backport of Fix rendering unknown values in map and null string primitives into v1.4 (#33033)
* backport of commit a654a76 * backport of commit f81afc3 * backport of commit e6d421a --------- Co-authored-by: Liam Cervante <liam.cervante@hashicorp.com>
1 parent 5387b1c commit 3ea88aa

File tree

5 files changed

+240
-5
lines changed

5 files changed

+240
-5
lines changed

‎internal/command/jsonformat/computed/renderers/primitive.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (renderer primitiveRenderer) renderStringDiff(diff computed.Diff, indent in
8888
}
8989

9090
if !str.IsMultiline {
91-
return fmt.Sprintf("%q%s", str.String, forcesReplacement(diff.Replace, opts))
91+
return fmt.Sprintf("%s%s", str.RenderSimple(), forcesReplacement(diff.Replace, opts))
9292
}
9393

9494
// We are creating a single multiline string, so let's split by the new
@@ -102,13 +102,18 @@ func (renderer primitiveRenderer) renderStringDiff(diff computed.Diff, indent in
102102
lines[0] = fmt.Sprintf("%s%s%s", formatIndent(indent+1), writeDiffActionSymbol(plans.NoOp, opts), lines[0])
103103
case plans.Delete:
104104
str := evaluatePrimitiveString(renderer.before, opts)
105+
if str.IsNull {
106+
// We don't put the null suffix (-> null) here because the final
107+
// render or null -> null would look silly.
108+
return fmt.Sprintf("%s%s", str.RenderSimple(), forcesReplacement(diff.Replace, opts))
109+
}
105110

106111
if str.Json != nil {
107112
return renderer.renderStringDiffAsJson(diff, indent, opts, str, evaluatedString{})
108113
}
109114

110115
if !str.IsMultiline {
111-
return fmt.Sprintf("%q%s%s", str.String, nullSuffix(diff.Action, opts), forcesReplacement(diff.Replace, opts))
116+
return fmt.Sprintf("%s%s%s", str.RenderSimple(), nullSuffix(diff.Action, opts), forcesReplacement(diff.Replace, opts))
112117
}
113118

114119
// We are creating a single multiline string, so let's split by the new
@@ -141,7 +146,7 @@ func (renderer primitiveRenderer) renderStringDiff(diff computed.Diff, indent in
141146
}
142147

143148
if !beforeString.IsMultiline && !afterString.IsMultiline {
144-
return fmt.Sprintf("%q %s %q%s", beforeString.String, opts.Colorize.Color("[yellow]->[reset]"), afterString.String, forcesReplacement(diff.Replace, opts))
149+
return fmt.Sprintf("%s %s %s%s", beforeString.RenderSimple(), opts.Colorize.Color("[yellow]->[reset]"), afterString.RenderSimple(), forcesReplacement(diff.Replace, opts))
145150
}
146151

147152
beforeLines := strings.Split(beforeString.String, "\n")

‎internal/command/jsonformat/computed/renderers/renderer_test.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,103 @@ func TestRenderers_Human(t *testing.T) {
2424
expected string
2525
opts computed.RenderHumanOpts
2626
}{
27+
// We're using the string "null" in these tests to demonstrate the
28+
// difference between rendering an actual string and rendering a null
29+
// value.
30+
"primitive_create_string": {
31+
diff: computed.Diff{
32+
Renderer: Primitive(nil, "null", cty.String),
33+
Action: plans.Create,
34+
},
35+
expected: "\"null\"",
36+
},
37+
"primitive_delete_string": {
38+
diff: computed.Diff{
39+
Renderer: Primitive("null", nil, cty.String),
40+
Action: plans.Delete,
41+
},
42+
expected: "\"null\" -> null",
43+
},
44+
"primitive_update_string_to_null": {
45+
diff: computed.Diff{
46+
Renderer: Primitive("null", nil, cty.String),
47+
Action: plans.Update,
48+
},
49+
expected: "\"null\" -> null",
50+
},
51+
"primitive_update_string_from_null": {
52+
diff: computed.Diff{
53+
Renderer: Primitive(nil, "null", cty.String),
54+
Action: plans.Update,
55+
},
56+
expected: "null -> \"null\"",
57+
},
58+
"primitive_update_multiline_string_to_null": {
59+
diff: computed.Diff{
60+
Renderer: Primitive("nu\nll", nil, cty.String),
61+
Action: plans.Update,
62+
},
63+
expected: `
64+
<<-EOT
65+
- nu
66+
- ll
67+
+ null
68+
EOT
69+
`,
70+
},
71+
"primitive_update_multiline_string_from_null": {
72+
diff: computed.Diff{
73+
Renderer: Primitive(nil, "nu\nll", cty.String),
74+
Action: plans.Update,
75+
},
76+
expected: `
77+
<<-EOT
78+
- null
79+
+ nu
80+
+ ll
81+
EOT
82+
`,
83+
},
84+
"primitive_update_json_string_to_null": {
85+
diff: computed.Diff{
86+
Renderer: Primitive("[null]", nil, cty.String),
87+
Action: plans.Update,
88+
},
89+
expected: `
90+
jsonencode(
91+
[
92+
- null,
93+
]
94+
) -> null
95+
`,
96+
},
97+
"primitive_update_json_string_from_null": {
98+
diff: computed.Diff{
99+
Renderer: Primitive(nil, "[null]", cty.String),
100+
Action: plans.Update,
101+
},
102+
expected: `
103+
null -> jsonencode(
104+
[
105+
+ null,
106+
]
107+
)
108+
`,
109+
},
110+
"primitive_create_null_string": {
111+
diff: computed.Diff{
112+
Renderer: Primitive(nil, nil, cty.String),
113+
Action: plans.Create,
114+
},
115+
expected: "null",
116+
},
117+
"primitive_delete_null_string": {
118+
diff: computed.Diff{
119+
Renderer: Primitive(nil, nil, cty.String),
120+
Action: plans.Delete,
121+
},
122+
expected: "null",
123+
},
27124
"primitive_create": {
28125
diff: computed.Diff{
29126
Renderer: Primitive(nil, 1.0, cty.Number),

‎internal/command/jsonformat/computed/renderers/string.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package renderers
22

33
import (
44
"encoding/json"
5+
"fmt"
56
"strings"
67

78
"github.com/hashicorp/terraform/internal/command/jsonformat/computed"
@@ -12,11 +13,15 @@ type evaluatedString struct {
1213
Json interface{}
1314

1415
IsMultiline bool
16+
IsNull bool
1517
}
1618

1719
func evaluatePrimitiveString(value interface{}, opts computed.RenderHumanOpts) evaluatedString {
1820
if value == nil {
19-
return evaluatedString{String: opts.Colorize.Color("[dark_gray]null[reset]")}
21+
return evaluatedString{
22+
String: opts.Colorize.Color("[dark_gray]null[reset]"),
23+
IsNull: true,
24+
}
2025
}
2126

2227
str := value.(string)
@@ -42,3 +47,10 @@ func evaluatePrimitiveString(value interface{}, opts computed.RenderHumanOpts) e
4247
String: str,
4348
}
4449
}
50+
51+
func (e evaluatedString) RenderSimple() string {
52+
if e.IsNull {
53+
return e.String
54+
}
55+
return fmt.Sprintf("%q", e.String)
56+
}

‎internal/command/jsonformat/differ/change_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2552,6 +2552,95 @@ func TestRelevantAttributes(t *testing.T) {
25522552
}
25532553
}
25542554

2555+
func TestSpecificCases(t *testing.T) {
2556+
// This is a special test that can contain any combination of individual
2557+
// cases and will execute against them. For testing/fixing specific issues
2558+
// you can generally put the test case in here.
2559+
tcs := map[string]struct {
2560+
input Change
2561+
block *jsonprovider.Block
2562+
validate renderers.ValidateDiffFunction
2563+
}{
2564+
"issues/33016/unknown": {
2565+
input: Change{
2566+
Before: nil,
2567+
After: map[string]interface{}{
2568+
"triggers": map[string]interface{}{},
2569+
},
2570+
Unknown: map[string]interface{}{
2571+
"id": true,
2572+
"triggers": map[string]interface{}{
2573+
"rotation": true,
2574+
},
2575+
},
2576+
BeforeSensitive: false,
2577+
AfterSensitive: map[string]interface{}{
2578+
"triggers": map[string]interface{}{},
2579+
},
2580+
ReplacePaths: attribute_path.Empty(false),
2581+
RelevantAttributes: attribute_path.AlwaysMatcher(),
2582+
},
2583+
block: &jsonprovider.Block{
2584+
Attributes: map[string]*jsonprovider.Attribute{
2585+
"id": {
2586+
AttributeType: unmarshalType(t, cty.String),
2587+
},
2588+
"triggers": {
2589+
AttributeType: unmarshalType(t, cty.Map(cty.String)),
2590+
},
2591+
},
2592+
},
2593+
validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{
2594+
"id": renderers.ValidateUnknown(nil, plans.Create, false),
2595+
"triggers": renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{
2596+
"rotation": renderers.ValidateUnknown(nil, plans.Create, false),
2597+
}, plans.Create, false),
2598+
}, nil, nil, nil, nil, plans.Create, false),
2599+
},
2600+
"issues/33016/null": {
2601+
input: Change{
2602+
Before: nil,
2603+
After: map[string]interface{}{
2604+
"triggers": map[string]interface{}{
2605+
"rotation": nil,
2606+
},
2607+
},
2608+
Unknown: map[string]interface{}{
2609+
"id": true,
2610+
"triggers": map[string]interface{}{},
2611+
},
2612+
BeforeSensitive: false,
2613+
AfterSensitive: map[string]interface{}{
2614+
"triggers": map[string]interface{}{},
2615+
},
2616+
ReplacePaths: attribute_path.Empty(false),
2617+
RelevantAttributes: attribute_path.AlwaysMatcher(),
2618+
},
2619+
block: &jsonprovider.Block{
2620+
Attributes: map[string]*jsonprovider.Attribute{
2621+
"id": {
2622+
AttributeType: unmarshalType(t, cty.String),
2623+
},
2624+
"triggers": {
2625+
AttributeType: unmarshalType(t, cty.Map(cty.String)),
2626+
},
2627+
},
2628+
},
2629+
validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{
2630+
"id": renderers.ValidateUnknown(nil, plans.Create, false),
2631+
"triggers": renderers.ValidateMap(map[string]renderers.ValidateDiffFunction{
2632+
"rotation": renderers.ValidatePrimitive(nil, nil, plans.Create, false),
2633+
}, plans.Create, false),
2634+
}, nil, nil, nil, nil, plans.Create, false),
2635+
},
2636+
}
2637+
for name, tc := range tcs {
2638+
t.Run(name, func(t *testing.T) {
2639+
tc.validate(t, tc.input.ComputeDiffForBlock(tc.block))
2640+
})
2641+
}
2642+
}
2643+
25552644
// unmarshalType converts a cty.Type into a json.RawMessage understood by the
25562645
// schema. It also lets the testing framework handle any errors to keep the API
25572646
// clean.

‎internal/command/jsonformat/differ/map.go‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,39 @@ import (
1212

1313
func (change Change) computeAttributeDiffAsMap(elementType cty.Type) computed.Diff {
1414
mapValue := change.asMap()
15-
elements, current := collections.TransformMap(mapValue.Before, mapValue.After, func(key string) computed.Diff {
15+
16+
// The jsonplan package will have stripped out unknowns from our after value
17+
// so we're going to add them back in here.
18+
//
19+
// This only affects attributes and not nested attributes or blocks, so we
20+
// only perform this fix in this function and not the equivalent map
21+
// functions for nested attributes and blocks.
22+
23+
// There is actually a difference between a null map and an empty map for
24+
// purposes of calculating a delete, create, or update operation.
25+
26+
var after map[string]interface{}
27+
if mapValue.After != nil {
28+
after = make(map[string]interface{})
29+
}
30+
31+
for key, value := range mapValue.After {
32+
after[key] = value
33+
}
34+
for key := range mapValue.Unknown {
35+
if _, ok := after[key]; ok {
36+
// Then this unknown value was in after, this probably means it has
37+
// a child that is unknown rather than being unknown itself. As
38+
// such, we'll skip over it. Note, it doesn't particularly matter if
39+
// an element is in both places - it's just important we actually
40+
// do cover all the elements. We want a complete union and therefore
41+
// duplicates are no cause for concern as long as we dedupe here.
42+
continue
43+
}
44+
after[key] = nil
45+
}
46+
47+
elements, current := collections.TransformMap(mapValue.Before, after, func(key string) computed.Diff {
1648
value := mapValue.getChild(key)
1749
if !value.RelevantAttributes.MatchesPartial() {
1850
// Mark non-relevant attributes as unchanged.

0 commit comments

Comments
 (0)