WIP: Bump to v1.36.2 - #665
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe build environment and Go module stack move to Go 1.26 and newer Kubernetes dependencies. Route validation options are removed from validation wiring, apiserver initialization parameters are updated, and deploy-log pod and deployment handling uses direct watches. ChangesPlatform and dependency refresh
Route validation interface simplification
Apiserver initialization and admission wiring
Deploy-log watch handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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`:
- Line 156: Update the go.opentelemetry.io/otel/sdk dependency in go.mod from
v1.40.0 to v1.43.0 or later, and synchronize the corresponding module checksums
in go.sum if needed.
- Line 208: Replace the temporary github.com/jacobsee/kubernetes-apiserver
override in go.mod with an approved upstream or OpenShift source, and update the
corresponding k8s.io/apiserver entry in goodmod.yaml so goodmod replace --apply
preserves it; if the fork must remain, document its provenance and provide SBOM
and signing coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.40.0 // indirect | ||
| go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.40.0 // indirect | ||
| go.opentelemetry.io/otel/metric v1.41.0 // indirect | ||
| go.opentelemetry.io/otel/sdk v1.40.0 // indirect |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go.mod excerpt =="
nl -ba go.mod | sed -n '130,180p'
echo
echo "== OpenTelemetry mentions in go.mod =="
rg -n "go\.opentelemetry\.io/otel" go.mod
echo
echo "== module dependency graph hints =="
# Show any references in go.sum too, without huge output
rg -n "go\.opentelemetry\.io/otel(/sdk)?\s" go.sum | sed -n '1,80p'Repository: openshift/openshift-apiserver
Length of output: 228
🌐 Web query:
OSV GHSA-hfvc-g4fc-pqhx go.opentelemetry.io/otel/sdk v1.40.0 fixed in v1.43.0
💡 Result:
The vulnerability GHSA-hfvc-g4fc-pqhx, also identified as CVE-2026-39883, affects the go.opentelemetry.io/otel/sdk package [1][2][3]. It is an incomplete fix for a previous vulnerability (GHSA-9h8m-3fm2-qjrq / CVE-2026-24051) where the application used a bare command name rather than an absolute path to execute system utilities [1][4]. Specifically, in versions from v1.15.0 up to and including v1.42.0, the SDK used the bare command kenv to retrieve host information on BSD and Solaris platforms [1][2][4]. Because the command was not referenced by its absolute path (e.g., /bin/kenv), an attacker with local access could place a malicious kenv binary earlier in the system's $PATH, resulting in arbitrary code execution in the context of the Go application [2][5][4]. This vulnerability was resolved in version v1.43.0, which updated the code to use the absolute path /bin/kenv [1][2][6]. Users of the OpenTelemetry Go SDK are advised to upgrade to v1.43.0 or later to mitigate this risk [2][5][6].
Citations:
- 1: GHSA-hfvc-g4fc-pqhx
- 2: https://osv.dev/vulnerability/GHSA-hfvc-g4fc-pqhx
- 3: https://nvd.nist.gov/vuln/detail/CVE-2026-39883
- 4: GHSA-hfvc-g4fc-pqhx
- 5: https://mondoo.com/vulnerability-intelligence/vulnerability/GHSA-hfvc-g4fc-pqhx
- 6: https://guide.sonatype.com/vulnerability/CVE-2026-39883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== go.mod around line 156 =="
sed -n '148,162p' go.mod
echo
echo "== all OpenTelemetry module entries in go.mod =="
grep -n "go.opentelemetry.io/otel" go.mod || trueRepository: openshift/openshift-apiserver
Length of output: 1460
Upgrade go.opentelemetry.io/otel/sdk to v1.43.0 or later. v1.40.0 is affected by GHSA-hfvc-g4fc-pqhx / CVE-2026-39883; on BSD/Solaris, resource detection can execute a local attacker-controlled kenv from PATH.
🧰 Tools
🪛 OSV Scanner (2.4.0)
[HIGH] 156-156: go.opentelemetry.io/otel/sdk 1.40.0: opentelemetry-go: BSD kenv command not using absolute path enables PATH hijacking
🤖 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 156, Update the go.opentelemetry.io/otel/sdk dependency in
go.mod from v1.40.0 to v1.43.0 or later, and synchronize the corresponding
module checksums in go.sum if needed.
Sources: Path instructions, Linters/SAST tools
| k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.36.2 | ||
| k8s.io/apimachinery => k8s.io/apimachinery v0.36.2 | ||
| // k8s.io/apiserver => github.com/openshift/kubernetes-apiserver v0.0.0-20260415154523-acdcc04896b5 // points to openshift-apiserver-4.22-kubernetes-1.34.1 | ||
| k8s.io/apiserver => github.com/jacobsee/kubernetes-apiserver v0.0.0-20260721191758-685ad32f88c0 // temporary: add-carries-to-1.36.2 branch |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## go.mod references\n'
git ls-files go.mod goodmod.yaml '**/goodmod.yaml' | cat
printf '\n## relevant replacement lines\n'
for f in go.mod goodmod.yaml; do
if [ -f "$f" ]; then
echo "--- $f ---"
nl -ba "$f" | sed -n '180,230p'
fi
done
printf '\n## search for the fork/module path\n'
rg -n --hidden --no-ignore-vcs 'jacobsee/kubernetes-apiserver|k8s.io/apiserver|goodmod replace|replace .*apiserver' .
printf '\n## module metadata hints\n'
rg -n --hidden --no-ignore-vcs 'OpenShift|openshift|temporary: add-carries-to-1\.36\.2|carry|fork' go.mod goodmod.yaml .github . || trueRepository: openshift/openshift-apiserver
Length of output: 296
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## go.mod replacement block'
sed -n '200,216p' go.mod | cat -n
echo
echo '## goodmod.yaml relevant block'
sed -n '1,220p' goodmod.yaml | cat -n | sed -n '/apiserver/,+12p'
echo
echo '## search for module/fork references'
rg -n --hidden --no-ignore-vcs 'jacobsee/kubernetes-apiserver|k8s.io/apiserver|goodmod replace|add-carries-to-1\.36\.2|temporary' go.mod goodmod.yaml .github .Repository: openshift/openshift-apiserver
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '## go.mod line with replacement'
grep -n -C 2 'jacobsee/kubernetes-apiserver\|k8s.io/apiserver' go.mod | sed -n '1,40p'
echo
echo '## goodmod.yaml lines mentioning apiserver or replacement'
grep -n -C 3 'apiserver\|jacobsee\|replace' goodmod.yaml | sed -n '1,120p'
echo
echo '## any direct note about the temporary carry in tracked config'
grep -RIn --exclude-dir=vendor --exclude=go.mod --exclude=goodmod.yaml 'add-carries-to-1\.36\.2\|temporary: add-carries\|jacobsee/kubernetes-apiserver' . | sed -n '1,80p'Repository: openshift/openshift-apiserver
Length of output: 2251
Replace the temporary fork before merge.
go.mod now routes k8s.io/apiserver to github.com/jacobsee/kubernetes-apiserver, but goodmod.yaml still points k8s.io/apiserver at openshift-apiserver-4.3-kubernetes-1.17.0, so goodmod replace --apply can revert the carry. Move this module to an approved upstream/OpenShift source and update goodmod.yaml, or document the exception with provenance/SBOM/signing coverage.
🤖 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 208, Replace the temporary
github.com/jacobsee/kubernetes-apiserver override in go.mod with an approved
upstream or OpenShift source, and update the corresponding k8s.io/apiserver
entry in goodmod.yaml so goodmod replace --apply preserves it; if the fork must
remain, document its provenance and provide SBOM and signing coverage.
Source: Path instructions
6f8acb3 to
9ba713a
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
images/Dockerfile.rhel (1)
1-10: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRun the runtime image as a non-root user.
The final stage has no
USERdirective, so the resulting API server container defaults to root. Add a non-root UID compatible with OpenShift SCCs and verify the binary and filesystem permissions under that UID. As per path instructions, containers must useUSERnon-root and never run as root.🤖 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 `@images/Dockerfile.rhel` around lines 1 - 10, The final runtime stage in Dockerfile.rhel currently runs as root because it lacks a USER directive. Add a non-root USER with an OpenShift-compatible UID, and ensure the copied binaries and required filesystem paths are readable and executable by that UID so the openshift-apiserver ENTRYPOINT works without root.Sources: Path instructions, Linters/SAST tools
🤖 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.
Outside diff comments:
In `@images/Dockerfile.rhel`:
- Around line 1-10: The final runtime stage in Dockerfile.rhel currently runs as
root because it lacks a USER directive. Add a non-root USER with an
OpenShift-compatible UID, and ensure the copied binaries and required filesystem
paths are readable and executable by that UID so the openshift-apiserver
ENTRYPOINT works without root.
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 (2)
pkg/apps/apiserver/registry/deploylog/wait.go (2)
73-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
%Tone.Typealways printswatch.EventType, not the actual event type value.
e.Typeis awatch.EventType(string-based type).fmt.Errorf("unexpected event type: %T", e.Type)formats the type of the value, so this error will always read"unexpected event type: watch.EventType"regardless of what the unexpected type actually was, defeating the diagnostic's purpose.🐛 Proposed fix
default: - return true, fmt.Errorf("unexpected event type: %T", e.Type) + return true, fmt.Errorf("unexpected event type: %v", e.Type) }🤖 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/apps/apiserver/registry/deploylog/wait.go` around lines 73 - 75, Update the default branch in the event handling logic to format e.Type as its value rather than its Go type, so the error identifies the specific unexpected event type. Keep the existing error context and return behavior unchanged.
58-79: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftTimeout handling now misses interrupted watches.
watchtools.UntilWithoutRetryreturnswait.ErrorInterrupted(nil)onctx.Done(), butpkg/apps/apiserver/registry/deploylog/rest.gostill checkserr == wait.ErrWaitTimeout. That comparison is false here, so deploy-log timeouts fall through toNewBadRequestinstead ofNewServerTimeout. Switch the caller towait.Interrupted(err)or translate the error before returning.🤖 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/apps/apiserver/registry/deploylog/wait.go` around lines 58 - 79, Update the deploy-log wait error handling in the caller of the watch flow around UntilWithoutRetry and checkDeploymentStatus to recognize interrupted context termination with wait.Interrupted(err), or translate it to the existing timeout error before returning. Preserve NewServerTimeout for deploy-log timeouts instead of allowing them to fall through to NewBadRequest.
🤖 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 `@hack/update-generated-deep-copies.sh`:
- Line 28: Quote the path arguments in the deepcopy-gen invocation: wrap the
GOPATH-based executable path and the SCRIPT_ROOT-based header-file path in
double quotes, while preserving the existing arguments and "$@" forwarding.
In `@pkg/apps/apiserver/registry/deploylog/wait.go`:
- Around line 29-30: Replace the context.WithTimeout call in the wait flow with
watchtools.ContextWithOptionalTimeout so a zero or unset timeout waits
indefinitely while positive timeouts retain their deadline behavior. Keep the
existing cancellation cleanup and surrounding wait logic unchanged.
---
Outside diff comments:
In `@pkg/apps/apiserver/registry/deploylog/wait.go`:
- Around line 73-75: Update the default branch in the event handling logic to
format e.Type as its value rather than its Go type, so the error identifies the
specific unexpected event type. Keep the existing error context and return
behavior unchanged.
- Around line 58-79: Update the deploy-log wait error handling in the caller of
the watch flow around UntilWithoutRetry and checkDeploymentStatus to recognize
interrupted context termination with wait.Interrupted(err), or translate it to
the existing timeout error before returning. Preserve NewServerTimeout for
deploy-log timeouts instead of allowing them to fall through to NewBadRequest.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d7fd2587-71b3-4f75-9f7e-59f35b3fe909
📒 Files selected for processing (2)
hack/update-generated-deep-copies.shpkg/apps/apiserver/registry/deploylog/wait.go
5c8cf24 to
3929899
Compare
3929899 to
219ed14
Compare
|
/test verify |
With WatchListClient enabled by default in client-go 1.35+, UntilWithSync requires the server to send an initial events bookmark, which fake clients do not provide, causing TestRESTGet/complete_deployment to hang for 60s and time out. Upstream kubectl replaced the informer-based implementation with a simple List followed by a Watch+UntilWithoutRetry fallback, avoiding the WatchListClient dependency entirely.
…tPodList Avoids the UntilWithSync/informer path that requires WatchListClient bookmark support and may not be available in tests. This is a one-shot wait, not setting up a long-lived watch.
219ed14 to
27479b3
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
/retest |
1 similar comment
|
/retest |
…tect from .ci-operator.yaml
|
/retest |
1 similar comment
|
/retest |
|
@jacobsee: The following tests failed, say
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. |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. 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. |
|
/close |
|
@jubittajohn: Closed this PR. DetailsIn response to this:
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. |
2 participants
Summary by CodeRabbit