Skip to content

Conversation

@praveenkumar
Copy link
Member

@praveenkumar praveenkumar commented Nov 27, 2025

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

  • Chores
    • Updated cluster authentication configuration to use an explicit kubeconfig path during login operations.

✏️ Tip: You can customize this high-level summary in your review settings.

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.
@openshift-ci openshift-ci bot requested review from anjannath and gbraad November 27, 2025 16:08
@openshift-ci
Copy link

openshift-ci bot commented Nov 27, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign anjannath for approval. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Walkthrough

The change adds the --kubeconfig=/tmp/kubeconfig flag to the oc login command within the try_login function in the systemd cluster status script, specifying an explicit kubeconfig path for the login operation.

Changes

Cohort / File(s) Summary
Login kubeconfig path isolation
systemd/crc-cluster-status.sh
Added --kubeconfig=/tmp/kubeconfig flag to the oc login invocation in the try_login function to isolate kubeadmin token handling to a temporary kubeconfig file

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single file with minimal flag parameter addition
  • No new logic or control flow changes
  • Straightforward configuration modification

Possibly related issues

Poem

🐰 A flag so small, yet oh so keen,
/tmp/kubeconfig makes tokens unseen,
One tiny change, security's gain,
Isolation flows through the login chain! 🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: using /tmp/kubeconfig instead of /opt/kubeconfig. It's specific, concise, and clearly communicates the primary purpose of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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/kubeconfig after each successful login. Depending on your systemd/container setup, consider whether explicit cleanup is needed or if the standard /tmp lifecycle 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/kubeconfig or 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2914074 and c44f7be.

📒 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:

  1. Code Change Analysis: The addition of --kubeconfig=/tmp/kubeconfig flag is syntactically correct and properly placed in the oc login command.

  2. Review Comment Structure: The original review already identifies the key concern (downstream dependencies) and provides a verification script to check for them.

  3. 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/kubeconfig flag correctly redirects kubeadmin token storage to temporary storage, aligning with the PR objective. The subshell with set +x and set +e properly 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/kubeconfig in systemd configurations and scripts
  • Other uses of kubeadmin credentials that might expect the persistent location
  • Cleanup mechanism for /tmp/kubeconfig after the login session

@openshift-ci
Copy link

openshift-ci bot commented Nov 27, 2025

@praveenkumar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-snc c44f7be link true /test e2e-snc

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant