-
Notifications
You must be signed in to change notification settings - Fork 409
[Do-not-review][ANSIENG-4255] | Adding first class support for file based mds user store #1958
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.9.x
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
roles/variables/defaults/main.yml
Outdated
@@ -1559,7 +1559,16 @@ sso_cli: false | |||
### Device Authorization endpoint of Idp, Required to enable SSO in cli. | |||
sso_device_authorization_uri: none | |||
|
|||
### Authorization mode on all cp components. Possible values are ldap, oauth, ldap_with_oauth and none. Set this to oauth for OAuth cluster and ldap_with_oauth for cluster with both ldap and oauth support. When set to oauth or ldap_with_oauth, you must set oauth_jwks_uri, oauth_token_uri, oauth_issuer_url, oauth_superuser_client_id, oauth_superuser_client_password. | |||
### Path to the file containing the user store for MDS. This must be definde when auth_mode is set to file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo found: 'definde' should be 'defined'.
### Path to the file containing the user store for MDS. This must be definde when auth_mode is set to file. | |
### Path to the file containing the user store for MDS. This must be defined when auth_mode is set to file. |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's correct the typo here
confluent.metadata.server.user.store.file.path: /tmp/credentials | ||
confluent_metadata_server_user_store_file_src: /tmp/credentials | ||
confluent_metadata_server_user_store_file_dest_path: /etc/kafka | ||
confluent_metadata_server_user_store_file_remote_src: true # credentials file already on target node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test when credentials file is already on mds server
confluent.metadata.server.user.store.file.path: /tmp/credentials | ||
confluent_metadata_server_user_store_file_src: "{{ lookup('env', 'MOLECULE_SCENARIO_DIRECTORY') }}/credentials" | ||
confluent_metadata_server_user_store_file_dest_path: /etc/kafka | ||
confluent_metadata_server_user_store_file_remote_src: false # credentials file on ansible control node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to test when credentials file is on ansible control node
…ing config validations
… directory for keeping mds user store file
…on along with mds_file_based_user_store_enable
Description
To use file based mds user store earlier kafka broker custom properties needed to be used and the credentials file needed to be present on all brokers where mds was running.
Adding this first class support allows user to define these variables
confluent_metadata_server_user_store_file_src, confluent_metadata_server_user_store_file_dest_path, confluent_metadata_server_user_store_file_remote_src
and it would either send the credentials from ansible control node to broker or from one path to another in broker based onconfluent_metadata_server_user_store_file_remote_src
's value.Fixes # (issue)
Type of change
How Has This Been Tested?
semaphore
Checklist: