Add support for ClusterRole AggregationRule during bundle generation
Description of the change:
ClusterRoles that use an AggregationRule often do not have any rules defined directly. Instead, their rules are aggregated from other ClusterRoles that match the AggregationRule’s label selector.
The existing generator logic only included rules from ClusterRoles that were explicitly bound via ClusterRoleBindings to the ServiceAccounts used by Deployments. Since ClusterRoles with an AggregationRule typically lack direct rule definitions, the resulting permission bundle ended up being empty.
Motivation for the change:
Improve user experience of bundle generator for users using ClusterRole AggregationRule.
Checklist
If the pull request includes user-facing changes, extra documentation is required:
- [x] Add a new changelog fragment in
changelog/fragments(seechangelog/fragments/00-template.yaml) - [ ] Add or update relevant sections of the docs website in
website/content/en/docs
Fixes #6977
@zimnx - please sign the DCO.
@zimnx - please sign the DCO.
Done
@jberkhahn @rashmigottipati Kind reminder about this PR, I would appreciate feedback.
This is on our path for OpenShift certification. It'd be great if it can get reviewed.
@grokspawn perhaps you could take a look? It didn't get much attention. Thanks in advance!
Along with the unit test changes, we'd also want to update our integration tests to make sure that an operator with these rules in their config/rbac has a bundle that is generated properly, and that the operator can run in a cluster. to update the unit integration tests, you can look at hack/generate/samples/internal/go/memcached-with-customization/memcached_with_customization.go to see how we post process the operator's directories and add in the necessary things to fully test different scaffoling options, ontop of what you get from kubebuilder (which operator-sdk is based on).
Thanks for hints, I've added ClusterRole having AggregatedRule to integration tests.
@zimnx Can you create a new changlog fragment as an addition for this? The initial comment lays out how to accomplish that.
Can you create a new changlog fragment as an addition for this? The initial comment lays out how to accomplish that.
yes, added.
Hi @jberkhahn @rashmigottipati @acornett21 @joelanford
~All comments have been addressed and the tests are passing - are we good to merge this?~
Hi @jberkhahn @rashmigottipati @acornett21 @joelanford
All comments have been addressed and the tests are passing - are we good to merge this?
Hi @mflendrich All comments haven't been addressed, the below is still open https://github.com/operator-framework/operator-sdk/pull/6978#discussion_r2334704456
Hi @zimnx If you'd still like this PR to land in this repo, could you address the comment from Sept 9th? Thanks
- https://github.com/operator-framework/operator-sdk/pull/6978#discussion_r2334704456
Hi @acornett21,
Speaking on behalf of the project that has active interest in getting this change upstreamed (that @zimnx has recently left, but the project remains) - I'll be happy to pick this up soon (no later than next week).
Unless you intend to take this yourself soon, @zimnx? In that case I'll stay out of the way.
@acornett21 - fyi, I've picked this up and responded in https://github.com/operator-framework/operator-sdk/pull/6978#discussion_r2528695678.