-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
put ConflictsWith on blocks that changed from ExactlyOneOf to AtLeastOneOf #2856
put ConflictsWith on blocks that changed from ExactlyOneOf to AtLeastOneOf #2856
Conversation
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in terraform-google-conversion. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
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 seems like a reasonable workaround to me, as long as it works with dynamic blocks!
+alex for ansible |
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.
I'm comfortable with this change as well
Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.
a8c0918
to
6dfa28f
Compare
#2837 changed a bunch of ExactlyOneOfs to AtLeastOneOfs, which broke a HealthCheck test that confirmed that we fail when setting two of the conflicting fields (because it started failing API-side instead of at plan-time).
When I checked what we had been doing before ExactlyOneOf (so I could grab the right error message), they had all been ConflictsWith, and since AtLeastOneOf + ConflictsWith = ExactlyOneOf, this actually gives us back ExactlyOneOf behavior but without the issues with dynamic blocks.
Technically this is a breaking change, but since it's just moving an API-side failure to a plan-time one (and there was only one release where the restrictions went away), I'm fine with including it in a minor release.
Release Note Template for Downstream PRs (will be copied)