[SANDBOX-1703] Upgrade deps to OCP 4.21#737
[SANDBOX-1703] Upgrade deps to OCP 4.21#737metlos wants to merge 9 commits intocodeready-toolchain:masterfrom
Conversation
- Update openshift/api to release-4.21, library-go to release-4.21 - Update k8s.io dependencies to v0.34.3 - Update controller-runtime to v0.22.5 - Add replace directives for api and toolchain-common forks - Fix useraccount test: normalize TypeMeta and ResourceVersion in noise object comparison (fake client Get no longer populates TypeMeta in v0.22) - Fix webhook deploy tests: use NewFakeClientWithManagedFields and add simulateUpdateManagedFields helper to properly test SSA migration flow with accurate managed fields after regular Updates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated controller-gen kubebuilder annotation from v0.18.0 → v0.19.0 across CRD manifests (nine files) and upgraded Go module dependencies (multiple direct and indirect bumps, replacement directives added in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 133-135: The go.mod contains personal-fork replace directives
pointing github.com/codeready-toolchain/api and
github.com/codeready-toolchain/toolchain-common to metlos forks; remove those
replace lines before merging to master. Fix by either (A) reverting/deleting the
two replace directives for "github.com/codeready-toolchain/api" and
"github.com/codeready-toolchain/toolchain-common" once the upstream PRs are
merged, running go mod tidy to update dependencies, or (B) move these local
overrides out of versioned go.mod into a local-only mechanism (e.g., a
git-ignored go.mod.local or a developer-only replace workflow) so master never
contains personal fork replaces. Ensure you run go mod tidy/go mod download
after the change to validate module resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c207f51d-e0b7-45e3-94bb-d19ca8d1a1aa
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
config/crd/bases/toolchain.dev.openshift.com_idlers.yamlconfig/crd/bases/toolchain.dev.openshift.com_memberoperatorconfigs.yamlconfig/crd/bases/toolchain.dev.openshift.com_memberstatuses.yamlconfig/crd/bases/toolchain.dev.openshift.com_nstemplatesets.yamlconfig/crd/bases/toolchain.dev.openshift.com_spacebindingrequests.yamlconfig/crd/bases/toolchain.dev.openshift.com_spacerequests.yamlconfig/crd/bases/toolchain.dev.openshift.com_toolchainclusters.yamlconfig/crd/bases/toolchain.dev.openshift.com_useraccounts.yamlconfig/crd/bases/toolchain.dev.openshift.com_workspaces.yamlcontrollers/useraccount/useraccount_controller_test.gogo.modpkg/webhook/deploy/deployment_test.go
| replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260318102359-237e681a639a | ||
|
|
||
| replace github.com/codeready-toolchain/toolchain-common => github.com/metlos/toolchain-common v0.0.0-20260318113116-fbd4bba9beb1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking replace directives in go.mod..."
rg -n '^replace\s+github.com/codeready-toolchain/(api|toolchain-common)\s*=>\s*github.com/metlos/' go.mod || true
API_COMMIT=237e681a639a
COMMON_COMMIT=fbd4bba9beb1
echo
echo "Verifying whether pinned commits exist only on fork remotes:"
echo "[api upstream]"
git ls-remote https://github.com/codeready-toolchain/api.git | rg "${API_COMMIT}" || true
echo "[api fork]"
git ls-remote https://github.com/metlos/api.git | rg "${API_COMMIT}" || true
echo
echo "[toolchain-common upstream]"
git ls-remote https://github.com/codeready-toolchain/toolchain-common.git | rg "${COMMON_COMMIT}" || true
echo "[toolchain-common fork]"
git ls-remote https://github.com/metlos/toolchain-common.git | rg "${COMMON_COMMIT}" || trueRepository: codeready-toolchain/member-operator
Length of output: 758
Remove personal-fork replace directives before merging to master.
Lines 133–135 reroute core dependencies to personal forks, creating maintainability and supply-chain risk on shared branches. While verification shows these commits exist as pending upstream pull requests (API #501, toolchain-common #522), using replace directives to circumvent the upstream review process is not appropriate for master.
Action: Either merge the upstream pull requests (which will release the commits to canonical modules) and remove these replace directives, or restrict the replace directives to feature branches only via .gitignore or a separate local go.mod.local workflow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 133 - 135, The go.mod contains personal-fork replace
directives pointing github.com/codeready-toolchain/api and
github.com/codeready-toolchain/toolchain-common to metlos forks; remove those
replace lines before merging to master. Fix by either (A) reverting/deleting the
two replace directives for "github.com/codeready-toolchain/api" and
"github.com/codeready-toolchain/toolchain-common" once the upstream PRs are
merged, running go mod tidy to update dependencies, or (B) move these local
overrides out of versioned go.mod into a local-only mechanism (e.g., a
git-ignored go.mod.local or a developer-only replace workflow) so master never
contains personal fork replaces. Ensure you run go mod tidy/go mod download
after the change to validate module resolution.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, metlos 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 |
…o the controller-runtime update that is now no longer happening
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
go.mod (1)
133-135:⚠️ Potential issue | 🟠 MajorRemove personal-fork
replacedirectives before merging.The
replacedirectives redirect core dependencies (github.com/codeready-toolchain/apiandtoolchain-common) to personal fork repositories at specific commit hashes. Merging these tomastercreates significant supply-chain and maintainability risks:
- Supply-chain risk: Dependencies are sourced from personal repositories rather than the canonical upstream, bypassing the upstream review process.
- Maintainability burden: Future updates require manual tracking of fork commits; the dependency graph becomes opaque to standard Go tooling.
- Critical dependency scope: The context snippets show these modules are used extensively across controllers for condition management (
condition.FindConditionByType,condition.AddOrUpdateStatusConditions), client utilities (applycl.NewApplyClient,commonclient.SameGVKandName), controller patterns (commoncontroller), and event filtering (commonpredicates).Action required: Either (A) merge the corresponding upstream pull requests and remove these
replacedirectives, updating dependencies viago get, or (B) restrict fork-basedreplacedirectives to feature branches only (e.g., via.gitignoreor local-onlygo.mod.localworkflow).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 133 - 135, Remove the personal-fork replace directives from go.mod (the two replace lines pointing github.com/codeready-toolchain/* to github.com/metlos/*) before merging; either merge the corresponding upstream PRs and update the modules using `go get`/`go mod tidy` so the canonical github.com/codeready-toolchain modules are used, or keep these replaces only in a local/feature-only mechanism (e.g., ignored go.mod.local) to avoid committing them. Look for usages in code paths referencing condition.FindConditionByType, condition.AddOrUpdateStatusConditions, applycl.NewApplyClient, commonclient.SameGVKandName, commoncontroller and commonpredicates to ensure compatibility after switching back to upstream, and run `go test`/`go vet` and `go mod tidy` to fix any version mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Around line 6-22: Update the controller-runtime dependency to a v0.22.x
release to match k8s.io v0.34.x: replace the current module entry
"sigs.k8s.io/controller-runtime v0.21.0" in go.mod with a v0.22.x version (e.g.,
v0.22.0 or newer v0.22.patch), run "go get
sigs.k8s.io/controller-runtime@v0.22.x" and "go mod tidy" to refresh
dependencies, and ensure the module's Go version (go directive) is set to 1.24
or higher to satisfy k8s.io v0.34.x requirements.
---
Duplicate comments:
In `@go.mod`:
- Around line 133-135: Remove the personal-fork replace directives from go.mod
(the two replace lines pointing github.com/codeready-toolchain/* to
github.com/metlos/*) before merging; either merge the corresponding upstream PRs
and update the modules using `go get`/`go mod tidy` so the canonical
github.com/codeready-toolchain modules are used, or keep these replaces only in
a local/feature-only mechanism (e.g., ignored go.mod.local) to avoid committing
them. Look for usages in code paths referencing condition.FindConditionByType,
condition.AddOrUpdateStatusConditions, applycl.NewApplyClient,
commonclient.SameGVKandName, commoncontroller and commonpredicates to ensure
compatibility after switching back to upstream, and run `go test`/`go vet` and
`go mod tidy` to fix any version mismatches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9c4f819-e725-40d5-8a3f-b7f20b4707e4
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
| github.com/go-logr/logr v1.4.3 | ||
| github.com/google/go-cmp v0.7.0 | ||
| // using latest commit from 'github.com/openshift/api branch release-4.20' | ||
| github.com/openshift/api v0.0.0-20251202204302-1cb53e34ca33 | ||
| // using latest commit from 'github.com/openshift/api branch release-4.21' | ||
| github.com/openshift/api v0.0.0-20260304172252-b0658d22beea | ||
| github.com/pkg/errors v0.9.1 | ||
| github.com/redhat-cop/operator-utils v1.3.8 | ||
| github.com/stretchr/testify v1.10.0 | ||
| github.com/stretchr/testify v1.11.1 | ||
| go.uber.org/zap v1.27.0 | ||
| gopkg.in/h2non/gock.v1 v1.0.14 | ||
| k8s.io/api v0.33.4 | ||
| k8s.io/client-go v0.33.4 | ||
| k8s.io/api v0.34.1 | ||
| k8s.io/client-go v0.34.1 | ||
| k8s.io/klog v1.0.0 | ||
| k8s.io/klog/v2 v2.130.1 | ||
| k8s.io/metrics v0.33.4 | ||
| k8s.io/metrics v0.34.1 | ||
| sigs.k8s.io/controller-runtime v0.21.0 | ||
| sigs.k8s.io/controller-tools v0.18.0 | ||
| sigs.k8s.io/kustomize/kustomize/v5 v5.6.0 | ||
| sigs.k8s.io/controller-tools v0.19.0 | ||
| sigs.k8s.io/kustomize/kustomize/v5 v5.7.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify k8s 0.34.x and controller-runtime 0.21.0 compatibility
echo "=== Checking for known breaking changes in Kubernetes 0.34.x ==="
echo "Searching CHANGELOG and release notes..."
# Check if there are references to v0.34 upgrade issues in test files
echo -e "\n=== Checking test files for upgrade-related changes ==="
rg -n -C3 --type=go 'fake.*[Cc]lient|managed.*field' --glob='*_test.go' | head -50
# Look for controller-runtime usage patterns that might be affected
echo -e "\n=== Checking controller-runtime client usage ==="
rg -n --type=go 'Client\.Status\(\)\.Update|Client\.Update|Client\.Patch' --glob='controllers/**/*.go' | head -20Repository: codeready-toolchain/member-operator
Length of output: 1818
🌐 Web query:
What are the breaking changes in Kubernetes 1.34 (k8s.io v0.34.x) client-go and apimachinery?
💡 Result:
k8s.io/client-go (v0.34.x) — breaking changes
- Minimum Go version bumped to Go 1.24:
k8s.io/client-go@v0.34.0declaresgo 1.24.0(andgodebug default=go1.24). If you build with an older Go toolchain, your build will fail. [1]
I couldn’t find any other explicitly documented client-go API removals / signature breaks specific to the v0.34.0 bump in upstream release notes/changelogs (the rest of the notable items I found were fixes or internal dependency refactors, not consumer-facing breaks).
k8s.io/apimachinery (v0.34.x) — breaking changes
- Minimum Go version bumped to Go 1.24:
k8s.io/apimachinery@v0.34.0declaresgo 1.24.0(andgodebug default=go1.24). This is a build-breaking change for consumers on older Go versions. [2]
Notable (but not breaking yet): apimachinery deprecations in the 1.34 line
- Deprecated
MessageCountMapandCreateAggregateFromMessageCountMapin apimachinery. (Deprecation is not an immediate breaking change, but it’s the main “API change” called out in 1.34-related notes I found.) [3]
Sources
[1] k8s.io/client-go v0.34.0 go.mod (Go 1.24 requirement) (raw.githubusercontent.com)
[2] k8s.io/apimachinery v0.34.0 go.mod (Go 1.24 requirement) (raw.githubusercontent.com)
[3] Kubernetes 1.34 changelog note: apimachinery deprecations (raw.githubusercontent.com)
Citations:
- 1: https://raw.githubusercontent.com/kubernetes/client-go/v0.34.0/go.mod
- 2: https://raw.githubusercontent.com/kubernetes/apimachinery/v0.34.0/go.mod
- 3: https://raw.githubusercontent.com/kubernetes/kubernetes/master/CHANGELOG/CHANGELOG-1.34.md
🌐 Web query:
Is controller-runtime v0.21.0 compatible with Kubernetes 1.34 (k8s.io v0.34.x)?
💡 Result:
No—controller-runtime v0.21.0 is not the supported/tested match for Kubernetes 1.34 (k8s.io v0.34.x).
- The controller-runtime project states that each minor controller-runtime release is tested against a specific k8s.io/ (client-go) minor*, and other combinations are “by chance” and not supported nor tested. In its compatibility table, controller-runtime v0.21 → k8s.io/ v0.33* (i.e., Kubernetes 1.33), while v0.22 → k8s.io/ v0.34* (i.e., Kubernetes 1.34). [1]
- The controller-runtime release notes for the v0.22 line explicitly include updating k8s.io/ from v0.33.0 to v0.34.0*. [2]
Recommendation: For Kubernetes 1.34 / k8s.io v0.34.x, use controller-runtime v0.22.x (or newer), not v0.21.0. [1][2]
Sources:
[1] controller-runtime README “Compatibility” table and policy (github.com)
[2] controller-runtime GitHub Releases (k8s.io/* v0.33 → v0.34 change) (github.com)
Citations:
- 1: https://github.com/kubernetes-sigs/controller-runtime
- 2: https://github.com/kubernetes-sigs/controller-runtime/releases
Upgrade controller-runtime to v0.22.x to match Kubernetes 1.34 (k8s.io v0.34.x).
The current go.mod pairs controller-runtime v0.21.0 with k8s.io v0.34.x, but this is an unsupported combination. According to the controller-runtime compatibility matrix, v0.21 is tested against k8s.io v0.33, while v0.22.x is the supported release for k8s.io v0.34.x. This mismatch is not tested and introduces risk of subtle runtime failures.
Additionally, k8s.io v0.34.x requires Go 1.24—verify that your build targets Go 1.24 or later.
Upgrade to controller-runtime v0.22.x to align with the Kubernetes 1.34 ecosystem.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 6 - 22, Update the controller-runtime dependency to a
v0.22.x release to match k8s.io v0.34.x: replace the current module entry
"sigs.k8s.io/controller-runtime v0.21.0" in go.mod with a v0.22.x version (e.g.,
v0.22.0 or newer v0.22.patch), run "go get
sigs.k8s.io/controller-runtime@v0.22.x" and "go mod tidy" to refresh
dependencies, and ensure the module's Go version (go directive) is set to 1.24
or higher to satisfy k8s.io v0.34.x requirements.
|
|
Postponed until operator-sdk is upgradable, too. |
2 participants
$TITLENote that this required slight updates to the test logic to account for the still imperfect managed fields handling in the fake client.
Summary by CodeRabbit