Copilot commented on code in PR #712:
URL: https://github.com/apache/ranger/pull/712#discussion_r2456009814
##########
security-admin/scripts/setup.sh:
##########
@@ -263,6 +263,16 @@ updatePropertyToFilePy(){
check_ret_status $? "Update property failed for: " $1
}
+#Update Properties to File if value is not empty
+#$1 -> propertyName $2 -> newPropertyValue $3 -> fileName
+updatePropertyToFilePyIfNotEmpty(){
+ if [ "${2}" != "" ]
Review Comment:
Use more robust shell string comparison with `-n` or `-z` operators instead
of comparing to empty string. Change to `if [ -n \"${2}\" ]` for better
portability and clarity.
```suggestion
if [ -n "${2}" ]
```
##########
security-admin/scripts/setup.sh:
##########
@@ -975,6 +985,15 @@ update_properties() {
fi
fi
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.loginModuleName' "$(get_prop_or_default
'audit_jaas_client_loginModuleName' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.loginModuleControlFlag' "$(get_prop_or_default
'audit_jaas_client_loginModuleControlFlag' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.useKeyTab' "$(get_prop_or_default
'audit_jaas_client_option_useKeyTab' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.storeKey' "$(get_prop_or_default
'audit_jaas_client_option_storeKey' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.useTicketCache' "$(get_prop_or_default
'audit_jaas_client_option_useTicketCache' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.serviceName' "$(get_prop_or_default
'audit_jaas_client_option_serviceName' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.keyTab' "$(get_prop_or_default
'audit_jaas_client_option_keyTab' $PROPFILE '')" $to_file_ranger
+ updatePropertyToFilePyIfNotEmpty
'xasecure.audit.jaas.Client.option.principal' "$(get_prop_or_default
'audit_jaas_client_option_principal' $PROPFILE '')" $to_file_ranger
Review Comment:
[nitpick] These 8 consecutive property updates follow an identical pattern
and could be refactored into a loop or function that accepts an array of
property name mappings. This would reduce code duplication and make future
additions easier to maintain.
```suggestion
# Refactored property updates using a loop for maintainability
declare -a jaas_properties=(
"xasecure.audit.jaas.Client.loginModuleName
audit_jaas_client_loginModuleName"
"xasecure.audit.jaas.Client.loginModuleControlFlag
audit_jaas_client_loginModuleControlFlag"
"xasecure.audit.jaas.Client.option.useKeyTab
audit_jaas_client_option_useKeyTab"
"xasecure.audit.jaas.Client.option.storeKey
audit_jaas_client_option_storeKey"
"xasecure.audit.jaas.Client.option.useTicketCache
audit_jaas_client_option_useTicketCache"
"xasecure.audit.jaas.Client.option.serviceName
audit_jaas_client_option_serviceName"
"xasecure.audit.jaas.Client.option.keyTab
audit_jaas_client_option_keyTab"
"xasecure.audit.jaas.Client.option.principal
audit_jaas_client_option_principal"
)
for mapping in "${jaas_properties[@]}"; do
prop_name=$(echo "$mapping" | awk '{print $1}')
var_name=$(echo "$mapping" | awk '{print $2}')
prop_value="$(get_prop_or_default "$var_name" $PROPFILE '')"
updatePropertyToFilePyIfNotEmpty "$prop_name" "$prop_value"
$to_file_ranger
done
```
--
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]
