Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plan renderer: Support users indexing integer paths with stringified numbers #33475

Merged
merged 1 commit into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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