Operator RBAC roles testing with LocallyRunOperatorExtension

[k-wall]

Is your feature request related to a problem? Please describe.

We want to test our operator's RBAC roles from test cases written using LocallyRunOperatorExtension.

why: we found that we weren't finding RBAC misconfigurations until quite late in the development cycle - until the operator was deployed to Kubernetes with the real RBAC resource deployed. We want a way to shift left. We already had a suite of operator tests written using LocallyRunOperatorExtension, but these weren't flagging the errors, because the framework makes no consideration for RBAC.

To work around, we wrote a Junit extension LocallyRunningOperatorRbacHandler that cooperates with LocallyRunOperatorExtension. It configures the Kubernetes client passed to the LocallyRunOperatorExtension so that it uses an impersonated user. It also takes responsibility to load the RBAC roles and creates bindings between the impersonated user and the roles. It also exposes a second Kubernetes client that is effectively root - so that the test can create the resources it needs without being impeded.

You can see it in action https://github.com/kroxylicious/kroxylicious/blob/edfc2267cac9853169e0a32c7c1aa00cb11189ae/kroxylicious-operator/src/test/java/io/kroxylicious/kubernetes/operator/KafkaProtocolFilterReconcilerIT.java#L56.

Basically the key parts:

    @RegisterExtension
    static LocallyRunningOperatorRbacHandler rbacHandler = new LocallyRunningOperatorRbacHandler("install", "*.ClusterRole.kroxylicious-operator-watched.yaml");

    @RegisterExtension
    LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder()
            .withReconciler(new KafkaProtocolFilterReconciler(Clock.systemUTC(), SecureConfigInterpolator.DEFAULT_INTERPOLATOR))
            .withAdditionalCustomResourceDefinition(KafkaProtocolFilter.class)
            .withKubernetesClient(rbacHandler.operatorClient()) // <--- injects impersonated user.
            .waitForNamespaceDeletion(true)
            .withConfigurationService(x -> x.withCloseClientOnStop(false))
            .build();

    private final LocallyRunningOperatorRbacHandler.TestActor testActor = rbacHandler.testActor(extension);

    @Test
    void myTest() {
        testActor.create(myCr());
        makeAssertions();
    }

The solution described above is working reasonable well for us. We wanted to share it, and also ask if you can suggest improvements.

There is a shortcoming. The kubernetesClient passed to the LocallyRunOperatorExtension has to have the RBAC required and the operator and the RBAC required by the LocallyRunOperatorExtension itself. This is a bit weak as it means the operator runs with more permissions than ought.

Describe the solution you'd like

Ideas for changes to LocallyRunOperatorExtension that would help this pattern:

1. Have LocallyRunOperatorExtension distinguish between the kubernetesClient is uses for its own purposes and the client given to the operator. That would resolve the shortcoming I describe above.

 LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder()
         ...
         .withOperatorKubernetesClient(aClientConfiguredWithAnImpersonatedUser())
         .withFrameworkKubernetesClient(aClient())

2. Give LocallyRunOperatorExtension (or son of LocallyRunOperatorExtension) the smarts to handle RBAC and user impersonation itself. This would make our LocallyRunningOperatorRbacHandler redundant. I appreciate this would be a deeper change.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

[csviri]

Hi @k-wall thank you very much for sharing, yes, this absolutely makes sense for me.

Maybe we should have both, like having the API you mentioned:

 LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder()
         ...
         .withOperatorKubernetesClient(aClientConfiguredWithAnImpersonatedUser())
         .withFrameworkKubernetesClient(aClient())

Thus there will be an option to configure both clients, if not explicitly configured the operator will use the standard client as now.
But also just have an option to set RBACK, and use that for the operator client

 LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder()
         ...
         .withOperatorClientRBAC(...)
         

This would create the additional client in the background with the passed RBAC, and use it for the operator.

In addition to this, also thinking a bit ahead, I would like to eventually do the RBAC generator in the core JOSDK:
#1765

The RBAC will be generated here based on managed DependentResources and additional annotation on the controller.
Note that with that present, we could automatically set the RBAC for the operator client in test.

[csviri]

@k-wall do you plan to create a PR for this, or we should take care of it? :)

[k-wall]

Thanks for the quick reply.

I've got no capacity to look at this right now, so if you were able to add the feature soon, that'd be awesome.
OTOH If it is still open in a month or so, I might see what I can contribute.