Skip to content

Comments

operator: itemize RBAC#580

Merged
chrisseto merged 1 commit intomainfrom
chris/p/operator-itemize-rbac
Mar 28, 2025
Merged

operator: itemize RBAC#580
chrisseto merged 1 commit intomainfrom
chris/p/operator-itemize-rbac

Conversation

@chrisseto
Copy link
Contributor

@chrisseto chrisseto commented Mar 27, 2025

Prior to this commit the RBAC declarations of the various controllers were littered across the repo. This made it exceptionally difficult to understand where permissions came from. It also made any attempt to test that the redpanda and operator charts' RBACs were correctly configured nearly impossible as the charts themselves had to itemize permissions while their sources did not.

This commit divides each distinct controller into its own package which allows controller-gen to build the (Cluster)Roles for a specific controller.

The vast majority of changes in the commit is just code movement. Notable exceptions are:

  • Duplication of redpanda_controller_utils.go. This file is cursed and will be removed soon any how. I'm accepting the evil of duplicating it.
  • RBAC is now spit into ./operator/config/rbac/itemized/
  • An empty rpkdebugbundle package has been added to track the permissions required for rpk debug bundle.

https://redpandadata.atlassian.net/browse/K8S-495

@chrisseto
Copy link
Contributor Author

My apologies for the massive diff. Everything I've been doing this sprint somehow resulted in me needing to update permission checking tests and it's abysmal to do without this.

@chrisseto chrisseto force-pushed the chris/p/operator-itemize-rbac branch from 64752d5 to eb18640 Compare March 27, 2025 19:30
Copy link
Contributor

@andrewstucki andrewstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks fine to me so long as the tests still pass. Only potential change would be to move the decommissioning package to follow everything else, i.e. internal/controller/decommission or something like that.

@chrisseto
Copy link
Contributor Author

Huh, chart integration tests are failing but the controller tests are passing.

Only potential change would be to move the decommissioning package to follow everything else, i.e. internal/controller/decommission or something like that.

I was trying to minimize changes but it does stick out like a sore thumb now doesn't it. I'll add move it as well if any other changes are required otherwise I might try to dance around CI times...

@chrisseto chrisseto force-pushed the chris/p/operator-itemize-rbac branch from eb18640 to 7b22e36 Compare March 27, 2025 21:25
@chrisseto
Copy link
Contributor Author

I moved the decommissioning package as well. There's no changes out side of the operator so I have to assume the chart failures are pre-existing :( Going to flick on auto merge as I've seen that integration tests work as expected for the operator package.

TFTR!

@chrisseto chrisseto enabled auto-merge (rebase) March 27, 2025 21:27
Prior to this commit the RBAC declarations of the various controllers were
littered across the repo. This made it exceptionally difficult to understand
where permissions came from. It also made any attempt to test that the redpanda
and operator charts' RBACs were correctly configured nearly impossible as the
charts themselves had to itemize permissions while their sources did not.

This commit divides each distinct controller into its own package which allows
`controller-gen` to build the (Cluster)Roles for a specific controller.

The vast majority of changes in the commit is just code movement. Notable
exceptions are:

- Duplication of `redpanda_controller_utils.go`. This file is cursed and will
  be removed soon any how. I'm accepting the evil of duplicating it.
- RBAC is now spit into `./operator/config/rbac/itemized/`
- An empty `rpkdebugbundle` package has been added to track the permissions
  required for `rpk debug bundle`.

See also: [K8S-537], [K8S-495]
@RafalKorepta RafalKorepta force-pushed the chris/p/operator-itemize-rbac branch from 7b22e36 to 8da779b Compare March 28, 2025 11:03
@chrisseto chrisseto merged commit ec56b44 into main Mar 28, 2025
12 checks passed
@RafalKorepta
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
release/v25.1.x
release/v2.4.x
release/v2.3.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@chrisseto chrisseto deleted the chris/p/operator-itemize-rbac branch March 28, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants