Enhance Kubernetes configuration by setting kubeconfig permissions#3048
Enhance Kubernetes configuration by setting kubeconfig permissions#3048ks-ci-bot merged 1 commit intokubesphere:mainfrom
Conversation
- Added tasks to set permissions for the kubeconfig file in both the renewal and join processes, ensuring proper access control. - Introduced a function to retrieve the current local user, improving user context handling in the local connector. - Updated the SSH connector to pass the SUDO_USER environment variable, enhancing command execution context. These changes improve security and user management in Kubernetes operations. Signed-off-by: redscholar <blacktiledhouse@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: redscholar The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the security and user management aspects of Kubernetes operations. It introduces robust mechanisms for setting appropriate permissions on kubeconfig files during cluster setup and renewal, preventing unauthorized access. Furthermore, it refines how user context is handled within the system's connectors, ensuring that commands executed with Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
There was a problem hiding this comment.
Code Review
This pull request introduces changes to explicitly set the SUDO_USER environment variable for sudo commands executed via local and SSH connectors, and adds chown tasks in Kubernetes Ansible playbooks to manage kubeconfig file permissions. However, the local_connector.go implementation for setting SUDO_USER is problematic as command.SetEnv replaces the entire environment, potentially breaking sudo by removing essential variables like PATH. Furthermore, the chown commands in the Ansible playbooks contain a logical flaw where ~ expands to /root when run with sudo, causing both chown operations to target /root/.kube/config and thus making the intended user-specific permission change ineffective.
| klog.V(5).InfoS("exec local command", "cmd", cmd) | ||
| // in | ||
| command := c.Cmd.CommandContext(ctx, "sudo", "-SE", c.shell, "-c", cmd) | ||
| command.SetEnv([]string{"SUDO_USER=" + c.User}) |
There was a problem hiding this comment.
command.SetEnv replaces the entire environment for the child process. This will likely cause the sudo command to fail because essential environment variables like PATH will be missing. You should append to the current environment instead of replacing it to ensure the command executes in a valid context.
| command.SetEnv([]string{"SUDO_USER=" + c.User}) | |
| command.SetEnv(append(os.Environ(), "SUDO_USER="+c.User)) |
| chown "$SUDO_USER":root ~/.kube/config | ||
| chown root:root /root/.kube/config No newline at end of file |
There was a problem hiding this comment.
The chown command for the user's kubeconfig is flawed. When the command is executed with sudo, the shell runs as root, causing ~ to expand to /root. As a result, both chown commands in this task operate on the same file, /root/.kube/config. The second command immediately overwrites the first, making the permission change for $SUDO_USER ineffective. You should use a more reliable method to get the user's home directory.
if [ -n "$SUDO_USER" ] && [ "$SUDO_USER" != "root" ]; then
USER_HOME=$(getent passwd "$SUDO_USER" | cut -d: -f6)
if [ -n "$USER_HOME" ]; then
chown "$SUDO_USER":root "$USER_HOME"/.kube/config
fi
fi
chown root:root /root/.kube/config| chown "$SUDO_USER":root ~/.kube/config | ||
| chown root:root /root/.kube/config |
There was a problem hiding this comment.
This command has a logical flaw. When executed with sudo, the shell runs as root, so ~ expands to /root. This means both chown commands target /root/.kube/config. The second command (chown root:root ...) nullifies the effect of the first one. To correctly set permissions for the user who invoked sudo, you need to determine their home directory explicitly rather than using ~.
if [ -n "$SUDO_USER" ] && [ "$SUDO_USER" != "root" ]; then
USER_HOME=$(getent passwd "$SUDO_USER" | cut -d: -f6)
if [ -n "$USER_HOME" ]; then
chown "$SUDO_USER":root "$USER_HOME"/.kube/config
fi
fi
chown root:root /root/.kube/config


What type of PR is this?
/kind bug
What this PR does / why we need it:
These changes improve security and user management in Kubernetes operations.
Which issue(s) this PR fixes:
Fixes #
Special notes for reviewers:
Does this PR introduced a user-facing change?
Additional documentation, usage docs, etc.: