Copilot commented on code in PR #6: URL: https://github.com/apache/ranger-tools/pull/6#discussion_r2655809236
########## docker/create_users_and_groups.sh: ########## @@ -0,0 +1,88 @@ +#!/bin/bash + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Script to create users and groups in ranger containers +# This script is designed to be run during container initialization + +# Function to create a group if it doesn't exist +create_group_if_not_exists() { + local groupname=$1 + + if ! getent group "$groupname" &>/dev/null; then + echo "Creating group: $groupname" + groupadd "$groupname" + echo "Group $groupname created successfully" + else + echo "Group $groupname already exists" + fi +} + +# Function to create a user for testing. +create_user_if_not_exists() { + local username=$1 + local home_dir=$2 + local primary_group=$3 + + if ! id "$username" &>/dev/null; then + echo "Creating user: $username" + useradd -g "$primary_group" -m -d "$home_dir" -s /bin/bash "$username" + + # Set a default password + echo "$username:$username" | chpasswd Review Comment: Setting default passwords to be the same as usernames (e.g., 'testuser1:testuser1') is a security vulnerability, especially for test users that will be included in production images. Even for testing purposes, consider using stronger default passwords or requiring password changes on first login, or document that these are test-only credentials that should be changed in production environments. ```suggestion # Configure password policy: # - If DEFAULT_USER_PASSWORD is set, use it and force a password change on first login. # - Otherwise, lock the account to avoid embedding a weak default password. if [ -n "${DEFAULT_USER_PASSWORD:-}" ]; then echo "$username:${DEFAULT_USER_PASSWORD}" | chpasswd # Expire password so user must change it at first login chage -d 0 "$username" else # Lock the account; an administrator must set a password explicitly if needed passwd -l "$username" fi ``` ########## docker/Dockerfile: ########## @@ -47,23 +47,13 @@ ENV RANGER_SCRIPTS=/home/ranger/scripts ENV RANGER_HOME=/opt/ranger ENV PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin -# setup groups, users, directories -RUN groupadd ranger \ - && for u in ranger rangeradmin rangerusersync rangertagsync rangerkms; do \ - useradd -g ranger -ms /bin/bash $u; \ - done - -RUN groupadd hadoop \ - && for u in hdfs yarn hive hbase kafka ozone; do \ - useradd -g hadoop -ms /bin/bash $u; \ - done - -RUN groupadd knox \ - && useradd -g knox -ms /bin/bash knox - # setup directories RUN mkdir -p /home/ranger/dist /home/ranger/scripts /opt/ranger && \ chown -R ranger:ranger /home/ranger /opt/ranger && \ chmod +rx /home/ranger /home/ranger/dist /home/ranger/scripts +# setup groups, users, directories +COPY create_users_and_groups.sh /${RANGER_SCRIPTS}/create_users_and_groups.sh +RUN chmod +x /${RANGER_SCRIPTS}/create_users_and_groups.sh && /${RANGER_SCRIPTS}/create_users_and_groups.sh && rm /${RANGER_SCRIPTS}/create_users_and_groups.sh Review Comment: The Dockerfile tries to set ownership to 'ranger:ranger' before the ranger user and group are created. The user creation script is executed after this chown command (lines 56-57), which will cause the chown command to fail. The directories should be created first, then users/groups should be created, and finally ownership should be set. ```suggestion chmod +rx /home/ranger /home/ranger/dist /home/ranger/scripts # setup groups, users, directories COPY create_users_and_groups.sh /${RANGER_SCRIPTS}/create_users_and_groups.sh RUN chmod +x /${RANGER_SCRIPTS}/create_users_and_groups.sh && \ /${RANGER_SCRIPTS}/create_users_and_groups.sh && \ chown -R ranger:ranger /home/ranger /opt/ranger && \ rm /${RANGER_SCRIPTS}/create_users_and_groups.sh ``` ########## docker/Dockerfile: ########## @@ -47,23 +47,13 @@ ENV RANGER_SCRIPTS=/home/ranger/scripts ENV RANGER_HOME=/opt/ranger ENV PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin -# setup groups, users, directories -RUN groupadd ranger \ - && for u in ranger rangeradmin rangerusersync rangertagsync rangerkms; do \ - useradd -g ranger -ms /bin/bash $u; \ - done - -RUN groupadd hadoop \ - && for u in hdfs yarn hive hbase kafka ozone; do \ - useradd -g hadoop -ms /bin/bash $u; \ - done - -RUN groupadd knox \ - && useradd -g knox -ms /bin/bash knox - # setup directories RUN mkdir -p /home/ranger/dist /home/ranger/scripts /opt/ranger && \ chown -R ranger:ranger /home/ranger /opt/ranger && \ chmod +rx /home/ranger /home/ranger/dist /home/ranger/scripts +# setup groups, users, directories +COPY create_users_and_groups.sh /${RANGER_SCRIPTS}/create_users_and_groups.sh +RUN chmod +x /${RANGER_SCRIPTS}/create_users_and_groups.sh && /${RANGER_SCRIPTS}/create_users_and_groups.sh && rm /${RANGER_SCRIPTS}/create_users_and_groups.sh Review Comment: The script is being copied to and executed from a path using the RANGER_SCRIPTS environment variable with a leading slash, but RANGER_SCRIPTS is already an absolute path (/home/ranger/scripts). This results in an incorrect path: //home/ranger/scripts/create_users_and_groups.sh. Remove the leading slash before ${RANGER_SCRIPTS}. ```suggestion COPY create_users_and_groups.sh ${RANGER_SCRIPTS}/create_users_and_groups.sh RUN chmod +x ${RANGER_SCRIPTS}/create_users_and_groups.sh && ${RANGER_SCRIPTS}/create_users_and_groups.sh && rm ${RANGER_SCRIPTS}/create_users_and_groups.sh ``` ########## docker/create_users_and_groups.sh: ########## @@ -0,0 +1,88 @@ +#!/bin/bash + +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Script to create users and groups in ranger containers +# This script is designed to be run during container initialization + +# Function to create a group if it doesn't exist +create_group_if_not_exists() { + local groupname=$1 + + if ! getent group "$groupname" &>/dev/null; then + echo "Creating group: $groupname" + groupadd "$groupname" + echo "Group $groupname created successfully" + else + echo "Group $groupname already exists" + fi +} + +# Function to create a user for testing. Review Comment: The function comment says "Function to create a user for testing" but this function is used to create all users (including production users like 'ranger', 'rangeradmin', 'hdfs', etc.), not just test users. Update the comment to accurately reflect that this is a general-purpose user creation function. ```suggestion # General-purpose function to create a user if it doesn't exist. ``` ########## docker/create_users_and_groups.sh: ########## @@ -0,0 +1,88 @@ +#!/bin/bash Review Comment: The script lacks error handling for critical operations. If useradd or groupadd commands fail (e.g., due to insufficient permissions or system errors), the script continues execution silently. Consider adding 'set -e' at the beginning of the script to exit on errors, or add explicit error checking after critical commands to ensure the container build fails if user/group creation fails. -- 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]
