v2: correct and test controller RBAC #301
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrisseto
requested review from
RafalKorepta and
andrewstucki
as code owners
chrisseto
force-pushed
the
chris/p/correct-rbac
branch
from
6b2498a
to
f07f723
Compare
| role := roles[1].(*rbacv1.Role) | ||
| clusterRole := roles[0].(*rbacv1.ClusterRole) | ||
|
|
||
| for _, typ := range redpandachart.Types() { |
There was a problem hiding this comment.
Do we have this same test equivalent in the helm chart-defined role? Seems like it'd be nice since we're relying on the generated role.yaml in our tests here, but the RBAC rules are actually a part of the operator chart currently.
There was a problem hiding this comment.
There's a WIP PR from Rafal that tests based off the operator's role.yaml: redpanda-data/helm-charts#1593 that I've been patching up to work with these updates. It's a bit less than ideal because the dependency is external to the repo but it'll work for now :/
There was a problem hiding this comment.
Updated PR here: redpanda-data/helm-charts#1595
andrewstucki
approved these changes
Nov 11, 2024
chrisseto
force-pushed
the
chris/p/correct-rbac
branch
2 times, most recently
from
4ef451c
to
61ccdd8
Compare
|
chrisseto
force-pushed
the
chris/p/correct-rbac
branch
from
61ccdd8
to
80f144c
Compare
RafalKorepta
approved these changes
Nov 12, 2024
Prior to this commit the declared permissions for the RedpandaReconciler had become out of date. This went unnoticed due to tests utilizing admin permissions or the inflated permissions required for executing `rpk debug bundle`. This commit corrects the permission declaration of the RedpandaReconciler, updates its tests to use the ClusterRole and Role generated by controller-gen, and adds a test to statically assert the correctness of the permissions.
chrisseto
force-pushed
the
chris/p/correct-rbac
branch
from
80f144c
to
616a923
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this commit the declared permissions for the RedpandaReconciler had become out of date. This went unnoticed due to tests utilizing admin permissions or the inflated permissions required for executing
rpk debug bundle.This commit corrects the permission declaration of the RedpandaReconciler, updates its tests to use the ClusterRole and Role generated by controller-gen, and adds a test to statically assert the correctness of the permissions.