-
Notifications
You must be signed in to change notification settings - Fork 56
Use /tmp/kubeconfig to avoid modifying /opt/kubeconfig #1194
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: release-4.20
Are you sure you want to change the base?
Conversation
Currently, the crc-cluster-status.sh script logs into OpenShift using the kubeadmin account, which writes authentication tokens to the default kubeconfig file at /opt/kubeconfig. This results in kubeadmin token entries adding to the system:admin kubeconfig file, where the tokens eventually expire and may cause authentication or usability issues for system administrators. This PR use --kubeconfig=/tmp/kubeconfig to avoid modifying the /opt/kubeocnfig file.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe change adds the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
systemd/crc-cluster-status.sh (1)
32-36: Consider documenting /tmp/kubeconfig lifecycle.The kubeconfig will now persist at
/tmp/kubeconfigafter each successful login. Depending on your systemd/container setup, consider whether explicit cleanup is needed or if the standard/tmplifecycle is sufficient. This is a minor operational detail but worth noting if debugging or troubleshooting is needed later.If you'd like, I can help add a cleanup step (e.g.,
rm -f /tmp/kubeconfigor similar) either within this script or as part of post-execution housekeeping, depending on your operational requirements. Would you like me to suggest a solution?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
systemd/crc-cluster-status.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-02T12:58:51.079Z
Learnt from: kpouget
Repo: crc-org/snc PR: 1168
File: systemd/crc-pullsecret.sh:11-11
Timestamp: 2025-10-02T12:58:51.079Z
Learning: In systemd/crc-pullsecret.sh, the wait_for_resource_or_die secret call is intentionally checking that the Kubernetes secret API is operational (part of the OCP/K8s boot process), not waiting for a specific secret resource to exist. The script relies on subsequent oc commands and systemd retry logic to handle cases where specific resources aren't available yet.
Applied to files:
systemd/crc-cluster-status.sh
📚 Learning: 2025-10-07T07:03:06.884Z
Learnt from: kpouget
Repo: crc-org/snc PR: 1168
File: systemd/crc-self-sufficient-env.sh:10-17
Timestamp: 2025-10-07T07:03:06.884Z
Learning: In the crc-org/snc repository, the CRC_CLOUD environment variable has been removed from the codebase. The self-sufficient environment check in systemd/crc-self-sufficient-env.sh now only validates CRC_SELF_SUFFICIENT=1, not CRC_CLOUD.
Applied to files:
systemd/crc-cluster-status.sh
🔇 Additional comments (1)
systemd/crc-cluster-status.sh (1)
28-46: I'm unable to execute the verification scripts due to a repository clone failure. However, let me analyze the provided review comment and code context to determine if the verification is necessary and rewrite the comment appropriately.Based on the information provided in your review comment:
Code Change Analysis: The addition of
--kubeconfig=/tmp/kubeconfigflag is syntactically correct and properly placed in theoc logincommand.Review Comment Structure: The original review already identifies the key concern (downstream dependencies) and provides a verification script to check for them.
Available Context: The code snippet shows an isolated
try_login()function with no apparent external dependencies visible in the provided lines.Since I cannot execute the verification script on the actual repository, the original review comment's request for manual verification remains valid. However, I can refine it to be more actionable:
Verify no downstream code expects kubeadmin credentials in
/opt/kubeconfig.The
--kubeconfig=/tmp/kubeconfigflag correctly redirects kubeadmin token storage to temporary storage, aligning with the PR objective. The subshell withset +xandset +eproperly handles password masking and error isolation. However, confirm that no other systemd units, scripts, or tooling depend on the previous behavior where kubeadmin credentials were written to/opt/kubeconfig.Before approving, check:
- References to
/opt/kubeconfigin systemd configurations and scripts- Other uses of kubeadmin credentials that might expect the persistent location
- Cleanup mechanism for
/tmp/kubeconfigafter the login session
|
@praveenkumar: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Currently, the crc-cluster-status.sh script logs into OpenShift using the kubeadmin account, which writes authentication tokens to the default kubeconfig file at /opt/kubeconfig. This results in kubeadmin token entries adding to the system:admin kubeconfig file, where the tokens eventually expire and may cause authentication or usability issues for system administrators.
This PR use --kubeconfig=/tmp/kubeconfig to avoid modifying the /opt/kubeocnfig file.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.