strimzi-kafka-operator
strimzi-kafka-operator copied to clipboard
Add support for custom CC API users
Type of change
- Enhancement / new feature
Description
Based on this proposal, this PR allows advanced users the ability to create REST API users for the Cruise Control REST API. This would allow roles and permissions to be defined to allow advanced users and third-party applications to access the Cruise Control REST API without having to disable HTTP basic authentication.
For example, advanced users could define their custom API users in a secret called cruise-control-api-users-secret putting their API user credentials in the Jetty's HashLoginService's file format like this:
userOne: passwordOne, USER
userTwo: passwordTwo, VIEWER
Create the secret:
kubectl create secret generic cruise-control-api-users-secret --from-file=key=cruise-control-auth.txt
Then update their Kafka custom resource like this:
apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
metadata:
...
spec:
cruiseControl:
apiUsers:
type: hashLoginService (1)
valueFrom: (2)
secretKeyRef:
name: cruise-control-api-users-secret
key: key
...
(1) A type field is added here to describe the format of the data and for configuration flexibility in the future. This gives us the option to add different data format types in the future. In this example we use Jetty's HashLoginService format since that is the format which Cruise Control uses for its API user configuration.
(2) The valueFrom construct allows us to add more sources in the future if needed. This is also a pattern used in other Strimzi APIs already, for example Strimzi's logging configuration, password configuration in KafkaUser resources, metrics configuration, and more.
For more information, checkout the proposal.
Checklist
Please go through this checklist and make sure all applicable tasks have been done
- [x] Write tests
- [x] Make sure all tests pass
- [x] Update documentation
- [x] Check RBAC rights for Kubernetes / OpenShift roles
- [x] Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
- [x] Reference relevant issue(s) and close them after merging
- [x] Update CHANGELOG.md
- [ ] Supply screenshots for visual changes, such as Grafana dashboards
/packit test --labels sanity
I just wanted to check something with Packit. The failure is not connected to this PR but to Packit/TF config, you can ignore the failed TF checks, thanks.
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
@kyguy The code looks good to me. But there seems to be some issue as all tests with CC failed in regression.
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
@kyguy You seem to have some Spotbugs issue that failed the build.
Based on the feedback I moved a bunch of the methods of ApiCredentials class into the ApiUsers class messing around with the access controls and attempting to simplify the logic. Please take another look to make sure everything looks ok!
@scholzj @ppatierno Could you take a look at the recent changes when you get a chance?
Can't we just revert to the point when I approved it and start again from there? 😞 To be honest, I'm not sure how did we managed to mess up so much something what looked good :-(. Sorry that I missed the comments and changes here earlier @kyguy.
Can't we just revert to the point when I approved it and start again from there? 😞 To be honest, I'm not sure how did we managed to mess up so much something what looked good :-(. Sorry that I missed the comments and changes here earlier @kyguy.
I think that "mess up" is a too strong wording (at least for how it actually translates to Italian). I asked for some abstraction and Kyle just followed my suggestions. Maybe there is something still to tweak but saying that it's a mess (and the one before was good) sounds to be not fair to me.
Refactored based on feedback, can you both take another pass when you get a chance @ppatierno @scholzj
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
@kyguy FYI: The system test that failed is flaky, so while it uses Cruise Control, it is possibly not related to your PR.
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
@scholzj Just addressed nits, once the base unit tests pass again we are are ready for another round of system tests
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
@kyguy Could you please resolve the conflict?
@scholzj COnflict resolved and now have all approvals, could you execute regression tests one more time
Yeah, I need to first merge #10415 that fixes some flaky tests.
/azp run regression
Azure Pipelines successfully started running 1 pipeline(s).
Thanks for the PR @kyguy