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

config generation: escape map keys with whitespace #35754

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

liamcervante
Copy link
Member

This PR updates the config generation package so map keys with whitespace are escaped with quotes. This is already handled automatically for the normal attribute generation, and nested blocks are also escaped by default. This PR updates the nested attributes so the keys of nested maps are validated and escaped if necessary. It is only maps that have this problem, attributes in objects cannot contain whitespace and other types (lists, sets, tuples) do not render keys anyway.

The rendering package has this same problem, and so far we've not handled it perfectly but just well enough. We'll continue to do so here, but the code calls out we could modify the HCL library so it exposes the internal functions it uses to detect when escaping is necessary and share those between HCL and Terraform. But so far, we've not had any need to take this too seriously.

Fixes #35752

Target Release

1.9.x

Draft CHANGELOG entry

BUG FIXES

  • config generation: escape map keys with whitespaces

@liamcervante liamcervante added the 1.9-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Sep 20, 2024
@liamcervante liamcervante requested a review from a team September 20, 2024 07:18
@DanielMSchmidt DanielMSchmidt merged commit d205fd0 into main Sep 24, 2024
6 checks passed
@DanielMSchmidt DanielMSchmidt deleted the liamcervante/35752 branch September 24, 2024 10:51
Copy link
Contributor

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

@apparentlymart
Copy link
Contributor

I guess my experiment over in #34823 would've unintentionally fixed this due to hclwrite already knowing how to properly quote/escape literal strings in the way HCL expects. Maybe I should've pushed a little harder to get the remaining problems with that solved, rather than aborting due the misconception that I wasn't actually fixing anything. 😀

That other (more-ambitious-than-necessary) PR aside, it might be interesting to note that hclwrite's quoting is currently exposed as TokensForValue followed by a call to Tokens.Bytes to get the generated expression source code. Though we already ended up needing to do it for instance keys in package addrs, where all the overhead of hclwrite seemed undesirable, and so we have an inline utility function for HCL-friendly string quoting which you could potentially factor out somewhere so that other code like this could use it.

Copy link
Contributor

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 Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.9-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 plan -generate-config-out fails for map nested objects with keys with spaces
3 participants