WIP: Rebase 1.37 - #2408
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jubittajohn The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThe change updates Go dependencies, adds conditions-aware authorization methods, returns authorization union construction errors, and removes manifest client feature-gate initialization. ChangesAuthorization and dependency updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Around line 41-49: Replace the Kubernetes v0.37.0-beta.0 dependencies in
go.mod, including the modules shown around k8s.io/api and related packages, with
approved stable versions and synchronize vendor/modules.txt. If a beta
dependency is intentionally retained, document the approved exception and ensure
this dependency graph is excluded from production builds.
- Around line 153-155: Remove the non-canonical github.com/openshift/api and
github.com/openshift/client-go replace directives so canonical OpenShift modules
are used. If fork usage is explicitly approved, retain them only with documented
owner approval and verified commits, module declarations, go.sum hashes,
licensing, OSV status, SBOM provenance, and signed artifacts.
- Line 140: Upgrade the google.golang.org/grpc dependency from v1.81.1 to
v1.82.1 or later, ensuring the module graph does not downgrade it. Refresh the
corresponding go.sum entries and vendor/modules.txt metadata to match the
upgraded dependency.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| k8s.io/api v0.37.0-beta.0 | ||
| k8s.io/apiextensions-apiserver v0.37.0-beta.0 | ||
| k8s.io/apimachinery v0.37.0-beta.0 | ||
| k8s.io/apiserver v0.37.0-beta.0 | ||
| k8s.io/client-go v0.37.0-beta.0 | ||
| k8s.io/component-base v0.37.0-beta.0 | ||
| k8s.io/klog/v2 v2.140.0 | ||
| k8s.io/kms v0.36.2 | ||
| k8s.io/kube-aggregator v0.36.2 | ||
| k8s.io/kms v0.37.0-beta.0 | ||
| k8s.io/kube-aggregator v0.37.0-beta.0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
if go list -m all | rg -- '-(alpha|beta|rc)([.-]|$)'; then
echo "Pre-release modules are present in the resolved graph."
exit 1
fiRepository: openshift/library-go
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- go.mod context ---'
sed -n '1,90p' go.mod
printf '%s\n' '--- repository metadata relevant to release intent ---'
for f in README.md CONTRIBUTING.md RELEASE.md .github/workflows/go.yml .github/workflows/ci.yml; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
rg -n -i 'release|production|stable|beta|vendor|mod=' "$f" || true
fi
done
printf '%s\n' '--- vendor metadata ---'
if [ -f vendor/modules.txt ]; then
rg -n 'k8s.io/(api|apiextensions-apiserver|apimachinery|apiserver|client-go|component-base|klog/v2|kms|kube-aggregator)' vendor/modules.txt | head -30
else
echo 'vendor/modules.txt not present'
fiRepository: openshift/library-go
Length of output: 5201
Security Misconfiguration (CWE-1104)
Replace the Kubernetes beta dependencies before merging.
go.mod and vendor/modules.txt resolve multiple Kubernetes modules to v0.37.0-beta.0. Replace them with approved stable versions, or document an approved exception and exclude this dependency graph from production builds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` around lines 41 - 49, Replace the Kubernetes v0.37.0-beta.0
dependencies in go.mod, including the modules shown around k8s.io/api and
related packages, with approved stable versions and synchronize
vendor/modules.txt. If a beta dependency is intentionally retained, document the
approved exception and ensure this dependency graph is excluded from production
builds.
Source: Path instructions
| golang.org/x/tools v0.47.0 // indirect | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect | ||
| google.golang.org/grpc v1.81.1 // indirect |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
go list -m all | rg '^google.golang.org/grpc '
go run golang.org/x/vuln/cmd/govulncheck@latest ./...
make precommitRepository: openshift/library-go
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- module declarations ---'
sed -n '1,180p' go.mod | nl -ba | sed -n '125,150p'
printf '%s\n' '--- grpc references ---'
rg -n --glob '!vendor/**' --glob '*.go' 'google.golang.org/grpc|xds|RBAC' .
printf '%s\n' '--- vendored grpc version ---'
rg -n 'google.golang.org/grpc|v1\.81\.1|v1\.82\.1' vendor/modules.txt vendor 2>/dev/null | head -80 || true
printf '%s\n' '--- go.sum entries ---'
rg -n '^google\.golang\.org/grpc ' go.sum 2>/dev/null || true
printf '%s\n' '--- OSV advisory ---'
curl -fsSL 'https://api.osv.dev/v1/vulns/GO-2026-6061' | python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({k:d.get(k) for k in ("id","summary","details","affected","references")}, indent=2))'Repository: openshift/library-go
Length of output: 226
🌐 Web query:
OSV GO-2026-6061 google.golang.org/grpc v1.81.1 affected versions fixed version
💡 Result:
Vulnerability GO-2026-6061 affects the google.golang.org/grpc package [1][2]. This vulnerability covers issues within the xDS RBAC (Role-Based Access Control) authorization engine and the HTTP/2 transport server implementation [1][2]. Versions of google.golang.org/grpc prior to v1.82.1 are affected [2]. Users are advised to upgrade to v1.82.1 or later to address the issue [2][3].
Citations:
- 1: https://osv.dev/vulnerability/GO-2026-6061
- 2: https://pkg.go.dev/vuln/GO-2026-6061
- 3: go-steer/core-agent@3724f7a
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- module declarations ---'
sed -n '125,150p' go.mod
printf '%s\n' '--- grpc references ---'
rg -n --glob '!vendor/**' --glob '*.go' 'google.golang.org/grpc|xds|RBAC' . || true
printf '%s\n' '--- vendored grpc version ---'
rg -n 'google.golang.org/grpc|v1\.81\.1|v1\.82\.1' vendor/modules.txt vendor 2>/dev/null | head -80 || true
printf '%s\n' '--- go.sum entries ---'
rg -n '^google\.golang\.org/grpc ' go.sum 2>/dev/null || true
printf '%s\n' '--- OSV advisory ---'
curl -fsSL 'https://api.osv.dev/v1/vulns/GO-2026-6061' \
| python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({k:d.get(k) for k in ("id","summary","details","affected","references")}, indent=2))' \
|| trueRepository: openshift/library-go
Length of output: 14895
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- package dependency reachability ---'
go list -mod=vendor -deps ./... 2>/tmp/go-list-errors \
| rg '^google\.golang\.org/grpc($|/)' \
| sort -u || true
printf '%s\n' '--- package-loading errors ---'
cat /tmp/go-list-errors
printf '%s\n' '--- direct source imports ---'
rg -n --glob '*.go' --glob '!vendor/**' \
'^[[:space:]]*"google\.golang\.org/grpc($|/)' . || trueRepository: openshift/library-go
Length of output: 3081
Vulnerable GRPC HTTP/2 Transport Dependency (CWE-400): Uncontrolled Resource Consumption
Upgrade google.golang.org/grpc and refresh vendored dependencies.
v1.81.1 is affected by GO-2026-6061, which is fixed in v1.82.1. The package graph includes the vulnerable HTTP/2 transport. Upgrade to v1.82.1 or later, then update go.sum and vendor/modules.txt without allowing a downgrade. (OSV)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` at line 140, Upgrade the google.golang.org/grpc dependency from
v1.81.1 to v1.82.1 or later, ensuring the module graph does not downgrade it.
Refresh the corresponding go.sum entries and vendor/modules.txt metadata to
match the upgraded dependency.
Sources: Path instructions, Learnings, Linters/SAST tools
| replace ( | ||
| github.com/openshift/api => github.com/jacobsee/openshift-api v0.0.0-20260723063153-cebda3324a87 | ||
| github.com/openshift/client-go => github.com/jubittajohn/client-go v0.0.0-20260805200440-5e8c00d8a4c0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
go list -m -json github.com/openshift/api github.com/openshift/client-go
go mod graph | rg 'github.com/(openshift|jacobsee|jubittajohn)'
test -f go.sum
go mod verifyRepository: openshift/library-go
Length of output: 21922
Security Misconfiguration (CWE-829): Inclusion of Functionality from Untrusted Control Sphere
Remove or formally approve the non-canonical OpenShift module replacements.
These replace directives make builds consume external fork modules while retaining canonical github.com/openshift/... imports. Restore the canonical modules unless explicit owner approval exists. Otherwise, document the justification and verify the pinned commits, module declarations, go.sum hashes, license compatibility, OSV status, SBOM provenance, and signed artifacts. go mod verify does not establish fork ownership or supply-chain provenance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` around lines 153 - 155, Remove the non-canonical
github.com/openshift/api and github.com/openshift/client-go replace directives
so canonical OpenShift modules are used. If fork usage is explicitly approved,
retain them only with documented owner approval and verified commits, module
declarations, go.sum hashes, licensing, OSV status, SBOM provenance, and signed
artifacts.
Source: Path instructions
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/authorization/hardcodedauthorizer/metrics.go (1)
35-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for conditions-aware authorization.
TestAuthorizeronly callsAuthorize. TestConditionsAwareAuthorizewith allow and no-opinion attributes, and testEvaluateConditionsforDecisionDeny, an empty reason, andauthorizer.ErrorConditionEvaluationNotSupported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/authorization/hardcodedauthorizer/metrics.go` around lines 35 - 42, Add regression tests for metricsAuthorizer.ConditionsAwareAuthorize using both allow and no-opinion attributes, verifying the corresponding conditions-aware decisions. Add coverage for EvaluateConditions asserting DecisionDeny, an empty reason, and authorizer.ErrorConditionEvaluationNotSupported.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/authorization/hardcodedauthorizer/metrics.go`:
- Around line 35-42: Add regression tests for
metricsAuthorizer.ConditionsAwareAuthorize using both allow and no-opinion
attributes, verifying the corresponding conditions-aware decisions. Add coverage
for EvaluateConditions asserting DecisionDeny, an empty reason, and
authorizer.ErrorConditionEvaluationNotSupported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 172763d3-c758-4e40-aabe-3f83abc170b2
📒 Files selected for processing (4)
pkg/authorization/hardcodedauthorizer/metrics.gopkg/controller/controllercmd/builder.gopkg/manifestclient/read_roundtripper.gopkg/manifestclient/testing/list_test.go
💤 Files with no reviewable changes (2)
- pkg/manifestclient/testing/list_test.go
- pkg/manifestclient/read_roundtripper.go
Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
…nges in k8s 1.37 Signed-off-by: jubittajohn <jujohn@redhat.com>
The feature gate graduated to GA and was removed in k8s 1.37. LabelSelector is now populated unconditionally in RequestInfo. Signed-off-by: jubittajohn <jujohn@redhat.com>
5080e73 to
8dd0468
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
go.mod (2)
140-140:⚠️ Potential issue | 🟠 MajorUpgrade
google.golang.org/grpcbefore merge.Line 140 selects
v1.81.1. GO-2026-6061 affects versions beforev1.82.1and covers the HTTP/2 transport and xDS RBAC packages. (osv.dev) Upgrade tov1.82.1or later, then refreshgo.sumand vendored metadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go.mod` at line 140, Upgrade the google.golang.org/grpc dependency from v1.81.1 to v1.82.1 or later, then regenerate go.sum and all vendored metadata to match the selected version.Source: Linters/SAST tools
41-49:⚠️ Potential issue | 🟠 MajorDo not merge the Kubernetes beta dependencies without an approved exception.
The module graph selects
v0.37.0-beta.0for multiple Kubernetes modules, includingk8s.io/streaming. Replace these versions with approved stable releases, or document an approved exception and prevent this graph from entering production builds.Also applies to: 146-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go.mod` around lines 41 - 49, Replace the v0.37.0-beta.0 Kubernetes dependencies in go.mod, including the selected k8s.io/streaming version, with approved stable releases across the module graph. If beta versions are required, document the approved exception and add a production-build safeguard preventing this dependency graph from being used.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@go.mod`:
- Around line 153-155: Remove the non-canonical replace directives for
github.com/openshift/api and github.com/openshift/client-go from the go.mod
replace block; if they are required, obtain formal owner approval and document
the requested provenance, licensing, vulnerability, hash, SBOM, and signature
verification before retaining them.
---
Duplicate comments:
In `@go.mod`:
- Line 140: Upgrade the google.golang.org/grpc dependency from v1.81.1 to
v1.82.1 or later, then regenerate go.sum and all vendored metadata to match the
selected version.
- Around line 41-49: Replace the v0.37.0-beta.0 Kubernetes dependencies in
go.mod, including the selected k8s.io/streaming version, with approved stable
releases across the module graph. If beta versions are required, document the
approved exception and add a production-build safeguard preventing this
dependency graph from being used.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| replace ( | ||
| github.com/openshift/api => github.com/jacobsee/openshift-api v0.0.0-20260806204542-cfe07ec0f95b | ||
| github.com/openshift/client-go => github.com/jubittajohn/client-go v0.0.0-20260806210316-cf531a43afcf |
There was a problem hiding this comment.
Remove or formally approve the non-canonical module replacements.
These directives redirect canonical OpenShift imports to external fork modules. Remove them before merge, or document owner approval and verify commit provenance, module declarations, licenses, OSV status, hashes, SBOM provenance, and artifact signatures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@go.mod` around lines 153 - 155, Remove the non-canonical replace directives
for github.com/openshift/api and github.com/openshift/client-go from the go.mod
replace block; if they are required, obtain formal owner approval and document
the requested provenance, licensing, vulnerability, hash, SBOM, and signature
verification before retaining them.
Source: Path instructions
|
@jubittajohn: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Summary by CodeRabbit