confluentinc / cp-ansible

Ansible playbooks for the Confluent Platform
Apache License 2.0
43 stars 405 forks source link

[ANSIENG-3807] | Rbac over mTLS #1804

Closed rrbadiani closed 3 weeks ago

rrbadiani commented 1 month ago

Description

This Pr aims to add support for RBAC over mTLS. The includes changes for


Fixes # (issue)

Type of change

How Has This Been Tested?

zookeeper

kraft

Checklist:

CLAassistant commented 1 month ago

CLA assistant check
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.

confluent-cla-assistant[bot] commented 1 month ago

: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.

mansisinha commented 1 month ago

Added some comments, other than those, Open question:

  1. Why are we using SASL_SSL listener with MDS /register endpoint? Please share the doc reference

Suggestions:

  1. Please rename the <component>_to_mds_send_certs_only to a better variable name for code quality. Something like rp_mds_certs_only, connect_mds_certs_only and add the description in docs.md
  2. 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.
  3. Please add check to validate if mds_super_user_external_cert_path is defined in a 2 cluster setup. Adding to this, the variable should be renamed to mds_superuser_cert_path and kafka_broker_cert_path should 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.
rrbadiani commented 1 month ago
  1. 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:

  1. Please rename the <component>_to_mds_send_certs_only to a better variable name for code quality. Something like rp_mds_certs_only, connect_mds_certs_only and add the description in docs.md

The variable name should make the purpose clear. If we shorten the name to _mds_certs_only by this name it cant convey full meaning. How about the name <component>_mds_cert_auth_only ?

  1. 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

  1. Please add check to validate if mds_super_user_external_cert_path is defined in a 2 cluster setup. Adding to this, the variable should be renamed to mds_superuser_cert_path and kafka_broker_cert_path should 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.