-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
terraform: use hcl.MergeBodies instead of configs.MergeBodies for pro… #29000
Conversation
…vider configuration Previously, Terraform would return an error if the user supplied provider configuration via interactive input iff the configuration provided on the command line was missing any required attributes - even if those attributes were already set in config. That error came from configs.MergeBody, which was designed for overriding valid configuration. It expects that the first ("base") body has all required attributes. However in the case of interactive input for provider configuration, it is perfectly valid if either or both bodies are missing required attributes, as long as the final body has all required attributes. hcl.MergeBodies works very similarly to configs.MergeBodies, with a key difference being that it only checks that all required attributes are present after the two bodies are merged. I've updated the existing test to use interactive input vars and a schema with all required attributes. This test failed before switching from configs.MergeBodies to hcl.MergeBodies.
On second thought, |
Reopening! Any dynamic blocks have already been expanded by the time we get to |
…s with dynamic configuration + required + interactive input merging This test failed when buildProviderConfig still used configs.MergeBodies instead of hcl.MergeBodies
64195ab
to
50cb740
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks plausible to me!
Since the bug this is addressing is such an edge case and has been encountered only once over several years, I'd vote for letting this one soak in the 1.1 branch for a bit (and asking the reporter to explicitly test from there) rather than immediately backporting this to 1.0, even though it's a bug fix. I trust the code you've written here, but hcl.MergeBodies
is some old code that we've not made extensive use of so far and so while I don't have any specific knowledge of any misbehavior in its implementation, hopefully it living through the 1.1 alpha/beta releases will give an opportunity for a variety of people to exercise it and see if they encounter anything odd that we might not have considered here.
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. |
…vider configuration
Previously, Terraform would return an error if the user supplied provider configuration via interactive input iff the configuration provided on the command line was missing any required attributes - even if those attributes were already set in config.
That error came from configs.MergeBody, which was designed for overriding valid configuration. It expects that the first ("base") body has all required attributes. However in the case of interactive input for provider configuration, it is perfectly valid if either or both bodies are missing required attributes, as long as the final body has all required attributes. hcl.MergeBodies works very similarly to configs.MergeBodies, with a key difference being that it only checks that all required attributes are present after the two bodies are merged.
I've updated the existing test to use interactive input vars and a schema with all required attributes. This test failed before switching from configs.MergeBodies to hcl.MergeBodies.
Fixes #28956