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

Ignore account_member role ID ordering #128

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

jacobbednarz
Copy link
Member

After we imported our users to Terraform we ordered the roles
alphabetically using a variable map for readability. Once we started
applying changes we noticed that the resource would be marked as needing
replacement despite nothing changing. Turns out the reason was the role
ID ordering.

There are a few ways I could have solved this however I opted for the
simplest of just checking that the old and new values are both present
somewhere in the roles and if they were, suppress the change diff.

This doesn't impact any functionality however it does allow a little
more flexibility if you have a particular way you want the roles ordered
and not want to follow Cloudflare's API response of lexically sorting on
the ID.

After we imported our users to Terraform we ordered the roles
alphabetically using a variable map for readability. Once we started
applying changes we noticed that the resource would be marked as needing
replacement despite nothing changing. Turns out the reason was the role
ID ordering.

There are a few ways I could have solved this however I opted for the
simplest of just checking that the old and new values are both present
somewhere in the roles and if they were, suppress the change diff.

This doesn't impact any functionality however it does allow a little
more flexibility if you have a particular way you want the roles ordered
and not want to follow Cloudflare's API response of lexically sorting on
the ID.
@ghost ghost added the size/XS label Oct 19, 2018
@patryk patryk merged commit 23874d5 into cloudflare:master Oct 19, 2018
@patryk
Copy link
Contributor

patryk commented Oct 19, 2018

Yup, nice find, thanks!

jacobbednarz referenced this pull request in jacobbednarz/terraform-provider-cloudflare Nov 25, 2020
When I created this resource, `TypeList` seemed to be the correct choice
for the data type we store. However, after a while we discovered an
issue whereby a change in the ordering of the `role_ids`, resulted in the
account member wanting to be recreated. This was addressed in #128
however it meant that new roles had to be added to the end otherwise it
would flap. It also had the extra legwork of a custom `DiffSuppressFunc`
to ensure that we compared the values using a consistent ordering to
avoid the trap where the value wasn't added to the end.

Now that we understand the internal types a little better, `TypeSet` is
a more appropriate choice as it is an unordered list which fits the
`role_ids` perfectly and solves the initial issues we encountered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants