[SANDBOX-1701] chore: upgrade to OCP 4.21 dependencies#522
[SANDBOX-1701] chore: upgrade to OCP 4.21 dependencies#522metlos wants to merge 3 commits intocodeready-toolchain:masterfrom
Conversation
Update openshift/api and openshift/library-go to release-4.21, k8s.io deps to v0.34.3, controller-runtime to v0.22.5. Adapt test FakeClient to work with real SSA support: - Remove Apply→Merge workaround (SSA is now natively supported) - Handle generation bump for SSA-created objects - Work around controller-runtime fake client bug where Update discards caller-set managedFields (needed for csaupgrade migration) - Add NewFakeClientWithManagedFields for tests that verify SSA managed fields behavior - Fix SSA migration test to provide APIVersion in ManagedFieldsEntry and use a non-empty Service spec for proper field tracking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughUpdates Go module dependencies to newer versions across OpenShift, Kubernetes, Prometheus, and related packages. Adds a replace directive for the codeready-toolchain API module. Adds managed-fields support to the test fake client and expands SSA-related tests to validate managed fields behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
📝 Coding Plan
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 126-127: The go.mod contains a replace directive "replace
github.com/codeready-toolchain/api => github.com/metlos/api
v0.0.0-20260311111036-f94bc6f41690" that points to a personal fork; either
remove this replace or replace it with the official module, and if keeping it
temporarily, add an inline comment and a linked issue/PR that documents why the
fork is required and the plan to revert (e.g., add a TODO with issue/PR number
and an expiration rationale next to the replace directive), and update the
commit message to explain the temporary nature and timeline for removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef7f35c2-5aec-455e-9990-5b5b276c6e61
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.modpkg/client/ssa_client_test.gopkg/test/client.go
go.mod
Outdated
|
|
||
| replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260311111036-f94bc6f41690 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's an open PR in the codeready-toolchain/api repo that this fork might be based on
echo "Checking the fork commit referenced in the replace directive..."
echo "Fork: github.com/metlos/api @ v0.0.0-20260311111036-f94bc6f41690"
echo ""
echo "This appears to be a personal fork. Verify that:"
echo "1. The required API changes are being upstreamed"
echo "2. This replace directive will be removed before merge or shortly after"
echo ""
# Check if the main API repo has recent activity that might include these changes
gh api repos/codeready-toolchain/api/pulls --jq '.[0:5] | .[] | {number, title, state, user: .user.login}' 2>/dev/null || echo "Could not fetch PR info"Repository: codeready-toolchain/toolchain-common
Length of output: 516
🏁 Script executed:
# Check the actual content of go.mod at the specified lines
cat -n go.mod | sed -n '120,135p'Repository: codeready-toolchain/toolchain-common
Length of output: 502
🏁 Script executed:
# Check git history for this change
git log --oneline -20 -- go.modRepository: codeready-toolchain/toolchain-common
Length of output: 127
🏁 Script executed:
# Look for any recent commits that added this replace directive
git log -p -S "metlos/api" -- go.mod | head -100Repository: codeready-toolchain/toolchain-common
Length of output: 62
🏁 Script executed:
# Get the full commit message and diff context
git show bbfe574 --stat | head -50Repository: codeready-toolchain/toolchain-common
Length of output: 2799
🏁 Script executed:
# Check if there's any branch information or PR context
git log bbfe574 -1 --format=fullerRepository: codeready-toolchain/toolchain-common
Length of output: 1104
🏁 Script executed:
# Check if there are any CHANGELOG, PR templates, or issue references
git log bbfe574 -1 --format=%BRepository: codeready-toolchain/toolchain-common
Length of output: 814
🏁 Script executed:
# Check if the replace directive is still in the current HEAD
git rev-parse HEADRepository: codeready-toolchain/toolchain-common
Length of output: 120
🏁 Script executed:
# Verify this is the actual current state
cat go.mod | grep -A 2 -B 2 "replace github.com/codeready-toolchain/api"Repository: codeready-toolchain/toolchain-common
Length of output: 185
🏁 Script executed:
# Check if there are any recent commits after this that might address it
git log --oneline -10 -- go.modRepository: codeready-toolchain/toolchain-common
Length of output: 127
Remove or document the replace directive for the personal fork of codeready-toolchain/api.
The replace directive points to a personal fork (github.com/metlos/api) instead of the official repository. While this was included in the OCP 4.21 dependency upgrade commit, there is no explanation in the commit message for why this fork is necessary. This creates maintenance and security risks:
- Personal forks can be deleted, archived, or diverge from the official repo
- It creates an undocumented dependency that complicates dependency management
- It may cause resolution issues in CI/CD pipelines or for downstream consumers
Either document the temporary nature of this fork and the plan to remove it, or replace it with the official github.com/codeready-toolchain/api repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` around lines 126 - 127, The go.mod contains a replace directive
"replace github.com/codeready-toolchain/api => github.com/metlos/api
v0.0.0-20260311111036-f94bc6f41690" that points to a personal fork; either
remove this replace or replace it with the official module, and if keeping it
temporarily, add an inline comment and a linked issue/PR that documents why the
fork is required and the plan to revert (e.g., add a TODO with issue/PR number
and an expiration rationale next to the replace directive), and update the
commit message to explain the temporary nature and timeline for removal.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
go.mod (1)
126-127:⚠️ Potential issue | 🟠 MajorThe personal fork replace directive persists.
The replace directive still points to a personal fork (
github.com/metlos/api), though updated to a newer commit. The concerns raised in the previous review comment regarding maintainability, security, and dependency resolution remain applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` around lines 126 - 127, Remove the personal-fork replace directive that references "github.com/metlos/api" in go.mod and instead point the module to the official upstream module (restore or add a versioned requirement for "github.com/codeready-toolchain/api" with a proper semantic version or a vetted commit pseudo-version); if a forked module is absolutely required for a temporary fix, replace it with an organizationally-approved fork and document the reason in a comment, and ensure the replace line referencing "github.com/metlos/api" is deleted or replaced accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@go.mod`:
- Around line 126-127: Remove the personal-fork replace directive that
references "github.com/metlos/api" in go.mod and instead point the module to the
official upstream module (restore or add a versioned requirement for
"github.com/codeready-toolchain/api" with a proper semantic version or a vetted
commit pseudo-version); if a forked module is absolutely required for a
temporary fix, replace it with an organizationally-approved fork and document
the reason in a comment, and ensure the replace line referencing
"github.com/metlos/api" is deleted or replaced accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc918ed0-04a8-425d-8a72-c1b9acbe4598
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
go.mod
|
| } | ||
|
|
||
| return cl.Client.Update(ctx, obj, opts...) | ||
| // Work around a controller-runtime fake client bug where Update unconditionally |
There was a problem hiding this comment.
interesting, is there a ling to an issue we could track?
3 participants
Update openshift/api and openshift/library-go to release-4.21, k8s.io deps to v0.34.1, controller-runtime to v0.22.0.
Adapt test FakeClient to work with real SSA support:
Assisted-by: Claude Code
Summary by CodeRabbit
Chores
Tests