build: upgrade to Go 1.26.5 - #1313
Conversation
also, convert all `.adoc` files to `.md` Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔀 Multi-repo context codeready-toolchain/api, codeready-toolchain/toolchain-common, codeready-toolchain/host-operatorLinked repositories findings
🔇 Additional comments (1)
WalkthroughThe PR replaces repository AsciiDoc guidance with Markdown, adds E2E and performance-testing instructions, updates Go 1.26 configuration and terminal APIs, adds a Go formatting target, and removes vulnerability suppressions. ChangesRepository refresh
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
chore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@multicluster_setup.md`:
- Line 93: Update the ksctl adm register-member command to pass the previously
defined HOST_CONFIG and MEMBER_CONFIG variables directly to --host-kubeconfig
and --member-kubeconfig, replacing the hardcoded HOME-based paths.
In `@openshift_local.md`:
- Line 184: Update the oc login command in openshift_local.md to remove the -p
${KUBEADMIN_PASSWORD} password argument; use interactive password prompting or
token-based authentication instead, ensuring credentials are not exposed in
process arguments or shell history.
In `@README.md`:
- Line 87: Update the README examples using os.Setenv for KUBECONFIG to replace
the tilde-prefixed paths with concrete absolute paths, including both referenced
examples, so they demonstrate a path format that os.Setenv resolves literally.
- Line 10: Replace the ambiguous Go prerequisite wording with “Go 1.26.5 or
newer” in README.md at lines 10-10 and required_tools.md at lines 3-3, keeping
the existing download link where applicable.
In `@setup/README.md`:
- Line 120: The setup README documents an unsupported --skip-csvgen option.
Update the Note 3 instruction to use a flag registered by the root command, or
add/register --skip-csvgen in the root command if CSV generation must be
independently skipped; do not replace it with --skip-install-operators unless
that broader behavior is intended.
- Around line 9-11: Update the setup procedure around the OCP provisioning
instructions to require recording the exact OCP 4.21.x release,
openshift-install version, and template commit or tag used for the benchmark
baseline; replace unpinned “latest” or master references with those recorded,
reproducible inputs.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 9b5ddf72-94c4-4efd-8d0e-6cc2abfde578
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
.govulncheck.yamlREADME.adocREADME.mdgo.modmulticluster_setup.mdopenshift_local.mdquay.adocquay.mdrequired_tools.adocrequired_tools.mdsetup/README.adocsetup/README.mdsetup/cmd/root.gosetup/results/results.gosetup/terminal/terminal.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
💤 Files with no reviewable changes (4)
- required_tools.adoc
- quay.adoc
- README.adoc
- setup/README.adoc
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Unit Tests
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
quay.mdsetup/cmd/root.gosetup/results/results.gorequired_tools.mdopenshift_local.mdgo.modsetup/README.mdsetup/terminal/terminal.gomulticluster_setup.mdREADME.md
🪛 GitHub Actions: ci-check-gomod / 0_go.mod replacements.txt
go.mod
[error] 1-1: CI validation failed: protected Go modules have disallowed replacements. Found replacements for github.com/codeready-toolchain/api and github.com/codeready-toolchain/toolchain-common pointing to github.com/xcoulon. Remove or update these replacements in go.mod. Command failed with exit code 1.
🪛 GitHub Actions: ci-check-gomod / go.mod replacements
go.mod
[error] 1-1: Protected Go modules have disallowed replacements: github.com/codeready-toolchain/api => github.com/xcoulon/api and github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common. The validation command failed with exit code 1.
🪛 LanguageTool
quay.md
[style] ~4-~4: Consider using a different verb to strengthen your wording.
Context: ... operators to OpenShift cluster. Please make sure that the repositories exist in your Qua...
(MAKE_SURE_ENSURE)
openshift_local.md
[style] ~4-~4: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...fter the release. Because of this it is very important to always run the latest release of Ope...
(EN_WEAK_ADJECTIVE)
[style] ~50-~50: Consider a more concise word here.
Context: ...cleanup`: bash crc setup Also, in order to run the tests seamlessly and without an...
(IN_ORDER_TO_PREMIUM)
[style] ~64-~64: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...rt the cluster. The first time you will need to provide the pull secret you were presen...
(REP_NEED_TO_VB)
README.md
[grammar] ~14-~14: Use a hyphen to join words.
Context: ...ub.com/golang/go/wiki/Modules). ## Step by step guide - running in CodeReady Contai...
(QB_NEW_EN_HYPHEN)
[style] ~67-~67: Use ‘will’ instead of ‘going to’ if the following action is certain.
Context: ...espace names where the end-to-end tests are going to be executed - eg.: `make test-e2e HOST_...
(GOING_TO_WILL)
[style] ~76-~76: Consider a more concise word here.
Context: ...ning/Debugging e2e tests from your IDE In order to run/debug tests from your IDE you'll ne...
(IN_ORDER_TO_PREMIUM)
[grammar] ~77-~77: Ensure spelling is correct
Context: ...es and the other toolchain resources in you cluster. Following snippet of code shou...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~126-~126: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...lchain-e2eandhost-operator`, so you need to run e2e tests against your current code...
(REP_NEED_TO_VB)
[style] ~128-~128: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...operatorandmember-operator`, so you need to run e2e tests against your current code...
(REP_NEED_TO_VB)
🪛 markdownlint-cli2 (0.23.2)
required_tools.md
[warning] 7-7: Link text should be descriptive
(MD059, descriptive-link-text)
setup/README.md
[warning] 71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 83-83: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 112-112: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 147-147: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 163-163: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 169-169: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
README.md
[warning] 10-10: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 50-50: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 68-68: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 70-70: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 72-72: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 159-159: Blank line inside blockquote
(MD028, no-blanks-blockquote)
[warning] 161-161: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🪛 OSV Scanner (2.4.0)
go.mod
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking key constraints not enforced in golang.org/x/crypto/ssh/agent
(GO-2026-5005)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking agent constraints dropped when forwarding keys in golang.org/x/crypto/ssh/agent
(GO-2026-5006)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking byte arithmetic causes underflow and panic in golang.org/x/crypto/ssh
(GO-2026-5013)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking bypass of certificate restrictions in golang.org/x/crypto/ssh
(GO-2026-5014)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking server panic during CheckHostKey/Authenticate in golang.org/x/crypto/ssh
(GO-2026-5015)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking memory leak when rejecting channels can lead to DoS in golang.org/x/crypto/ssh
(GO-2026-5016)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking client can cause server deadlock on unexpected responses in golang.org/x/crypto/ssh
(GO-2026-5017)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking pathological RSA/DSA parameters may cause DoS in golang.org/x/crypto/ssh
(GO-2026-5018)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking bypass of FIDO/U2F security keys physical interaction in golang.org/x/crypto/ssh
(GO-2026-5019)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking infinite loop on large channel writes in golang.org/x/crypto/ssh
(GO-2026-5020)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking auth bypass via unenforced @revoked status in golang.org/x/crypto/ssh/knownhosts
(GO-2026-5021)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking VerifiedPublicKeyCallback permissions skip enforcement in golang.org/x/crypto/ssh
(GO-2026-5023)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: Invoking pathological inputs can lead to client panic in golang.org/x/crypto/ssh/agent
(GO-2026-5033)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: The golang.org/x/crypto/openpgp package is unmaintained, unsafe by design, and has known security issues
(GO-2026-5932)
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto vulnerable to invoking bypass of certificate restrictions
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto vulnerable to auth bypass via unenforced @revoked status
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto is vulnerable to invoking server panic during CheckHostKey/Authenticate flow
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: FIDO/U2F security key physical presence check can be bypassed
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking pathological inputs can lead to client panic
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto doesn't drop invoking agent constraints when forwarding keys
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto doesn't enforce invoking key constraints
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking byte arithmetic causes underflow and panic
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking memory leak when rejecting channels can lead to DoS
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto vulnerable to infinite loop on large channel writes
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking client can cause server deadlock on unexpected responses
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking pathological RSA/DSA parameters may cause DoS
[CRITICAL] 119-119: golang.org/x/crypto 0.51.0: golang.org/x/crypto: Invoking VerifiedPublicKeyCallback permissions skip enforcement
🔇 Additional comments (16)
README.md (1)
1-9: LGTM!Also applies to: 11-64, 66-86, 88-106, 108-177
openshift_local.md (1)
1-183: LGTM!Also applies to: 185-216
multicluster_setup.md (1)
1-92: LGTM!Also applies to: 94-128
quay.md (1)
1-2: LGTM!Also applies to: 5-42
required_tools.md (1)
1-2: LGTM!Also applies to: 4-16
.govulncheck.yaml (1)
1-1: LGTM!go.mod (3)
3-6: LGTM!Also applies to: 38-41, 147-147
120-125: LGTM!
119-119: 🔒 Security & PrivacyReproduce the reported
golang.org/x/crypto v0.51.0advisories.The dependency is indirect, and the queried OSV database reports no vulnerabilities for this version. Do not require an upgrade based only on the current finding.
setup/terminal/terminal.go (2)
60-60: LGTM!Also applies to: 71-81, 90-109, 118-118
19-26: 🗄️ Data Integrity & IntegrationNo alternate
Terminalimplementations or test doubles require updates.> Likely an incorrect or invalid review comment.setup/cmd/root.go (1)
156-160: LGTM!setup/results/results.go (1)
85-85: LGTM!setup/README.md (3)
1-8: LGTM!Also applies to: 12-48, 55-59, 61-119, 121-172
54-54: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-295): Improper Certificate Validation
Reachability: External
Do not recommend disabling TLS verification.
If certificate validation fails, configure the trusted cluster CA or correct the server configuration. Do not add
--insecure-skip-tls-verify=truewhile sending the bearer token. A network attacker could impersonate the API server and capture the token.
49-53: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-522): Insufficiently Protected Credentials
Reachability: External
Keep the API token out of command-line arguments.
The instructions tell users to run
oc login --token=<token>and copy that command. A real token can remain in shell history and can be read from process arguments. The setup workflow later retrieves the active token withoc whoami -tinsetup/auth/token.go:25-33.Document a supported kubeconfig or interactive credential flow that does not expose the token in
argv.
| ---- | ||
| ```bash | ||
| # create/configure the ToolchainCluster resources on host and member clusters | ||
| $ ksctl adm register-member --host-ns="${HOST_NS}" --member-ns="${MEMBER_NS}" --host-kubeconfig="${HOME}/.kube/host-config" --member-kubeconfig="${HOME}/.kube/member-config" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the multicluster_setup.md file
fd -type f -name "multicluster_setup.md"Repository: codeready-toolchain/toolchain-e2e
Length of output: 404
🏁 Script executed:
#!/bin/bash
# Locate the multicluster_setup.md file
find . -name "multicluster_setup.md" -type fRepository: codeready-toolchain/toolchain-e2e
Length of output: 195
🏁 Script executed:
#!/bin/bash
# Read the multicluster_setup.md file to check context around line 93
wc -l multicluster_setup.mdRepository: codeready-toolchain/toolchain-e2e
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Read the entire multicluster_setup.md file
cat -n multicluster_setup.mdRepository: codeready-toolchain/toolchain-e2e
Length of output: 6354
Use the kubeconfig variables defined earlier in the procedure.
The procedure defines ${HOST_CONFIG} and ${MEMBER_CONFIG} to store arbitrary kubeconfig paths (line 10). The ksctl adm register-member command on line 93 uses hardcoded paths ${HOME}/.kube/host-config and ${HOME}/.kube/member-config instead. Users must place their kubeconfigs at those exact paths for registration to succeed, even if they were configured elsewhere. Pass the variables directly to --host-kubeconfig and --member-kubeconfig instead.
Proposed fix
-ksctl adm register-member --host-ns="${HOST_NS}" --member-ns="${MEMBER_NS}" --host-kubeconfig="${HOME}/.kube/host-config" --member-kubeconfig="${HOME}/.kube/member-config"
+ksctl adm register-member --host-ns="${HOST_NS}" --member-ns="${MEMBER_NS}" --host-kubeconfig="${HOST_CONFIG}" --member-kubeconfig="${MEMBER_CONFIG}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $ ksctl adm register-member --host-ns="${HOST_NS}" --member-ns="${MEMBER_NS}" --host-kubeconfig="${HOME}/.kube/host-config" --member-kubeconfig="${HOME}/.kube/member-config" | |
| $ ksctl adm register-member --host-ns="${HOST_NS}" --member-ns="${MEMBER_NS}" --host-kubeconfig="${HOST_CONFIG}" --member-kubeconfig="${MEMBER_CONFIG}" |
🤖 Prompt for 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.
In `@multicluster_setup.md` at line 93, Update the ksctl adm register-member
command to pass the previously defined HOST_CONFIG and MEMBER_CONFIG variables
directly to --host-kubeconfig and --member-kubeconfig, replacing the hardcoded
HOME-based paths.
| [source,bash] | ||
| ---- | ||
| ```bash | ||
| oc login -u kubeadmin -p ${KUBEADMIN_PASSWORD} https://api.crc.testing:6443 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the file exists and check line 184 and surrounding context
if [ -f "openshift_local.md" ]; then
sed -n '180,190p' openshift_local.md
else
echo "File not found"
fi
</parameter>Repository: codeready-toolchain/toolchain-e2e
Length of output: 645
🌐 Web query:
oc login password command-line arguments security exposure
💡 Result:
Using the --password flag with the oc login command is considered a security risk and is generally discouraged [1]. While the functionality is provided in the OpenShift CLI to allow for automation in specific, controlled environments, it exposes your credentials to potential discovery [1]. The primary security exposures associated with using command-line arguments for passwords include: 1. Process Table Exposure: On many multi-user operating systems, command-line arguments are visible to other users on the system. Tools like ps (process status) can display the full command lines of running processes, potentially revealing your password to anyone with permission to list active processes [1]. 2. Shell History: When you run a command in a terminal, it is often recorded in your shell history file (e.g.,.bash_history,.zsh_history). If you include a password as an argument, it will be stored in plain text in these files, making it accessible to anyone who gains access to your user account or the history file [1]. Best Practices: - Prefer Interactive Login: By running oc login without the password flag, the CLI will prompt you for input. This interaction ensures the password is not echoed to the terminal or saved in command history, and it is not exposed via the process table [2][3]. - Use Tokens: For automation, CI/CD pipelines, or scripts, avoid using passwords entirely. Instead, use an authentication token or a service account token, which can be managed more securely and revoked if necessary [4][5]. - Environment Security: If you must automate, consider using environment variables or configuration files that are strictly protected with restrictive file permissions, rather than passing sensitive information directly on the command line [1]. In summary, while oc login supports the --password flag for convenience, its use in interactive sessions or untrusted environments is a security vulnerability [1]. Always prioritize interactive entry or secure token-based authentication to protect your credentials [2][3].
Citations:
- 1: Accepting a password on the command line is unsafe openshift/origin#22997
- 2: https://docs.redhat.com/fr/documentation/openshift_container_platform/3.10/html/developer_guide/dev-guide-authentication
- 3: https://docs.redhat.com/en/documentation/openshift_container_platform/3.11/html/developer_guide/dev-guide-authentication
- 4: https://github.com/openshift/oc/blob/master/pkg/cli/login/login.go
- 5: https://goto/?url=CAESdgHuR6pNXlc-TQNdmSSis8EITRg3Sw7hyx5dKHZC5aPfB7LDCiV3nsaUPxmy9z9Igu6unxdqPjJ7uW5gXjtPCWfYqRUgJY-XnabSZqjvp1hdlq8gMX-bba3ObAFqOR0Fgn0EJsINu2uhfN3rkOo5mp8qZOGoA0Q%3D
🌐 Web query:
oc login stdin password input method security
💡 Result:
Using the --password flag with the oc login command is considered insecure and is generally discouraged because it exposes credentials in plain text to the command line environment [1]. When you pass a password via command line arguments, it can often be observed by other users on the same system through process monitoring tools like ps [1]. While the oc login command supports providing a password via the --password flag [2][3][4], security best practices recommend avoiding this method in shared or automated environments. If you need to authenticate non-interactively, the following alternatives are generally preferred: 1. Use an existing session token: If you have already authenticated, you can use the token directly with oc login --token= or configure it via your kubeconfig [4]. 2. Use credential helpers or OIDC flows: Modern OpenShift clusters often support OIDC-based authentication or external credential plugins that manage tokens more securely without requiring you to handle raw passwords manually [4]. 3. Interactive prompt: The most secure default is to run oc login without providing a password, allowing the CLI to prompt you for input securely, which prevents the password from appearing in process lists or shell history files [5][6][7]. If you are writing scripts, prioritize using short-lived tokens or service account tokens rather than static user passwords [4]. If you must use a password and are concerned about security, ensure the process environment is restricted and consider using tools like secret managers to inject credentials safely rather than passing them as plain-text arguments.
Citations:
- 1: Accepting a password on the command line is unsafe openshift/origin#22997
- 2: https://github.com/openshift/oc/blob/master/pkg/cli/login/login.go
- 3: https://github.com/ardaguclu/oc/blob/v4.2.0-alpha.0/pkg/cli/login/login.go
- 4: https://manpages.opensuse.org/Tumbleweed/oc/oc-login.1.en.html
- 5: https://docs.redhat.com/en/documentation/openshift_container_platform/3.11/html/developer_guide/dev-guide-authentication
- 6: https://docs.redhat.com/en/documentation/openshift_container_platform/3.0/single/cli_reference/index
- 7: https://github.com/openshift/origin/blob/master/docs/cli.md
Sensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal
Remove the password from command-line arguments to prevent credential exposure.
The shell expands ${KUBEADMIN_PASSWORD} before oc login starts. The password then appears in the oc process arguments. Other users and processes on the local machine can read command-line arguments using tools like ps. This also stores the password in your shell history file in plain text.
Omit the -p flag and allow oc to prompt interactively for the password, or use token-based authentication instead:
Proposed fix
-oc login -u kubeadmin -p ${KUBEADMIN_PASSWORD} https://api.crc.testing:6443
+oc login -u kubeadmin https://api.crc.testing:6443For non-interactive authentication, use a token instead:
oc login --token=<service-account-token> https://api.crc.testing:6443📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| oc login -u kubeadmin -p ${KUBEADMIN_PASSWORD} https://api.crc.testing:6443 | |
| oc login -u kubeadmin https://api.crc.testing:6443 |
🤖 Prompt for 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.
In `@openshift_local.md` at line 184, Update the oc login command in
openshift_local.md to remove the -p ${KUBEADMIN_PASSWORD} password argument; use
interactive password prompting or token-based authentication instead, ensuring
credentials are not exposed in process arguments or shell history.
|
|
||
| ## Build | ||
|
|
||
| Requires Go version 1.26.x (1.26.5 or higher) - download for your development environment [here](https://golang.org/dl/). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- README.md ---'
cat -n README.md | sed -n '1,20p'
printf '%s\n' '--- required_tools.md ---'
cat -n required_tools.md | sed -n '1,12p'
printf '%s\n' '--- Go-version references ---'
rg -n -i --glob '!vendor/**' --glob '!node_modules/**' 'go[[:space:]_-]*1\.[0-9]+(\.[0-9]+)?|golang|go-version|go version|go\.mod' .
printf '%s\n' '--- repository metadata ---'
find . -maxdepth 2 -type f \( -name 'go.mod' -o -name '.tool-versions' -o -name 'go.work' -o -name 'Dockerfile*' \) -printRepository: codeready-toolchain/toolchain-e2e
Length of output: 34298
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- go.mod version directives ---'
cat -n go.mod | sed -n '1,12p'
printf '%s\n' '--- all prerequisite-document references ---'
rg -n -i --glob '*.md' --glob '*.yaml' --glob '*.yml' --glob '*.toml' --glob 'Dockerfile*' '1\.26(\.[0-9]+)?|go[[:space:]_-]*version|toolchain go' .
printf '%s\n' '--- deterministic comparison ---'
python3 - <<'PY'
from pathlib import Path
import re
docs = {name: Path(name).read_text() for name in ("README.md", "required_tools.md")}
for name, text in docs.items():
matches = re.findall(r'(?i)(?:go\s+version\s+|[*]\s*go\s+)([^)\n.]+(?:\.[^)\n ]+)*)', text)
print(name, matches)
mod = Path("go.mod").read_text()
print("go directive:", re.search(r'(?m)^go\s+(\S+)', mod).group(1))
print("toolchain directive:", re.search(r'(?m)^toolchain\s+(\S+)', mod).group(1))
PYRepository: codeready-toolchain/toolchain-e2e
Length of output: 1348
🏁 Script executed on selected repositories:
#!/bin/bash
printf '%s\n' '--- related repository Go directives ---'
for f in $(find . -name go.mod -type f); do
printf '%s: ' "$f"
rg -n '^(go|toolchain) ' "$f" || true
done
Repositories: codeready-toolchain/api, codeready-toolchain/toolchain-common, codeready-toolchain/host-operator
Length of output: 905
Use one unambiguous Go prerequisite in both documents. Use Go 1.26.5 or newer in README.md and required_tools.md. The current 1.26.x (1.26.5 or higher) wording mixes a fixed patch series with an open-ended range.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 10-10: Link text should be descriptive
(MD059, descriptive-link-text)
📍 Affects 2 files
README.md#L10-L10(this comment)required_tools.md#L3-L3
🤖 Prompt for 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.
In `@README.md` at line 10, Replace the ambiguous Go prerequisite wording with “Go
1.26.5 or newer” in README.md at lines 10-10 and required_tools.md at lines 3-3,
keeping the existing download link where applicable.
| os.Setenv("MEMBER_NS_2","toolchain-member2-18161051") | ||
| os.Setenv("HOST_NS","toolchain-host-18161051") | ||
| os.Setenv("REGISTRATION_SERVICE_NS","toolchain-host-18161051") | ||
| os.Setenv("KUBECONFIG", "~/aws-cluster-test/my-devsandbox/auth/kubeconfig") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine README.md
fd README.md -type f
# Then check the specific lines mentioned
cat -n README.md | sed -n '80,115p'Repository: codeready-toolchain/toolchain-e2e
Length of output: 1910
🏁 Script executed (no clone):
# Verify os.Setenv behavior with tilde expansion in Go
python3 - <<'PY'
# os.Setenv in Go does not expand ~ like the shell does
# Let's verify this is true by checking Go documentation and behavior
info = """
In Go, os.Setenv() sets an environment variable to a literal string value.
The tilde (~) character is NOT expanded by os.Setenv().
Only the shell expands ~ to the user's home directory during command parsing.
When os.Setenv("KUBECONFIG", "~/path") is called:
- The environment variable KUBECONFIG gets the literal string "~/path"
- This is NOT the same as the user's home directory path
- Programs that read KUBECONFIG may not recognize ~/path as a valid path
- The kubectl tool, for example, does expand ~, but other tools may not
This is a real issue if the test framework or tooling doesn't expand ~.
"""
print(info)
PY
Length of output: 733
Use absolute paths for KUBECONFIG in documentation examples.
os.Setenv does not expand the tilde (~) character. The example code at lines 87 and 107 sets a literal tilde-prefixed path, which does not resolve to the user's home directory. Developers copying this example may not realize that ~/path fails when used with os.Setenv. Replace the example with an absolute path to clarify the correct usage pattern.
Suggested fix
Replace the tilde path with a concrete absolute path in the example (e.g., /home/user/aws-cluster-test/my-devsandbox/auth/kubeconfig or suggest developers compute the path with os.UserHomeDir() and filepath.Join()).
🤖 Prompt for 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.
In `@README.md` at line 87, Update the README examples using os.Setenv for
KUBECONFIG to replace the tilde-prefixed paths with concrete absolute paths,
including both referenced examples, so they demonstrate a path format that
os.Setenv resolves literally.
| 2. Provision the **latest available** GA version of **OCP 4.21.x** on AWS with sufficient resources: 3 `m5.8xlarge` master nodes and 3 `m5.2xlarge` worker nodes. | ||
|
|
||
| The latest version of openshift-install can be downloaded from https://mirror.openshift.com/pub/openshift-v4/clients/ocp/ |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Pin the benchmark inputs.
The procedure uses the latest OCP 4.21.x release, the latest installer, and a template from master. These inputs can change after this document is merged. Performance results can then differ from the baseline for reasons unrelated to the operator.
Record the exact OCP release, openshift-install version, and template commit or tag used for the baseline.
Also applies to: 60-60
🤖 Prompt for 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.
In `@setup/README.md` around lines 9 - 11, Update the setup procedure around the
OCP provisioning instructions to require recording the exact OCP 4.21.x release,
openshift-install version, and template commit or tag used for the benchmark
baseline; replace unpinned “latest” or master references with those recorded,
reproducible inputs.
|
|
||
| Note 2: The `--workloads` flag tells the tool to capture the CPU and memory of a deployment and include the results in the summary upon completion of the setup. Use this for including any deployments related to the onboarding operator. The format must follow `--workloads namespace:name` | ||
|
|
||
| Note 3: CSV resources are automatically created for each default user as well. An all-namespaces scoped operator will be installed as part of the 'preparing' step. This operator will create a CSV resource in each namespace to mimic the behaviour observed in the production cluster. This operator install step can be skipped with the `--skip-csvgen` flag but should not be skipped without good reason. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Replace the unsupported --skip-csvgen flag.
The documented flag is not registered in setup/cmd/root.go:61-94. That command defines --skip-install-operators, but it does not define --skip-csvgen. A user who follows this instruction will receive an unknown-flag error.
Add the intended flag or update the note to use a supported option. Do not substitute --skip-install-operators unless skipping all operator installation is intended.
🤖 Prompt for 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.
In `@setup/README.md` at line 120, The setup README documents an unsupported
--skip-csvgen option. Update the Note 3 instruction to use a flag registered by
the root command, or add/register --skip-csvgen in the root command if CSV
generation must be independently skipped; do not replace it with
--skip-install-operators unless that broader behavior is intended.
|
|
||
| replace github.com/codeready-toolchain/api => github.com/xcoulon/api v0.0.0-20260805080027-7512d38caa3b | ||
|
|
||
| replace github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common v0.0.0-20260805080255-7db4efab1c46 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MatousJobanek, MikelAlejoBR, xcoulon 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 |
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
There was a problem hiding this comment.
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 `@make/go.mk`:
- Around line 21-27: Update the check-go-format recipe around the gofmt find
command to redirect its output directly to $(OUT_DIR)/gofmt-errors instead of
using tee | read, then test the file with [ -s "$(OUT_DIR)/gofmt-errors" ]
before printing the error and exiting. Remove the trailing || true so find,
gofmt, and output-file write failures propagate and cause the target to fail.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 97122fe3-aa76-4c3e-85c1-95d01bdb3752
📒 Files selected for processing (3)
gofmt_excludemake/format.mkmake/go.mk
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/api(manual) → reviewed against open PR#521upgrade-go-1.26-apiinstead of the default branchcodeready-toolchain/toolchain-common(manual) → reviewed against open PR#543upgrade-go-1.26-commoninstead of the default branchcodeready-toolchain/host-operator(manual) → reviewed against open PR#1290upgrade-go-1.26-hostinstead of the default branchcodeready-toolchain/toolchain-e2e(manual)
💤 Files with no reviewable changes (2)
- gofmt_exclude
- make/format.mk
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
make/go.mk
🔇 Additional comments (1)
make/go.mk (1)
16-16: 🎯 Functional CorrectnessKeep the existing
find -exec ... +invocation.GNU
findreturns a non-zero status when the batchedgofmtcommand fails, somake format-go-codedoes not hide this failure.> Likely an incorrect or invalid review comment.
| $(Q)find . -name '*.go' -not -path '*/vendor/*' -not -path '*/.git/*' -exec gofmt -s -l {} + 2>&1 \ | ||
| | tee $(OUT_DIR)/gofmt-errors \ | ||
| | read \ | ||
| && echo "ERROR: These files differ from gofmt's style (run 'make format-go-code' to fix this):" \ | ||
| && cat $(OUT_DIR)/gofmt-errors \ | ||
| && exit 1 \ | ||
| || true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -u
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
set +e
bash -c 'false | tee "$1" | read || true' _ "$tmp/errors"
status=$?
set -e
printf 'status=%s\n' "$status"Repository: codeready-toolchain/toolchain-e2e
Length of output: 214
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- make/go.mk ---'
cat -n make/go.mk | sed -n '1,55p'
printf '%s\n' '--- shell configuration ---'
rg -n '^(SHELL|\.SHELLFLAGS|MAKEFLAGS)|gofmt-errors|format-go-code' Makefile make .github 2>/dev/null || true
printf '%s\n' '--- available tools ---'
command -v bash || true
command -v sh || true
command -v tee || true
command -v busybox || trueRepository: codeready-toolchain/toolchain-e2e
Length of output: 2762
🏁 Script executed (no clone):
#!/usr/bin/env bash
set -u
printf '%s\n' '--- pipeline behavior with available shell tools ---'
for shell in bash sh; do
if command -v "$shell" >/dev/null 2>&1; then
"$shell" -c '
set +e
tmp=$(mktemp -d)
trap "rm -rf \"$tmp\"" EXIT
command -v tee >/dev/null 2>&1
tee_available=$?
false | tee "$tmp/errors" | read
pipeline_status=$?
false | tee "$tmp/errors-guarded" | read || true
guarded_status=$?
printf "shell=%s tee_available=%s pipeline_status=%s guarded_status=%s file_size=%s\n" \
"$0" "$tee_available" "$pipeline_status" "$guarded_status" \
"$(wc -c < "$tmp/errors-guarded" 2>/dev/null || printf unavailable)"
' "$shell"
fi
done
printf '%s\n' '--- shell command lookup ---'
type -a tee 2>&1 || true
Length of output: 705
🏁 Script executed:
#!/usr/bin/env bash
set -u
python3 - <<'PY'
from pathlib import Path
p = Path("make/go.mk")
text = p.read_text()
start = text.find("gofmt-errors")
print("gofmt occurrence count:", text.count("gofmt-errors"))
for i, line in enumerate(text.splitlines(), 1):
if "gofmt-errors" in line or "format-go-code" in line:
print(f"{i}: {line}")
PYRepository: codeready-toolchain/toolchain-e2e
Length of output: 423
🏁 Script executed (no clone):
#!/usr/bin/env bash
set -u
python3 - <<'PY'
import os
import shutil
import subprocess
import tempfile
print("tee:", shutil.which("tee"))
# Use an explicit temporary directory and an executable producer so that
# producer output and the downstream read behavior are observable.
with tempfile.TemporaryDirectory() as d:
for shell in ("/bin/bash", "/bin/sh"):
if not os.path.exists(shell):
continue
script = r'''
set +e
producer() {
printf '%s\n' first
printf '%s\n' second
return 1
}
producer | tee "$1" | read
printf 'plain_status=%s\n' "$?"
producer | tee "$2" | read || true
printf 'guarded_status=%s\n' "$?"
printf 'plain_bytes=%s guarded_bytes=%s\n' \
"$(wc -c < "$3")" "$(wc -c < "$4")"
'''
plain = os.path.join(d, os.path.basename(shell) + "-plain")
guarded = os.path.join(d, os.path.basename(shell) + "-guarded")
out = subprocess.run(
[shell, "-c", script, shell, plain, guarded],
text=True, capture_output=True,
)
print(shell, "exit=", out.returncode, "stdout=", out.stdout.strip(),
"stderr=", out.stderr.strip())
PY
Length of output: 675
Replace the tee | read status check.
read is the last pipeline command, so Bash can hide find, gofmt, or tee failures. The trailing || true then allows check-go-format to succeed. Redirect output directly to $(OUT_DIR)/gofmt-errors, test [ -s "$(OUT_DIR)/gofmt-errors" ], and allow command and file-write failures to fail the target.
🤖 Prompt for 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.
In `@make/go.mk` around lines 21 - 27, Update the check-go-format recipe around
the gofmt find command to redirect its output directly to
$(OUT_DIR)/gofmt-errors instead of using tee | read, then test the file with [
-s "$(OUT_DIR)/gofmt-errors" ] before printing the error and exiting. Remove the
trailing || true so find, gofmt, and output-file write failures propagate and
cause the target to fail.
also, `remove check-go-format` make goal Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
coderabbitai
Bot
added
the
dependencies
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
3 participants
also, convert all
.adocfiles to.mdSee also:
Signed-off-by: Xavier Coulon xcoulon@redhat.com
Summary by CodeRabbit
Documentation
Maintenance