Add k8s-rebase plugin for Kubernetes dependency rebases#617
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dfarrell07 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @dfarrell07. Thanks for your PR. I'm waiting for a openshift-eng member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
openshift-ci
Bot
added
approved
WalkthroughAdds the ChangesKubernetes rebase plugin
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant RebaseScript
participant GoModules
participant Autofix
participant Validator
participant ReviewAgent
Developer->>RebaseScript: provide target Kubernetes version
RebaseScript->>GoModules: update dependencies and regenerate code
RebaseScript->>Autofix: run deterministic post-rebase fixes
Autofix->>Validator: run build, lint, test, and vet validation
Validator->>ReviewAgent: submit fix commit evidence
ReviewAgent-->>Developer: return approval or rejection verdict
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh (1)
111-114: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueHarden the in-container
jqdownload. The binary is fetched over the network and executed without integrity verification, and/tmp/jqis a predictable path (symlink/TOCTOU-prone on shared hosts). Since it's the official jqlang release running in an ephemeral container the risk is limited, but consider pinning and checking a known SHA-256 (and/ormktemp) beforechmod +x.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh` around lines 111 - 114, Harden the jq installation block by downloading to a unique temporary path via mktemp, verifying the downloaded binary against a pinned known SHA-256 checksum, and only then making it executable and adding its directory to PATH. Update the existing command chain guarded by command -v jq; remove the predictable /tmp/jq path and ensure verification failure prevents execution or PATH export.Source: Linters/SAST tools
plugins/k8s-rebase/scripts/k8s-rebase-validate.sh (2)
79-88: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope
--privilegedto modes that actually need it.The container is launched with
--privileged(and--security-opt label=disable) unconditionally, including--quick(build+vet) and--no-test, which never need elevated capabilities. Only privileged/namespace tests (--full, and the default mode's network tests) require it. Gate--privilegedon the mode to reduce the blast radius for the common build/vet path.♻️ Suggested approach
+ PRIV_FLAG="" + case "$MODE" in + full|default) PRIV_FLAG="--privileged" ;; + esac exec $CONTAINER_RT run --rm \ --security-opt label=disable \ - --privileged \ + $PRIV_FLAG \ $USERNS_FLAG \🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh` around lines 79 - 88, Update the container invocation in the validation script so --privileged and its associated security-opt label=disable are included only for modes requiring namespace or network tests: --full and the default mode. Omit both options for --quick and --no-test while preserving the existing run arguments and behavior.
240-241: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winHarden env extraction from
test-go.sh(avoideval).
eval "$(grep "^export KUBE_FEATURE_" "$TEST_GO_SH")"re-parses lines from the target repo'shack/test-go.shas shell code; a craftedexport KUBE_FEATURE_X=...; <cmd>line would execute. The same concatenate-into-bash -cpattern recurs at Lines 405 and 588. Since the tool already runs that repo's build/test targets the incremental risk is limited, but prefer parsingKEY=valuepairs and exporting them directly rather than executing arbitrary content.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh` around lines 240 - 241, Replace the eval-based environment extraction in the TEST_GO_SH handling and the corresponding logic at the later occurrences with safe parsing of only KUBE_FEATURE_ KEY=value assignments, then export each validated key/value directly. Ensure crafted values or appended commands from test-go.sh are treated as data and never executed, while preserving the intended feature-variable imports.plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md (1)
7-10: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTie the KubeVirt requirement to the target version matrix.
“Latest stable release” is a moving acceptance criterion: it can reject a valid pinned version or force an unrelated dependency upgrade. Require the exact version prescribed by the patterns document or target repository matrix, with “not nightly” as the additional constraint.
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md` around lines 7 - 10, Update the KubeVirt checklist item in the step3-autofix requirements to require the exact version specified by the patterns document or target repository version matrix, while retaining the constraint that it must not be a nightly release; remove the vague “latest stable release” wording.
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/dep-release-notes.md`:
- Around line 6-13: The release-note sources in the step 3 instructions are
incomplete and mutable. Update the KIND and KubeVirt GitHub API retrieval to
paginate through all release pages, pin MetalLB and golangci-lint raw URLs to
the requested version’s tag or commit instead of main, and explicitly report
when a requested version cannot be found.
In `@plugins/k8s-rebase/gates/step3-autofix/feature-gates.md`:
- Around line 1-6: Update the feature-gate verification instructions to filter
GATE_DEPS using the same active-gate logic as k8s-rebase-autofix.sh, considering
only gate symbols present under vendor/k8s.io. Count missing gates only among
those active dependencies while preserving the existing comparisons across
SetFromMap, os.Setenv/t.Setenv, and hack/test-go.sh exports.
In `@plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md`:
- Line 11: Update the verification command in the build-and-vet step so go vet
./... executes regardless of whether go build ./... fails; replace the &&
chaining between these checks with independent execution while preserving the
existing output handling, directory change, and combined failure reporting.
In `@plugins/k8s-rebase/gates/step4-verification/ci-prediction.md`:
- Around line 18-23: Update the stale codegen guidance in step4 verification to
remove the requirement for a dedicated codegen commit. Prefer running the
repository’s verify-update-codegen or equivalent diff verification when
available, and treat git log history only as supporting evidence while allowing
generated output in squashed or combined commits.
In `@plugins/k8s-rebase/gates/step4-verification/cleanliness.md`:
- Around line 3-4: Update the root-owned entry count command in the cleanliness
verification step to include the find predicate -type f, while preserving the
existing .git and vendor exclusions and user filter.
In `@plugins/k8s-rebase/gates/step4-verification/dep-cve-check.md`:
- Line 6: Update the dependency scan command in the verification step to process
every unique changed module instead of truncating output with head -50. Remove
the cap or implement batching/pagination that still queries the complete module
list.
In `@plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md`:
- Around line 11-12: Replace the broad golang.org/x/exp grep in
plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md:11-12 with
checks derived from the package-level x/exp migration map, flagging only
packages that have explicit replacements. Update
plugins/k8s-rebase/gates/step4-verification/go-version-check.md:20-23 to reuse
that same map while keeping x/slices and x/maps checks conditional on the
detected Go version.
In `@plugins/k8s-rebase/gates/step4-verification/gomod-diff-analysis.md`:
- Around line 17-19: Update the step4-verification reporting instructions to
require output for every classification the prompt asks agents to evaluate,
including pseudo-version pins, dependency additions/removals, and go directive
changes, rather than only counts for third-party minor-version jumps and
pre-release direct dependencies. Preserve the informational review guidance for
each reported category.
In `@plugins/k8s-rebase/gates/step4-verification/k8s-changelog.md`:
- Around line 1-2: Update the changelog retrieval command to use curl’s --fail
and --show-error options, explicitly report and fail when the changelog is
unavailable or empty, and replace the mutable master URL with a stable
Kubernetes release tag or commit while preserving the K8S_MINOR-based changelog
path.
In `@plugins/k8s-rebase/gates/step4-verification/version-completeness.md`:
- Line 3: Update the file-scope statement in version-completeness.md to include
.yaml alongside .yml, ensuring the scan covers both YAML extensions while
preserving the existing exclusions and count-gate behavior.
In `@plugins/k8s-rebase/README.md`:
- Line 45: Update the gate prompt file count in the README table to 29 files,
matching the gate totals documented in SKILL.md and the PR description; leave
the path and description unchanged.
---
Nitpick comments:
In `@plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md`:
- Around line 7-10: Update the KubeVirt checklist item in the step3-autofix
requirements to require the exact version specified by the patterns document or
target repository version matrix, while retaining the constraint that it must
not be a nightly release; remove the vague “latest stable release” wording.
In `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh`:
- Around line 111-114: Harden the jq installation block by downloading to a
unique temporary path via mktemp, verifying the downloaded binary against a
pinned known SHA-256 checksum, and only then making it executable and adding its
directory to PATH. Update the existing command chain guarded by command -v jq;
remove the predictable /tmp/jq path and ensure verification failure prevents
execution or PATH export.
In `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh`:
- Around line 79-88: Update the container invocation in the validation script so
--privileged and its associated security-opt label=disable are included only for
modes requiring namespace or network tests: --full and the default mode. Omit
both options for --quick and --no-test while preserving the existing run
arguments and behavior.
- Around line 240-241: Replace the eval-based environment extraction in the
TEST_GO_SH handling and the corresponding logic at the later occurrences with
safe parsing of only KUBE_FEATURE_ KEY=value assignments, then export each
validated key/value directly. Ensure crafted values or appended commands from
test-go.sh are treated as data and never executed, while preserving the intended
feature-variable imports.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ec127345-a4c1-4949-b911-15f3f492325c
📒 Files selected for processing (41)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/k8s-rebase/.claude-plugin/plugin.jsonplugins/k8s-rebase/OWNERSplugins/k8s-rebase/README.mdplugins/k8s-rebase/docs/k8s-rebase-patterns.mdplugins/k8s-rebase/gates/step1-rebase/rebase-completeness.mdplugins/k8s-rebase/gates/step2-compilation/build-vet.mdplugins/k8s-rebase/gates/step2-compilation/diff-scope.mdplugins/k8s-rebase/gates/step2-compilation/fix-correctness.mdplugins/k8s-rebase/gates/step2-compilation/type-conversions.mdplugins/k8s-rebase/gates/step2-compilation/version-consistency.mdplugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.mdplugins/k8s-rebase/gates/step3-autofix/autofix-result.mdplugins/k8s-rebase/gates/step3-autofix/crd-validation.mdplugins/k8s-rebase/gates/step3-autofix/dep-release-notes.mdplugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.mdplugins/k8s-rebase/gates/step3-autofix/e2e-infra.mdplugins/k8s-rebase/gates/step3-autofix/feature-gates.mdplugins/k8s-rebase/gates/step3-autofix/logical-completeness.mdplugins/k8s-rebase/gates/step3-autofix/patterns-completeness.mdplugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdplugins/k8s-rebase/gates/step4-verification/ci-prediction.mdplugins/k8s-rebase/gates/step4-verification/ci-readiness.mdplugins/k8s-rebase/gates/step4-verification/cleanliness.mdplugins/k8s-rebase/gates/step4-verification/commit-messages.mdplugins/k8s-rebase/gates/step4-verification/correctness.mdplugins/k8s-rebase/gates/step4-verification/dep-cve-check.mdplugins/k8s-rebase/gates/step4-verification/deprecated-imports.mdplugins/k8s-rebase/gates/step4-verification/go-version-check.mdplugins/k8s-rebase/gates/step4-verification/gomod-diff-analysis.mdplugins/k8s-rebase/gates/step4-verification/k8s-changelog.mdplugins/k8s-rebase/gates/step4-verification/logical-consistency.mdplugins/k8s-rebase/gates/step4-verification/maintainer-review.mdplugins/k8s-rebase/gates/step4-verification/version-completeness.mdplugins/k8s-rebase/scripts/k8s-rebase-autofix.shplugins/k8s-rebase/scripts/k8s-rebase-review-prompt.mdplugins/k8s-rebase/scripts/k8s-rebase-review.shplugins/k8s-rebase/scripts/k8s-rebase-validate.shplugins/k8s-rebase/scripts/k8s-rebase.shplugins/k8s-rebase/skills/k8s-rebase/SKILL.md
| - KIND: gh api repos/kubernetes-sigs/kind/releases (has explicit | ||
| "Breaking Changes" headings in .body) | ||
| - MetalLB: curl the in-repo release notes at | ||
| raw.githubusercontent.com/metallb/metallb/main/website/content/release-notes/_index.md | ||
| - KubeVirt: gh api repos/kubevirt/kubevirt/releases (tagged by | ||
| SIG — focus on SIG-network, Deprecation, API change) | ||
| - golangci-lint: curl CHANGELOG.md from the repo | ||
| raw.githubusercontent.com/golangci/golangci-lint/main/CHANGELOG.md |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make release-note retrieval complete and reproducible.
The GitHub API calls omit pagination, so releases outside the default first page can be missed. The raw URLs also point at mutable main branches, making gate results non-reproducible. Paginate API results, use tag/commit-pinned sources, and report when the requested versions cannot be found.
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/dep-release-notes.md` around lines 6 -
13, The release-note sources in the step 3 instructions are incomplete and
mutable. Update the KIND and KubeVirt GitHub API retrieval to paginate through
all release pages, pin MetalLB and golangci-lint raw URLs to the requested
version’s tag or commit instead of main, and explicitly report when a requested
version cannot be found.
| If test files use feature gates (SetFromMap or KUBE_FEATURE_ | ||
| env vars): find `k8s-rebase-autofix.sh` and read the GATE_DEPS | ||
| map near the top. Count files missing any gate from that map. Verify | ||
| gates match between SetFromMap calls, os.Setenv/t.Setenv | ||
| calls, and hack/test-go.sh exports. Report count of files | ||
| with missing gates. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Filter GATE_DEPS to gates active in the vendored Kubernetes version.
k8s-rebase-autofix.sh only considers parent/dependent gates whose symbols exist under vendor/k8s.io. This gate currently says to check “any gate from that map,” which can produce false failures for future gates on older targets. Mirror the script’s active-gate filtering before counting missing files.
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/feature-gates.md` around lines 1 - 6,
Update the feature-gate verification instructions to filter GATE_DEPS using the
same active-gate logic as k8s-rebase-autofix.sh, considering only gate symbols
present under vendor/k8s.io. Count missing gates only among those active
dependencies while preserving the existing comparisons across SetFromMap,
os.Setenv/t.Setenv, and hack/test-go.sh exports.
| - Stale codegen output? If hack/update-codegen.sh or a | ||
| Makefile codegen/generate/manifests target exists, check | ||
| that git log shows a codegen commit. If the repo has a | ||
| `verify-update-codegen` or `verify` CI job, stale output | ||
| will fail `git diff --exit-code`. Look for controller-gen | ||
| version annotations in CRD manifests matching the vendored |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not require a dedicated codegen commit.
A valid rebase may include generated output in a squashed or combined commit, so checking git log for a codegen commit can report a false failure. Prefer running the repository’s verify-update-codegen/diff verification when available, using commit history only as supporting evidence.
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/ci-prediction.md` around lines 18
- 23, Update the stale codegen guidance in step4 verification to remove the
requirement for a dedicated codegen commit. Prefer running the repository’s
verify-update-codegen or equivalent diff verification when available, and treat
git log history only as supporting evidence while allowing generated output in
squashed or combined commits.
| 3. "golang.org/x/exp/" — promoted to stdlib | ||
| grep -rn '"golang.org/x/exp/' --include='*.go' . | grep -v vendor |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- file map ---\n'
git ls-files 'plugins/k8s-rebase/gates/step4-verification/*'
printf '\n--- outline: deprecated-imports.md ---\n'
wc -l plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md
cat -n plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md | sed -n '1,120p'
printf '\n--- outline: go-version-check.md ---\n'
wc -l plugins/k8s-rebase/gates/step4-verification/go-version-check.md
cat -n plugins/k8s-rebase/gates/step4-verification/go-version-check.md | sed -n '1,140p'Repository: openshift-eng/ai-helpers
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- file map ---'
git ls-files 'plugins/k8s-rebase/gates/step4-verification/*'
echo
echo '--- deprecated-imports.md ---'
wc -l plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md
cat -n plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md | sed -n '1,120p'
echo
echo '--- go-version-check.md ---'
wc -l plugins/k8s-rebase/gates/step4-verification/go-version-check.md
cat -n plugins/k8s-rebase/gates/step4-verification/go-version-check.md | sed -n '1,140p'Repository: openshift-eng/ai-helpers
Length of output: 3910
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- x/exp references ---'
rg -n '"golang.org/x/(exp|slices|maps|constraints)|x/exp|x/slices|x/maps' \
plugins/k8s-rebase -g '*.md' -g '*.go' -g '*.sh' -g '*.yaml' -g '*.yml' || true
echo
echo '--- potential migration maps / package lists ---'
rg -n 'maps|slices|constraints|cmp|explicit migration map|promoted to stdlib' \
plugins/k8s-rebase -g '*.md' -g '*.go' || trueRepository: openshift-eng/ai-helpers
Length of output: 5030
Use the package-level x/exp migration map
plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md#L11-L12:golang.org/x/exp/is broader than the stdlib promotions; only flag packages with explicit replacements.plugins/k8s-rebase/gates/step4-verification/go-version-check.md#L20-L23: keep thex/slices/x/mapscheck tied to the detected Go version and reuse the same package map.
📍 Affects 2 files
plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md#L11-L12(this comment)plugins/k8s-rebase/gates/step4-verification/go-version-check.md#L20-L23
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md` around
lines 11 - 12, Replace the broad golang.org/x/exp grep in
plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md:11-12 with
checks derived from the package-level x/exp migration map, flagging only
packages that have explicit replacements. Update
plugins/k8s-rebase/gates/step4-verification/go-version-check.md:20-23 to reuse
that same map while keeping x/slices and x/maps checks conditional on the
detected Go version.
| Read the Kubernetes changelog for the target minor version: | ||
| curl -sL "https://raw.githubusercontent.com/kubernetes/kubernetes/master/CHANGELOG/CHANGELOG-1.${K8S_MINOR}.md" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make changelog retrieval fail closed.
curl -sL does not fail on HTTP 404, so an unavailable changelog can be treated as an empty result and incorrectly pass the gate. Use --fail --show-error, report unavailable data explicitly, and pin the source to a stable Kubernetes tag or commit instead of master.
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/k8s-changelog.md` around lines 1
- 2, Update the changelog retrieval command to use curl’s --fail and
--show-error options, explicitly report and fail when the changelog is
unavailable or empty, and replace the mutable master URL with a stable
Kubernetes release tag or commit while preserving the K8S_MINOR-based changelog
path.
| | `scripts/k8s-rebase-validate.sh` | Build/lint/vet/test across all modules | | ||
| | `scripts/k8s-rebase-review.sh` | Antagonistic review via `claude -p` | | ||
| | `scripts/k8s-rebase-review-prompt.md` | Review agent prompt template | | ||
| | `gates/step{1,2,3,4}-*/*.md` | Subagent verification prompts (23 files) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Gate file count is inaccurate. SKILL.md enumerates 29 gate prompts (step1=1, step2=5, step3=9, step4=14), and the PR description also says 29. Update "23 files" to match.
✏️ Proposed fix
-| `gates/step{1,2,3,4}-*/*.md` | Subagent verification prompts (23 files) |
+| `gates/step{1,2,3,4}-*/*.md` | Subagent verification prompts (29 files) |📝 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.
| | `gates/step{1,2,3,4}-*/*.md` | Subagent verification prompts (23 files) | | |
| | `gates/step{1,2,3,4}-*/*.md` | Subagent verification prompts (29 files) | |
🤖 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 `@plugins/k8s-rebase/README.md` at line 45, Update the gate prompt file count
in the README table to 29 files, matching the gate totals documented in SKILL.md
and the PR description; leave the path and description unchanged.
| CONTRIBUTING.md). If the PR already exists, suggest `gh pr edit`. | ||
|
|
||
| Always suggest the CI monitoring loop alongside the PR command: | ||
| `/loop 5m check CI on the PR, explore any failures max carefully, find root causes` |
There was a problem hiding this comment.
As per our meeting you mentioned that the loop check the CI logs and errors.
However these are my findings from this updated PR which is the same as raised earlier that I don't find a logic for the loop.
From what I see is that the skill doesn't catch e2e-level breakage. It is not even defined in ai-helpers.
I also see there is no gh run, gh pr checks or or CI-log fetching in this plugin.
I even check the gates ci-prediction and ci-readiness and saw that it greps the repo and does not read CI.
I see that the skill only checks build, vet, lint and unit tests and it never runs e2e.
k8s-rebase-validate.sh runs go build (L346), go vet, golangci-lint (L365) and go test on the packages (L310). The test/e2e module is skipped at L339–342 because its vendor dir is gitignored, and nothing in the plugin ever starts a kind cluster or runs ginkgo. right ?
The problem is that the agent decides it's finished when these checks pass. So the rebase can look completely done and green, while still being broken. This is because the things that actually break an ovnk rebase don't show up in a build or a lint.
These issues will only show up when a real cluster is running the e2e suite like the test for RelaxedServiceNameValidation from k8s 1.36 binary, the KubeVirt live-migration lanes, the MetalLB/FRR image mismatch issue, kubeadm settings silently not applying, etc.
However please correct if I am wrong. cc @vinnie1110 @Jaanki
There was a problem hiding this comment.
I see that the Step 5 in SKILL.md is where the skill stops and has the following:
"Generate a gh pr create command for the user. Do NOT run it. Read git log to build the title and body, then present the complete command for the user to review, edit, and execute."
then
"Always suggest the CI monitoring loop alongside the PR command:
/loop 5m check CI on the PR, explore any failures max carefully, find root causes"
then rm -rf .rebase-tmp/, and the skill ends.
So the skill prints two strings and exits. It doesn't open the PR, and it doesn't run /loop.
So someone has to manually create a PR using gh pr create and type type /loop 5m ... into the session, which re-prompts the agent every 5 minutes with that sentence.
Then I see agent does things by itself like run gh pr checks to find failing jobs, gh run view --log-failed to pull the logs, gh run download for the ginkgo artifacts, then reads them and decides what broke.
So, the point is this step is not really a part of the skill. Nothing calls it, no subagent runs it, nothing checks what it found. The skill only does build, vet, lint and unit tests locally. It skips e2e test whose failures can break ovn-k rebase.
In short this is manual process done by human who executes this command may be in claude code console which keeps on running by agent.
But I can understand that sometimes running e2e needs real clusters and running it locally isn't realistic.
However I think CI is where the actual validation happens and right now it is outside the skill.
Can we add it in a deterministic stage instead of a suggestion ?
There was a problem hiding this comment.
For k8s 1.36, all the breakage patterns you listed are already encoded in the autofix scripts. fix_relaxed_service_name_validation, fix_kubevirt_version, fix_metallb_version, fix_kubeadm_v1beta4 all fire on every rebase (no-op on repos without the relevant files). The ovnk PR is CI green including kv-live-migration, BGP, and conformance. The skill doesn't need to run e2e to catch these, it prevents the failures before CI ever runs.
The harder question is k8s 1.37, where there will be new patterns nobody has seen yet. That's where the gate subagents matter most. The k8s-changelog gate reads the 1.37 release notes filtered by SIG Network and API Machinery and flags breaking changes that have no existing autofix pattern. The dep-release-notes gate reads changelogs for ecosystem deps like KIND, MetalLB, KubeVirt, and golangci-lint and flags incompatibilities between the old and new versions. The dep-cve gate scans whatever dependency versions 1.37 pins against the OSV vulnerability database. None of this content exists until the release, so it can't be scripted in advance.
For 1.36 the skill already prevents several issues that human reviewers had to flag on the manual PRs. The SKILL.md has explicit guidelines against adding config file comments that explain the PR (which danwinship flagged on openshift/cluster-network-operator#3017) and against commit messages that reference issue/PR numbers (which danwinship flagged for spamming GitHub notifications on the same PR). The version-consistency gate (step2) prevents partial module skew like openshift/cloud-network-config-controller#223 where kube-aggregator was left at v0.36.1 while all other k8s.io modules were at v0.36.2.
When something does slip through to CI, a genuinely novel pattern, the user runs /loop 5m check CI on the PR, explore any failures max carefully, find root causes and the agent reads the failure logs and investigates. The fix gets documented in the patterns doc and encoded as an autofix function. The next repo gets it automatically. That's how the 1.36 patterns got built in the first place: by running the skill on ovnk, finding what broke, fixing it, and documenting it. Now all 6 CORENET repos benefit from those patterns without discovering them independently.
Running e2e locally would need clusters and hours of compute. CI is the right place for that. The skill maximizes local coverage (build, vet, lint, unit tests, 30 gate checks) and uses /loop to close the CI feedback loop.
There was a problem hiding this comment.
Making CI monitoring a deterministic stage inside the skill would mean the skill pushes code to remote repos and triggers CI runs autonomously. The skill avoids this by design: the gh pr create command is generated but the skill does not run it, so the human controls what gets pushed and when. A rebase can produce a correct branch that the human decides not to push yet (waiting for library-go, coordinating with other PRs, etc.). Baking push+monitor into the skill removes that control.
/loop is a Claude Code built-in. The SKILL.md suggests /loop 5m check CI on the PR, explore any failures max carefully, find root causes after the user pushes. The agent runs gh pr checks, reads failure logs, and investigates root causes every 5 minutes until stopped. The user types one command and the agent handles the monitoring cycle.
There was a problem hiding this comment.
Agreed that once a pattern is generated it fires on every rebase, but that's exactly the problem for testing 1.37. These patterns are specific to a handful of ovnk issues (NetworkQoS int64, RelaxedServiceNameValidation, kubeadm v1beta4, MetalLB/FRR). They don't generalise to other repos, and they were seeded from my PR, not discovered by the skill.
You confirmed this yourself:
ovn-kubernetes/ovn-kubernetes#6525 (comment)
ovn-kubernetes/ovn-kubernetes#6525 (comment)
So 1.36 being green just shows those patterns replay. It doesn't test whether the skill can find an ovnk e2e failure on its own because the answers were already in the doc.
The /loop might well fix the 1.37 issues on its own next time that's the thing we actually want to test. So I think we should:
Empty the ovnk e2e patterns that came from my PR, so 1.37 is a real test of whether the skill discovers them rather than replays them.
First ask the human what steps are needed to bump for this repo, and feed that in.
Have the agent look at previous merged commits, not open PRs.
If /loop rediscovers the failures with the patterns emptied, that proves the capability and I'll be the first to say so. If it doesn't, we've learned the skill needs a human e2e pass per new version which is worth knowing before the fleet relies on it.
There was a problem hiding this comment.
Making CI monitoring a deterministic stage inside the skill would mean the skill pushes code to remote repos and triggers CI runs autonomously. The skill avoids this by design: the
gh pr createcommand is generated but the skill does not run it, so the human controls what gets pushed and when. A rebase can produce a correct branch that the human decides not to push yet (waiting for library-go, coordinating with other PRs, etc.). Baking push+monitor into the skill removes that control.
/loopis a Claude Code built-in. The SKILL.md suggests/loop 5m check CI on the PR, explore any failures max carefully, find root causesafter the user pushes. The agent runsgh pr checks, reads failure logs, and investigates root causes every 5 minutes until stopped. The user types one command and the agent handles the monitoring cycle.
The problem with /loop is not that it can't check CI. It can. And I'm fine with the human controlling the push.
The problem is /loop sits outside the skill. The fixes it pushes don't go into summary.txt, get no gate, get no review, and nothing records what it did. So if it fixes an e2e test and commits, we don't know what broke or why it picked that fix.
Also the skill deletes its state (rm -rf .rebase-tmp/) at Step 5, before CI even runs. So when CI fails the to-do list is already gone and /loop starts cold.
I think a Step 6 fixes this and keeps your push design:
Step 6 — check CI, after the human pushes the PR
Wait for CI: gh pr checks --watch. It waits till CI finishes and returns. No 5-minute loop, and it keeps the state.
Get the logs: gh run view --log-failed and gh run download for ginkgo (GitHub Actions). For prow, reuse the ci plugin.
Write each failure into summary.txt, same as a build failure. So it's tracked, not a loose suggestion.
Fix it the same way Steps 2–4 do: patterns doc first, one commit per fix, run the gate on it, run the review on it.
Give the fixes to the human to push. Same control as today. Then wait for CI again.
Delete .rebase-tmp/ only after CI is green. Right now it's deleted at Step 5, which is why the track is lost.
Human still controls the push. Nothing pushes on its own. The only change is the CI fixes now go through the same gates and review as everything else, and we have a record of what broke and why.
If you don't want a long-running run, Step 6 can be a separate command (/k8s-rebase:ci-check ) the user runs when CI is done.
There was a problem hiding this comment.
Also regarding clearing patterns to test 1.36 wont work because there might be a change the /loop agent to check exisiting open bump/rebase PRs.
So, as of now to test skills for bump will be testing k8s 1.37 for ovnk.
There was a problem hiding this comment.
point 6 if implemented can solve the issue of testing 1.36 as it will use the skills which should have, don't look for open PRs, look for only past commits. Also the main re-requisite is the patterns must be clear and the skills must be provided with info on how a human will bump or rebase k8s.
There was a problem hiding this comment.
right now under /loop it's the Claude agent working the e2e tests, not our skill or a subagent. Nothing in the skill drives it or checks it, so we also have no track of which commits were made for which issue when the loop is involved.
0b218ca to
c30d11d
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@plugins/k8s-rebase/gates/step1-rebase/rebase-completeness.md`:
- Around line 6-9: Update the uncommitted-changes counting guidance in
rebase-completeness.md to include relevant untracked (??) generated or vendored
outputs, while filtering only unrelated pre-existing untracked files. Preserve
detection of staged-but-uncommitted go.mod, vendor, and generated files as
commit-step failures.
- Around line 10-16: Update the commit-counting procedure in the “Rebase
commits” section to inspect only commits from the current rebase attempt, using
the recorded rebase range or a run-specific manifest instead of the entire
branch history. Apply this scope consistently to Rebase, Codegen, and Version
refs commit checks.
In `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md`:
- Around line 3-4: Update the file-counting step in the CRD validation
instructions to inspect parsed YAML schema objects rather than relying on
adjacent lines. Identify objects containing both format: int32 and maximum:
4294967295 regardless of comments, blank lines, key order, or intervening
properties, and count those requiring format: int64.
- Around line 5-8: Update the CRD comparison instructions to derive the actual
merge base or target ref from the repository state instead of hardcoding a
main-then-master fallback. Use that resolved ref with git show when checking the
original CRD definitions, preserving the metadata.name pattern validation
comparison.
- Around line 1-2: Update the CRD discovery instructions in the “If CRD YAMLs
exist in helm/*/crds/” section so a missing helm/*/crds/ directory does not
produce a zero result or skip validation. Discover CRDs across the repository,
or require explicit target paths, and preserve validation for CRDs located
outside Helm directories.
In `@plugins/k8s-rebase/scripts/k8s-rebase-review-prompt.md`:
- Around line 7-23: Update the prompt template around ORIGINAL_ERROR, DIFF,
K8S_CHANGELOG, and PATTERN_HINT to explicitly classify all interpolated content
as untrusted data, never instructions. Add clear, unambiguous delimiters around
each rendered evidence block and instruct Claude to ignore any directives
embedded within them before determining the final verdict.
- Around line 11-15: Update the review prompt around the `${DIFF}` placeholder
to state that the input may contain only selected files or be truncated, and
require the reviewer to fail closed when the diff is incomplete. Ensure the
prompt requests all relevant dependency/build files and later hunks, and defines
an explicit truncation marker that must trigger rejection rather than approval.
In `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh`:
- Around line 90-99: Update the auto-containerize re-exec command in the
validation script to pass --privileged only when MODE_FLAG selects --full, using
the same conditional gating pattern as USERNS_FLAG. Ensure quick, no-test,
test-only, and default modes omit the privileged option while preserving
full-mode behavior.
In `@plugins/k8s-rebase/scripts/k8s-rebase.sh`:
- Around line 643-649: Restrict the bare-version replacement in the Pass 2 loop
to Kubernetes-specific documentation or lines with Kubernetes context, rather
than all Markdown files under docs/. Update both the file-selection grep and
replacement matching around OLD_SHORT so unrelated values such as percentages or
longer versions like 1.35.0 remain unchanged, while preserving replacements for
standalone Kubernetes prose versions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 564aa843-28a9-4051-8865-5728bbf2a0bc
📒 Files selected for processing (41)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/k8s-rebase/.claude-plugin/plugin.jsonplugins/k8s-rebase/OWNERSplugins/k8s-rebase/README.mdplugins/k8s-rebase/docs/k8s-rebase-patterns.mdplugins/k8s-rebase/gates/step1-rebase/rebase-completeness.mdplugins/k8s-rebase/gates/step2-compilation/build-vet.mdplugins/k8s-rebase/gates/step2-compilation/diff-scope.mdplugins/k8s-rebase/gates/step2-compilation/fix-correctness.mdplugins/k8s-rebase/gates/step2-compilation/type-conversions.mdplugins/k8s-rebase/gates/step2-compilation/version-consistency.mdplugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.mdplugins/k8s-rebase/gates/step3-autofix/autofix-result.mdplugins/k8s-rebase/gates/step3-autofix/crd-validation.mdplugins/k8s-rebase/gates/step3-autofix/dep-release-notes.mdplugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.mdplugins/k8s-rebase/gates/step3-autofix/e2e-infra.mdplugins/k8s-rebase/gates/step3-autofix/feature-gates.mdplugins/k8s-rebase/gates/step3-autofix/logical-completeness.mdplugins/k8s-rebase/gates/step3-autofix/patterns-completeness.mdplugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdplugins/k8s-rebase/gates/step4-verification/ci-prediction.mdplugins/k8s-rebase/gates/step4-verification/ci-readiness.mdplugins/k8s-rebase/gates/step4-verification/cleanliness.mdplugins/k8s-rebase/gates/step4-verification/commit-messages.mdplugins/k8s-rebase/gates/step4-verification/correctness.mdplugins/k8s-rebase/gates/step4-verification/dep-cve-check.mdplugins/k8s-rebase/gates/step4-verification/deprecated-imports.mdplugins/k8s-rebase/gates/step4-verification/go-version-check.mdplugins/k8s-rebase/gates/step4-verification/gomod-diff-analysis.mdplugins/k8s-rebase/gates/step4-verification/k8s-changelog.mdplugins/k8s-rebase/gates/step4-verification/logical-consistency.mdplugins/k8s-rebase/gates/step4-verification/maintainer-review.mdplugins/k8s-rebase/gates/step4-verification/version-completeness.mdplugins/k8s-rebase/scripts/k8s-rebase-autofix.shplugins/k8s-rebase/scripts/k8s-rebase-review-prompt.mdplugins/k8s-rebase/scripts/k8s-rebase-review.shplugins/k8s-rebase/scripts/k8s-rebase-validate.shplugins/k8s-rebase/scripts/k8s-rebase.shplugins/k8s-rebase/skills/k8s-rebase/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (22)
- plugins/k8s-rebase/gates/step4-verification/dep-cve-check.md
- plugins/k8s-rebase/.claude-plugin/plugin.json
- plugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.md
- plugins/k8s-rebase/gates/step4-verification/maintainer-review.md
- .claude-plugin/marketplace.json
- plugins/k8s-rebase/gates/step2-compilation/fix-correctness.md
- plugins/k8s-rebase/gates/step3-autofix/patterns-completeness.md
- plugins/k8s-rebase/gates/step2-compilation/version-consistency.md
- plugins/k8s-rebase/gates/step4-verification/commit-messages.md
- plugins/k8s-rebase/README.md
- plugins/k8s-rebase/gates/step4-verification/cleanliness.md
- plugins/k8s-rebase/OWNERS
- plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md
- plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md
- plugins/k8s-rebase/gates/step2-compilation/type-conversions.md
- plugins/k8s-rebase/gates/step3-autofix/feature-gates.md
- plugins/k8s-rebase/gates/step4-verification/ci-readiness.md
- plugins/k8s-rebase/gates/step3-autofix/logical-completeness.md
- plugins/k8s-rebase/gates/step4-verification/logical-consistency.md
- plugins/k8s-rebase/gates/step4-verification/k8s-changelog.md
- docs/index.html
- plugins/k8s-rebase/gates/step3-autofix/dep-release-notes.md
| 2. Uncommitted changes: count from `git status --short` | ||
| (exclude untracked files with `?`). Any staged-but- | ||
| uncommitted go.mod, vendor, or generated files indicate | ||
| the script's commit step failed. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not ignore relevant untracked outputs.
Excluding every status entry beginning with ? allows newly generated or vendored files to remain uncommitted while producing count 0, contradicting the failure condition described here. Count relevant ?? paths while filtering only unrelated pre-existing files.
🤖 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 `@plugins/k8s-rebase/gates/step1-rebase/rebase-completeness.md` around lines 6
- 9, Update the uncommitted-changes counting guidance in rebase-completeness.md
to include relevant untracked (??) generated or vendored outputs, while
filtering only unrelated pre-existing untracked files. Preserve detection of
staged-but-uncommitted go.mod, vendor, and generated files as commit-step
failures.
| 3. Rebase commits: check `git log --oneline` on the current | ||
| branch. Count MISSING expected commits: | ||
| - "Rebase" commits (at least 1 per go.mod with k8s.io deps, | ||
| excluding vendor/) | ||
| - Codegen commit (expected if hack/update-codegen.sh or | ||
| Makefile has generate/manifests/codegen targets) | ||
| - Version refs commit |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Scope commit checks to the current rebase attempt.
Searching the entire branch with git log --oneline can satisfy the Rebase, codegen, and version-reference checks using commits from an earlier run. Use the recorded rebase range or a run-specific manifest instead.
🤖 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 `@plugins/k8s-rebase/gates/step1-rebase/rebase-completeness.md` around lines 10
- 16, Update the commit-counting procedure in the “Rebase commits” section to
inspect only commits from the current rebase attempt, using the recorded rebase
range or a run-specific manifest instead of the entire branch history. Apply
this scope consistently to Rebase, Codegen, and Version refs commit checks.
| 1. Count files where `format: int32` immediately precedes | ||
| `maximum: 4294967295` (these need format: int64). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match CRD schema semantics, not line adjacency.
This detects only the exact adjacent sequence format: int32 followed by maximum: 4294967295. Comments, blank lines, reordered keys, or other schema properties will produce a false zero count. Parse the YAML or inspect the containing schema object.
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md` around lines 3 - 4,
Update the file-counting step in the CRD validation instructions to inspect
parsed YAML schema objects rather than relying on adjacent lines. Identify
objects containing both format: int32 and maximum: 4294967295 regardless of
comments, blank lines, key order, or intervening properties, and count those
requiring format: int64.
| ## Original Error | ||
|
|
||
| ${ORIGINAL_ERROR} | ||
|
|
||
| ## Fix Diff | ||
|
|
||
| ```diff | ||
| ${DIFF} | ||
| ``` | ||
|
|
||
| ## K8s Release Notes (relevant excerpt) | ||
|
|
||
| ${K8S_CHANGELOG} | ||
|
|
||
| ## Matching Pattern (if any) | ||
|
|
||
| ${PATTERN_HINT} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat interpolated sections as untrusted evidence.
Commit messages, diffs, changelogs, and pattern hints can contain instructions such as APPROVE:. Since the rendered prompt is sent directly to Claude, a malicious change can steer the verdict. Explicitly state that these blocks are data, never instructions, and use unambiguous delimiters before evaluating the final verdict.
🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-review-prompt.md` around lines 7 - 23,
Update the prompt template around ORIGINAL_ERROR, DIFF, K8S_CHANGELOG, and
PATTERN_HINT to explicitly classify all interpolated content as untrusted data,
never instructions. Add clear, unambiguous delimiters around each rendered
evidence block and instruct Claude to ignore any directives embedded within them
before determining the final verdict.
| ## Fix Diff | ||
|
|
||
| ```diff | ||
| ${DIFF} | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make the review fail closed on partial diffs.
The downstream plugins/k8s-rebase/scripts/k8s-rebase-review.sh only supplies selected file types and truncates ${DIFF} to 2,000 lines, while this template presents it as the complete fix. Dependency/build changes such as go.mod, go.sum, Makefile, or later diff hunks can therefore be approved without review. Include all relevant files and add an explicit truncation marker or reject incomplete input.
🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-review-prompt.md` around lines 11 - 15,
Update the review prompt around the `${DIFF}` placeholder to state that the
input may contain only selected files or be truncated, and require the reviewer
to fail closed when the diff is incomplete. Ensure the prompt requests all
relevant dependency/build files and later hunks, and defines an explicit
truncation marker that must trigger rejection rather than approval.
| # Pass 2: bare version in doc prose (1.35 without v-prefix) | ||
| while IFS= read -r file; do | ||
| [[ -z "$file" ]] && continue | ||
| sed -i -E "s/\b${OLD_SHORT//./\\.}\b/${NEW_SHORT}/g" "$file" | ||
| CHANGED_FILES+="$file"$'\n' | ||
| info " Updated (short): $file" | ||
| done < <(grep -rln "\b${OLD_SHORT}\b" --include="*.md" docs/ 2>/dev/null | grep -v vendor || true) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant region with line numbers
sed -n '600,690p' plugins/k8s-rebase/scripts/k8s-rebase.sh | cat -n
printf '\n--- SEARCH OLD_SHORT/NEW_SHORT ---\n'
rg -n "OLD_SHORT|NEW_SHORT|OLD_MAJOR|OLD_MINOR|docs/" plugins/k8s-rebase/scripts/k8s-rebase.sh
printf '\n--- DOC FILE PATTERNS IN SCRIPT ---\n'
rg -n "grep -rln|sed -i -E|k8s|kubernetes|docs/" plugins/k8s-rebase/scripts/k8s-rebase.shRepository: openshift-eng/ai-helpers
Length of output: 13032
🏁 Script executed:
python3 - <<'PY'
import subprocess, textwrap, os, tempfile, pathlib, json
samples = [
"Kubernetes 1.35 release notes",
"tool version 1.35%",
"version 1.35.0",
"abc1.35def",
"1.35",
]
pattern = r"\b1\.35\b"
for s in samples:
p = subprocess.run(
["bash", "-lc", f"printf '%s\n' {s!r} | grep -n '{pattern}'"],
capture_output=True, text=True
)
print("GREP", repr(s), "=>", "MATCH" if p.returncode == 0 else "NO")
p2 = subprocess.run(
["bash", "-lc", f"printf '%s\n' {s!r} | sed -E 's/{pattern}/1.36/g'"],
capture_output=True, text=True
)
print("SED ", repr(s), "=>", repr(p2.stdout.strip()))
PYRepository: openshift-eng/ai-helpers
Length of output: 539
🏁 Script executed:
python3 - <<'PY'
import subprocess
samples = [
"Kubernetes 1.35 release notes",
"tool version 1.35%",
"version 1.35.0",
"abc1.35def",
"1.35",
]
pattern = r"\b1\.35\b"
for s in samples:
grep = subprocess.run(
["bash", "-lc", f"printf '%s\n' {s!r} | grep -n '{pattern}'"],
capture_output=True, text=True
)
sed = subprocess.run(
["bash", "-lc", f"printf '%s\n' {s!r} | sed -E 's/{pattern}/1.36/g'"],
capture_output=True, text=True
)
print(repr(s), "GREP=", grep.returncode == 0, "SED=", repr(sed.stdout.strip()))
PYRepository: openshift-eng/ai-helpers
Length of output: 423
Restrict the bare-version docs replace to Kubernetes-specific prose. docs/**/*.md can still contain unrelated 1.35 tokens, and this pass rewrites them too (for example 1.35% and the leading part of 1.35.0). Narrow the match to k8s-specific docs or surrounding context.
🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase.sh` around lines 643 - 649, Restrict
the bare-version replacement in the Pass 2 loop to Kubernetes-specific
documentation or lines with Kubernetes context, rather than all Markdown files
under docs/. Update both the file-selection grep and replacement matching around
OLD_SHORT so unrelated values such as percentages or longer versions like 1.35.0
remain unchanged, while preserving replacements for standalone Kubernetes prose
versions.
c30d11d to
f06d1b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (2)
plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md (1)
11-11: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun
go veteven whengo buildfails.Because the commands are chained with
&&, a build failure skipsgo vet, violating the gate’s requirement to run both checks and producing an incomplete error count.
plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md#L11-L11: replace the command chain with a subshell that executes both checks independently.plugins/k8s-rebase/gates/step2-compilation/build-vet.md#L11-L11: apply the identical subshell replacement to ensurego vetis not skipped on build failure.🛠️ Proposed fix (apply to both files)
- cd "$mod_dir" && go build ./... 2>&1 && go vet ./... 2>&1; cd - + ( + cd "$mod_dir" || exit 1 + go build ./... 2>&1 + go vet ./... 2>&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 `@plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md` at line 11, Update the command at plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md:11-11 to run go build and go vet independently within a subshell, ensuring go vet executes even if the build fails. Apply the identical replacement at plugins/k8s-rebase/gates/step2-compilation/build-vet.md:11-11.plugins/k8s-rebase/gates/step4-verification/ci-prediction.md (1)
19-24: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not require a dedicated codegen commit in
git log.A valid rebase may include generated output in a squashed or combined commit. Checking
git logcan report a false failure. Update the guidance to prefer diff verification over checking the commit history.🛠️ Proposed update
- Makefile codegen/generate/manifests target exists, check - that git log shows a codegen commit. If the repo has a - `verify-update-codegen` or `verify` CI job, stale output - will fail `git diff --exit-code`. Look for controller-gen - version annotations in CRD manifests matching the vendored - controller-tools version. + Makefile codegen/generate/manifests target exists, verify + that stale output would not fail `git diff --exit-code` if + the repo has a `verify-update-codegen` or `verify` CI job. + Prefer running these verification scripts over checking + git history, as generated output may be part of squashed or + combined commits. Look for controller-gen version annotations + in CRD manifests matching the vendored controller-tools version.🤖 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 `@plugins/k8s-rebase/gates/step4-verification/ci-prediction.md` around lines 19 - 24, Update the guidance in the codegen verification section to remove the requirement that git log contain a dedicated codegen commit. Instruct reviewers to prefer verifying generated output through git diff --exit-code or the repository’s existing verify-update-codegen/verify CI job, while retaining the controller-gen version annotation check.
🧹 Nitpick comments (4)
plugins/k8s-rebase/gates/step2-compilation/diff-scope.md (1)
5-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix markdown formatting for directory paths.
The unescaped underscores in
_output/andthird_party/can trigger markdown rendering issues (MD037: spaces inside emphasis markers). Wrap the paths in backticks to treat them as literal code.🛠️ Proposed fix
-.sh, .j2). Changes in generated/managed directories are also -expected: vendor/, LICENSES/, _output/, third_party/. +.sh, .j2). Changes in generated/managed directories are also +expected: `vendor/`, `LICENSES/`, `_output/`, `third_party/`.🤖 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 `@plugins/k8s-rebase/gates/step2-compilation/diff-scope.md` around lines 5 - 6, Update the directory-path list in the compilation diff-scope documentation to wrap `_output/` and `third_party/` in backticks, preserving the existing wording and formatting for the other paths.plugins/k8s-rebase/docs/k8s-rebase-patterns.md (1)
154-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifiers to fenced code blocks.
These code blocks lack a language specifier, which triggers
markdownlint(MD040) and degrades syntax highlighting.
plugins/k8s-rebase/docs/k8s-rebase-patterns.md#L154-L154: change```to```textplugins/k8s-rebase/docs/k8s-rebase-patterns.md#L266-L266: change```to```textplugins/k8s-rebase/docs/k8s-rebase-patterns.md#L270-L270: change```to```textplugins/k8s-rebase/docs/k8s-rebase-patterns.md#L294-L294: change```to```textplugins/k8s-rebase/docs/k8s-rebase-patterns.md#L329-L329: change```to```textplugins/k8s-rebase/docs/k8s-rebase-patterns.md#L463-L463: change```to```go.mod🤖 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 `@plugins/k8s-rebase/docs/k8s-rebase-patterns.md` at line 154, Update the fenced code blocks in plugins/k8s-rebase/docs/k8s-rebase-patterns.md at lines 154, 266, 270, 294, and 329 to use the text language specifier, and use the go.mod specifier at line 463. No other content changes are needed.plugins/k8s-rebase/gates/step4-verification/go-version-check.md (1)
20-27: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlign this gate's output with the "count" contract other step4 gates use.
SKILL.md lists this as a
(count)gate and requires "All count-checks must be 0," but the prompt only asks to "Report any mismatches or migration opportunities" — unlike sibling count gates (version-completeness.md,gomod-diff-analysis.md) which explicitly say "Report count of X." Free-form prose output makes it harder for the main agent to mechanically verify the zero/non-zero gate result.♻️ Suggested wording
-Report any mismatches or migration opportunities. +Report the count of mismatches and migration opportunities found above (0 = pass).🤖 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 `@plugins/k8s-rebase/gates/step4-verification/go-version-check.md` around lines 20 - 27, Update the x/ package opportunity check in the step4 verification prompt to explicitly require reporting the count of detected mismatches or migration opportunities, while preserving the existing read-only rule and grep scope. Ensure the output is a numeric count so SKILL.md can enforce the zero/non-zero gate contract.plugins/k8s-rebase/scripts/k8s-rebase.sh (1)
37-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftContainerize re-exec + commit-style helper logic is copy-pasted across all three scripts with no shared source of truth.
_detect_commit_style/format_msg(k8s-rebase.sh, k8s-rebase-autofix.sh) and the "check Go version → pick podman/docker → re-exec in container" block (all three scripts) are near-identical, hand-duplicated blocks. This isn't just style: the previously-flagged--privilegedscoping fix landed only ink8s-rebase-validate.sh's copy and never propagated, which is exactly the kind of drift this duplication invites.
plugins/k8s-rebase/scripts/k8s-rebase.sh#L37-L59: extract_detect_commit_style/format_msginto a smalllib/rebase-common.shsourced by all three scripts.plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L42-L61: source the same shared helper instead of redefining it.plugins/k8s-rebase/scripts/k8s-rebase.sh#L225-L263: extract the podman/docker-detection-and-re-exec block into a shared function parameterized by the script path and extra args.plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L79-L113: use the shared re-exec function instead of its own copy.plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L66-L102: use the shared re-exec function (with a mode-aware--privileged/--userns=keep-idparameter) instead of its own copy.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase.sh` around lines 37 - 59, Create plugins/k8s-rebase/scripts/lib/rebase-common.sh with shared _detect_commit_style, format_msg, and a parameterized container re-exec function. In plugins/k8s-rebase/scripts/k8s-rebase.sh#37-59 and k8s-rebase-autofix.sh#42-61, remove duplicated commit-style helpers and source the shared library; replace re-exec blocks at k8s-rebase.sh#225-263, k8s-rebase-autofix.sh#79-113, and k8s-rebase-validate.sh#66-102 with the shared function, preserving script-specific arguments and mode-aware --privileged/--userns=keep-id behavior.
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/skill-improvement.md`:
- Around line 1-6: Update
plugins/k8s-rebase/gates/step4-verification/skill-improvement.md lines 1-6 to
require inspecting commit history with git log/git show before classifying
commits, using commit bodies to distinguish manual fixes from commits containing
“Applied:”. Update
plugins/k8s-rebase/gates/step4-verification/maintainer-review.md lines 1-10 and
21-25 to use commit history when reporting scope, commit messages, SHAs, and
per-commit file/line findings.
- Around line 1-4: Update the manual-fix commit classification in the
branch-diff verification instructions to avoid treating absence of “Applied:” as
definitive. Use an explicit autofix commit trailer, or require the autofix
workflow—including post-vet fix_uncommitted commits—to add the marker
consistently before relying on it for classification.
In `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh`:
- Around line 122-125: Replace the predictable /tmp/jq download location in the
jq-install blocks of plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh (lines
122-125) and plugins/k8s-rebase/scripts/k8s-rebase-validate.sh (lines 123-126)
with a directory created by mktemp -d, download jq there, and prepend that
directory to PATH in both scripts.
In `@plugins/k8s-rebase/scripts/k8s-rebase-review.sh`:
- Around line 30-59: Align the review workflow with the script’s per-commit
design: update the SKILL.md Step 4c invocation to run the review for each fix
commit with the actual error context, rather than once at the final HEAD with
the generic “k8s rebase” argument. Preserve the existing evidence collection in
the script, including PATTERN_HINT, K8S_CHANGELOG, and commit-scoped DIFF
behavior.
In `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh`:
- Around line 271-289: The root package classification in run_test_only uses an
unanchored regex and can incorrectly skip packages with matching substrings.
Make this check exact-match the same way as the make test sudo-fallback path,
preferably by reusing a shared helper or pattern based on ROOT_PKGS so both
paths cannot drift.
- Around line 248-253: Replace the eval-based loading in the feature-gate export
block with a loop over the matching export lines from TEST_GO_SH, parsing and
applying only literal KUBE_FEATURE_ assignments. Preserve the existing file
discovery and skip behavior when TEST_GO_SH is empty, while avoiding execution
of arbitrary shell code.
In `@plugins/k8s-rebase/skills/k8s-rebase/SKILL.md`:
- Around line 28-35: Choose one consistent container policy and apply it across
the documented and automated flows: either make podman with --userns=keep-id a
hard requirement by removing the docker fallback and making the
container-selection logic in k8s-rebase.sh, k8s-rebase-autofix.sh, and
k8s-rebase-validate.sh fail when podman is unavailable, or retain the fallback
and update SKILL.md plus all three scripts to visibly warn about docker’s
root-owned-file caveat when it is selected. Ensure all four affected files
reflect the same policy.
---
Duplicate comments:
In `@plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md`:
- Line 11: Update the command at
plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md:11-11 to run go
build and go vet independently within a subshell, ensuring go vet executes even
if the build fails. Apply the identical replacement at
plugins/k8s-rebase/gates/step2-compilation/build-vet.md:11-11.
In `@plugins/k8s-rebase/gates/step4-verification/ci-prediction.md`:
- Around line 19-24: Update the guidance in the codegen verification section to
remove the requirement that git log contain a dedicated codegen commit. Instruct
reviewers to prefer verifying generated output through git diff --exit-code or
the repository’s existing verify-update-codegen/verify CI job, while retaining
the controller-gen version annotation check.
---
Nitpick comments:
In `@plugins/k8s-rebase/docs/k8s-rebase-patterns.md`:
- Line 154: Update the fenced code blocks in
plugins/k8s-rebase/docs/k8s-rebase-patterns.md at lines 154, 266, 270, 294, and
329 to use the text language specifier, and use the go.mod specifier at line
463. No other content changes are needed.
In `@plugins/k8s-rebase/gates/step2-compilation/diff-scope.md`:
- Around line 5-6: Update the directory-path list in the compilation diff-scope
documentation to wrap `_output/` and `third_party/` in backticks, preserving the
existing wording and formatting for the other paths.
In `@plugins/k8s-rebase/gates/step4-verification/go-version-check.md`:
- Around line 20-27: Update the x/ package opportunity check in the step4
verification prompt to explicitly require reporting the count of detected
mismatches or migration opportunities, while preserving the existing read-only
rule and grep scope. Ensure the output is a numeric count so SKILL.md can
enforce the zero/non-zero gate contract.
In `@plugins/k8s-rebase/scripts/k8s-rebase.sh`:
- Around line 37-59: Create plugins/k8s-rebase/scripts/lib/rebase-common.sh with
shared _detect_commit_style, format_msg, and a parameterized container re-exec
function. In plugins/k8s-rebase/scripts/k8s-rebase.sh#37-59 and
k8s-rebase-autofix.sh#42-61, remove duplicated commit-style helpers and source
the shared library; replace re-exec blocks at k8s-rebase.sh#225-263,
k8s-rebase-autofix.sh#79-113, and k8s-rebase-validate.sh#66-102 with the shared
function, preserving script-specific arguments and mode-aware
--privileged/--userns=keep-id behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e3591aa-e874-46c0-a843-96728207649f
📒 Files selected for processing (42)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/k8s-rebase/.claude-plugin/plugin.jsonplugins/k8s-rebase/OWNERSplugins/k8s-rebase/README.mdplugins/k8s-rebase/docs/k8s-rebase-patterns.mdplugins/k8s-rebase/gates/step1-rebase/rebase-completeness.mdplugins/k8s-rebase/gates/step2-compilation/build-vet.mdplugins/k8s-rebase/gates/step2-compilation/diff-scope.mdplugins/k8s-rebase/gates/step2-compilation/fix-correctness.mdplugins/k8s-rebase/gates/step2-compilation/type-conversions.mdplugins/k8s-rebase/gates/step2-compilation/version-consistency.mdplugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.mdplugins/k8s-rebase/gates/step3-autofix/autofix-result.mdplugins/k8s-rebase/gates/step3-autofix/crd-validation.mdplugins/k8s-rebase/gates/step3-autofix/dep-release-notes.mdplugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.mdplugins/k8s-rebase/gates/step3-autofix/e2e-infra.mdplugins/k8s-rebase/gates/step3-autofix/feature-gates.mdplugins/k8s-rebase/gates/step3-autofix/logical-completeness.mdplugins/k8s-rebase/gates/step3-autofix/patterns-completeness.mdplugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdplugins/k8s-rebase/gates/step4-verification/ci-prediction.mdplugins/k8s-rebase/gates/step4-verification/ci-readiness.mdplugins/k8s-rebase/gates/step4-verification/cleanliness.mdplugins/k8s-rebase/gates/step4-verification/commit-messages.mdplugins/k8s-rebase/gates/step4-verification/correctness.mdplugins/k8s-rebase/gates/step4-verification/dep-cve-check.mdplugins/k8s-rebase/gates/step4-verification/deprecated-imports.mdplugins/k8s-rebase/gates/step4-verification/go-version-check.mdplugins/k8s-rebase/gates/step4-verification/gomod-diff-analysis.mdplugins/k8s-rebase/gates/step4-verification/k8s-changelog.mdplugins/k8s-rebase/gates/step4-verification/logical-consistency.mdplugins/k8s-rebase/gates/step4-verification/maintainer-review.mdplugins/k8s-rebase/gates/step4-verification/skill-improvement.mdplugins/k8s-rebase/gates/step4-verification/version-completeness.mdplugins/k8s-rebase/scripts/k8s-rebase-autofix.shplugins/k8s-rebase/scripts/k8s-rebase-review-prompt.mdplugins/k8s-rebase/scripts/k8s-rebase-review.shplugins/k8s-rebase/scripts/k8s-rebase-validate.shplugins/k8s-rebase/scripts/k8s-rebase.shplugins/k8s-rebase/skills/k8s-rebase/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (25)
- plugins/k8s-rebase/gates/step2-compilation/type-conversions.md
- plugins/k8s-rebase/scripts/k8s-rebase-review-prompt.md
- plugins/k8s-rebase/gates/step3-autofix/logical-completeness.md
- plugins/k8s-rebase/gates/step4-verification/k8s-changelog.md
- plugins/k8s-rebase/gates/step4-verification/cleanliness.md
- plugins/k8s-rebase/gates/step4-verification/ci-readiness.md
- plugins/k8s-rebase/gates/step4-verification/commit-messages.md
- plugins/k8s-rebase/OWNERS
- plugins/k8s-rebase/.claude-plugin/plugin.json
- plugins/k8s-rebase/gates/step2-compilation/version-consistency.md
- plugins/k8s-rebase/gates/step2-compilation/fix-correctness.md
- plugins/k8s-rebase/gates/step3-autofix/crd-validation.md
- plugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.md
- plugins/k8s-rebase/gates/step3-autofix/dep-release-notes.md
- plugins/k8s-rebase/gates/step4-verification/logical-consistency.md
- plugins/k8s-rebase/gates/step1-rebase/rebase-completeness.md
- plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md
- plugins/k8s-rebase/gates/step3-autofix/feature-gates.md
- plugins/k8s-rebase/gates/step4-verification/dep-cve-check.md
- plugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.md
- plugins/k8s-rebase/gates/step3-autofix/patterns-completeness.md
- .claude-plugin/marketplace.json
- plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md
- docs/index.html
- plugins/k8s-rebase/README.md
| Read the branch diff (git diff of merge-base..HEAD). Identify | ||
| manual fix commits — those that do NOT have "Applied:" in the | ||
| commit body. Autofix commits always contain "Applied:" followed | ||
| by fix descriptions; commits without it are manual agent work. | ||
|
|
||
| For each manual fix commit, classify the change: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add an explicit commit-history inspection step.
Both gates require commit-level classification, messages, SHAs, and bodies, but git diff <merge-base>..HEAD only shows an aggregate patch.
plugins/k8s-rebase/gates/step4-verification/skill-improvement.md#L1-L6: requiregit log/git showbefore classifying manual versus autofix commits.plugins/k8s-rebase/gates/step4-verification/maintainer-review.md#L1-L10,21-L25: use commit history to support scope, message, SHA, and per-commit file/line findings.
📍 Affects 2 files
plugins/k8s-rebase/gates/step4-verification/skill-improvement.md#L1-L6(this comment)plugins/k8s-rebase/gates/step4-verification/maintainer-review.md#L1-L10
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/skill-improvement.md` around
lines 1 - 6, Update
plugins/k8s-rebase/gates/step4-verification/skill-improvement.md lines 1-6 to
require inspecting commit history with git log/git show before classifying
commits, using commit bodies to distinguish manual fixes from commits containing
“Applied:”. Update
plugins/k8s-rebase/gates/step4-verification/maintainer-review.md lines 1-10 and
21-25 to use commit history when reporting scope, commit messages, SHAs, and
per-commit file/line findings.
| # Install jq if missing (needed by verify-third-party-licenses) | ||
| if ! command -v jq &>/dev/null; then | ||
| curl -sL https://github.com/jqlang/jq/releases/download/jq-1.7.1/jq-linux-amd64 -o /tmp/jq 2>/dev/null && chmod +x /tmp/jq && export PATH="/tmp:$PATH" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Same predictable /tmp/jq TOCTOU pattern copy-pasted into two scripts. Both scripts download jq to a hardcoded, predictable /tmp/jq path, chmod +x it, and prepend /tmp to PATH — a local attacker (or leftover file) can pre-place /tmp/jq and have it executed as jq for the rest of the run.
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L122-L125: download to amktemp -ddirectory and prepend that toPATHinstead of/tmp.plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L123-L126: apply the identicalmktemp -dfix here too.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 123-123: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/jq
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 123-123: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/jq
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
📍 Affects 2 files
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L122-L125(this comment)plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L123-L126
🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh` around lines 122 - 125,
Replace the predictable /tmp/jq download location in the jq-install blocks of
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh (lines 122-125) and
plugins/k8s-rebase/scripts/k8s-rebase-validate.sh (lines 123-126) with a
directory created by mktemp -d, download jq there, and prepend that directory to
PATH in both scripts.
Source: Linters/SAST tools
| # Filter out root_pkgs (need CAP_NET_ADMIN, always fail unprivileged) | ||
| local test_go_sh | ||
| test_go_sh=$(find . -name "test-go.sh" -path "*/hack/*" -not -path "*/vendor/*" 2>/dev/null | head -1) | ||
| if [[ -n "$test_go_sh" ]]; then | ||
| local root_pkgs_pattern | ||
| root_pkgs_pattern=$(sed -n '/root_pkgs=(/,/)/p' "$test_go_sh" | grep -oE 'pkg/[^"]+' | tr '\n' '|' || true) | ||
| if [[ -n "$root_pkgs_pattern" ]]; then | ||
| local filtered="" | ||
| for pkg in $TEST_ONLY_PKGS; do | ||
| if echo "$pkg" | grep -qE "(${root_pkgs_pattern%|})"; then | ||
| echo ":: Skipping root_pkg $pkg (needs CAP_NET_ADMIN)" | ||
| else | ||
| filtered="$filtered $pkg" | ||
| fi | ||
| done | ||
| TEST_ONLY_PKGS="${filtered# }" | ||
| [[ -z "$TEST_ONLY_PKGS" ]] && { echo "All packages are root_pkgs — nothing to test unprivileged"; exit 0; } | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
root_pkgs filtering is unanchored here, but exact-matched elsewhere in this same file — pick one.
run_test_only() skips a package as privileged via an unanchored substring match:
if echo "$pkg" | grep -qE "(${root_pkgs_pattern%|})"; then
Later in the same file (the make test sudo-fallback path, ~lines 433/443) the equivalent check anchors the match: grep -qE "^(${ROOT_PKGS%|})$". Because the first check has no boundaries, a root_pkgs entry like pkg/ovn will also match an unrelated package such as pkg/ovnwebhook, silently skipping a non-privileged package from --test-only runs with no error — just a "Skipping root_pkg" message that looks correct. Consider extracting one shared helper/pattern for this classification so the two code paths can't drift.
🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh` around lines 271 - 289,
The root package classification in run_test_only uses an unanchored regex and
can incorrectly skip packages with matching substrings. Make this check
exact-match the same way as the make test sudo-fallback path, preferably by
reusing a shared helper or pattern based on ROOT_PKGS so both paths cannot
drift.
| **Container commands:** When running containers, always use | ||
| `podman` with `--userns=keep-id`. Never use `docker run` — | ||
| it creates root-owned files that break subsequent operations. | ||
| `--security-opt label=disable` is required for SELinux hosts | ||
| (container writes to bind-mounted repo dirs fail without it). | ||
| ``` | ||
| podman run --rm --security-opt label=disable --userns=keep-id -v "$(pwd):$(pwd)" -w "$(pwd)" docker.io/library/golang:VERSION ... | ||
| ``` |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
SKILL.md's "Never use docker run" rule is silently contradicted by all three automation scripts. SKILL.md states plainly that docker must never be used because it "creates root-owned files that break subsequent operations," but every script that auto-containerizes falls back to docker automatically whenever podman isn't installed — exactly the failure mode SKILL.md warns about, without the human/agent ever being told it happened.
plugins/k8s-rebase/skills/k8s-rebase/SKILL.md#L28-L35: acknowledge the automatic docker fallback in the deterministic scripts (and its root-owned-file caveat), or state that podman is a hard requirement and the scripts should fail instead of silently falling back to docker.plugins/k8s-rebase/scripts/k8s-rebase.sh#L236-L241: either drop the docker fallback (diewhen only docker is available) to match the documented policy, or emit a visible warning about root-owned files when it engages.plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L87-L89: same choice — align with whichever policy is chosen for k8s-rebase.sh.plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L81-L83: same choice — align with whichever policy is chosen for k8s-rebase.sh.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 4 files
plugins/k8s-rebase/skills/k8s-rebase/SKILL.md#L28-L35(this comment)plugins/k8s-rebase/scripts/k8s-rebase.sh#L236-L241plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L87-L89plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L81-L83
🤖 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 `@plugins/k8s-rebase/skills/k8s-rebase/SKILL.md` around lines 28 - 35, Choose
one consistent container policy and apply it across the documented and automated
flows: either make podman with --userns=keep-id a hard requirement by removing
the docker fallback and making the container-selection logic in k8s-rebase.sh,
k8s-rebase-autofix.sh, and k8s-rebase-validate.sh fail when podman is
unavailable, or retain the fallback and update SKILL.md plus all three scripts
to visibly warn about docker’s root-owned-file caveat when it is selected.
Ensure all four affected files reflect the same policy.
Automates k8s.io/* dependency rebases for Go projects. Handles dep bumps, codegen, version references, compilation fixes, lint migration, feature gate management, and antagonistic review. The skill improves itself across runs: when a rebase surfaces a new breakage pattern (via gates, local validation, or CI), the agent fixes it and documents the pattern. The next repo or k8s version benefits from that pattern automatically — through autofix functions, gate checks, or patterns doc entries. Scripts: - k8s-rebase.sh: deterministic rebase (deps, codegen, version refs) - k8s-rebase-autofix.sh: named fix functions for known breakage patterns (x/exp, reflect.Ptr, klog v1, FieldsV1, Eventf, CRDs, feature gates, KIND, MetalLB, KubeVirt, golangci-lint v1→v2) - k8s-rebase-validate.sh: build/vet/lint/test with skip detection - k8s-rebase-review.sh: antagonistic review via separate Claude instance 29 gate subagents across 4 verification steps verify the rebase before the agent proceeds. Count gates enforce hard constraints (build errors, version mismatches, test skips). Judge gates review diffs for correctness. Discovery gates read external signals (dep release notes, k8s changelog, CVE databases, go.mod risk classification) to catch issues that build/lint/test cannot detect. Patterns doc catalogs 30+ breakage patterns with fix recipes and extension guide for future k8s versions. Signed-off-by: Daniel Farrell <dfarrell@redhat.com>
f06d1b9 to
c0724a4
Compare
|
The OWNERS file contains untrusted users, which makes it INVALID. The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the openshift-eng org. You can then trigger verification by writing
|
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (5)
plugins/k8s-rebase/scripts/k8s-rebase.sh (1)
686-695: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBare-version doc replace still isn't restricted to Kubernetes-specific prose.
The perl lookbehind/lookahead now correctly guards against IP-address and patch-version corruption (the two concrete examples from the earlier review), but the match is still purely numeric — any standalone
1.35-style token indocs/**/*.mdunrelated to Kubernetes (e.g. a percentage, section/step number) will still be rewritten, since there's no check that the surrounding text is actually about Kubernetes.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase.sh` around lines 686 - 695, Restrict the bare-version replacement in the Pass 2 loop to Kubernetes-specific documentation instead of matching every standalone numeric token under docs/. Update the grep/perl selection around OLD_SHORT and NEW_SHORT to require an appropriate Kubernetes context, while preserving the existing protections for IP addresses, compound versions, and patch versions.plugins/k8s-rebase/gates/step3-autofix/crd-validation.md (1)
6-7: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCRD
format: int32/maximumcheck still relies on line adjacency, not schema semantics.This only detects the exact adjacent sequence
format: int32followed immediately bymaximum: 4294967295. Comments, blank lines, reordered keys, or other intervening schema properties will still produce a false zero count for a read-only gate that's required to report exact counts.🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md` around lines 6 - 7, Update the CRD validation instructions in step 3 so the count identifies schema objects containing both format: int32 and maximum: 4294967295 regardless of key order, comments, blank lines, or intervening properties; remove the line-adjacency requirement and preserve exact reporting for the read-only gate.plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh (1)
122-128: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winPredictable
/tmp/jqpath is copy-pasted into both scripts — same TOCTOU root cause. Both download jq to a hardcoded, predictable path,chmod +xit, and prepend/tmptoPATH, allowing a local attacker (or leftover file) to hijack the binary.
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh#L122-L128: download jq into amktemp -ddirectory and prepend that directory toPATHinstead of/tmp.plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L125-L131: apply the identicalmktemp -dfix here too.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh` around lines 122 - 128, The jq bootstrap blocks in plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh lines 122-128 and plugins/k8s-rebase/scripts/k8s-rebase-validate.sh lines 125-131 use a predictable shared /tmp/jq path. In both scripts, create a unique temporary directory with mktemp -d, download and verify jq within that directory, make that file executable, and prepend only the temporary directory to PATH.plugins/k8s-rebase/scripts/k8s-rebase-review.sh (1)
30-59: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEvidence-gathering (
PATTERN_HINT,K8S_CHANGELOG, commit-scopedDIFF) is still unreachable in practice.SKILL.md's Step 4c still calls this script once at final
HEADwith the literal string"k8s rebase"asORIGINAL_ERROR— that string never matches any of thePATTERN_HINTkeywords (L53),K8S_CHANGELOG(L48) is just whatever the last commit message says (not a real changelog), andDIFF(L40-42) covers the entire rebase range truncated to 2000 lines rather than one commit's diff. The antagonistic review still runs with much weaker context than the script's own design intends.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-review.sh` around lines 30 - 59, Update the Step 4c invocation and the evidence preparation around PATTERN_HINT, K8S_CHANGELOG, and DIFF so the review runs per rebased commit with that commit’s actual error output, commit message, and scoped diff rather than once at final HEAD with the literal “k8s rebase” value. Preserve the existing filtering and fallback behavior while ensuring each invocation receives commit-specific context before the review runs.plugins/k8s-rebase/scripts/k8s-rebase-validate.sh (1)
277-295: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUnanchored root_pkgs regex duplicated between the script and its documentation snippet — same substring-match root cause. Both build a
(pkg/a|pkg/b|...)alternation fromroot_pkgsand match it with only a leading^\./anchor, so a shorter root_pkgs entry (e.g.pkg/ovn) substring-matches an unrelated package (pkg/ovnwebhook) and silently drops it from testing. The correct anchored form already exists elsewhere ink8s-rebase-validate.sh(L439, L449:^(...)$).
plugins/k8s-rebase/scripts/k8s-rebase-validate.sh#L277-L295: add the trailing$anchor to thegrep -qEpattern inrun_test_only(), matching the anchored form used at L439/L449.plugins/k8s-rebase/skills/k8s-rebase/SKILL.md#L426-L436: add the same trailing$anchor to the Step 4b package-discovery snippet'sgrep -qEpattern.🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh` around lines 277 - 295, Anchor the root package regex to the full package name by adding a trailing $ to the grep -qE pattern in run_test_only() within plugins/k8s-rebase/scripts/k8s-rebase-validate.sh lines 277-295. Apply the same trailing anchor to the corresponding Step 4b package-discovery snippet in plugins/k8s-rebase/skills/k8s-rebase/SKILL.md lines 426-436; both sites must reject only exact root package paths.
🧹 Nitpick comments (2)
plugins/k8s-rebase/scripts/k8s-rebase.sh (1)
251-263: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueQuote/array-ify the container
execargs instead of relying on unquoted command substitution.
$([[ "$BUMP_TOOLS" == true ]] && echo "--bump-tools")at Line 263 is unquoted, which both ast-grep (flagged as CWE-78-style pattern) and shellcheck (SC2046) call out. In practiceBUMP_TOOLSis fully internal (only evertrue/falsefrom the script's own arg parser), so this isn't currently exploitable, but the pattern is fragile and worth hardening to an explicit args array for clarity and defense against future refactors that might route less-trusted input through this path.🛡️ Proposed fix using an args array
+ EXTRA_ARGS=() + [[ "$BUMP_TOOLS" == true ]] && EXTRA_ARGS+=("--bump-tools") exec $CONTAINER_RT run --rm \ --security-opt label=disable \ $USERNS_FLAG \ -v "$REPO_ROOT:$REPO_ROOT" \ -v "$(dirname "$SCRIPT_PATH"):$(dirname "$SCRIPT_PATH"):ro" \ -w "$REPO_ROOT" \ -e GIT_AUTHOR_NAME="$(git config user.name)" \ -e GIT_AUTHOR_EMAIL="$(git config user.email)" \ -e GIT_COMMITTER_NAME="$(git config user.name)" \ -e GIT_COMMITTER_EMAIL="$(git config user.email)" \ -e K8S_REBASE_IN_CONTAINER=1 \ "$GO_IMAGE" \ - bash "$SCRIPT_PATH" $([[ "$BUMP_TOOLS" == true ]] && echo "--bump-tools") "$VERSION_INPUT" + bash "$SCRIPT_PATH" "${EXTRA_ARGS[@]}" "$VERSION_INPUT"🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase.sh` around lines 251 - 263, Replace the unquoted conditional command substitution in the container execution command with an explicit argument array, adding --bump-tools only when BUMP_TOOLS is true and then expanding the array safely. Update the exec invocation in the container run block while preserving the existing script, image, and version arguments.plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh (1)
172-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGate-parent/dep computation duplicated 3× (
run_checkstwice,fix_feature_gatesonce).
_all_gate_names(L174-183) and_sfm_gates(L204-210) compute the identical set — activeGATE_DEPSparents plus their vendor-present deps — twice withinrun_checks(), andfix_feature_gates()(L1112-1125) recomputes it a third time with arrays instead of strings. Any future change to how gates/deps are filtered risks drifting between the three copies.♻️ Suggested direction
Extract one helper (e.g.
_compute_active_gates) that echoes space-separated gate names, and reuse it for_all_gate_names,_sfm_gates, andfix_feature_gates'sall_gates(viaread -rainto an array where an array is needed).🤖 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 `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh` around lines 172 - 217, Extract the duplicated active gate/ dependency discovery into a single helper, such as _compute_active_gates, that returns the vendor-present parent and dependency names in a space-separated form. Update run_checks to reuse this result for both _all_gate_names and _sfm_gates, and update fix_feature_gates to populate its all_gates array from the helper output via read -ra, removing its separate computation while preserving existing filtering behavior.
🤖 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 `@plugins/k8s-rebase/docs/k8s-rebase-patterns.md`:
- Line 154: Add language identifiers to every fenced code block in
k8s-rebase-patterns.md, including the blocks at the referenced locations. Use
text for diagnostic/output blocks and the appropriate language identifier for
each code or configuration block, eliminating all MD040 violations.
- Around line 459-470: Update the replace directive workaround guidance near the
example to require a trusted, reviewed fork and an immutable pinned version or
commit reference instead of the generic FORK placeholder. Retain the instruction
to remove the replace directive once the official library-go fix is available.
In `@plugins/k8s-rebase/gates/step3-autofix/autofix-result.md`:
- Around line 1-9: Update the autofix verification guidance to treat matching
git-log commit counts only as supporting evidence, not proof of execution.
Require corroborating diff, result, or script-output evidence so no-op and
squashed/combined successful runs are recognized, while preserving the
instruction to ignore stale gitignored vendor directories and avoid escalating
them.
In `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md`:
- Around line 8-11: Replace the brittle main/master merge-base fallback with
resolved default/target branch detection, mirroring the existing detection in
k8s-rebase.sh before comparing against the base. In
plugins/k8s-rebase/gates/step3-autofix/crd-validation.md lines 8-11, use the
resolved base for git show metadata.name comparisons; in
plugins/k8s-rebase/gates/step4-verification/go-version-check.md lines 4-9, reuse
the same resolved base for both go and toolchain directive git diff checks,
ensuring unresolved bases fail rather than silently producing zero differences.
In `@plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md`:
- Line 11: Update the subshell command in the build-and-vet verification step to
exit when changing to mod_dir fails, then run go build and go vet while
capturing each command’s status and return a failure if either command fails.
Preserve both commands’ output and ensure the combined status reflects both
checks.
In `@plugins/k8s-rebase/skills/k8s-rebase/SKILL.md`:
- Around line 1-9: Add an overlap-review summary to the k8s-rebase skill
documentation, covering the four-phase comparison against open PRs and existing
skills/commands. Include the required HIGH/MODERATE/LOW classification and
retain the existing front matter and skill instructions unchanged.
---
Duplicate comments:
In `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md`:
- Around line 6-7: Update the CRD validation instructions in step 3 so the count
identifies schema objects containing both format: int32 and maximum: 4294967295
regardless of key order, comments, blank lines, or intervening properties;
remove the line-adjacency requirement and preserve exact reporting for the
read-only gate.
In `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh`:
- Around line 122-128: The jq bootstrap blocks in
plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh lines 122-128 and
plugins/k8s-rebase/scripts/k8s-rebase-validate.sh lines 125-131 use a
predictable shared /tmp/jq path. In both scripts, create a unique temporary
directory with mktemp -d, download and verify jq within that directory, make
that file executable, and prepend only the temporary directory to PATH.
In `@plugins/k8s-rebase/scripts/k8s-rebase-review.sh`:
- Around line 30-59: Update the Step 4c invocation and the evidence preparation
around PATTERN_HINT, K8S_CHANGELOG, and DIFF so the review runs per rebased
commit with that commit’s actual error output, commit message, and scoped diff
rather than once at final HEAD with the literal “k8s rebase” value. Preserve the
existing filtering and fallback behavior while ensuring each invocation receives
commit-specific context before the review runs.
In `@plugins/k8s-rebase/scripts/k8s-rebase-validate.sh`:
- Around line 277-295: Anchor the root package regex to the full package name by
adding a trailing $ to the grep -qE pattern in run_test_only() within
plugins/k8s-rebase/scripts/k8s-rebase-validate.sh lines 277-295. Apply the same
trailing anchor to the corresponding Step 4b package-discovery snippet in
plugins/k8s-rebase/skills/k8s-rebase/SKILL.md lines 426-436; both sites must
reject only exact root package paths.
In `@plugins/k8s-rebase/scripts/k8s-rebase.sh`:
- Around line 686-695: Restrict the bare-version replacement in the Pass 2 loop
to Kubernetes-specific documentation instead of matching every standalone
numeric token under docs/. Update the grep/perl selection around OLD_SHORT and
NEW_SHORT to require an appropriate Kubernetes context, while preserving the
existing protections for IP addresses, compound versions, and patch versions.
---
Nitpick comments:
In `@plugins/k8s-rebase/scripts/k8s-rebase-autofix.sh`:
- Around line 172-217: Extract the duplicated active gate/ dependency discovery
into a single helper, such as _compute_active_gates, that returns the
vendor-present parent and dependency names in a space-separated form. Update
run_checks to reuse this result for both _all_gate_names and _sfm_gates, and
update fix_feature_gates to populate its all_gates array from the helper output
via read -ra, removing its separate computation while preserving existing
filtering behavior.
In `@plugins/k8s-rebase/scripts/k8s-rebase.sh`:
- Around line 251-263: Replace the unquoted conditional command substitution in
the container execution command with an explicit argument array, adding
--bump-tools only when BUMP_TOOLS is true and then expanding the array safely.
Update the exec invocation in the container run block while preserving the
existing script, image, and version arguments.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8bf0d47c-2dbc-44f3-bbac-534fbd62c92a
📒 Files selected for processing (42)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/k8s-rebase/.claude-plugin/plugin.jsonplugins/k8s-rebase/OWNERSplugins/k8s-rebase/README.mdplugins/k8s-rebase/docs/k8s-rebase-patterns.mdplugins/k8s-rebase/gates/step1-rebase/rebase-completeness.mdplugins/k8s-rebase/gates/step2-compilation/build-vet.mdplugins/k8s-rebase/gates/step2-compilation/diff-scope.mdplugins/k8s-rebase/gates/step2-compilation/fix-correctness.mdplugins/k8s-rebase/gates/step2-compilation/type-conversions.mdplugins/k8s-rebase/gates/step2-compilation/version-consistency.mdplugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.mdplugins/k8s-rebase/gates/step3-autofix/autofix-result.mdplugins/k8s-rebase/gates/step3-autofix/crd-validation.mdplugins/k8s-rebase/gates/step3-autofix/dep-release-notes.mdplugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.mdplugins/k8s-rebase/gates/step3-autofix/e2e-infra.mdplugins/k8s-rebase/gates/step3-autofix/feature-gates.mdplugins/k8s-rebase/gates/step3-autofix/logical-completeness.mdplugins/k8s-rebase/gates/step3-autofix/patterns-completeness.mdplugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdplugins/k8s-rebase/gates/step4-verification/ci-prediction.mdplugins/k8s-rebase/gates/step4-verification/ci-readiness.mdplugins/k8s-rebase/gates/step4-verification/cleanliness.mdplugins/k8s-rebase/gates/step4-verification/commit-messages.mdplugins/k8s-rebase/gates/step4-verification/correctness.mdplugins/k8s-rebase/gates/step4-verification/dep-cve-check.mdplugins/k8s-rebase/gates/step4-verification/deprecated-imports.mdplugins/k8s-rebase/gates/step4-verification/go-version-check.mdplugins/k8s-rebase/gates/step4-verification/gomod-diff-analysis.mdplugins/k8s-rebase/gates/step4-verification/k8s-changelog.mdplugins/k8s-rebase/gates/step4-verification/logical-consistency.mdplugins/k8s-rebase/gates/step4-verification/maintainer-review.mdplugins/k8s-rebase/gates/step4-verification/skill-improvement.mdplugins/k8s-rebase/gates/step4-verification/version-completeness.mdplugins/k8s-rebase/scripts/k8s-rebase-autofix.shplugins/k8s-rebase/scripts/k8s-rebase-review-prompt.mdplugins/k8s-rebase/scripts/k8s-rebase-review.shplugins/k8s-rebase/scripts/k8s-rebase-validate.shplugins/k8s-rebase/scripts/k8s-rebase.shplugins/k8s-rebase/skills/k8s-rebase/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (26)
- plugins/k8s-rebase/gates/step2-compilation/fix-correctness.md
- plugins/k8s-rebase/gates/step4-verification/cleanliness.md
- plugins/k8s-rebase/gates/step3-autofix/autofix-diff-review.md
- plugins/k8s-rebase/scripts/k8s-rebase-review-prompt.md
- plugins/k8s-rebase/gates/step3-autofix/patterns-completeness.md
- plugins/k8s-rebase/.claude-plugin/plugin.json
- plugins/k8s-rebase/gates/step1-rebase/rebase-completeness.md
- plugins/k8s-rebase/gates/step3-autofix/logical-completeness.md
- plugins/k8s-rebase/gates/step2-compilation/type-conversions.md
- plugins/k8s-rebase/gates/step3-autofix/deprecated-api-remnants.md
- plugins/k8s-rebase/gates/step3-autofix/e2e-infra.md
- .claude-plugin/marketplace.json
- plugins/k8s-rebase/gates/step3-autofix/feature-gates.md
- plugins/k8s-rebase/gates/step3-autofix/dep-release-notes.md
- plugins/k8s-rebase/gates/step4-verification/logical-consistency.md
- plugins/k8s-rebase/gates/step2-compilation/version-consistency.md
- plugins/k8s-rebase/gates/step4-verification/maintainer-review.md
- plugins/k8s-rebase/gates/step4-verification/commit-messages.md
- plugins/k8s-rebase/gates/step4-verification/dep-cve-check.md
- plugins/k8s-rebase/README.md
- plugins/k8s-rebase/gates/step4-verification/skill-improvement.md
- plugins/k8s-rebase/OWNERS
- plugins/k8s-rebase/gates/step4-verification/ci-readiness.md
- plugins/k8s-rebase/gates/step4-verification/k8s-changelog.md
- plugins/k8s-rebase/gates/step4-verification/deprecated-imports.md
- docs/index.html
| The v0.2.0 conformance suite also expects `ClusterNetworkPolicy` | ||
| resources in `v1alpha2` API version. If the project only installs | ||
| `v1alpha1` CRDs, conformance tests fail with: | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to all fenced blocks.
markdownlint reports six MD040 violations. Mark diagnostic blocks as text and code/configuration blocks with their appropriate language to keep documentation checks green.
Also applies to: 266-266, 270-270, 294-294, 329-329, 463-463
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@plugins/k8s-rebase/docs/k8s-rebase-patterns.md` at line 154, Add language
identifiers to every fenced code block in k8s-rebase-patterns.md, including the
blocks at the referenced locations. Use text for diagnostic/output blocks and
the appropriate language identifier for each code or configuration block,
eliminating all MD040 violations.
Source: Linters/SAST tools
| **Replace directive workaround:** If library-go hasn't merged | ||
| its k8s bump yet, use a `replace` directive in go.mod pointing | ||
| to a fork that has the fix: | ||
|
|
||
| ``` | ||
| replace github.com/openshift/library-go => github.com/FORK/library-go v0.0.0-DATE-HASH | ||
| ``` | ||
|
|
||
| This is the standard approach used by manual rebases (e.g., | ||
| CNO PR #3017 uses `jubittajohn/library-go`). The `replace` | ||
| survives `go mod vendor` because it directs the module system | ||
| to fetch from the fork. Remove the `replace` when the official |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require trusted, immutable fork references for the replace workaround.
A generic github.com/FORK/library-go replacement changes dependency provenance. Document that the fork must be trusted, reviewed, pinned to an immutable version, and removed once the upstream fix lands.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 463-463: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@plugins/k8s-rebase/docs/k8s-rebase-patterns.md` around lines 459 - 470,
Update the replace directive workaround guidance near the example to require a
trusted, reviewed fork and an immutable pinned version or commit reference
instead of the generic FORK placeholder. Retain the instruction to remove the
replace directive once the official library-go fix is available.
| Check the autofix results by examining git log for the autofix | ||
| commits. Look for commits matching patterns like "Migrate x/exp | ||
| imports", "Disable new default-true feature gates", "Update KIND | ||
| image", "Update version references and lint". Count how many | ||
| autofix commits were created. If zero, the autofix may not have | ||
| run. Ignore stale vendor in gitignored directories | ||
| (`git check-ignore -q <dir>/vendor`) — these are expected and | ||
| not maintained by the rebase. Do NOT escalate gitignored vendor | ||
| staleness as a blocker. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not infer autofix execution from commit-message patterns alone.
A no-op run or squashed/combined commit can produce zero matching commits even when autofix completed successfully. Require corroborating diff, result, or script-output evidence; treat the git-log count as supporting evidence only.
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: Ensure spelling is correct
Context: ...intained by the rebase. Do NOT escalate gitignored vendor staleness as a blocker. Report ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_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 `@plugins/k8s-rebase/gates/step3-autofix/autofix-result.md` around lines 1 - 9,
Update the autofix verification guidance to treat matching git-log commit counts
only as supporting evidence, not proof of execution. Require corroborating diff,
result, or script-output evidence so no-op and squashed/combined successful runs
are recognized, while preserving the instruction to ignore stale gitignored
vendor directories and avoid escalating them.
| 2. Count CRDs that lost metadata.name pattern validation | ||
| compared to the base branch. Detect the base branch with | ||
| `git merge-base HEAD main 2>/dev/null || git merge-base HEAD master`, | ||
| then use `git show <base>:path` to check the original. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Same brittle main/master merge-base assumption repeated across gate prompts. Both gates resolve the comparison base with git merge-base HEAD main 2>/dev/null || git merge-base HEAD master; when the default branch is neither, the substitution is empty and git diff $(...)..HEAD degenerates to git diff ..HEAD (interpreted as HEAD..HEAD), silently reporting zero differences instead of erroring — the opposite of what a "read-only, must report exact counts" gate should do on an unresolvable base.
plugins/k8s-rebase/gates/step3-autofix/crd-validation.md#L8-L11: derive the actual default/target branch (e.g. viagit symbolic-ref refs/remotes/origin/HEAD, mirroring the detection already done ink8s-rebase.shat Line 300) before usinggit show <base>:pathfor the metadata.name comparison.plugins/k8s-rebase/gates/step4-verification/go-version-check.md#L4-L9: apply the same resolved-base-ref approach to both thego directiveandtoolchain directivegit diffinvocations instead of hardcodingmain/master.
📍 Affects 2 files
plugins/k8s-rebase/gates/step3-autofix/crd-validation.md#L8-L11(this comment)plugins/k8s-rebase/gates/step4-verification/go-version-check.md#L4-L9
🤖 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 `@plugins/k8s-rebase/gates/step3-autofix/crd-validation.md` around lines 8 -
11, Replace the brittle main/master merge-base fallback with resolved
default/target branch detection, mirroring the existing detection in
k8s-rebase.sh before comparing against the base. In
plugins/k8s-rebase/gates/step3-autofix/crd-validation.md lines 8-11, use the
resolved base for git show metadata.name comparisons; in
plugins/k8s-rebase/gates/step4-verification/go-version-check.md lines 4-9, reuse
the same resolved base for both go and toolchain directive git diff checks,
ensuring unresolved bases fail rather than silently producing zero differences.
| continue | ||
| fi | ||
| echo "CHECK $mod_dir" | ||
| (cd "$mod_dir" && go build ./... 2>&1; go vet ./... 2>&1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
sed -n '1,120p' plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdRepository: openshift-eng/ai-helpers
Length of output: 1100
🏁 Script executed:
sed -n '1,80p' plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.mdRepository: openshift-eng/ai-helpers
Length of output: 1100
Preserve both exit codes in the subshell.
(cd "$mod_dir" && go build ./... 2>&1; go vet ./... 2>&1) still returns only go vet’s status, so a failed build can be reported as success. go vet also runs from the caller’s directory if cd fails. Guard the directory change and combine both exit codes before returning.
Suggested adjustment
- (cd "$mod_dir" && go build ./... 2>&1; go vet ./... 2>&1)
+ (
+ cd "$mod_dir" || exit 1
+ build_rc=0
+ go build ./... 2>&1 || build_rc=$?
+ vet_rc=0
+ go vet ./... 2>&1 || vet_rc=$?
+ (( build_rc == 0 && vet_rc == 0 ))
+ )📝 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.
| (cd "$mod_dir" && go build ./... 2>&1; go vet ./... 2>&1) | |
| ( | |
| cd "$mod_dir" || exit 1 | |
| build_rc=0 | |
| go build ./... 2>&1 || build_rc=$? | |
| vet_rc=0 | |
| go vet ./... 2>&1 || vet_rc=$? | |
| (( build_rc == 0 && vet_rc == 0 )) | |
| ) |
🤖 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 `@plugins/k8s-rebase/gates/step4-verification/build-vet-recheck.md` at line 11,
Update the subshell command in the build-and-vet verification step to exit when
changing to mod_dir fails, then run go build and go vet while capturing each
command’s status and return a failure if either command fails. Preserve both
commands’ output and ensure the combined status reflects both checks.
| --- | ||
| name: k8s-rebase | ||
| description: Rebase a Go project to a new Kubernetes version by bumping all k8s.io/* dependencies, running codegen, updating version references, fixing build breakage with antagonistic review, and presenting a gh pr create command. | ||
| argument-hint: "[--bump-tools] <version> (e.g., 1.36.0 or --bump-tools 1.36.0)" | ||
| user-invocable: true | ||
| allowed-tools: Bash, Read, Agent | ||
| --- | ||
|
|
||
| # Kubernetes Rebase |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
gh pr list --repo openshift-eng/ai-helpers --state open --json number,title,files \
--jq '.[] | select(.files[].path | test("k8s-rebase|SKILL\\.md")) | {number, title}'Repository: openshift-eng/ai-helpers
Length of output: 6075
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Repo files ==\n'
git ls-files 'plugins/*/skills/*/SKILL.md' 'plugins/*/commands/*.md' | sed -n '1,200p'
printf '\n== Candidate related files ==\n'
git ls-files 'plugins/rebase-library-go/skills/*/SKILL.md' 'plugins/k8s-rebase/skills/*/SKILL.md' | sed -n '1,200p'
printf '\n== Outline k8s-rebase skill ==\n'
ast-grep outline plugins/k8s-rebase/skills/k8s-rebase/SKILL.md --view expanded || true
printf '\n== Outline rebase-library-go skill ==\n'
ast-grep outline plugins/rebase-library-go/skills/rebase-library-go/SKILL.md --view expanded || trueRepository: openshift-eng/ai-helpers
Length of output: 9494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== PR 617 metadata ==\n'
gh pr view 617 --repo openshift-eng/ai-helpers --json number,title,body,files,comments,reviews \
--jq '{number,title,body,files:[.files[].path],comments:[.comments[].body],reviews:[.reviews[].body]}'Repository: openshift-eng/ai-helpers
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## related files"
git ls-files 'plugins/*/skills/*/SKILL.md' | sed -n '1,120p'
echo "## k8s-rebase skill"
cat -n plugins/k8s-rebase/skills/k8s-rebase/SKILL.md | sed -n '1,120p'
echo "## rebase-library-go skill"
cat -n plugins/rebase-library-go/skills/rebase-library-go/SKILL.md | sed -n '1,220p'Repository: openshift-eng/ai-helpers
Length of output: 11672
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## rebase-related files"
git ls-files 'plugins/*/*rebase*' 'plugins/*/commands/*rebase*.md' 'plugins/*/skills/*rebase*/SKILL.md' | sed -n '1,200p'
echo
echo "## search for k8s rebase references"
rg -n --glob 'plugins/**' -S 'k8s rebase|k8s-rebase|k8s\.io/\*|dependency rebase|rebase a Go project|bump-deps|rebase' plugins | sed -n '1,240p'Repository: openshift-eng/ai-helpers
Length of output: 29890
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in plugins/openshift/commands/rebase.md plugins/openshift/commands/bump-deps.md plugins/openshift/commands/create-cluster.md; do
if [ -f "$f" ]; then
echo "## $f"
cat -n "$f" | sed -n '1,220p'
echo
fi
doneRepository: openshift-eng/ai-helpers
Length of output: 25755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
gh pr view 617 --repo openshift-eng/ai-helpers --json files --jq '.files[].path' | sed -n '1,80p'Repository: openshift-eng/ai-helpers
Length of output: 2500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## rebase-related files"
git ls-files 'plugins/*/*rebase*' 'plugins/*/commands/*rebase*.md' 'plugins/*/skills/*rebase*/SKILL.md' | sed -n '1,200p'
echo
echo "## k8s-rebase PR files"
gh pr view 617 --repo openshift-eng/ai-helpers --json files --jq '.files[].path' | sed -n '1,120p'
echo
echo "## search for overlap-relevant wording"
rg -n -S 'overlap|related PR|possibly related|similar|duplicate|rebase|k8s\.io/\*|dependency rebase|bump-deps' \
plugins/k8s-rebase plugins/openshift plugins/ci plugins/code-review plugins/golang | sed -n '1,260p'Repository: openshift-eng/ai-helpers
Length of output: 39182
Add the overlap-review summary for this new skill. PR #617 already notes related PRs, but this file still needs the 4-phase comparison against open PRs and existing skills/commands, plus the HIGH/MODERATE/LOW classification.
🤖 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 `@plugins/k8s-rebase/skills/k8s-rebase/SKILL.md` around lines 1 - 9, Add an
overlap-review summary to the k8s-rebase skill documentation, covering the
four-phase comparison against open PRs and existing skills/commands. Include the
required HIGH/MODERATE/LOW classification and retain the existing front matter
and skill instructions unchanged.
Source: Coding guidelines
2 participants
Automates k8s.io/* dependency rebases for Go projects. Handles dep bumps, codegen, version references, compilation fixes, lint migration, feature gate management, and antagonistic review.
The skill improves itself across runs: when a rebase surfaces a new breakage pattern (via gates, local validation, or CI), the agent fixes it and documents the pattern. The next repo or k8s version benefits from that pattern automatically — through autofix functions, gate checks, or patterns doc entries.
Scripts:
29 gate subagents across 4 verification steps verify the rebase before the agent proceeds. Count gates enforce hard constraints (build errors, version mismatches, test skips). Judge gates review diffs for correctness. Discovery gates read external signals (dep release notes, k8s changelog, CVE databases, go.mod risk classification) to catch issues that build/lint/test cannot detect.
Patterns doc catalogs 30+ breakage patterns with fix recipes and extension guide for future k8s versions.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
k8s-rebasemarketplace plugin (v0.1.0) to automate Kubernetesk8s.io/*dependency rebases for Go repositories, including deterministic steps, code generation updates, repo reference syncing, validation, and fix review.