[WIP]OCPBUGS-85429: Bump to 1.36.2 - #666
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 project baseline is updated to Go 1.26, Kubernetes 1.36.2, and OpenShift 5.0. API-generation metadata, route validation interfaces, quota initialization, and OpenAPI tooling are also updated. ChangesPlatform and API-generation baseline
Route and admission behavior
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: 4
🤖 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 177: Update the google.golang.org/grpc dependency in go.mod from v1.79.3
to a version newer than v1.82.1, preserving the existing indirect dependency
declaration.
- Line 210: Remove the forked k8s.io/apiserver replacement from go.mod and
eliminate the matching override in vendor/modules.txt. Before release, use the
approved carry source if one exists; otherwise ensure the forked module is
absent from release artifacts.
In `@hack/update-generated-deep-copies.sh`:
- Line 28: Update the deepcopy-gen invocation in update-generated-deep-copies.sh
to quote both the GOPATH-based generator path and the SCRIPT_ROOT-based
boilerplate path, preserving the existing arguments and generation behavior.
In `@images/Dockerfile.rhel`:
- Around line 7-10: Add a non-root USER directive and a HEALTHCHECK to the
runtime stage of Dockerfile.rhel after the binary COPY instructions and before
ENTRYPOINT; configure the health check to probe the running openshift-apiserver
readiness endpoint and ensure the selected user can execute the entrypoint and
access required files.
🪄 Autofix (Beta)
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 => k8s.io/api v0.36.2 | ||
| 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/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
# Inspect the surrounding go.mod context
nl -ba go.mod | sed -n '190,225p'
# Find any references to the replacement or the fork
rg -n "jacobsee/kubernetes-apiserver|k8s.io/apiserver|temporary: add-carries-to-1.36.2 branch|release approval|provenance|sigstore|cosign" .
# Capture the module replacement details if present elsewhere
git ls-files | rg '(^|/)go\.mod$|(^|/)go\.sum$|(^|/)README|(^|/)docs?/'Repository: openshift/openshift-apiserver
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant go.mod section using portable tools
sed -n '190,225p' go.mod
echo
echo "---- matches ----"
rg -n "jacobsee/kubernetes-apiserver|k8s.io/apiserver|temporary: add-carries-to-1.36.2 branch|release approval|provenance|sigstore|cosign" .
echo
echo "---- module files ----"
git ls-files | rg '(^|/)go\.mod$|(^|/)go\.sum$|(^|/)README|(^|/)docs?/'Repository: openshift/openshift-apiserver
Length of output: 50386
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- go.mod around replacement ----"
sed -n '205,215p' go.mod
echo
echo "---- replace directives in go.mod ----"
rg -n '^\s*replace\s+.*k8s\.io/apiserver|jacobsee/kubernetes-apiserver|add-carries-to-1\.36\.2' go.mod
echo
echo "---- repo guidance mentions ----"
rg -n 'release approval|provenance|cosign|sigstore|SBOM|supply chain|temporary replacement|fork' README* docs* .github* . 2>/dev/null | head -n 200Repository: openshift/openshift-apiserver
Length of output: 31041
go.mod:210 — Remove this forked k8s.io/apiserver replacement before release. It redirects the module to github.com/jacobsee/kubernetes-apiserver, and vendor/modules.txt carries the same override. Route it through the approved carry source or keep it out of release artifacts.
🤖 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 210, Remove the forked k8s.io/apiserver replacement from
go.mod and eliminate the matching override in vendor/modules.txt. Before
release, use the approved carry source if one exists; otherwise ensure the
forked module is absent from release artifacts.
Source: Path instructions
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/api/validation/register.go (1)
84-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the adapters now that they no longer adapt options.
routeOptionsAdapterandrouteUpdateOptionsAdapterare now misleading. Rename them to reflect route validation and update the registration reference accordingly.Proposed rename
-Validator.MustRegister(&routeapi.Route{}, true, routeOptionsAdapter, routeUpdateOptionsAdapter) +Validator.MustRegister(&routeapi.Route{}, true, routeValidationAdapter, routeValidationUpdateAdapter) -func routeOptionsAdapter(route *routeapi.Route) field.ErrorList { +func routeValidationAdapter(route *routeapi.Route) field.ErrorList { -func routeUpdateOptionsAdapter(route *routeapi.Route, oldRoute *routeapi.Route) field.ErrorList { +func routeValidationUpdateAdapter(route *routeapi.Route, oldRoute *routeapi.Route) field.ErrorList {🤖 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/api/validation/register.go` around lines 84 - 89, Rename routeOptionsAdapter and routeUpdateOptionsAdapter to names reflecting route validation and route update validation, respectively, and update every registration or call-site reference to use the new names consistently.
🤖 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/api/validation/register.go`:
- Around line 84-89: Rename routeOptionsAdapter and routeUpdateOptionsAdapter to
names reflecting route validation and route update validation, respectively, and
update every registration or call-site reference to use the new names
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2005b6fc-2dbb-4eab-85af-3d791247b15e
⛔ Files ignored due to path filters (1)
pkg/openapi/zz_generated.openapi.gois excluded by!**/zz_generated*
📒 Files selected for processing (6)
hack/openapi-violation.listpkg/api/validation/register.gopkg/cmd/openshift-apiserver/openshiftadmission/plugin_initializer.gopkg/route/apis/route/validation/validation.gopkg/route/apiserver/admission/routehostassignment/adapter.gopkg/route/apiserver/registry/route/strategy.go
🚧 Files skipped from review as they are similar to previous changes (1)
- hack/openapi-violation.list
Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
Signed-off-by: jubittajohn <jujohn@redhat.com>
openshift-ci-robot
added
jira/valid-reference
|
@jubittajohn: This pull request references Jira Issue OCPBUGS-85429, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@jubittajohn: This pull request references Jira Issue OCPBUGS-85429, which is valid. 3 validation(s) were run on this bug
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-openapi-spec.sh`:
- Around line 145-152: Update the OPENSHIFT_VERSION detection probe in
hack/update-openapi-spec.sh to tolerate a grep pipeline with no openshift-X.Y
match under errexit and pipefail. Add the failure suppression to the
grep/head/sed assignment so execution reaches the existing empty-value check and
error message, while preserving successful version detection.
🪄 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: 417345d4-8173-4b4b-8153-fd7036ae80d3
📒 Files selected for processing (1)
hack/update-openapi-spec.sh
| if [[ -f "${SCRIPT_ROOT}/.ci-operator.yaml" ]]; then | ||
| OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "${SCRIPT_ROOT}/.ci-operator.yaml" | head -1 | sed 's/openshift-//') | ||
| fi | ||
| if [[ -z "${OPENSHIFT_VERSION}" ]]; then | ||
| echo "ERROR: Could not detect OpenShift version from .ci-operator.yaml" | ||
| echo "Please set OPENSHIFT_RELEASE environment variable manually, e.g.:" | ||
| echo " OPENSHIFT_RELEASE=registry.ci.openshift.org/ocp/release:5.0 $0" | ||
| exit 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section of the script
sed -n '132,170p' hack/update-openapi-spec.sh
printf '\n--- shell probe ---\n'
tmpdir="$(mktemp -d)"
cat >"$tmpdir/probe.sh" <<'EOF'
#!/bin/bash
set -euo pipefail
f="$1"
OPENSHIFT_VERSION=""
if [[ -f "$f" ]]; then
OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "$f" | head -1 | sed 's/openshift-//')
fi
echo "OPENSHIFT_VERSION=${OPENSHIFT_VERSION:-<empty>}"
echo "reached end"
EOF
chmod +x "$tmpdir/probe.sh"
cat >"$tmpdir/match.yaml" <<'EOF'
something openshift-4.16 other
EOF
cat >"$tmpdir/nomatch.yaml" <<'EOF'
something else
EOF
echo "match case:"
bash "$tmpdir/probe.sh" "$tmpdir/match.yaml" ; echo "exit=$?"
echo "no-match case:"
bash "$tmpdir/probe.sh" "$tmpdir/nomatch.yaml" ; echo "exit=$?"Repository: openshift/openshift-apiserver
Length of output: 1958
🏁 Script executed:
sed -n '132,170p' hack/update-openapi-spec.shRepository: openshift/openshift-apiserver
Length of output: 1868
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '132,170p' hack/update-openapi-spec.sh
tmpdir="$(mktemp -d)"
cat >"$tmpdir/probe.sh" <<'EOF'
#!/bin/bash
set -euo pipefail
f="$1"
OPENSHIFT_VERSION=""
if [[ -f "$f" ]]; then
OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "$f" | head -1 | sed 's/openshift-//')
fi
echo "OPENSHIFT_VERSION=${OPENSHIFT_VERSION:-<empty>}"
echo "reached end"
EOF
chmod +x "$tmpdir/probe.sh"
cat >"$tmpdir/match.yaml" <<'EOF'
something openshift-4.16 other
EOF
cat >"$tmpdir/nomatch.yaml" <<'EOF'
something else
EOF
echo "match case:"
bash "$tmpdir/probe.sh" "$tmpdir/match.yaml"; echo "exit=$?"
echo "no-match case:"
bash "$tmpdir/probe.sh" "$tmpdir/nomatch.yaml"; echo "exit=$?"Repository: openshift/openshift-apiserver
Length of output: 1937
Preserve the version-detection fallback.
With errexit and pipefail, a .ci-operator.yaml without an openshift-X.Y match makes the grep pipeline abort before the empty-value check can emit the intended error. Add || true to the probe so the script reaches that branch.
Proposed fix
- OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "${SCRIPT_ROOT}/.ci-operator.yaml" | head -1 | sed 's/openshift-//')
+ OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "${SCRIPT_ROOT}/.ci-operator.yaml" | head -1 | sed 's/openshift-//' || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -f "${SCRIPT_ROOT}/.ci-operator.yaml" ]]; then | |
| OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "${SCRIPT_ROOT}/.ci-operator.yaml" | head -1 | sed 's/openshift-//') | |
| fi | |
| if [[ -z "${OPENSHIFT_VERSION}" ]]; then | |
| echo "ERROR: Could not detect OpenShift version from .ci-operator.yaml" | |
| echo "Please set OPENSHIFT_RELEASE environment variable manually, e.g.:" | |
| echo " OPENSHIFT_RELEASE=registry.ci.openshift.org/ocp/release:5.0 $0" | |
| exit 1 | |
| if [[ -f "${SCRIPT_ROOT}/.ci-operator.yaml" ]]; then | |
| OPENSHIFT_VERSION=$(grep -o 'openshift-[0-9]*\.[0-9]*' "${SCRIPT_ROOT}/.ci-operator.yaml" | head -1 | sed 's/openshift-//' || true) | |
| fi | |
| if [[ -z "${OPENSHIFT_VERSION}" ]]; then | |
| echo "ERROR: Could not detect OpenShift version from .ci-operator.yaml" | |
| echo "Please set OPENSHIFT_RELEASE environment variable manually, e.g.:" | |
| echo " OPENSHIFT_RELEASE=registry.ci.openshift.org/ocp/release:5.0 $0" | |
| exit 1 |
🤖 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 `@hack/update-openapi-spec.sh` around lines 145 - 152, Update the
OPENSHIFT_VERSION detection probe in hack/update-openapi-spec.sh to tolerate a
grep pipeline with no openshift-X.Y match under errexit and pipefail. Add the
failure suppression to the grep/head/sed assignment so execution reaches the
existing empty-value check and error message, while preserving successful
version detection.
| echo " OPENSHIFT_RELEASE=quay.io/openshift-release-dev/ocp-release:4.22.0-x86_64 $0" | ||
| exit 1 | ||
| # Determine which release image to use. | ||
| # Priority: OPENSHIFT_RELEASE env var > RELEASE_IMAGE_LATEST (set by ci-operator) > auto-detect from .ci-operator.yaml |
There was a problem hiding this comment.
Can OPENSHIFT_RELEASE be assigned in the job invocation instead of baking knowledge of RELEASE_IMAGE_LATEST into this script?
| elif [[ -n "${REGISTRY_AUTH_FILE:-}" && -f "${REGISTRY_AUTH_FILE}" ]]; then | ||
| if [[ -n "${REGISTRY_AUTH_FILE:-}" && -f "${REGISTRY_AUTH_FILE}" ]]; then | ||
| echo "Using pull secret from ${REGISTRY_AUTH_FILE}" | ||
| REGISTRY_AUTH_OPTS="--registry-config=${REGISTRY_AUTH_FILE}" |
There was a problem hiding this comment.
Is passing the --registry-config option necessary? It seems like oc itself respects REGISTRY_AUTH_FILE (https://github.com/openshift/oc/blob/a4c90c71ea5562102352df426c71e03381b7a02f/pkg/cli/image/manifest/dockercredentials/auth_resolver.go#L38).
| elif [[ -n "${CLUSTER_PROFILE_DIR:-}" && -f "${CLUSTER_PROFILE_DIR}/pull-secret" ]]; then | ||
| echo "Using pull secret from ${CLUSTER_PROFILE_DIR}/pull-secret" | ||
| REGISTRY_AUTH_OPTS="--registry-config=${CLUSTER_PROFILE_DIR}/pull-secret" | ||
| else |
There was a problem hiding this comment.
We can remove the CLUSTER_PROFILE_DIR bits if cluster profiles don't include CI registry pull secrets.
|
[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 |
|
@jubittajohn: The following test 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. |
4 participants
Summary by CodeRabbit