cp-ansible
cp-ansible copied to clipboard
[ANSIENG-3807] | Rbac over mTLS
Description
This Pr aims to add support for RBAC over mTLS. The includes changes for
- MDS
- Broker
- Controller
- SR
- ERP
- RP
- Connect
- KSQL
- C3
- Using cert based token retrival
- Extracting Principal from certs to assign role bindings
- Molecule tests
- Setting Principal Mapping Rules
- Setting impersonation super users for principal propogation
Replicator changes will be raised in seperate PR
Fixes # (issue)
Type of change
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
How Has This Been Tested?
Checklist:
- [ ] Any variable/code changes have been validated to be backwards compatible (doesn't break upgrade)
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] If required, I have ensured the changes can be discovered by cp-ansible discovery codebase
- [ ] My code follows the style guidelines of this project
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] Any dependent changes have been merged and published in downstream modules
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
:tada: All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.
Added some comments, other than those, Open question:
- Why are we using SASL_SSL listener with MDS /register endpoint? Please share the doc reference
Suggestions:
- Please rename the
<component>_to_mds_send_certs_onlyto a better variable name for code quality. Something likerp_mds_certs_only,connect_mds_certs_onlyand add the description in docs.md - Please add creating connectors in the molecule tests to validate Connect server mtls and dual auth. Similarly, please add kafka produce/consumer commands in verify.yml over different listeners to validate the listeners.
- Please add check to validate if
mds_super_user_external_cert_pathis defined in a 2 cluster setup. Adding to this, the variable should be renamed tomds_superuser_cert_pathandkafka_broker_cert_pathshould default to this and its principal should be added to super.users. This is similar to the current flow where customer defines mds_super_user or oauth_superuser_principal. This super user principal is the principal that can provide rolebindings and which may or may not be equal to kafka principal, specially in external mds setup.
- Why are we using SASL_SSL listener with MDS /register endpoint? Please share the doc reference
The doc reference https://confluentinc.atlassian.net/wiki/spaces/OAAC/pages/3653567526/mTLS+with+RBAC+Gotchas
Suggestions:
- Please rename the
<component>_to_mds_send_certs_onlyto a better variable name for code quality. Something likerp_mds_certs_only,connect_mds_certs_onlyand add the description in docs.md
The variable name should make the purpose clear. If we shorten the name to <component>_mds_cert_auth_only ?
- Please add creating connectors in the molecule tests to validate Connect server mtls and dual auth. Similarly, please add kafka produce/consumer commands in verify.yml over different listeners to validate the listeners.
Sure will add that
- Please add check to validate if
mds_super_user_external_cert_pathis defined in a 2 cluster setup. Adding to this, the variable should be renamed tomds_superuser_cert_pathandkafka_broker_cert_pathshould default to this and its principal should be added to super.users. This is similar to the current flow where customer defines mds_super_user or oauth_superuser_principal. This super user principal is the principal that can provide rolebindings and which may or may not be equal to kafka principal, specially in external mds setup.
- Yes agreed validation should be added and principal of this cert should be added to super.user of 2nd cluster
- On default value of kafka broker cert path being same as mds super user cert. That part I don't get it. Like in case when there are 2 clusters that makes sense for custom certs. but in case when there is single cluster this variable cant be defined or even for 2nd clusters in cases of provided keystore,truststore this cant be defined.