Skip to content

Commit

Permalink
Plan renderer: Support users indexing integer paths with stringified …
Browse files Browse the repository at this point in the history
…numbers (#33475)
  • Loading branch information
liamcervante committed Jul 5, 2023
1 parent ad2298b commit 3613e97
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 3 deletions.
109 changes: 109 additions & 0 deletions internal/command/jsonformat/differ/differ_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2742,6 +2742,115 @@ func TestSpecificCases(t *testing.T) {
}, plans.Create, false),
}, nil, nil, nil, nil, plans.Create, false),
},

// The following tests are from issue 33472. Basically Terraform allows
// callers to treat numbers as strings in references and expects us
// to coerce the strings into numbers. For example the following are
// equivalent.
// - test_resource.resource.list[0].attribute
// - test_resource.resource.list["0"].attribute
//
// We need our attribute_path package (used within the ReplacePaths and
// RelevantAttributes fields) to handle coercing strings into numbers
// when it's expected.

"issues/33472/expected": {
input: structured.Change{
Before: map[string]interface{}{
"list": []interface{}{
map[string]interface{}{
"number": -1,
},
},
},
After: map[string]interface{}{
"list": []interface{}{
map[string]interface{}{
"number": 2,
},
},
},
Unknown: false,
BeforeSensitive: false,
AfterSensitive: false,
ReplacePaths: attribute_path.Empty(false),
RelevantAttributes: &attribute_path.PathMatcher{
Propagate: true,
Paths: [][]interface{}{
{
"list",
0.0, // This is normal and expected so easy case.
"number",
},
},
},
},
block: &jsonprovider.Block{
Attributes: map[string]*jsonprovider.Attribute{
"list": {
AttributeType: unmarshalType(t, cty.List(cty.Object(map[string]cty.Type{
"number": cty.Number,
}))),
},
},
},
validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{
"list": renderers.ValidateList([]renderers.ValidateDiffFunction{
renderers.ValidateObject(map[string]renderers.ValidateDiffFunction{
"number": renderers.ValidatePrimitive(-1, 2, plans.Update, false),
}, plans.Update, false),
}, plans.Update, false),
}, nil, nil, nil, nil, plans.Update, false),
},

"issues/33472/coerce": {
input: structured.Change{
Before: map[string]interface{}{
"list": []interface{}{
map[string]interface{}{
"number": -1,
},
},
},
After: map[string]interface{}{
"list": []interface{}{
map[string]interface{}{
"number": 2,
},
},
},
Unknown: false,
BeforeSensitive: false,
AfterSensitive: false,
ReplacePaths: attribute_path.Empty(false),
RelevantAttributes: &attribute_path.PathMatcher{
Propagate: true,
Paths: [][]interface{}{
{
"list",
"0", // Difficult but allowed, we need to handle this.
"number",
},
},
},
},
block: &jsonprovider.Block{
Attributes: map[string]*jsonprovider.Attribute{
"list": {
AttributeType: unmarshalType(t, cty.List(cty.Object(map[string]cty.Type{
"number": cty.Number,
}))),
},
},
},
validate: renderers.ValidateBlock(map[string]renderers.ValidateDiffFunction{
"list": renderers.ValidateList([]renderers.ValidateDiffFunction{
renderers.ValidateObject(map[string]renderers.ValidateDiffFunction{
"number": renderers.ValidatePrimitive(-1, 2, plans.Update, false),
}, plans.Update, false),
}, plans.Update, false),
}, nil, nil, nil, nil, plans.Update, false),
},
}
for name, tc := range tcs {
t.Run(name, func(t *testing.T) {
Expand Down
34 changes: 31 additions & 3 deletions internal/command/jsonformat/structured/attribute_path/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@

package attribute_path

import "encoding/json"
import (
"encoding/json"
"fmt"
"strconv"
)

// Matcher provides an interface for stepping through changes following an
// attribute path.
Expand Down Expand Up @@ -173,8 +177,32 @@ func (p *PathMatcher) GetChildWithIndex(index int) Matcher {
continue
}

if int(path[0].(float64)) == index {
child.Paths = append(child.Paths, path[1:])
// Terraform actually allows user to provide strings into indexes as
// long as the string can be interpreted into a number. For example, the
// following are equivalent and we need to support them.
// - test_resource.resource.list[0].attribute
// - test_resource.resource.list["0"].attribute
//
// Note, that Terraform will raise a validation error if the string
// can't be coerced into a number, so we will panic here if anything
// goes wrong safe in the knowledge the validation should stop this from
// happening.

switch val := path[0].(type) {
case float64:
if int(path[0].(float64)) == index {
child.Paths = append(child.Paths, path[1:])
}
case string:
f, err := strconv.ParseFloat(val, 64)
if err != nil {
panic(fmt.Errorf("found invalid type within path (%v:%T), the validation shouldn't have allowed this to happen; this is a bug in Terraform, please report it", val, val))
}
if int(f) == index {
child.Paths = append(child.Paths, path[1:])
}
default:
panic(fmt.Errorf("found invalid type within path (%v:%T), the validation shouldn't have allowed this to happen; this is a bug in Terraform, please report it", val, val))
}
}
return child
Expand Down

0 comments on commit 3613e97

Please sign in to comment.