-
Notifications
You must be signed in to change notification settings - Fork 994
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 cluster role binding namespace handling #2633
base: main
Are you sure you want to change the base?
Conversation
kubernetes/structures_rbac.go
Outdated
subject.Namespace = v.(string) | ||
|
||
// Handle namespace logic | ||
if subject.Kind == "Group" || subject.Kind == "User" { |
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.
seems like the logic can be simplified a bit more where the condition only handles the Kind that use the namespace. See if you can simplify this a bit more. Same goes for the flattener.
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 good, only thing missing is a changelog-entry. after that i can approve this 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.
so i'm unsure if people are still running into this. Despite the comments on the previous issue I went and attempted to reproduce the bug but came across the expected behavior:
terraform init
:
I even went ahead and downgrade from 1.31
->1.26
and still unable to reproduce the bug.
can you provide me with your kubectl configs as well as confirming that you can reproduce it yourself?
@BBBmau it looks like your test is making use of the workaround of providing |
That's what I get for attempting to reproduce a bug late at night. Thanks! |
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.
works great! I fixed up the test since it was explicitly setting the namespace to "". removed and tests pass as well as confirming by doing some manual tests.
Description
Fixes #710
This PR addresses and fixes the handling of namespaces in the kubernetes_cluster_role_binding_v1 resource
I am looking for feedback on my fix! Initially, I implemented logic in structures_rbac.go to handle namespace values directly. However, during acceptance testing, I encountered plan difference errors, where the namespace field was toggling between "default" and null. These differences caused Terraform to repeatedly attempt updates, believing the resource was not in the desired state.
To address this issue, I modified the schema to include diff suppression for the namespace field:
Diff suppression ensures that when the namespace field is either "default" (defaulted by Kubernetes) or null (irrelevant for Group and User), Terraform does not detect a difference.
Acceptance tests
Output from acceptance testing:
Release Note
Release note for CHANGELOG:
References
Community Note