build: upgrade to Go 1.26.5 - #1290
Conversation
also, build image using `registry.access.redhat.com/ubi10/ubi:latest` also, convert `README.adoc` to `README.md` also, remove the `check-format` target and move the formatting logic into `go.mk` 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: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (1)**⚙️ CodeRabbit configuration file
Files:
🔀 Multi-repo context codeready-toolchain/registration-service, codeready-toolchain/member-operator, codeready-toolchain/api, codeready-toolchain/toolchain-common, codeready-toolchain/toolchain-e2eLinked repositories findings
🔇 Additional comments (1)
WalkthroughThe repository updates its Go toolchain, dependencies, build image, formatting target, and vulnerability configuration. It replaces ChangesRepository maintenance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 5
🤖 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 10-15: Update the format-go-code target to pass discovered Go
files as safe arguments without expanding repository-controlled filenames
through the shell. Replace the GOFORMAT_FILES expansion or its use in gofmt with
a find -exec or NUL-delimited pipeline, while preserving the existing gofmt
flags and exclusion behavior.
In `@make/gofmt_exclude`:
- Around line 1-2: Update the exclusion patterns in gofmt_exclude to match
vendor and .git only as complete path components, using anchored path-component
patterns such as (^|/)vendor/ and (^|/)\.git/. Preserve exclusion of actual
vendor and Git directories while allowing similarly named legitimate paths.
In `@openshift-ci/Dockerfile.tools`:
- Line 1: Update the Dockerfile’s build-tools FROM instruction to use a reviewed
immutable digest instead of the mutable ubi:latest tag, and ensure package
installation uses controlled, pinned package sources. Preserve the UBI 10 base
image while making the build environment reproducible.
In `@README.md`:
- Around line 65-67: Update the local E2E setup documentation to require a
disposable OpenShift cluster, noting that cluster-admin access is limited to
this workflow unless a tested custom role replaces it. Document private-registry
support and imagePullSecrets configuration, or explicitly warn that images
published by the workflow must be public and contain no sensitive data.
- Around line 47-57: Update the E2E pairing workflow in make/test.mk to pin the
official toolchain-e2e and member-operator clones to explicit commits rather
than latest changes. Add verification or an approval gate before merging code
from a developer fork; otherwise document the trust boundary clearly. Update the
README to describe the pinned commits used locally before the merge step.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 93c90961-e3f4-4e03-a080-aff44cddf314
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.govulncheck.yamlREADME.adocREADME.mdgo.modmake/format.mkmake/go.mkmake/gofmt_excludeopenshift-ci/Dockerfile.tools
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual)codeready-toolchain/member-operator(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
💤 Files with no reviewable changes (2)
- make/format.mk
- README.adoc
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles & dashboard image for e2e tests
- GitHub Check: test
🧰 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/gofmt_excludemake/go.mkopenshift-ci/Dockerfile.toolsgo.modREADME.md
🪛 GitHub Actions: ci-check-gomod / 0_go.mod replacements.txt
go.mod
[error] 1-1: The dependency validation command failed because protected modules were replaced: github.com/codeready-toolchain/api => github.com/xcoulon/api and github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common. Remove these replacements from go.mod.
🪛 GitHub Actions: ci-check-gomod / go.mod replacements
go.mod
[error] 1-1: CI validation failed: protected modules have disallowed replacements in go.mod: github.com/codeready-toolchain/api => github.com/xcoulon/api and github.com/codeready-toolchain/toolchain-common => github.com/xcoulon/toolchain-common. Command failed with exit code 1.
🪛 LanguageTool
README.md
[style] ~80-~80: 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] ~98-~98: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...po:** 1. check the name of a branch you are going to create a PR for 2. create a branch ...
(REP_GOING_TO_VB)
🪛 markdownlint-cli2 (0.23.2)
README.md
[warning] 13-13: Link text should be descriptive
(MD059, descriptive-link-text)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Trivy (0.72.0)
openshift-ci/Dockerfile.tools
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🔇 Additional comments (8)
go.mod (3)
3-6: 🎯 Functional CorrectnessAlign the module minimum with the documented build requirement.
go 1.26.0declares Go 1.26.0 as the minimum.toolchain go1.26.5selects a preferred toolchain; it does not change that minimum. The suppliedREADME.mdandopenshift-ci/Dockerfile.toolsrequire Go 1.26.5 or newer. If 1.26.5 is required, set thegodirective to1.26.5. Otherwise, verify Go 1.26.0 support and update the documentation. (go.dev)
123-144: LGTM!
39-42: 🔒 Security & PrivacyConfirm authorization and compatibility of the fork replacements.
The pinned revisions have Go checksum entries and preserve the canonical module paths. The forked API differs from the declared canonical version in 23 files. Confirm owner approval and API compatibility before merging; otherwise remove the replacements.
openshift-ci/Dockerfile.tools (3)
1-1: 🩺 Stability & AvailabilityVerify package-manager compatibility with UBI 10.
The base image changed to UBI 10, but both installation blocks still use
yum. Red Hat’s UBI 10 instructions usednf. Verify thatyumand all listed packages resolve in the exact image, or replaceyumwith supporteddnfcommands before merging. (docs.redhat.com)
11-12: LGTM!
1-1: 🔒 Security & PrivacySet a non-root user for CI execution.
If CI uses this image for repository-controlled commands, add a dedicated unprivileged user and switch to it before
ENTRYPOINT. The Dockerfile currently starts/bin/bashas root..govulncheck.yaml (1)
1-1: 🩺 Stability & AvailabilityKeep the empty vulnerability suppression list.
The current toolchain and dependency versions meet the fixed versions for all 20 removed vulnerability IDs.
> Likely an incorrect or invalid review comment.README.md (1)
1-46: LGTM!Also applies to: 58-64, 68-103
| @@ -1,4 +1,4 @@ | |||
| FROM registry.access.redhat.com/ubi9/ubi:latest as build-tools | |||
| FROM registry.access.redhat.com/ubi10/ubi:latest as build-tools | |||
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Dockerfile.tools ---'
cat -n openshift-ci/Dockerfile.tools
printf '%s\n' '--- image references and package commands ---'
rg -n --glob 'Dockerfile.tools' '^(FROM|RUN|USER|ENTRYPOINT|CMD)|yum|dnf|microdnf|apk|apt' .
printf '%s\n' '--- relevant CI references ---'
rg -n 'Dockerfile\.tools|build-tools|ubi10/ubi' openshift-ci .github Makefile* 2>/dev/null || trueRepository: codeready-toolchain/host-operator
Length of output: 3559
Security Misconfiguration (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Reachability: External
Pin the UBI base image to an immutable digest.
registry.access.redhat.com/ubi10/ubi:latest and unpinned package installs can change the build environment without a source change. Pin a reviewed image digest and controlled package sources.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
🤖 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-ci/Dockerfile.tools` at line 1, Update the Dockerfile’s build-tools
FROM instruction to use a reviewed immutable digest instead of the mutable
ubi:latest tag, and ensure package installation uses controlled, pinned package
sources. Preserve the UBI 10 base image while making the build environment
reproducible.
| The logic that executes tests in openshift-ci automatically tries to pair PR opened for this (host-operator) repository with a branch that potentially exists in the developer's fork of the [toolchain-e2e](https://github.com/codeready-toolchain/toolchain-e2e) repo. This pairing is based on a branch name. | ||
|
|
||
| For example, if a developer with GH account `cooljohn` opens a PR (for host-operator repo) from a branch `fix-reconcile`, then the logic checks if there is a branch `fix-reconcile` also in the `cooljohn/toolchain-e2e` fork. | ||
| If there is, then the logic: | ||
|
|
||
| 1. clones latest changes from [codeready-toolchain/toolchain-e2e](https://github.com/codeready-toolchain/toolchain-e2e) | ||
| 2. fetches the `fix-reconcile` branch from `cooljohn/toolchain-e2e` fork | ||
| 3. merges `master` branch with the changes from `fix-reconcile` branch | ||
| 4. clones latest changes from [member-operator](https://github.com/codeready-toolchain/member-operator) repo and builds & deploys the `member-operator` image out of it | ||
| 5. builds & deploys the `host-operator` image from the code that is in the PR | ||
| 6. runs e2e tests against both operators from the merged branch of the `toolchain-e2e` repo |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, examine the README.md file around the specified lines
head -80 README.md | tail -40Repository: codeready-toolchain/host-operator
Length of output: 3960
🏁 Script executed:
#!/bin/bash
# Look for OpenShift CI configuration
find . -name ".ci" -o -name ".prow*" -o -name "Makefile*" | head -20Repository: codeready-toolchain/host-operator
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Check if there's a ci directory or configuration
ls -la | grep -i ciRepository: codeready-toolchain/host-operator
Length of output: 300
🏁 Script executed:
#!/bin/bash
# Examine openshift-ci directory
ls -la openshift-ci/
cat openshift-ci/*Repository: codeready-toolchain/host-operator
Length of output: 2318
🏁 Script executed:
#!/bin/bash
# Extract relevant sections from Makefile
grep -n "test-e2e\|toolchain-e2e\|member-operator\|clone\|fetch\|merge" Makefile | head -50Repository: codeready-toolchain/host-operator
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Look for any scripts that handle the pairing logic
find . -name "*.sh" -type f | head -20Repository: codeready-toolchain/host-operator
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Read the full Makefile
cat Makefile | head -150Repository: codeready-toolchain/host-operator
Length of output: 310
🏁 Script executed:
#!/bin/bash
# Search for references to test-e2e without exact match
rg -i "test.*e2e\|e2e.*test" --type makefileRepository: codeready-toolchain/host-operator
Length of output: 208
🏁 Script executed:
#!/bin/bash
# Look for OpenShift CI configuration in toolchain-e2e
find . -name ".ci" -o -name "*.prow*" -o -name "*prow*" -o -name "*.openshift*" 2>/dev/null | head -20Repository: codeready-toolchain/toolchain-e2e
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Search for pairing or branch logic
rg -i "pair\|fork\|developer.*fork" --type md --type sh --type yaml | head -30Repository: codeready-toolchain/toolchain-e2e
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Check the make directory structure
ls -la make/Repository: codeready-toolchain/host-operator
Length of output: 1005
🏁 Script executed:
#!/bin/bash
# Search for e2e related content in make files
grep -r "e2e\|toolchain-e2e\|member-operator" make/Repository: codeready-toolchain/host-operator
Length of output: 1814
🏁 Script executed:
#!/bin/bash
# Look for CI-related files more broadly
find . -type f \( -name ".github" -o -name "*.yml" -o -name "*.yaml" \) | grep -i "ci\|workflow\|action" | head -30Repository: codeready-toolchain/host-operator
Length of output: 585
🏁 Script executed:
#!/bin/bash
# Look for the actual test setup and branch pairing logic
ls -la
find . -maxdepth 2 -name "*.sh" -o -name "Makefile*" -o -name "*.mk"Repository: codeready-toolchain/toolchain-e2e
Length of output: 2131
🏁 Script executed:
#!/bin/bash
# Read the full test.mk file which contains the e2e logic
cat make/test.mkRepository: codeready-toolchain/host-operator
Length of output: 5542
🏁 Script executed:
#!/bin/bash
# Read the full README to understand credential and registry requirements
cat README.md | head -200Repository: codeready-toolchain/host-operator
Length of output: 8118
🏁 Script executed:
#!/antml:thinking
Let me check if there are GitHub Actions workflow files that show how the CI is actually configured and what protections exist around credentials.
`#/`
find .github -name "*.yml" -exec grep -l "e2e\|test-e2e\|openshift" {} \;Repository: codeready-toolchain/host-operator
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Check what the CI workflows do - especially the one related to E2E publishing
cat .github/workflows/publish-components-for-e2e-tests.ymlRepository: codeready-toolchain/host-operator
Length of output: 4089
🏁 Script executed:
#!/bin/bash
# Check the publish action from toolchain-cicd to understand what credentials it uses
curl -s https://api.github.com/repos/codeready-toolchain/toolchain-cicd/contents/publish-operators-for-e2e-tests | jq '.download_url' 2>/dev/null || echo "Check GitHub web interface"Repository: codeready-toolchain/host-operator
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Look for CLONEREFS_OPTIONS or other PROW configuration that shows the CI execution context
grep -r "CLONEREFS_OPTIONS\|PROW\|pull_request_target" .github/Repository: codeready-toolchain/host-operator
Length of output: 590
Security (CWE-494): Download of Code Without Integrity Check
Reachability: External · Exploitability: Moderate
Address unpinned dependencies and unverified fork code in E2E pairing.
The E2E pairing logic in make/test.mk fetches and merges code from a developer's fork without commit pinning or signature verification. The workflow uses pull_request_target to run with repository credentials, though it publishes to a segregated test namespace (codeready-toolchain-test) via an explicit TEST_QUAY_TOKEN.
Add commit pinning for the cloned repositories (official toolchain-e2e and member-operator). If the developer's fork branch cannot be verified, document the trust boundary explicitly or require an approval step before merging unverified fork code. For local runs, document the commits that will be used before the merge step executes.
🤖 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` around lines 47 - 57, Update the E2E pairing workflow in
make/test.mk to pin the official toolchain-e2e and member-operator clones to
explicit commits rather than latest changes. Add verification or an approval
gate before merging code from a developer fork; otherwise document the trust
boundary clearly. Update the README to describe the pinned commits used locally
before the merge step.
Source: Path instructions
| * Make sure you have set the `QUAY_NAMESPACE` variable: `export QUAY_NAMESPACE=<quay-username>` | ||
| * Log in to the target OpenShift cluster with cluster admin privileges | ||
| * The visibility of `host-operator` repository in quay is set to public (`https://quay.io/repository/<your-username>/host-operator?tab=settings`) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- README context ---'
sed -n '1,120p' README.md
printf '%s\n' '--- relevant files ---'
git ls-files | rg '(^|/)(Makefile|Dockerfile|.*e2e.*|.*rbac.*|.*role.*|.*clusterrole.*|.*quay.*|.*deploy.*|.*config.*|.*test.*)' | head -200
printf '%s\n' '--- privilege and image references ---'
rg -n -i 'cluster-admin|clusterrole|rolebinding|serviceaccount|quay|image:|imagePull|pull-secret|e2e|oc adm policy|kubectl auth' --glob '!vendor/**' --glob '!**/go.sum' . | head -400Repository: codeready-toolchain/host-operator
Length of output: 50391
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- E2E entry points and setup references ---'
rg -n -i 'test-e2e|dev_install|cluster-admin|clusterrole|rolebinding|imagePull|pull-secret|quay|HOST_NS|MEMBER_NS|oc |kubectl ' \
--glob '!vendor/**' --glob '!**/go.sum' . | head -500
printf '%s\n' '--- candidate Makefiles and manifests ---'
git ls-files | rg '(^|/)(Makefile|.*\.mk|.*\.yaml|.*\.yml|.*\.adoc|.*\.sh)$' | \
rg -i 'make|e2e|dev|deploy|rbac|role|cluster|quay|registry' | head -300Repository: codeready-toolchain/toolchain-e2e
Length of output: 50389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- host Makefile E2E target ---'
sed -n '65,135p' make/test.mk
printf '%s\n' '--- generated host RBAC declarations ---'
sed -n '1,130p' config/rbac/role_binding.yaml
sed -n '1,180p' config/rbac/kustomization.yaml
sed -n '80,115p' cmd/main.go
printf '%s\n' '--- host deployment image and service account ---'
sed -n '1,115p' config/manager/manager.yamlRepository: codeready-toolchain/host-operator
Length of output: 8783
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- E2E setup and deployment commands ---'
sed -n '270,470p' make/test.mk
printf '%s\n' '--- E2E RBAC and cluster-scoped resources ---'
rg -n -i '^(kind: (ClusterRole|ClusterRoleBinding|CustomResourceDefinition|Namespace|PriorityClass|MutatingWebhookConfiguration|ValidatingWebhookConfiguration)|apiVersion:|resources:|verbs:|roleRef:|subjects:)' \
deploy setup --glob '*.yaml' --glob '*.yml' | head -500
printf '%s\n' '--- operator installation and image pull configuration ---'
sed -n '1,290p' scripts/ci/manage-operator.sh
sed -n '1,150p' scripts/ci/manage-host-operator.sh
sed -n '1,140p' scripts/ci/manage-member-operator.sh
sed -n '1,90p' quay.adocRepository: codeready-toolchain/toolchain-e2e
Length of output: 40375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path(".")
files = list(root.glob("make/*.mk")) + list(root.glob("scripts/ci/*.sh"))
patterns = {
"cluster_scope_commands": re.compile(
r"\boc\s+(?:apply|create|delete|patch|get|wait|adm)\b[^\n]*"
r"(?:--all-namespaces|ClusterRole|ClusterRoleBinding|crd|crds|projects|"
r"PriorityClass|Webhook|oauths\.config|ingress\.config|must-gather)",
re.I,
),
"namespace_commands": re.compile(
r"\boc\s+(?:new-project|project|apply|create|delete|patch|get|wait)\b[^\n]*"
r"(?:-n\s+|\-\-namespace|PROJECT_NAME|HOST_NS|MEMBER_NS)",
re.I,
),
"pull_secret_configuration": re.compile(
r"(?:imagePullSecrets|dockerconfigjson|pull-secret|secret\s+generic|"
r"registry.*secret|oc\s+create\s+secret)",
re.I,
),
}
for name, pattern in patterns.items():
print(f"--- {name} ---")
for path in files:
try:
text = path.read_text()
except UnicodeDecodeError:
continue
for line_no, line in enumerate(text.splitlines(), 1):
if pattern.search(line):
print(f"{path}:{line_no}:{line.strip()}")
PYRepository: codeready-toolchain/toolchain-e2e
Length of output: 14251
Security Misconfiguration (CWE-250)
Exploitability: Theoretical
Restrict local E2E tests to disposable clusters.
The workflow creates CRDs, projects, OLM resources, and cluster-scoped RBAC, and its cleanup removes cluster-scoped resources. Keep cluster-admin only for this workflow unless a tested custom role replaces it. Document the disposable-cluster requirement. The workflow also publishes images without configuring imagePullSecrets; document private-registry support or warn that all public images must contain no sensitive data.
🤖 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` around lines 65 - 67, Update the local E2E setup documentation to
require a disposable OpenShift cluster, noting that cluster-admin access is
limited to this workflow unless a tested custom role replaces it. Document
private-registry support and imagePullSecrets configuration, or explicitly warn
that images published by the workflow must be public and contain no sensitive
data.
Source: Path instructions
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🤖 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 10-15: Update the format-go-code target to pass discovered Go
files as safe arguments without expanding repository-controlled filenames
through the shell. Replace the GOFORMAT_FILES expansion or its use in gofmt with
a find -exec or NUL-delimited pipeline, while preserving the existing gofmt
flags and exclusion behavior.
In `@make/gofmt_exclude`:
- Around line 1-2: Update the exclusion patterns in gofmt_exclude to match
vendor and .git only as complete path components, using anchored path-component
patterns such as (^|/)vendor/ and (^|/)\.git/. Preserve exclusion of actual
vendor and Git directories while allowing similarly named legitimate paths.
In `@openshift-ci/Dockerfile.tools`:
- Line 1: Update the Dockerfile’s build-tools FROM instruction to use a reviewed
immutable digest instead of the mutable ubi:latest tag, and ensure package
installation uses controlled, pinned package sources. Preserve the UBI 10 base
image while making the build environment reproducible.
In `@README.md`:
- Around line 65-67: Update the local E2E setup documentation to require a
disposable OpenShift cluster, noting that cluster-admin access is limited to
this workflow unless a tested custom role replaces it. Document private-registry
support and imagePullSecrets configuration, or explicitly warn that images
published by the workflow must be public and contain no sensitive data.
- Around line 47-57: Update the E2E pairing workflow in make/test.mk to pin the
official toolchain-e2e and member-operator clones to explicit commits rather
than latest changes. Add verification or an approval gate before merging code
from a developer fork; otherwise document the trust boundary clearly. Update the
README to describe the pinned commits used locally before the merge step.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 93c90961-e3f4-4e03-a080-aff44cddf314
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.govulncheck.yamlREADME.adocREADME.mdgo.modmake/format.mkmake/go.mkmake/gofmt_excludeopenshift-ci/Dockerfile.tools
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual)codeready-toolchain/member-operator(manual)codeready-toolchain/api(manual)codeready-toolchain/toolchain-common(manual)codeready-toolchain/host-operator(manual)codeready-toolchain/toolchain-e2e(manual)
💤 Files with no reviewable changes (2)
- make/format.mk
- README.adoc
📜 Review details
🔇 Additional comments (8)
go.mod (3)
3-6: 🎯 Functional CorrectnessAlign the module minimum with the documented build requirement.
go 1.26.0declares Go 1.26.0 as the minimum.toolchain go1.26.5selects a preferred toolchain; it does not change that minimum. The suppliedREADME.mdandopenshift-ci/Dockerfile.toolsrequire Go 1.26.5 or newer. If 1.26.5 is required, set thegodirective to1.26.5. Otherwise, verify Go 1.26.0 support and update the documentation. (go.dev)
123-144: LGTM!
39-42: 🔒 Security & PrivacyConfirm authorization and compatibility of the fork replacements.
The pinned revisions have Go checksum entries and preserve the canonical module paths. The forked API differs from the declared canonical version in 23 files. Confirm owner approval and API compatibility before merging; otherwise remove the replacements.
openshift-ci/Dockerfile.tools (3)
1-1: 🩺 Stability & AvailabilityVerify package-manager compatibility with UBI 10.
The base image changed to UBI 10, but both installation blocks still use
yum. Red Hat’s UBI 10 instructions usednf. Verify thatyumand all listed packages resolve in the exact image, or replaceyumwith supporteddnfcommands before merging. (docs.redhat.com)
11-12: LGTM!
1-1: 🔒 Security & PrivacySet a non-root user for CI execution.
If CI uses this image for repository-controlled commands, add a dedicated unprivileged user and switch to it before
ENTRYPOINT. The Dockerfile currently starts/bin/bashas root..govulncheck.yaml (1)
1-1: 🩺 Stability & AvailabilityKeep the empty vulnerability suppression list.
The current toolchain and dependency versions meet the fixed versions for all 20 removed vulnerability IDs.
> Likely an incorrect or invalid review comment.README.md (1)
1-46: LGTM!Also applies to: 58-64, 68-103
🛑 Comments failed to post (1)
make/gofmt_exclude (1)
1-2: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match complete path components.
The patterns match unintended directories.
vendor/.*excludes./myvendor/file.go..git/treats.as a wildcard and excludes./legit/file.go.Use patterns such as
(^|/)vendor/and(^|/)\.git/. This prevents the formatter from silently skipping legitimate Go files.🤖 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/gofmt_exclude` around lines 1 - 2, Update the exclusion patterns in gofmt_exclude to match vendor and .git only as complete path components, using anchored path-component patterns such as (^|/)vendor/ and (^|/)\.git/. Preserve exclusion of actual vendor and Git directories while allowing similarly named legitimate paths.
Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
coderabbitai
Bot
added
ci
| 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 |
There was a problem hiding this comment.
Same as in the other PR, seems like leftover stuff
There was a problem hiding this comment.
yeah, I usually run a first time the e2e tests on CI with the pending "upstream" changes, then once the associated PRs have been merged, I update go.mod accordingly
| .PHONY: format-go-code | ||
| ## Formats any go file that does not match formatting defined by gofmt | ||
| format-go-code: | ||
| $(Q)find . -name '*.go' -not -path '*/vendor/*' -not -path '*/.git/*' -exec gofmt -s -l -w {} + |
There was a problem hiding this comment.
btw, you moved the gofmt_exclude to make/gofmt_exclude but you don't use it here.
either use it or drop it
There was a problem hiding this comment.
the api PR uses it https://github.com/codeready-toolchain/toolchain-common/pull/543/changes#diff-fe1727af26e0329c2bb94f9d274ed74fa926a0a7af1392c8db761788949a0bc3R13
would be cool to have it consistent
There was a problem hiding this comment.
good point, let me fix that
|
[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>
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 to capture find/gofmt
output in $(OUT_DIR)/gofmt-errors, then explicitly test whether that file is
non-empty and exit 1 when formatting differences exist. Remove the trailing ||
true and preserve nonzero statuses from find or gofmt failures, while retaining
the existing error message and file listing.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: e2890330-6a3b-4634-b06c-e56da173c219
📒 Files selected for processing (1)
make/go.mk
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
codeready-toolchain/registration-service(manual) → reviewed against open PR#615upgrade-go-1.26-regsvcinstead of the default branchcodeready-toolchain/member-operator(manual) → reviewed against open PR#763upgrade-go-1.26-memberinstead of the default branchcodeready-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)codeready-toolchain/toolchain-e2e(manual) → reviewed against open PR#1313upgrade-go-1.26-e2einstead of the default branch
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: GolangCI Lint
- GitHub Check: go.mod replacements
- GitHub Check: test
- GitHub Check: govulncheck
- 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
🔀 Multi-repo context codeready-toolchain/registration-service, codeready-toolchain/member-operator, codeready-toolchain/api, codeready-toolchain/toolchain-common, codeready-toolchain/toolchain-e2e
Linked repositories findings
- The inspected PR branches for
registration-service,member-operator,api, andtoolchain-commonalready use Go 1.26/toolchain 1.26.5 and the newer formatting targets, so no cross-repository Makefile mismatch was found.[::codeready-toolchain/registration-service::][::codeready-toolchain/member-operator::][::codeready-toolchain/api::][::codeready-toolchain/toolchain-common::] apiandtoolchain-commonuse empty vulnerability ignore lists, consistent with this change.[::codeready-toolchain/api::][::codeready-toolchain/toolchain-common::]- The inspected
toolchain-e2ePR branch still uses older UBI 9/Go 1.24.13 tooling, indicating repository-wide build-image consistency is not yet complete.[::codeready-toolchain/toolchain-e2e::] - No consumers of the removed
check-formattarget orREADME.adocwere found.
All findings refer to the listed open-PR branch refs, not default branches.
|
/test e2e |
also, `remove check-go-format` make goal Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
|
3 participants
also, build image using
registry.access.redhat.com/ubi10/ubi:latestalso, convert
README.adoctoREADME.mdalso, remove the
check-formattarget and move the formatting logic intogo.mkSee also
Signed-off-by: Xavier Coulon xcoulon@redhat.com
Summary by CodeRabbit
Documentation
Security
Chores