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

Conversation

liamcervante
Copy link
Member

Terraform allows users to specify indexes as strings as long as the string can be converted into a number. For example, the following are equivalent:

  • test_resource.resource.list[0].attribute
  • test_resource.resource.list["0"].attribute
  • test_resource.resource.list["0.0"].attribute

While the following would all raise validation errors.

  • test_resource.resource.list[0.5].attribute
  • test_resource.resource.list["not a number"].attribute
  • test_resource.resource.list["0.5"].attribute

The plan renderer currently makes an assumption that the indices will be the appropriate type. This causes a panic when attributes like those listed above are included in the relevant attributes data, as the renderer will attempt to cast them into the wrong type.

This PR updates the logic for parsing paths within the renderer so that it will attempt to convert strings into integers and panic if it can't. The panic should be okay and never triggered as Terraform will fail earlier if the index is actually invalid.

Fixes #33472

Target Release

1.5.3

Draft CHANGELOG entry

BUG FIXES

  • Fixes crash when rendering the plan if a relevant attribute contains an integer index specified as a string.

@liamcervante liamcervante requested a review from a team July 5, 2023 08:40
Copy link
Member

@kmoe kmoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1.5-backport

@liamcervante liamcervante added the 1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Jul 5, 2023
@liamcervante liamcervante merged commit 3613e97 into main Jul 5, 2023
6 checks passed
@liamcervante liamcervante deleted the liamcervante/33472 branch July 5, 2023 09:11
@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Terraform crashed: interface {} is string, not float64
2 participants