Skip to content

WIP: Rebase 1.37 - #2408

Open
jubittajohn wants to merge 4 commits into
openshift:masterfrom
jubittajohn:rebase-1.37
Open

WIP: Rebase 1.37#2408
jubittajohn wants to merge 4 commits into
openshift:masterfrom
jubittajohn:rebase-1.37

Conversation

@jubittajohn

@jubittajohn jubittajohn commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Chores
    • Completed routine platform maintenance and compatibility updates across supported infrastructure components.
    • Improved authorization integration and handling for compatibility with newer platform capabilities.
    • Enhanced handling of authorization conditions and configuration errors.
    • Removed obsolete feature-gate initialization and outdated test documentation.
    • No new user-facing features were added; existing functionality remains unchanged aside from improved compatibility and error handling.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 5, 2026
@openshift-ci
openshift-ci Bot requested review from bertinatto and deads2k August 5, 2026 21:13
@openshift-ci

openshift-ci Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jubittajohn
Once this PR has been reviewed and has the lgtm label, please assign bertinatto for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Walkthrough

The change updates Go dependencies, adds conditions-aware authorization methods, returns authorization union construction errors, and removes manifest client feature-gate initialization.

Changes

Authorization and dependency updates

Layer / File(s) Summary
Go module dependency updates
go.mod
Updates direct and indirect dependencies, removes two indirect modules, and adds OpenShift API and client-go replacement directives.
Conditions-aware authorization and union error handling
pkg/authorization/hardcodedauthorizer/metrics.go, pkg/controller/controllercmd/builder.go
Adds conditions-aware methods to the metrics authorizer. Names the union authorizers and returns errors from union.New.
Manifest client feature-gate cleanup
pkg/manifestclient/read_roundtripper.go, pkg/manifestclient/testing/list_test.go
Removes AuthorizeWithSelectors initialization and its related imports. Removes the outdated test comment.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: needs-rebase

Suggested reviewers: bertinatto, deads2k


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Sensitive-Data-In-Logs ❌ Error The vendored etcd update adds zap.String("peer", p.String()) to a retry warning; gRPC Peer.String() logs remote and local addresses that may contain internal hostnames. Remove the peer field or redact endpoint values before logging. Log only a non-sensitive retry identifier.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title identifies a rebase to version 1.37, which relates to the dependency and compatibility updates but does not clearly describe the authorization changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The project has no Ginkgo dependency, imports, or DSL title calls; the only changed test uses the static Go title TestListWithLabelSelector_FindsExpectedConfigMaps.
Test Structure And Quality ✅ Passed The only changed test is a standard Go test, not Ginkgo; it reads static fixtures, creates no cluster resources, uses no waits, and includes diagnostic failure messages.
Microshift Test Compatibility ✅ Passed The full PR adds no Ginkgo e2e tests. The only changed test is a unit test with one comment removed, so this MicroShift compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The full PR adds no Ginkgo e2e tests. The only changed test is a standard Go unit test with one outdated comment removed, so no SNO multi-node assumption is introduced.
Topology-Aware Scheduling Compatibility ✅ Passed The PR changes dependency, authorization, and manifest-client code only; the controller diff adds no scheduling constraints, replicas, affinities, tolerations, topology spreads, selectors, or PDBs.
Ote Binary Stdout Contract ✅ Passed The PR adds no process-level stdout writes or suite setup. Existing fmt writes are in test execution helpers, and vendored klog documents stderr as its default output.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR adds no Ginkgo e2e tests; its only test-file change removes a comment from an existing unit test, and no network-sensitive test code was added.
No-Weak-Crypto ✅ Passed The PR adds no weak-crypto implementation or secret comparison; MD5/SHA1 additions are OpenTelemetry metadata, and x/crypto OpenPGP changes only add deprecation comments.
Container-Privileges ✅ Passed The PR adds no repository Kubernetes/container manifests. Changed manifest-like files are vendor metadata, and added-line scans found no prohibited privilege settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread go.mod
Comment on lines +41 to +49
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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
fi

Repository: 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'
fi

Repository: 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

Comment thread go.mod
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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 precommit

Repository: 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:


🏁 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))' \
  || true

Repository: 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($|/)' . || true

Repository: 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

Comment thread go.mod Outdated
Comment on lines +153 to +155
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 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 verify

Repository: 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

@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
pkg/authorization/hardcodedauthorizer/metrics.go (1)

35-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for conditions-aware authorization. TestAuthorizer only calls Authorize. Test ConditionsAwareAuthorize with allow and no-opinion attributes, and test EvaluateConditions for DecisionDeny, an empty reason, and authorizer.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

📥 Commits

Reviewing files that changed from the base of the PR and between 96c45c3 and 5080e73.

📒 Files selected for processing (4)
  • pkg/authorization/hardcodedauthorizer/metrics.go
  • pkg/controller/controllercmd/builder.go
  • pkg/manifestclient/read_roundtripper.go
  • pkg/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>
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
go.mod (2)

140-140: ⚠️ Potential issue | 🟠 Major

Upgrade google.golang.org/grpc before merge.

Line 140 selects v1.81.1. GO-2026-6061 affects versions before v1.82.1 and covers the HTTP/2 transport and xDS RBAC packages. (osv.dev) Upgrade to v1.82.1 or later, then refresh go.sum and 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 | 🟠 Major

Do not merge the Kubernetes beta dependencies without an approved exception.

The module graph selects v0.37.0-beta.0 for multiple Kubernetes modules, including k8s.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

Comment thread go.mod
Comment on lines +153 to +155
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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

@openshift-ci

openshift-ci Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@jubittajohn: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant