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

Correctly handle Access Group schema transformations #917

Merged
merged 15 commits into from
Jan 28, 2021

Conversation

jacobbednarz
Copy link
Member

This fixes the ability to have Access Policies be able to be imported and still
maintain the policy conditions. The cause of the bug is due to the change in
schema made many moons ago and the schema not matching the API responses. This
was an issue as out of the box, Terraform can do some conversion however the
Access Policy resources are complex structures. The issue was made worse by the
omission of error handling d.Set calls which can slightly fail.

Looking at this PR, I bet you're thinking "this looks really verbose, surely
there is a better way" and you'd be partially correct. This is pretty verbose.
It's essentially the counterpart to BuildAccessGroupCondition which
conditionally builds the API request whereas this focused on the API response =>
schema conversion. The reason it is so verbose is due to the remote API and the
flexibility this API has in the conditions and combinations. You can have a
myriad of include, exclude and require conditions that all work in unison
to provide a fitting policy.

Fixes #681

jacobbednarz added a commit that referenced this pull request Jan 22, 2021
…pForSchema`

Off the back of #917, this is another fix to ensure the state is able to be
compared against the remote API to correctly identify drift.

To be a good citizen, I've also swapped these `d.Set` calls to handle error
checking to prevent silent failures from slipping through.

Fixes #891
jacobbednarz added a commit that referenced this pull request Jan 27, 2021
…pForSchema`

Off the back of #917, this is another fix to ensure the state is able to be
compared against the remote API to correctly identify drift.

To be a good citizen, I've also swapped these `d.Set` calls to handle error
checking to prevent silent failures from slipping through.

Fixes #891
@jacobbednarz jacobbednarz merged commit c1ad4a9 into master Jan 28, 2021
@jacobbednarz jacobbednarz deleted the fix-access-import branch January 28, 2021 00:17
jacobbednarz added a commit that referenced this pull request Jan 28, 2021
…pForSchema`

Off the back of #917, this is another fix to ensure the state is able to be
compared against the remote API to correctly identify drift.

To be a good citizen, I've also swapped these `d.Set` calls to handle error
checking to prevent silent failures from slipping through.

Fixes #891
@jacobbednarz jacobbednarz added this to the 2.18.0 milestone Jan 28, 2021
@jacobbednarz jacobbednarz mentioned this pull request Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Access policy imports don't properly track include{} blocks
1 participant