Copilot commented on code in PR #787:
URL: https://github.com/apache/ranger/pull/787#discussion_r2659970195
##########
dev-support/ranger-docker/scripts/knox/ranger-knox-setup.sh:
##########
@@ -25,6 +25,9 @@ EOF
if [ "${KERBEROS_ENABLED}" == "true" ]
then
${RANGER_SCRIPTS}/wait_for_keytab.sh knox.keytab
+ ${RANGER_SCRIPTS}/wait_for_keytab.sh testuser1.keytab
+ ${RANGER_SCRIPTS}/wait_for_keytab.sh testuser2.keytab
+ ${RANGER_SCRIPTS}/wait_for_keytab.sh testuser3.keytab
Review Comment:
The same three lines for waiting on test user keytabs are duplicated across
seven different service setup scripts. Consider extracting this into a reusable
helper function or script (e.g., wait_for_testuser_keytabs.sh) to improve
maintainability and reduce code duplication. This would make future changes to
test user management easier and less error-prone.
##########
dev-support/ranger-docker/scripts/kdc/entrypoint.sh:
##########
@@ -118,6 +126,7 @@ if [ ! -f $DB_DIR/principal ]; then
echo "Database initialized"
create_keytabs
+ create_testusers ranger ranger-audit ranger-hadoop ranger-hive ranger-hbase
ranger-kafka ranger-knox ranger-solr ranger-kms ranger-ozone ranger-trino
Review Comment:
The create_testusers function is called with ranger-solr as one of the
containers, but the ranger-solr setup script
(dev-support/ranger-docker/scripts/solr/ranger-solr.sh) has not been updated to
wait for the test user keytabs. If ranger-solr needs access to test user
keytabs, similar wait_for_keytab.sh calls should be added to its setup script
after line 25 for consistency with other services. If test users are not needed
for ranger-solr, it should be removed from the create_testusers call.
```suggestion
create_testusers ranger ranger-audit ranger-hadoop ranger-hive
ranger-hbase ranger-kafka ranger-knox ranger-kms ranger-ozone ranger-trino
```
##########
dev-support/ranger-docker/scripts/kdc/entrypoint.sh:
##########
@@ -102,6 +102,14 @@ function create_keytabs() {
create_principal_and_keytab HTTP ranger-solr
}
+function create_testusers() {
+ for container in "$@"; do
+ create_principal_and_keytab testuser1 $container
+ create_principal_and_keytab testuser2 $container
+ create_principal_and_keytab testuser3 $container
Review Comment:
The variable `$container` in the loop should be quoted to handle potential
whitespace or special characters in container names. While unlikely in this
specific context, it's a best practice for shell scripts to quote variable
expansions.
```suggestion
create_principal_and_keytab testuser1 "$container"
create_principal_and_keytab testuser2 "$container"
create_principal_and_keytab testuser3 "$container"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
