SANDBOX-1357: update kube & openshift dependencies to 4.19 #1196
Conversation
WalkthroughMultiple CRD base YAMLs update the controller-gen Kubebuilder version annotation to v0.17.2. One CRD also tweaks description strings. Tooling and build updates include upgrading Go to 1.23.x, broad dependency bumps in go.mod, adding tools.go, adjusting Makefiles to install tools locally, enabling a new linter, and clearing govulncheck ignores. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
config/crd/bases/toolchain.dev.openshift.com_tiertemplates.yaml (1)
116-118: Doc casing tweak is harmless; consider consistent sentence case across CRDs.
Pure description change; optional style consistency follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
config/crd/bases/toolchain.dev.openshift.com_bannedusers.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_masteruserrecords.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_notifications.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_nstemplatetiers.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_proxyplugins.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_socialevents.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacebindings.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spaceprovisionerconfigs.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spaces.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_tiertemplaterevisions.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_tiertemplates.yaml(4 hunks)config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_toolchainstatuses.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_usersignups.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_usertiers.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Test with Coverage
🔇 Additional comments (24)
config/crd/bases/toolchain.dev.openshift.com_nstemplatetiers.yaml (1)
6-6: LGTM: controller-gen annotation consistently bumped to v0.17.2 across all CRDs
No schema deltas observed.config/crd/bases/toolchain.dev.openshift.com_spaceprovisionerconfigs.yaml (1)
6-6: LGTM: controller-gen annotation updated to v0.17.2Change is metadata-only; nothing else affected.
config/crd/bases/toolchain.dev.openshift.com_socialevents.yaml (1)
6-6: LGTM: controller-gen annotation updated to v0.17.2No other diffs in spec/schema.
config/crd/bases/toolchain.dev.openshift.com_toolchainstatuses.yaml (1)
6-6: LGTM: controller-gen annotation updated to v0.17.2Annotation-only change; CRD structure unchanged.
config/crd/bases/toolchain.dev.openshift.com_masteruserrecords.yaml (1)
6-6: LGTM: controller-gen annotation updated to v0.17.2No schema changes in this file.
config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml (1)
6-6: Annotation bump to controller-gen v0.17.2 — LGTM.
No schema/content diffs introduced in this file.config/crd/bases/toolchain.dev.openshift.com_notifications.yaml (1)
6-6: Annotation bump to controller-gen v0.17.2 — LGTM.
No other CRD changes detected.config/crd/bases/toolchain.dev.openshift.com_usertiers.yaml (1)
6-6: Annotation bump to controller-gen v0.17.2 — LGTM.
No schema/content changes in this file.config/crd/bases/toolchain.dev.openshift.com_spacebindings.yaml (1)
6-6: Annotation bump to controller-gen v0.17.2 — LGTM.
Looks consistent with the rest of the PR.config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml (1)
6-6: Annotation bump to controller-gen v0.17.2 — LGTM.
No functional changes observed.config/crd/bases/toolchain.dev.openshift.com_spaces.yaml (1)
6-6: LGTM – all CRD base manifests now reference controller-gen v0.17.2 with no v0.16.5 remnants.config/crd/bases/toolchain.dev.openshift.com_toolchainconfigs.yaml (1)
6-6: Annotation updated to v0.17.2 — LGTMconfig/crd/bases/toolchain.dev.openshift.com_proxyplugins.yaml (1)
6-6: Annotation updated to v0.17.2 — LGTMconfig/crd/bases/toolchain.dev.openshift.com_usersignups.yaml (1)
6-6: Annotation updated to v0.17.2 — LGTMconfig/crd/bases/toolchain.dev.openshift.com_tiertemplaterevisions.yaml (1)
6-6: Annotation updated to v0.17.2 — LGTMconfig/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml (2)
6-6: LGTM: controller-gen annotation bumped to v0.17.2.
No schema/content deltas observed.
6-6: Re-run CRD version consistency check with a PCRE2-enabled regex or alternate grep
The previousrgcommand failed due to unsupported look-around. Please verify no CRD base is left on a different version by either:
- Using PCRE2 with ripgrep:
rg --pcre2 -n 'controller-gen\.kubebuilder\.io/version:\s*v(?!0\.17\.2)\S+' config/crd/bases- Or a two-step grep workaround:
grep -R 'controller-gen\.kubebuilder\.io/version:' config/crd/bases | grep -v 'v0\.17\.2'Ensure the command returns no results.
config/crd/bases/toolchain.dev.openshift.com_bannedusers.yaml (2)
6-6: LGTM: controller-gen annotation bumped to v0.17.2.
No schema/content changes.
6-6: Verified cross-file versions: Allconfig/crd/basesfiles referencecontroller-gen.kubebuilder.io/version: v0.17.2. No stragglers remain.config/crd/bases/toolchain.dev.openshift.com_tiertemplates.yaml (5)
6-6: LGTM: controller-gen annotation bumped to v0.17.2.
No schema structural diffs beyond docstrings.
123-125: Doc casing tweak only.
No behavioral impact.
142-145: Doc casing tweak only.
No behavioral impact.
151-154: Doc casing tweak only.
No behavioral impact.
6-6: CRD base manifests regeneration with controller-gen v0.17.2 verified.
| metadata: | ||
| annotations: | ||
| controller-gen.kubebuilder.io/version: v0.16.5 | ||
| controller-gen.kubebuilder.io/version: v0.17.2 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify repo-wide consistency and tool pin.
Please confirm all CRD bases carry v0.17.2 and that the repo pins/uses controller-tools v0.17.2 in generation scripts.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) All base CRDs should have v0.17.2 annotation
ok=$(mktemp)
fd -t f -e yaml config/crd/bases | xargs -I{} rg -nH 'controller-gen\.kubebuilder\.io/version:\s*v0\.17\.2\b' {} \
| cut -d: -f1 | sort -u > "$ok"
comm -23 <(fd -t f -e yaml config/crd/bases | sort) <(cat "$ok") | sed 's/^/MISSING v0.17.2: /' || true
# 2) Check controller-tools pin (go.mod/tools or scripts)
rg -n 'sigs\.k8s\.io/controller-tools' -g 'go.mod' -S || true
rg -nP '\bcontroller-gen\b' -g 'Makefile' -S || trueLength of output: 567
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Verify all YAMLs under config/crd/bases include the v0.17.2 annotation
all_files=$(mktemp)
matched_files=$(mktemp)
find config/crd/bases -type f -name '*.yaml' | sort > "$all_files"
grep -RlnP 'controller-gen\.kubebuilder\.io/version:\s*v0\.17\.2\b' config/crd/bases | cut -d: -f1 | sort > "$matched_files"
comm -23 "$all_files" "$matched_files" | sed 's/^/MISSING v0.17.2: /' || true
# 2) Confirm controller-tools is pinned to v0.17.2 in go.mod
grep -n 'sigs.k8s.io/controller-tools' go.mod || true
# 3) Confirm controller-gen invocation in Makefile
grep -Pn '\bcontroller-gen\b.*' -n Makefile || trueLength of output: 457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Verify CRD annotations
find config/crd/bases -type f -name '*.yaml' | while read -r f; do
grep -qP 'controller-gen\.kubebuilder\.io/version:\s*v0\.17\.2\b' "$f" || echo "MISSING v0.17.2 in $f"
done
# 2) Search for controller-tools in go.mod
grep -R "sigs.k8s.io/controller-tools" -n go.mod || echo "controller-tools not referenced in go.mod"
# 3) Search for controller-gen usage and version pin in entire repo
grep -R "controller-gen" -n . || echo "controller-gen not referenced in repo"
# 4) Locate any Go tool management files (e.g., tools.go)
find . -type f -name '*tools.go'Length of output: 5418
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display the version pin for controller-gen in make/get-tool.mk
grep -n 'CONTROLLER_GEN_VERSION' -n make/get-tool.mk || true
# Show surrounding context to verify the pinned version
grep -n -C3 'CONTROLLER_GEN_VERSION' make/get-tool.mk || trueLength of output: 716
Pin controller-gen to v0.17.2
In make/get-tool.mk (line 2) update CONTROLLER_GEN_VERSION from v0.16.5 to v0.17.2 to match the CRD annotations.
🤖 Prompt for AI Agents
In config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml around line
6, the CRD annotation indicates controller-gen.kubebuilder.io/version: v0.17.2
but make/get-tool.mk still pins CONTROLLER_GEN_VERSION to v0.16.5; update
CONTROLLER_GEN_VERSION in make/get-tool.mk (line 2) from v0.16.5 to v0.17.2 so
the tool version used matches the CRD annotation.
✅ Addressed in commit 3022166
There was a problem hiding this comment.
@xcoulon Thank you! Maintaining consistency between the tool version used for generation and the annotations in the generated CRDs is crucial for reproducible builds and avoiding potential schema drift. It's always good to catch these version mismatches early.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
I think that this is gonna be part of the next PR - this PR contains only generated CRDs from the api PR.
Apart from that, it would be good to drop the make/get-tool.mk file and use the versions from the go.mod file
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@xcoulon @MatousJobanek thanks for your comments!
I usually do a separate PR since this one is generated by api PR. But I can join them from now on to make it easier for review
Apart from that, it would be good to drop the make/get-tool.mk file and use the versions from the go.mod file
I will try to address it, thanks!
There was a problem hiding this comment.
@MatousJobanek controller-tools is not presented on go.mod, so maybe we can not drop make/get-tool.mk
There was a problem hiding this comment.
this can be defined via the tool.go file - see an example from api repo:
https://github.com/codeready-toolchain/api/blob/master/tools.go
rsoaresd
changed the title
make/run-cicd-script.mk
Outdated
| @@ -1,4 +1,4 @@ | |||
| OWNER_AND_BRANCH_LOCATION=codeready-toolchain/toolchain-cicd/master | |||
| OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/openshift_4_19_toolchain_cicd | |||
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
.govulncheck.yaml (1)
1-1: Resetting ignored vulnerabilities to empty — confirm intent; add trailing newline.
- Confirm the previously ignored IDs are indeed resolved or acceptable to surface again in CI.
- Add a newline at EOF to satisfy linters.
Apply:
-ignored-vulnerabilities: [] +ignored-vulnerabilities: [] +openshift-ci/Dockerfile.tools (2)
33-39: Fail-fast on download and remove redundant untar to reduce layer size.
- Add -f to curl to fail on HTTP errors.
- The first “tar xzf …” is redundant; only extract to /usr/local.
Apply:
-RUN curl -Lo ${GOLANG_VERSION}.linux-amd64.tar.gz https://dl.google.com/go/${GOLANG_VERSION}.linux-amd64.tar.gz \ +RUN curl -fsSLo ${GOLANG_VERSION}.linux-amd64.tar.gz https://dl.google.com/go/${GOLANG_VERSION}.linux-amd64.tar.gz \ && echo "${GOLANG_SHA256} ${GOLANG_VERSION}.linux-amd64.tar.gz" > ${GOLANG_VERSION}.linux-amd64.sha256 \ && sha256sum -c ${GOLANG_VERSION}.linux-amd64.sha256 \ - && tar xzf ${GOLANG_VERSION}.linux-amd64.tar.gz \ && tar -C /usr/local -xzf ${GOLANG_VERSION}.linux-amd64.tar.gz \ && rm -f ${GOLANG_VERSION}.linux-amd64.tar.gz \ && go version
1-1: Reproducibility: consider pinning base image by digest.Optional: pin UBI to a digest to avoid surprise rebuilds.
make/run-cicd-script.mk (1)
11-13: Clarify log message and harden download path.
- Echo the actual ref used, not “master”.
- Add -f to curl and fail clearly on download errors.
Apply:
- @echo "running the script from GH api repo (using latest version in master)..." + @echo "running the script from GH repo (${OWNER_AND_BRANCH_LOCATION})..." $(eval SCRIPT_NAME := $(shell basename ${SCRIPT_PATH})) - curl -sSL ${GH_SCRIPTS_URL}/${SCRIPT_PATH} > /tmp/${SCRIPT_NAME} && chmod +x /tmp/${SCRIPT_NAME} && OWNER_AND_BRANCH_LOCATION=${OWNER_AND_BRANCH_LOCATION} /tmp/${SCRIPT_NAME} ${SCRIPT_PARAMS} + curl -fsSL ${GH_SCRIPTS_URL}/${SCRIPT_PATH} -o /tmp/${SCRIPT_NAME} \ + || (echo "ERROR: failed to download ${GH_SCRIPTS_URL}/${SCRIPT_PATH}"; exit 1) + chmod +x /tmp/${SCRIPT_NAME} && OWNER_AND_BRANCH_LOCATION=${OWNER_AND_BRANCH_LOCATION} /tmp/${SCRIPT_NAME} ${SCRIPT_PARAMS}go.mod (1)
11-14: OpenShift 4.19 pins look good; keep the comment or link to the commit.You’re pinned to specific pseudo-versions for release-4.19. Consider pasting the commit SHAs in the comment for traceability, but functionally this is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.golangci.yml(1 hunks).govulncheck.yaml(1 hunks)README.adoc(1 hunks)go.mod(3 hunks)make/get-tool.mk(1 hunks)make/run-cicd-script.mk(1 hunks)openshift-ci/Dockerfile.tools(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.adoc
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.govulncheck.yaml
[error] 1-1: no new line character at the end of file
(new-line-at-end-of-file)
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 106-106: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 106-106: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 108-108: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 108-108: golang.org/x/oauth2 0.23.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
🪛 GitHub Actions: ci-check-gomod
go.mod
[error] 1-1: Go.mod replacement not allowed. Replacement check failed (the above replacement(s) are not allowed in go.mod). Command: go list -m all | grep --color=never -E ${REGEX}.\s=>. Exit code: 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (8)
.golangci.yml (1)
33-33: Nice addition: copyloopvar enabled.Good call enabling this linter to catch loop-var capture bugs.
make/get-tool.mk (1)
2-2: controller-gen bumped to v0.17.2 — OK.Looks consistent with the CRD annotations in this PR. Ensure CRDs were regenerated with this exact version in CI.
openshift-ci/Dockerfile.tools (1)
11-12: Go toolchain pinned (1.23.12 + SHA) — LGTM.Thanks for pinning with checksum.
go.mod (5)
106-111: Patch known HIGH vulns in x/crypto and x/oauth2.Upgrade to fixed versions: x/crypto ≥ v0.35.0 (fixes CVE‑2025‑22869/GO‑2025‑3487) and x/oauth2 ≥ v0.27.0 (fixes CVE‑2025‑22868/GO‑2025‑3488). (github.com, pkg.go.dev, osv.dev, groups.google.com)
[ suggest_essential_refactor ]Apply:
- golang.org/x/crypto v0.32.0 // indirect + golang.org/x/crypto v0.35.0 // indirect @@ - golang.org/x/oauth2 v0.23.0 // indirect + golang.org/x/oauth2 v0.27.0 // indirectAfter bumping, run:
- go mod tidy -compat=1.23
- go mod vendor
21-21: zap bump acknowledged.v1.27.0 is safe and compatible with controller-runtime v0.20. No action.
136-138: Go toolchain directive OK; confirm CI image matches go1.23.12.Given the toolchain directive, verify openshift-ci Dockerfile/tools is already bumped to Go 1.23.12, or builds will fetch a toolchain on the fly.
28-29: Both klog v1 and v2 are explicitly imported and aliased in cmd/main.go, so both entries in go.mod are required.Likely an incorrect or invalid review comment.
24-27: K8s 1.32 alignment verified: core libraries at v0.32.1 and controller-runtime v0.20.4 in go.mod; all CRDs annotated with controller-gen v0.17.2; no admissionregistration.k8s.io/v1beta1 or policy/v1beta1 references found.
make/run-cicd-script.mk
Outdated
| @@ -1,4 +1,4 @@ | |||
| OWNER_AND_BRANCH_LOCATION=codeready-toolchain/toolchain-cicd/master | |||
| OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/openshift_4_19_toolchain_cicd | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Defaulting to a personal fork/branch is risky — revert to official default and allow override.
Use the official repo/branch by default; allow CI to override for temporary branches.
Apply:
-OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/openshift_4_19_toolchain_cicd
+OWNER_AND_BRANCH_LOCATION?=codeready-toolchain/toolchain-cicd/master🏁 Script executed:
#!/bin/bash
set -e
# Search for all occurrences of OWNER_AND_BRANCH_LOCATION to verify default usage and overrides
grep -R "OWNER_AND_BRANCH_LOCATION" -n .Length of output: 485
Use conditional assignment for OWNER_AND_BRANCH_LOCATION
Replace the hard-coded personal fork with the official repo/branch as a default and allow CI overrides.
Location: make/run-cicd-script.mk:1
- OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/openshift_4_19_toolchain_cicd
+ OWNER_AND_BRANCH_LOCATION?=codeready-toolchain/toolchain-cicd/master🤖 Prompt for AI Agents
In make/run-cicd-script.mk around line 1, the OWNER_AND_BRANCH_LOCATION variable
is hard-coded to a personal fork; change it to use a conditional assignment so
CI or env can override it. Replace the direct assignment with the Make
conditional operator (?:= or ?=) and set the default value to the official
repo/branch (e.g., org/repo/branch) so that when OWNER_AND_BRANCH_LOCATION is
already defined by CI it will not be overwritten while still providing the
correct default.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, rsoaresd, 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 |
go.mod
Outdated
| golang.org/x/crypto v0.32.0 // indirect | ||
| golang.org/x/net v0.34.0 // indirect | ||
| golang.org/x/oauth2 v0.23.0 // indirect | ||
| golang.org/x/sync v0.10.0 // indirect | ||
| golang.org/x/sys v0.28.0 // indirect | ||
| golang.org/x/term v0.27.0 // indirect | ||
| golang.org/x/sys v0.29.0 // indirect | ||
| golang.org/x/term v0.28.0 // indirect | ||
| golang.org/x/text v0.21.0 // indirect | ||
| golang.org/x/time v0.5.0 // indirect | ||
| golang.org/x/time v0.7.0 // indirect |
There was a problem hiding this comment.
the same comment - couldn't we go higher? #1168
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
make/manifests.mk (2)
44-51: KUSTOMIZE var is referenced but never defined.bundle target invokes $(KUSTOMIZE) (Line 49) but no variable assignment exists. Define it to avoid PATH/command-not-found failures.
Apply:
@@ bundle: clean-bundle generate-rbac kustomize ## Generate bundle manifests and metadata, then validate generated files. operator-sdk generate kustomize manifests -q $(eval TMP_MANIFEST_FILE := $(shell mktemp)) @echo "generating manifests to temporary file: ${TMP_MANIFEST_FILE}" - $(KUSTOMIZE) build config/manifests -o ${TMP_MANIFEST_FILE} + $(KUSTOMIZE) build config/manifests -o ${TMP_MANIFEST_FILE}And add the definition as below (see next comment diff).
59-65: Define KUSTOMIZE and pin tool versions for reproducible installs.
- Add KUSTOMIZE path variable.
- Optionally pin versions via @… to ensure local installs match PR-intended versions regardless of ambient module state.
Apply:
-CONTROLLER_GEN = $(PROJECT_DIR)/bin/controller-gen +CONTROLLER_GEN = $(PROJECT_DIR)/bin/controller-gen +KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize +CONTROLLER_GEN_VERSION ?= v0.17.2 +KUSTOMIZE_VERSION ?= v5.5.0 controller-gen: ## Download controller-gen locally if necessary. - GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_GEN_VERSION) kustomize: ## Download kustomize locally if necessary. - GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5@$(KUSTOMIZE_VERSION)
♻️ Duplicate comments (1)
go.mod (1)
153-156: Remove replace directives to forks — CI blocks them.Same as prior review; these break ci-check-gomod.
Apply:
-replace github.com/codeready-toolchain/api => github.com/rsoaresd/api v0.0.0-20250904111715-ce08642a4499 - -replace github.com/codeready-toolchain/toolchain-common => github.com/rsoaresd/toolchain-common v0.0.0-20250904123823-7c569c802618 -Then run: go mod tidy
🧹 Nitpick comments (1)
make/manifests.mk (1)
13-15: Use $(CURDIR) instead of spawning pwd.Slightly faster and more portable; avoids a subshell.
Apply:
-PROJECT_DIR := $(shell pwd) +PROJECT_DIR := $(CURDIR)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(4 hunks)make/get-tool.mk(0 hunks)make/manifests.mk(2 hunks)tools.go(1 hunks)
💤 Files with no reviewable changes (1)
- make/get-tool.mk
🧰 Additional context used
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 123-123: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 123-123: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Test with Coverage
- GitHub Check: GolangCI Lint
🔇 Additional comments (4)
tools.go (1)
1-18: LGTM: tools build tag and pinning via blank imports look correct.Matches go.mod updates; no executable logic; safe to merge.
go.mod (3)
41-45: Tooling deps block looks good.Matches tools.go blank imports; ensures deterministic go install.
157-160: Ensure CI/tooling images use Go 1.23.12.You set go 1.23.0 and toolchain go1.23.12; verify openshift-ci and local dev images match to avoid module resolution drift.
24-33: No action needed: controller-runtime v0.20.4 is officially compatible with k8s.io/ v0.32.x.*
go.mod
Outdated
| golang.org/x/crypto v0.32.0 // indirect | ||
| golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect | ||
| golang.org/x/mod v0.22.0 // indirect | ||
| golang.org/x/net v0.34.0 // indirect | ||
| golang.org/x/oauth2 v0.23.0 // indirect | ||
| golang.org/x/sync v0.10.0 // indirect | ||
| golang.org/x/sys v0.28.0 // indirect | ||
| golang.org/x/term v0.27.0 // indirect | ||
| golang.org/x/sys v0.29.0 // indirect | ||
| golang.org/x/term v0.28.0 // indirect | ||
| golang.org/x/text v0.21.0 // indirect | ||
| golang.org/x/time v0.5.0 // indirect | ||
| golang.org/x/time v0.7.0 // indirect | ||
| golang.org/x/tools v0.29.0 // indirect |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
High-severity vulns flagged for x/crypto and x/oauth2 — verify and bump to patched patch releases.
OSV hints: GO-2025-3487 (x/crypto v0.32.0) and GO-2025-3488 (x/oauth2 v0.23.0). If confirmed, bump to the latest non-breaking patch versions.
Run:
Also applies to: 127-128
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
# Check known vulns
curl -s https://api.osv.dev/v1/query -H 'content-type: application/json' \
-d '{"package":{"name":"golang.org/x/crypto","ecosystem":"Go"},"version":"0.32.0"}' | jq -r '.vulns[]?.id,.vulns[]?.summary'
curl -s https://api.osv.dev/v1/query -H 'content-type: application/json' \
-d '{"package":{"name":"golang.org/x/oauth2","ecosystem":"Go"},"version":"0.23.0"}' | jq -r '.vulns[]?.id,.vulns[]?.summary'
# If affected, update to latest patches:
go get -u=patch golang.org/x/crypto golang.org/x/oauth2
go mod tidy
Length of output: 9173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Fetch fixed versions for vulnerabilities
curl -s https://api.osv.dev/v1/vulns/GO-2025-3487 | jq -r '.affected[].ranges[].events[] | select(.fixed) | .fixed'
curl -s https://api.osv.dev/v1/vulns/GO-2025-3488 | jq -r '.affected[].ranges[].events[] | select(.fixed) | .fixed'Length of output: 13
Bump x/crypto and x/oauth2 to patched releases
In go.mod (lines 123–133; also applies to 127–128), update
- golang.org/x/crypto → v0.35.0 (fixes GO-2025-3487)
- golang.org/x/oauth2 → v0.27.0 (fixes GO-2025-3488)
Rungo get -u=patch golang.org/x/[email protected] golang.org/x/[email protected] && go mod tidy.
🧰 Tools
🪛 OSV Scanner (2.1.0)
[HIGH] 123-123: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 123-123: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
🤖 Prompt for AI Agents
In go.mod around lines 123–133 (notably lines 127–128), the indirect module
versions for golang.org/x/crypto and golang.org/x/oauth2 are out of date; update
golang.org/x/crypto to v0.35.0 and golang.org/x/oauth2 to v0.27.0 to apply the
security patches, then run: go get -u=patch golang.org/x/[email protected]
golang.org/x/[email protected] && go mod tidy to update go.mod and go.sum
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
make/manifests.mk (3)
13-15: Prefer CURDIR and make-friendly defaults over spawning a shellAvoids an extra subshell and is more robust under make -C. Also, use ?= so callers can override.
-PROJECT_DIR := $(shell pwd) -GO?=go +PROJECT_DIR ?= $(CURDIR) +GO ?= go
49-51: Silence or drop the debug echo of the kustomize pathThis looks like leftover debug noise and clutters CI logs. Either remove it or keep the output without echoing the command.
Option A (remove):
- echo $(KUSTOMIZE)Option B (keep, but quiet and clearer):
- echo $(KUSTOMIZE) + @echo "Using kustomize: $(KUSTOMIZE)"
60-67: Harden tool installs in manifests.mk
- Ensure
$(PROJECT_DIR)/binexists and use:=for variable assignments.- Versions are already pinned in go.mod/tools.go (controller-gen v0.17.2, kustomize v5.5.0), so
go installwithout@versionwill be reproducible.- (Optional) Use file-targets (
$(CONTROLLER_GEN),$(KUSTOMIZE)) instead of phony targets to avoid unnecessary reinstalls.Minimal hardening:
-CONTROLLER_GEN = $(PROJECT_DIR)/bin/controller-gen -controller-gen: ## Download controller-gen locally if necessary. - GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen +CONTROLLER_GEN := $(PROJECT_DIR)/bin/controller-gen +controller-gen: ## Download controller-gen locally if necessary. + mkdir -p $(PROJECT_DIR)/bin + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen -KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize -kustomize: ## Download kustomize locally if necessary. - GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 +KUSTOMIZE := $(PROJECT_DIR)/bin/kustomize +kustomize: ## Download kustomize locally if necessary. + mkdir -p $(PROJECT_DIR)/bin + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
make/manifests.mk(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
go.mod (1)
153-156: Remove fork replaces — they break CI.The personal fork replaces reintroduced here will fail “Go.mod replacement not allowed.” Remove before merge.
Apply:
-replace github.com/codeready-toolchain/api => github.com/rsoaresd/api v0.0.0-20250908100603-312ce7567dd1 - -replace github.com/codeready-toolchain/toolchain-common => github.com/rsoaresd/toolchain-common v0.0.0-20250903095154-769c1bbc2835 -#!/bin/bash # Confirm no remaining replace directives to non-upstream rg -nP '^\s*replace\s+github\.com/codeready-toolchain/' go.mod -C0
🧹 Nitpick comments (2)
go.mod (2)
40-45: Tooling modules — ensure they’re isolated via tools.go.Keep these under a dedicated tools.go guarded by //go:build tools to avoid pulling them into production builds.
Example tools.go (outside this hunk):
//go:build tools package tools import ( _ "github.com/go-bindata/go-bindata/v3/go-bindata" _ "k8s.io/code-generator" _ "k8s.io/gengo" _ "k8s.io/kube-openapi/cmd/openapi-gen" )
24-27: Pick latest Kubernetes patch dependencies (v0.32.9)
Controller-runtime v0.20.4 targets k8s.io v0.32.x; update k8s.io/api, apiextensions-apiserver, apimachinery and client-go from v0.32.1 to v0.32.9 (latest patch) before merge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
config/crd/bases/toolchain.dev.openshift.com_usersignups.yaml(1 hunks)go.mod(4 hunks)make/manifests.mk(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- config/crd/bases/toolchain.dev.openshift.com_usersignups.yaml
- make/manifests.mk
🧰 Additional context used
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Test with Coverage
🔇 Additional comments (7)
go.mod (7)
11-14: LGTM: OpenShift 4.19 pins look correct.Pinning openshift/api and library-go to release-4.19 commits aligns with SANDBOX-1357 intent.
21-21: LGTM: zap bumped to v1.27.0.
36-38: LGTM: kubectl/utils aligned with K8s 0.32.x.
62-65: LGTM: routine bumps/indirect pins.These dependency refreshes are consistent with the K8s/OpenShift stack update.
Also applies to: 70-70, 72-74, 77-77, 92-92, 95-95, 97-99, 111-115, 136-137, 139-139, 143-149
157-157: LGTM: go 1.23 with toolchain go1.23.12.Matches the updated CI toolchain; no action needed.
Also applies to: 159-159
30-33: Regenerate code & manifests after bump
Rerunmake generate manifestslocally (or your project’s equivalent codegen commands) and commit any updated files to prevent API or manifest drift.
123-133: Upgradegolang.org/x/oauth2to v0.27.0
Fixes GHSA-6v2p-p543-phr9 / GO-2025-3488. Apply in go.mod:- golang.org/x/oauth2 v0.23.0 // indirect + golang.org/x/oauth2 v0.27.0 // indirect
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
go.mod (1)
127-127: Bump golang.org/x/oauth2 to patched release.v0.23.0 has HIGH vulns (GO-2025-3488). Move to ≥ v0.27.0.
Apply:
- golang.org/x/oauth2 v0.23.0 // indirect + golang.org/x/oauth2 v0.27.0 // indirectThen run:
go get golang.org/x/[email protected] && go mod tidy
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod(4 hunks)
🧰 Additional context used
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 127-127: golang.org/x/oauth2 0.23.0: golang.org/x/oauth2 Improper Validation of Syntactic Correctness of Input vulnerability
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Test with Coverage
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (4)
go.mod (4)
31-31: Controller-tools version mismatches PR description and CRD annotations.PR text/CRDs say v0.17.2, go.mod pins v0.17.3. Align one way or the other to avoid drift.
If you intended 0.17.2, revert here; if 0.17.3 is intended, update PR text and regenerate CRDs so
controller-gen.kubebuilder.io/versionmatches.
153-156: Toolchain pin: ensure CI uses Go 1.23.12.go.mod sets
go 1.23.0andtoolchain go1.23.12. Confirm CI images/build scripts install/use ≥ 1.23.12 everywhere.
123-123: x/crypto already ≥ patched.v0.36.0 supersedes earlier vulnerable releases. No action needed.
24-27: Confirmed — controller-runtime v0.20.4 is compatible with k8s.io v0.32.2.
Sanity-checked against the upstream controller-runtime compatibility matrix (v0.20.x → k8s.io/* v0.32.x). No changes required.
| // using latest commit from 'github.com/openshift/api branch release-4.19' | ||
| github.com/openshift/api v0.0.0-20250903165707-ce7baf0afbc4 | ||
| // using latest commit from 'github.com/openshift/library-go branch release-4.19' | ||
| github.com/openshift/library-go v0.0.0-20250826065405-6d18d1191f49 // indirect |
There was a problem hiding this comment.
🧩 Analysis chain
Verify “latest commit from release-4.19” claim for OpenShift deps.
Ensure these pseudo-versions match the heads of release-4.19.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
check() {
repo="$1"; branch="$2"; expected="$3"
head="$(git ls-remote --heads https://github.com/$repo "$branch" | awk '{print $1}')"
echo "$repo $branch head=$head expected=$expected"
test "$head" = "$expected" || { echo "Mismatch for $repo:$branch"; exit 1; }
}
check openshift/api release-4.19 ce7baf0afbc4
check openshift/library-go release-4.19 6d18d1191f49Length of output: 134
Mismatch: github.com/openshift/api pseudo-version != release-4.19 HEAD
- go.mod lists github.com/openshift/api v0.0.0-20250903165707-ce7baf0afbc4 (ce7baf0afbc4) but release-4.19 HEAD is 93b8cc2cf247366771a42d967d836f1968418540 — update the pseudo-version to the branch head or correct the comment. File: go.mod (lines 11-14).
- The library-go check exited early; verify github.com/openshift/library-go v0.0.0-20250826065405-6d18d1191f49 (6d18d1191f49) against release-4.19 HEAD.
🤖 Prompt for AI Agents
In go.mod around lines 11-14, the pseudo-version for github.com/openshift/api
(v0.0.0-20250903165707-ce7baf0afbc4) does not match the release-4.19 branch HEAD
(93b8cc2c...), and the comment for library-go should be verified; update the
github.com/openshift/api pseudo-version to the actual release-4.19 HEAD
pseudo-version (or change/remove the comment to reflect the current commit), and
confirm/update github.com/openshift/library-go to the release-4.19 HEAD
pseudo-version as well; regenerate go.mod/go.sum (e.g., run go get
github.com/openshift/api@<commit> and go get
github.com/openshift/library-go@<commit> or use go mod edit) so the
pseudo-versions and comments match the real branch heads.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1196 +/- ##
=======================================
Coverage 70.99% 70.99%
=======================================
Files 82 82
Lines 8022 8022
=======================================
Hits 5695 5695
Misses 2120 2120
Partials 207 207 🚀 New features to boost your workflow:
|
4 participants
Description
update controller-gen.kubebuilder.io/version to v0.17.2 generated by SANDBOX-1357: update kube & openshift dependencies to 4.19 api#484 in commit 2934127bca5e6f3ebf1777216503bb427c0a25b2
update dependencies
Related PRs
Issue ticket number and link
SANDBOX-1357
Summary by CodeRabbit
Chores
Documentation