Add support for ClusterRole AggregationRule during bundle generation

[zimnx]

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:

• Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
• Add or update relevant sections of the docs website in website/content/en/docs

Fixes #6977

[mykaul]

@zimnx - please sign the DCO.

[zimnx]

@zimnx - please sign the DCO.

Done

@jberkhahn @rashmigottipati Kind reminder about this PR, I would appreciate feedback.

[mykaul]

This is on our path for OpenShift certification. It'd be great if it can get reviewed.

[zimnx]

@grokspawn perhaps you could take a look? It didn't get much attention. Thanks in advance!

[acornett21]

Thanks for this PR and sorry for the delay, I only can look at this project in my free time.

I have a couple of comments, one about the unit test is marked as such. 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).

[zimnx]

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.

[acornett21]

@zimnx Can you create a new changlog fragment as an addition for this? The initial comment lays out how to accomplish that.

[joelanford]

The GoDoc comment mentions that this recursively collect rules from other ClusterRoles that match the aggregation label. But I think I only see one level of collection happening?

What if we have:

• ClusterRole A with AggregationRule that matches foo: "bar" with rules X
• ClusterRole B with label foo: "bar" and an AggregationRule that matches fizz: "buzz" with rules Y
• ClusterRole C with label fizz: "buzz" with rules Z

I think I would expect calling this function on ClusterRole A to return rules X, Y, and Z?

[zimnx]

Thanks for spotting. GoDoc comment is incorrect.

Policy rules are not aggregated recursively:
https://github.com/kubernetes/kubernetes/blob/release-1.34/pkg/controller/clusterroleaggregation/clusterroleaggregation_controller.go#L84-L125

Fixed the comment to reflect function semantic.

[joelanford]

Are you sure they are not aggregated recursively (at least in effect)?

https://github.com/kubernetes/kubernetes/blob/45a8851a7c64f7f05afdfad36ebfcca906816f2e/pkg/controller/clusterroleaggregation/clusterroleaggregation_controller.go#L131

It looks like a "sync" goes and fetches rules from cluster roles (let's call them C and D) that match the aggregation label in cluster role B and then syncs those rules into the aggregated role B. So at the end of a sync, B actually contains rules from C and D.

If there is another cluster role A that has an aggregation rule that selects B, then at some point A will sync, see the aggregated rules in B and pull those rules into A. So eventually, A will have rules from B, C, and D.

[zimnx]

Can you create a new changlog fragment as an addition for this? The initial comment lays out how to accomplish that.

yes, added.

[mflendrich]

Hi @jberkhahn @rashmigottipati @acornett21 @joelanford

All comments have been addressed and the tests are passing - are we good to merge this?

[acornett21]

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
#6978 (comment)