-
Notifications
You must be signed in to change notification settings - Fork 31
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
fix: convert User Policies to unordered set to prevent policy thrashing #577
Conversation
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.
it looks good to me. my only concern is that this type change (List -> Set) will show up as a change in the plan for all existing customers. could be worth to mention in the release changelog.
@peterdeme is the changelog something I need to do? Also is there a way to get the "Test the code" job triggered to conclude the PR requirements? |
acf3e91
to
85f1755
Compare
|
test: add test for different policy order doc: update user.md for new set
85f1755
to
167f1d8
Compare
@peterdeme ahh ok. Well I just rebased to a single signed commit to satisfy that requirement, but I guess that won't matter if you're opening a new PR. |
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.
Looks great to me as well @peterdeme.
@bushong1 thanks a lot for this contribution!
@peterdeme I agree with the concern about upgrading. Let's check what happens if you have a state using the current provider version, then switch to this version and apply before merging.
Description of the change
I’ve been having some trouble with the Spacelift Terraform provider when managing user access policies. Every time I add a new policy, Terraform tries to delete and recreate some of the existing ones—even though they haven’t changed at all. It’s like it’s getting confused about what’s actually new and what’s already there.
I think the problem is with how the policy field is set up in the spacelift_user resource. Right now, it’s defined as a TypeList, which is ordered. So when I insert a new policy somewhere other than the end of the list, it shifts the order, and Terraform thinks the policies have changed.
To fix this, I think we should change policy from a TypeList to a TypeSet. Sets are unordered, so Terraform won’t care about the order of the policies anymore. Also, by adding a custom hash function that combines space_id and role, we can uniquely identify each policy.
Type of change
Related issues
Checklists
Development
false
.)go generate
to make sure the docs are up to dateCode review