SANDBOX-1357: update kube & openshift dependencies to 4.19 #696
SANDBOX-1357: update kube & openshift dependencies to 4.19 #696rsoaresd merged 15 commits intocodeready-toolchain:masterfrom
Conversation
WalkthroughUpdates Go/toolchain versions and dependencies, enables a new linter, removes old tool-download Makefile, adds tooling installation to manifests Makefile, introduces tools.go for tool pinning, clears govulncheck ignore list, and bumps controller-gen annotation versions in multiple CRDs. Also updates CI Dockerfile Go version and README Go requirement. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used🪛 OSV Scanner (2.1.0)go.mod[HIGH] 106-106: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2 (GO-2025-3488) [HIGH] 106-106: 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)
🔇 Additional comments (5)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
xcoulon
left a comment
xcoulon
left a comment
There was a problem hiding this comment.
do you also need a change similar to codeready-toolchain/host-operator#1196 (comment) ?
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, thank you! |
go.mod
Outdated
| replace github.com/codeready-toolchain/api => github.com/rsoaresd/api v0.0.0-20250904111715-ce08642a4499 | ||
|
|
||
| toolchain go1.22.12 | ||
| replace github.com/codeready-toolchain/toolchain-common => github.com/rsoaresd/toolchain-common v0.0.0-20250506093954-2b65ad3a2e12 |
rsoaresd
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
make/get-tool.mk (1)
17-31: Fix TMP_DIR cleanup: wrong variable expansion leaks temp dirs.
rm -rf $$TMP_DIRexpands$$to PID, not the variable; the tmp dir isn’t removed.Apply:
- rm -rf $$TMP_DIR ;\ + rm -rf $${TMP_DIR} ;\
🧹 Nitpick comments (3)
make/get-tool.mk (1)
17-31: Optional: ensure temp cleanup on failure.Add a trap so the tmp dir is removed even if a command fails.
set -e ;\ -TMP_DIR=$$(mktemp -d) ;\ +TMP_DIR=$$(mktemp -d) ;\ +trap 'rm -rf "$${TMP_DIR}"' EXIT ;\.govulncheck.yaml (1)
1-1: Add trailing newline to satisfy YAML lintFile is missing a newline at EOF per lint hint.
-ignored-vulnerabilities: [] +ignored-vulnerabilities: [] +Also verify govulncheck still passes with the ignore list cleared.
openshift-ci/Dockerfile.tools (1)
12-13: Verify Go tarball SHA and consider image pinning
- Please confirm GOLANG_SHA256 matches the official go1.23.12 linux-amd64 tarball.
- Optional: pin the base UBI image (not “latest”) for reproducible builds.
📜 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 (15)
.golangci.yml(1 hunks).govulncheck.yaml(1 hunks)README.adoc(1 hunks)config/crd/bases/toolchain.dev.openshift.com_idlers.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_memberoperatorconfigs.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_memberstatuses.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_nstemplatesets.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_useraccounts.yaml(1 hunks)config/crd/bases/toolchain.dev.openshift.com_workspaces.yaml(1 hunks)go.mod(3 hunks)make/get-tool.mk(1 hunks)openshift-ci/Dockerfile.tools(1 hunks)
🧰 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] 89-89: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 89-89: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 91-91: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 91-91: 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: Disallowed go.mod replacement(s) detected: the above replacement(s) are not allowed in go.mod. Replacements include 'github.com/codeready-toolchain/api' => 'github.com/rsoaresd/api' and 'github.com/codeready-toolchain/toolchain-common' => 'github.com/rsoaresd/toolchain-common'. The CI step exited with 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). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Test with Coverage
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (15)
config/crd/bases/toolchain.dev.openshift.com_spacerequests.yaml (1)
6-6: Approved: All CRD annotations and generator pin updated to v0.17.2
Verified that every file under config/crd/bases and the CONTROLLER_GEN_VERSION in make/get-tool.mk are consistently set to v0.17.2.config/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yaml (1)
6-6: LGTM: controller-gen annotation bumped to v0.17.2Looks consistent with the repo-wide upgrade. Run the verification script from the other comment to ensure there are no stragglers.
config/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yaml (1)
6-6: LGTM: controller-gen annotation bumped to v0.17.2No spec changes; only the annotation. Covered by the same consistency check.
README.adoc (1)
13-13: Go version alignment validated: go.mod’s toolchain directive and CI images are both pinned to go1.23.12..golangci.yml (1)
33-33: Ensure CI golangci-lint ≥v1.41.1 for copyloopvar
No version is currently pinned in make/lint.mk or any CI workflow—add a version constraint or verify your CI installs golangci-lint v1.41.1+ (which introduced copyloopvar) to prevent lint failures.make/get-tool.mk (1)
2-2: Bump to controller-gen v0.17.2 looks good.Matches the CRD annotation updates across the repo.
config/crd/bases/toolchain.dev.openshift.com_idlers.yaml (1)
6-6: Annotation bump aligned with toolchain.No schema drift; safe metadata-only change.
config/crd/bases/toolchain.dev.openshift.com_workspaces.yaml (1)
6-6: Annotation version updated correctly.CRD content unchanged; OK.
config/crd/bases/toolchain.dev.openshift.com_nstemplatesets.yaml (1)
6-6: Controller-gen annotation update LGTM.No functional changes detected.
config/crd/bases/toolchain.dev.openshift.com_memberoperatorconfigs.yaml (1)
6-6: Annotation bump verified All CRD bases are using v0.17.2 and the Makefile’s CONTROLLER_GEN_VERSION matches.config/crd/bases/toolchain.dev.openshift.com_memberstatuses.yaml (1)
6-6: Annotation bump looks goodOnly the controller-gen version annotation changed; no schema drift apparent. Consider running your generators to confirm no additional diffs are needed.
config/crd/bases/toolchain.dev.openshift.com_useraccounts.yaml (1)
6-6: Annotation bump acknowledgedController-gen annotation updated to v0.17.2; no other CRD changes detected. Safe.
go.mod (3)
114-116: Go toolchain updated — LGTMgo 1.23.0 and toolchain go1.23.12 align with the Dockerfile change. No concerns.
8-9: OpenShift API bump — sanity checkYou’re pointing to a recent release-4.19 pseudo-version. Ensure it’s consistent with other OpenShift deps (eg, library-go) to avoid type mismatches.
15-21: Compatibility verified – no action required controller-runtime v0.20.x is officially supported with k8s.io v0.32.x (Kubernetes 1.32) per the controller-runtime compatibility matrix.
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.3.0 // indirect | ||
| golang.org/x/time v0.7.0 // indirect | ||
| gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Security: consider bumping golang.org/x modules flagged by OSV
OSV hints show issues for golang.org/x/crypto v0.32.0 and golang.org/x/oauth2 v0.23.0. Check if newer patched versions are available and update if applicable.
🌐 Web query:
What are the latest non-vulnerable versions for golang.org/x/crypto and golang.org/x/oauth2 as of today?
💡 Result:
As of 2025-09-04:
- golang.org/x/crypto — fixed in v0.35.0 (upgrade to v0.35.0 or later). [1]
- golang.org/x/oauth2 — fixed in v0.27.0 (upgrade to v0.27.0 or later). [2]
Sources:
[1] pkg.go.dev vuln GO-2025-3487 (CVE-2025-22869) — golang.org/x/crypto.
[2] pkg.go.dev vuln GO-2025-3488 (CVE-2025-22868) — golang.org/x/oauth2.
Update vulnerable golang.org/x dependencies
In go.mod (lines 89–97), bump:
- golang.org/x/crypto → v0.35.0 (fixes GO-2025-3487/CVE-2025-22869)
- golang.org/x/oauth2 → v0.27.0 (fixes GO-2025-3488/CVE-2025-22868)
- 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 // indirect📝 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.
| 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.3.0 // indirect | |
| golang.org/x/time v0.7.0 // indirect | |
| gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect | |
| golang.org/x/crypto v0.35.0 // indirect | |
| golang.org/x/net v0.34.0 // indirect | |
| golang.org/x/oauth2 v0.27.0 // indirect | |
| golang.org/x/sync v0.10.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.7.0 // indirect | |
| gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect |
🧰 Tools
🪛 OSV Scanner (2.1.0)
[HIGH] 89-89: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 89-89: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 91-91: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 91-91: 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 89–97, two indirect golang.org/x dependencies are
vulnerable; update golang.org/x/crypto from v0.32.0 to v0.35.0 and
golang.org/x/oauth2 from v0.23.0 to v0.27.0, then run go mod tidy (and go
test/build) to ensure module graph and go.sum are updated and there are no
import or compatibility issues.
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 | |||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #696 +/- ##
=======================================
Coverage 80.69% 80.69%
=======================================
Files 48 48
Lines 4507 4507
=======================================
Hits 3637 3637
Misses 709 709
Partials 161 161 🚀 New features to boost your workflow:
|
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.3.0 // indirect | ||
| golang.org/x/time v0.7.0 // indirect |
There was a problem hiding this comment.
the same applies also to the other repos
There was a problem hiding this comment.
On #663, the bump is done by bumping the toolchain to go1.24.1, which is not applicable for us. We are already using the latest Go 1.23 (1.23.12), so we can not do a similar approach.
From what I saw, golang.org/x/net comes from sigs.k8s.io/controller-tools. We are using version v0.17.2 (which is the one used by operator-sdk. If we upgrade to v0.17.3, golang.org/x/net will be bumped to v0.35.0. WDYT? @MatousJobanek
There was a problem hiding this comment.
ok, I missed that it updated also the toolchain version. I assumed that it could be compatible with 1.23 based on the Xavier's comment: #663 (comment)
There was a problem hiding this comment.
trying locally in member-operator
go get golang.org/x/net@v0.38.0
it seems to work 🤔
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MatousJobanek, rsoaresd 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
make/manifests.mk (1)
51-57: bundle target will fail: KUSTOMIZE variable is undefined.bundle uses $(KUSTOMIZE) but it’s never set. Define it to the installed binary path.
Apply:
CONTROLLER_GEN = $(PROJECT_DIR)/bin/controller-gen +KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize controller-gen: ## Download controller-gen locally if necessary. GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen kustomize: ## Download kustomize locally if necessary. GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5
♻️ Duplicate comments (3)
make/run-cicd-script.mk (1)
1-1: Revert hardcoded fork; default to canonical and fix messaging + curl robustness.Pointing to a personal branch will break CI/repro. Default to upstream and allow overrides; also fix the echo to reflect the actual source and add curl -f + quoting.
Apply:
-OWNER_AND_BRANCH_LOCATION=rsoaresd/toolchain-cicd/openshift_4_19_toolchain_cicd +OWNER_AND_BRANCH_LOCATION?=codeready-toolchain/toolchain-cicd/master @@ - @echo "running the script from GH api repo (using latest version in master)..." + @echo "running the script from remote ${OWNER_AND_BRANCH_LOCATION}..." @@ - 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}" && chmod +x "/tmp/${SCRIPT_NAME}" && OWNER_AND_BRANCH_LOCATION="${OWNER_AND_BRANCH_LOCATION}" "/tmp/${SCRIPT_NAME}" ${SCRIPT_PARAMS}Also applies to: 11-13
go.mod (2)
128-131: Remove disallowed replace directives in go.mod
Lines 128 and 130 still contain forks ofgithub.com/codeready-toolchain; these must be removed so CI passes and only upstream modules are used.-go.mod:128 -replace github.com/codeready-toolchain/api => github.com/rsoaresd/api v0.0.0-20250904111715-ce08642a4499 -go.mod:130 -replace github.com/codeready-toolchain/toolchain-common => github.com/rsoaresd/toolchain-common v0.0.0-20250904123823-7c569c802618
102-107: Bump golang.org/x/crypto and x/oauth2 to patched versions- 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 // indirectOptionally, upgrade to the latest upstream releases (v0.41.0 for x/crypto and v0.31.0 for x/oauth2).
Rungo mod tidyandgo list -m -u golang.org/x/crypto golang.org/x/oauth2to verify.
🧹 Nitpick comments (2)
tools.go (1)
1-18: Tools pinning looks good; add trailing newline to satisfy linters.Apply:
) +make/manifests.mk (1)
31-38: Optional: make installs idempotent and reduce repeated downloads.Use file targets so tools install only when missing; depend on the file paths in other targets.
Apply:
-.PHONY: generate-rbac -generate-rbac: build controller-gen +.PHONY: generate-rbac +generate-rbac: build $(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): ## Ensure controller-gen is installed locally. + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen @@ -kustomize: ## Download kustomize locally if necessary. - GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 +$(KUSTOMIZE): ## Ensure kustomize is installed locally. + GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 @@ -bundle: clean-bundle generate-rbac kustomize ## Generate bundle manifests and metadata, then validate generated files. +bundle: clean-bundle generate-rbac $(KUSTOMIZE) ## Generate bundle manifests and metadata, then validate generated files.Also applies to: 51-57
📜 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 (5)
go.mod(3 hunks)make/get-tool.mk(0 hunks)make/manifests.mk(2 hunks)make/run-cicd-script.mk(1 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] 102-102: golang.org/x/crypto 0.32.0: Potential denial of service in golang.org/x/crypto
(GO-2025-3487)
[HIGH] 102-102: golang.org/x/crypto 0.32.0: golang.org/x/crypto Vulnerable to Denial of Service (DoS) via Slow or Incomplete Key Exchange
[HIGH] 106-106: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 106-106: 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 (1)
go.mod (1)
17-17: Retain legacy k8s.io/klog v1 – it’s directly imported asklogv1in cmd/webhook/main.go (line 24) and cmd/main.go (line 42), so dropping v1 will break the build.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
make/manifests.mk (1)
11-13: Prefer CURDIR and add a reusable bin var.Avoid spawning a shell and normalize spacing; add TOOLS_BIN_DIR for reuse.
-PROJECT_DIR := $(shell pwd) -GO?=go +PROJECT_DIR ?= $(CURDIR) +GO ?= go +TOOLS_BIN_DIR ?= $(PROJECT_DIR)/bin
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
make/manifests.mk(2 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
| 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 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin controller-gen install for reproducibility and mark PHONY.
Without @Version, builds may drift. Pin to v0.17.2 (matches PR) and mark the target phony.
-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
+.PHONY: controller-gen
+CONTROLLER_GEN = $(PROJECT_DIR)/bin/controller-gen
+CONTROLLER_TOOLS_VERSION ?= v0.17.2
+controller-gen: ## Download controller-gen locally if necessary.
+ GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)🤖 Prompt for AI Agents
In make/manifests.mk around lines 51 to 54, the controller-gen install target
isn't pinned and isn't marked phony; update the install command to pin the
module to v0.17.2 by adding @v0.17.2 to the package path (e.g.,
GOBIN=$(PROJECT_DIR)/bin $(GO) install
sigs.k8s.io/controller-tools/cmd/controller-gen@v0.17.2) and add a .PHONY:
controller-gen declaration so the target is always rebuilt.
| KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize | ||
| kustomize: ## Download kustomize locally if necessary. | ||
| GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin kustomize install and mark PHONY.
Same drift risk as above. Derive version from go.mod (tools.go) when present; otherwise fall back to latest (preferably ensure it’s pinned in tools.go).
-KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize
-kustomize: ## Download kustomize locally if necessary.
- GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5
+.PHONY: kustomize
+KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize
+# Prefer pinning via tools.go; this reads the version selected in go.mod.
+KUSTOMIZE_VERSION ?= $(shell $(GO) list -m -f '{{.Version}}' sigs.k8s.io/kustomize/kustomize/v5 2>/dev/null)
+kustomize: ## Download kustomize locally if necessary.
+ GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5@$(or $(KUSTOMIZE_VERSION),latest)Run to verify kustomize/controller-gen are pinned in go.mod/tools.go:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tools.go references:"
rg -n 'sigs\.k8s\.io/(controller-tools|kustomize/kustomize/v5)' -- tools.go || true
echo "go.mod pinned versions:"
rg -nP 'sigs\.k8s\.io/controller-tools\s+v' go.mod || true
rg -nP 'sigs\.k8s\.io/kustomize/kustomize/v5\s+v' go.mod || trueLength of output: 230
Pin and mark the kustomize target as PHONY using the version from go.mod
Add a .PHONY: kustomize declaration, derive KUSTOMIZE_VERSION via go list -m from your pinned module version in go.mod (v5.5.0), and install with @$(or $(KUSTOMIZE_VERSION),latest) to prevent drift.
.PHONY: kustomize
KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize
# Prefer pinning via tools.go; this reads the version selected in go.mod.
KUSTOMIZE_VERSION ?= $(shell $(GO) list -m -f '{{.Version}}' sigs.k8s.io/kustomize/kustomize/v5 2>/dev/null)
kustomize: ## Download kustomize locally if necessary.
GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5@$(or $(KUSTOMIZE_VERSION),latest)📝 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.
| KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize | |
| kustomize: ## Download kustomize locally if necessary. | |
| GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5 | |
| .PHONY: kustomize | |
| KUSTOMIZE = $(PROJECT_DIR)/bin/kustomize | |
| # Prefer pinning via tools.go; this reads the version selected in go.mod. | |
| KUSTOMIZE_VERSION ?= $(shell $(GO) list -m -f '{{.Version}}' sigs.k8s.io/kustomize/kustomize/v5 2>/dev/null) | |
| kustomize: ## Download kustomize locally if necessary. | |
| GOBIN=$(PROJECT_DIR)/bin $(GO) install sigs.k8s.io/kustomize/kustomize/v5@$(or $(KUSTOMIZE_VERSION),latest) |
🤖 Prompt for AI Agents
In make/manifests.mk around lines 55-57, the kustomize target should be marked
phony and its version pinned from go.mod; add a .PHONY: kustomize line,
introduce a KUSTOMIZE_VERSION variable populated via go list -m for
sigs.k8s.io/kustomize/kustomize (so it picks up the pinned v5.5.0), and change
the install invocation to use @$(or $(KUSTOMIZE_VERSION),latest) when running
`go install` so the pinned module version is installed and the target won’t be
considered up-to-date.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
go.mod (1)
128-131: Remove disallowed replace directives (CI blocker).Local replaces to rsoaresd forks will fail CI and must be dropped before merge. Switch back to upstream codeready-toolchain modules once the related PRs are merged.
-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
🧹 Nitpick comments (1)
go.mod (1)
33-35: Prefer stable tags or tracked branch commits for tooling deps.Pseudo-versions for kube-openapi, sigs.k8s.io/json, and kustomize components are fine short-term; consider pinning to the versions recommended by controller-tools/controller-runtime for 1.32 to improve reproducibility.
Would you like me to look up the recommended combos for k8s 1.32 and propose exact pins?
Also applies to: 121-124
📜 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(3 hunks)
🧰 Additional context used
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 106-106: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 106-106: 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 (3)
go.mod (3)
15-23: K8s 1.32 alignment looks coherent.k8s.io/* v0.32.1 with controller-runtime v0.20.4 and controller-tools v0.17.2 is a consistent set. Nice.
Also applies to: 29-36, 119-126
22-22: Tool pin looks fine.Requiring kustomize/v5 explicitly to pin the CLI via tools.go is a common pattern. LGTM.
132-135: Go toolchain directives LGTM.go 1.23.0 with toolchain 1.23.12 is consistent with the project baseline.
| golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect | ||
| golang.org/x/mod v0.22.0 // indirect | ||
| golang.org/x/net v0.38.0 // indirect | ||
| golang.org/x/oauth2 v0.23.0 // indirect |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Upgrade vulnerable golang.org/x/oauth2.
v0.23.0 has known HIGH vulns; bump to a patched release (>= v0.27.0).
- golang.org/x/oauth2 v0.23.0 // indirect
+ golang.org/x/oauth2 v0.27.0 // indirect🌐 Web query:
What is the latest non-vulnerable version of golang.org/x/oauth2 as of today?
💡 Result:
Latest non-vulnerable version: v0.31.0. Fix for the vulnerability was released in v0.27.0. (pkg.go.dev, github.com)
Citations:
Upgrade golang.org/x/oauth2 to v0.31.0
v0.23.0 has a HIGH-severity vulnerability (GO-2025-3488); the fix was introduced in v0.27.0—upgrade to v0.31.0, the latest non-vulnerable release. cite12
- golang.org/x/oauth2 v0.23.0 // indirect
+ golang.org/x/oauth2 v0.31.0 // indirect🧰 Tools
🪛 OSV Scanner (2.1.0)
[HIGH] 106-106: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 106-106: 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 line 106, the project pins golang.org/x/oauth2 to v0.23.0 which
has a high-severity vulnerability; update the requirement to golang.org/x/oauth2
v0.31.0, then run go get golang.org/x/oauth2@v0.31.0 (or manually edit go.mod),
run go mod tidy to refresh go.sum, and run the test/build pipeline to ensure no
breakages.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
go.mod (2)
128-131: Remove disallowed replace directives (CI blocker).CI failed with “Disallowed module replacements detected.” These replaces to personal forks must be removed before merge; point requires to upstream codeready-toolchain commits instead.
-replace github.com/codeready-toolchain/api => github.com/rsoaresd/api v0.0.0-20250910155735-270447f471cd - -replace github.com/codeready-toolchain/toolchain-common => github.com/rsoaresd/toolchain-common v0.0.0-20250910164142-aabf15684683#!/bin/bash # Verify no disallowed replaces remain rg -n '^replace\s+github\.com/codeready-toolchain/' go.mod || echo "OK: no replaces"
106-106: Bump golang.org/x/oauth2 to a non‑vulnerable version.v0.23.0 has HIGH vulns; upgrade to >= v0.27.0 (suggest v0.31.0).
- golang.org/x/oauth2 v0.23.0 // indirect + golang.org/x/oauth2 v0.31.0 // indirect
🧹 Nitpick comments (2)
go.mod (2)
21-21: controller-tools version differs from PR description.PR text says v0.17.2; go.mod uses v0.17.3. Either align to 0.17.2 or update PR notes; confirm compatibility with operator-sdk toolchain.
15-16: Keep k8s patch levels consistent across modules.Minor mismatch: api/client-go/apiextensions/apimachinery at 0.32.2, but metrics/kubectl/cli-runtime at 0.32.1. Recommend bumping to 0.32.2 for parity to reduce surprise resolutions.
- k8s.io/metrics v0.32.1 + k8s.io/metrics v0.32.2 - k8s.io/kubectl v0.32.1 + k8s.io/kubectl v0.32.2 - k8s.io/cli-runtime v0.32.1 // indirect + k8s.io/cli-runtime v0.32.2 // indirectAlso applies to: 19-19, 34-35, 119-119
📜 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(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T10:28:33.280Z
Learnt from: fbm3307
PR: codeready-toolchain/member-operator#692
File: controllers/nstemplateset/nstemplatetier.go:137-141
Timestamp: 2025-09-02T10:28:33.280Z
Learning: In the member-operator codebase, MEMBER_OPERATOR_NAMESPACE is only used in OpenShift templates (with ${MEMBER_OPERATOR_NAMESPACE} syntax), not in go templates (which would use {{.MEMBER_OPERATOR_NAMESPACE}} syntax). Go templates in TierTemplateRevision resources use other parameters like SPACE_NAME, NAMESPACE, CONFIG_VALUE, etc., but not MEMBER_OPERATOR_NAMESPACE.
Applied to files:
go.mod
🪛 OSV Scanner (2.1.0)
go.mod
[HIGH] 106-106: golang.org/x/oauth2 0.23.0: Unexpected memory consumption during token parsing in golang.org/x/oauth2
(GO-2025-3488)
[HIGH] 106-106: 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: Disallowed module replacements detected in go.mod. The CI script reported: 'the above replacement(s) are not allowed in go.mod'. Build aborted with 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). (1)
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (2)
go.mod (2)
105-105: LGTM: x/net bumped to v0.38.0.Matches the discussion in the PR thread and works with Go 1.23.
132-134: LGTM: go/toolchain settings.go 1.23 with toolchain go1.23.12 aligns with the repo toolchain upgrades.
| // using latest commit from 'github.com/openshift/api branch release-4.19' | ||
| github.com/openshift/api v0.0.0-20250903165707-ce7baf0afbc4 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin openshift/api to a commit on release-4.19 (verify current SHA).
The pseudo-version may not belong to release-4.19. Ensure it resolves to that branch to avoid drift.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
remote=https://github.com/openshift/api
sha=$(printf '%s\n' 'ce7baf0afbc4')
git ls-remote "$remote" refs/heads/release-4.19 | awk '{print $1}' | grep -qi "$sha" && echo "OK on release-4.19" || echo "MISMATCH: $sha not on release-4.19"
echo "If mismatch, run: go get github.com/openshift/api@release-4.19 && go mod tidy"Length of output: 120
Pin openshift/api to a commit on release-4.19 — current pseudo-version mismatch
Verification: SHA ce7baf0afbc4 is NOT on release-4.19.
File: go.mod (lines 8-9).
Action: replace the pseudo-version with a commit that is actually on release-4.19 or point to the branch. Run:
go get github.com/openshift/api@release-4.19 && go mod tidy
Or pin to a specific release-4.19 commit:
go get github.com/openshift/api@ && go mod tidy
Verify the chosen SHA belongs to release-4.19 before committing.
🤖 Prompt for AI Agents
In go.mod around lines 8-9, the pseudo-version github.com/openshift/api
v0.0.0-20250903165707-ce7baf0afbc4 is not a commit on release-4.19; replace it
with either the branch ref or a commit SHA that actually belongs to
release-4.19. Run: go get github.com/openshift/api@release-4.19 && go mod tidy
to point to the branch, or find a specific full commit SHA on release-4.19 and
run go get github.com/openshift/api@<full-sha> && go mod tidy; verify the chosen
SHA is on release-4.19 before committing.
|
5 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 2934127
update dependencies
Related PRs
Issue ticket number and link
SANDBOX-1357
Summary by CodeRabbit
Documentation
Chores