-
Notifications
You must be signed in to change notification settings - Fork 108
operator: synchronize {Cluster,}Roles with operator manifests
#1595
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
Conversation
|
Wait on redpanda-data/redpanda-operator#301 to be merged and for a release to be cut. |
| kind: Kustomization | ||
| resources: | ||
| # TODO: Move these links back to main and then to the tag in appVersion. | ||
| - https://raw.githubusercontent.com/redpanda-data/redpanda-operator/chris/p/correct-rbac/operator/config/rbac/leader-election-role/role.yaml |
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 assuming this will get updated after redpanda-data/redpanda-operator#301 merges
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.
Yep! That's the plan.
charts/operator/rbac.go
Outdated
| { | ||
| Verbs: []string{"get", "list", "patch", "update", "watch"}, | ||
| APIGroups: []string{"cluster.redpanda.com"}, | ||
| Resources: []string{"users"}, |
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.
oh gosh, I just now saw the differentiation here between
values.Scope == Namespaceand
values.Scope == Clusterif you're adding this here (under Cluster) we'll likely want to do schemas as well. Then again is the Cluster scope intended to be for the v1 operator based on the bootup logic for the operator?
Either way I think that we'll want to either drop this (if it is solely for the v1 controller) or add in schemas as well.
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.
Fixed. To be on the safe side, I'm leaving the V2 permissions in V1 as they won't have an adverse affect if they're not installed or used AFAIK. Ideally, we'll eventually remove all the gating.
2f1f568 to
20ae4f6
Compare
Prior to this commit the operator chart's declared permissions had grown out of sync with those declared by the operator itself. This commit re-synchronizes the permissions, adds in regression tests to prevent such drift from happening again, and releases the updated chart. Co-authored-by: Rafal Korepta <rafal.korepta@gmail.com>
20ae4f6 to
c24ee97
Compare
|
It's already merged in #1600 |
Prior to this commit the operator chart's declared permissions had grown out of sync with those declared by the operator itself. This commit re-synchronizes the permissions, adds in regression tests to prevent such drift from happening again, and releases the updated chart.