Skip to content

fix(two-node): support proxy env file in bare metal diagnostic mode#242

Open
dhensel-rh wants to merge 1 commit into
openshift-eng:mainfrom
dhensel-rh:fix/two-node-proxy-env
Open

fix(two-node): support proxy env file in bare metal diagnostic mode#242
dhensel-rh wants to merge 1 commit into
openshift-eng:mainfrom
dhensel-rh:fix/two-node-proxy-env

Conversation

@dhensel-rh

@dhensel-rh dhensel-rh commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add PROXY_ENV variable to diagnose-cluster.sh for bare metal deployments
  • On bare metal (direct access mode), oc commands source the proxy env file before running, instead of routing through a bastion host as in the hypervisor/VM path

Test plan

  • Run PROXY_ENV=/path/to/proxy.env ./diagnose-cluster.sh on a bare metal two-node cluster
  • Verify existing hypervisor mode is unaffected

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added support for an optional proxy environment configuration (via a file setting) for cluster diagnostics, so oc commands load proxy settings in direct SSH mode.
  • Documentation
    • Added a comprehensive README for the Two-Node “TNF Cluster Diagnostic” skill, including installation, required environment variables, what it diagnoses, and example commands.
  • Chores
    • Bumped the two-node plugin version to 1.1.1 (marketplace/manifest).

@openshift-ci

openshift-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhensel-rh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 56f94d0c-2393-468f-b935-3addfee4b6ad

📥 Commits

Reviewing files that changed from the base of the PR and between 8471075 and 9faff2a.

📒 Files selected for processing (4)
  • .claude-plugin/marketplace.json
  • plugins/two-node/.claude-plugin/plugin.json
  • plugins/two-node/scripts/diagnose-cluster.sh
  • plugins/two-node/skills/cluster-diagnostic/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • plugins/two-node/skills/cluster-diagnostic/README.md
  • .claude-plugin/marketplace.json
  • plugins/two-node/.claude-plugin/plugin.json
  • plugins/two-node/scripts/diagnose-cluster.sh

Walkthrough

The two-node diagnosis script adds optional PROXY_ENV support for direct-mode oc commands, and a README documents the cluster diagnostic skill. Marketplace and plugin manifest versions are updated from 1.1.0 to 1.1.1.

Changes

Two-node diagnostics

Layer / File(s) Summary
Direct oc proxy handling
plugins/two-node/scripts/diagnose-cluster.sh
Documents PROXY_ENV and sources an existing file before invoking oc, while preserving KUBECONFIG and default fallback behavior.
Cluster diagnostic skill documentation
plugins/two-node/skills/cluster-diagnostic/README.md
Documents the skill’s read-only diagnostics, access configuration, fencing and Pacemaker checks, and command examples.
Plugin metadata version alignment
.claude-plugin/marketplace.json, plugins/two-node/.claude-plugin/plugin.json
Updates the two-node plugin version from 1.1.0 to 1.1.1 in both metadata files.

Estimated code review effort: 2 (Simple) | ~5 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai-Attribution ⚠️ Warning PR mentions Claude Code, and HEAD uses Co-Authored-By: Claude Opus 4.6 instead of Red Hat AI attribution trailers. Replace the AI co-author trailer with Red Hat-compliant Assisted-by or Generated-by trailer, and remove Co-Authored-By for AI use.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main functional change: adding PROXY_ENV support in bare-metal diagnostic mode.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed Touched files add PROXY_ENV support and version bumps only; no MD5/SHA1/DES/RC4/ECB or custom crypto/secret comparisons appear.
Container-Privileges ✅ Passed The PR only changes a shell script, version metadata, and docs; no container/K8s manifests or privileged settings are introduced.
No-Sensitive-Data-In-Logs ✅ Passed PASS: The patch adds no new logging of secrets; PROXY_ENV is sourced silently and the README only shows placeholder examples, not real credentials.
No-Hardcoded-Secrets ✅ Passed No actual hardcoded secrets were added; the only password literal is a README placeholder example, not a real credential.
No-Injection-Vectors ✅ Passed The PR adds only source "$PROXY_ENV" with a file-existence check; none of the flagged patterns (eval/exec, shell=True, os.system, yaml.load, etc.) appear in the changed files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 markdownlint-cli2 (0.23.0)
plugins/two-node/skills/cluster-diagnostic/README.md

markdownlint-cli2 v0.23.0 (markdownlint v0.41.0)
Finding: :plugins/two-node/skills/cluster-diagnostic/README.md !node_modules/** !two-node-toolbox/**
Linting: 1 file(s)
Summary: 0 error(s)
AggregateError: Unable to import module 'markdownlint-cli2-formatter-pretty'.
at importModule (file:///usr/local/lib/node_modules/markdownlint-cli2/markdownlint-cli2.mjs:92:11)
at async Promise.all (index 0)
at async outputResults (file:///usr/local/lib/node_modules/markdownlint-cli2/markdownlint-cli2.mjs:879:9)
at async main (file:///usr/local/lib/node_modules/markdownlint-cli2/markdownlint-cli2.mjs:1072:5)
at async file:///usr/local/lib/node_modules/markdownlint-cli2/markdownlint-cli2-bin.mjs:14:22 {
[errors]: [
Error: Cannot find module 'markdownlint-cli2-formatter-pretty'
Require stack:
- /usr/local/lib/node_modules/markdownlint-cli2/node_modules/markdownlint/lib/resolve-module.cjs
at Module._resolveFilename (node:internal/modules/cjs/loader:1476:15)

... [truncated 1101 characters] ...

node:internal/modules/esm/resolve:271:11)
at moduleResolve (node:internal/modules/esm/resolve:861:10)
at defaultResolve (node:internal/modules/esm/resolve:988:11)
at #cachedDefaultResolve (node:internal/modules/esm/loader:697:20)
at #resolveAndMaybeBlockOnLoaderThread (node:internal/modules/esm/loader:714:38)
at ModuleLoader.resolveSync (node:internal/modules/esm/loader:746:52)
at #resolve (node:internal/modules/esm/loader:679:17)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:599:35)
at node:internal/modules/esm/loader:628:32
at TracingChannel.tracePromise (node:diagnostics_channel:362:14) {
code: 'ERR_MODULE_NOT_FOUND',
url: 'file:///markdownlint-cli2-formatter-pretty'
}
]
}


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/two-node/scripts/diagnose-cluster.sh`:
- Around line 87-88: Add a ShellCheck directive immediately before the dynamic
source in the PROXY_ENV execution branch, marking the intentionally arbitrary
sourced file as /dev/null so SC1090 is suppressed while preserving the existing
source-and-oc behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 6350d3dd-33f1-4469-9c31-34c65410c9c3

📥 Commits

Reviewing files that changed from the base of the PR and between 81f6a9f and 8cb4f52.

📒 Files selected for processing (1)
  • plugins/two-node/scripts/diagnose-cluster.sh

Comment thread plugins/two-node/scripts/diagnose-cluster.sh

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/two-node/skills/cluster-diagnostic/README.md`:
- Around line 64-65: Remove the hardcoded BMC_PASS=changeme assignment from the
README example and instruct users to provide BMC_PASS securely through their
external environment or secret management instead. Keep the BMC_USER example and
surrounding setup guidance intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 3c57b91e-4cfd-457a-a569-8d5daec7e227

📥 Commits

Reviewing files that changed from the base of the PR and between 2bcc7aa and 8471075.

📒 Files selected for processing (1)
  • plugins/two-node/skills/cluster-diagnostic/README.md

Comment thread plugins/two-node/skills/cluster-diagnostic/README.md Outdated
…ostic README

Add PROXY_ENV support to diagnose-cluster.sh so bare metal deployments
can source a proxy env file for oc commands instead of routing through
a bastion host. Add getting started guide for the cluster-diagnostic
skill covering setup, BMC debugging, and usage examples.

Also bumps two-node plugin version to 1.1.1.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dhensel-rh
dhensel-rh force-pushed the fix/two-node-proxy-env branch from 8471075 to 9faff2a Compare July 24, 2026 14:33
@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant