Skip to content

ci: drop the cert-manager Kubernetes support-window gate - #458

Merged
shrinedogg merged 1 commit into
polarsquad:mainfrom
lmilbaum:ci/drop-cert-manager-k8s-support-gate
Oct 14, 2026
Merged

shrinedogg merged 1 commit into
polarsquad:mainfrom
lmilbaum:ci/drop-cert-manager-k8s-support-gate

Conversation

@lmilbaum

@lmilbaum lmilbaum commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Why

The gate was built on a misdiagnosis of #322. The failure was a chart/image version mismatch (1.21.1 chart, 1.21.2 images), fixed by #332 and guarded by tests/test-cert-manager-version-consistency.py, which stays. It was not cert-manager 1.21 running on Kubernetes v1.37. docs/dependencies.md already records that correction.

The scheduled air-gapped deploy then passed on 2026-09-21 with cert-manager 1.21.2 on Kubernetes v1.37.0, the exact pairing this gate rejects. So the gate was red on every PR touching bootstrap.toml or the local-host cluster.yaml (e.g. #377, merged through it), with no end date until cert-manager publishes a window that includes 1.37.

Test plan

  • No remaining references to the removed test (git grep).
  • mise run validate: all kustomize and pin checks pass. The only local failure is test-airgap-failure-notification.py missing PyYAML on the host, which this change doesn't touch.
  • CI green, with the Zarf mise asset pin job passing on a shallow checkout.

Fixes #384

Also adds a non-blocking replacement test that warns when the cert-manager/Kubernetes pairing hasn't been empirically validated.

🤖 Generated with Claude Code

@lmilbaum lmilbaum self-assigned this Sep 23, 2026
@lmilbaum
lmilbaum marked this pull request as ready for review September 23, 2026 12:53
@lmilbaum
lmilbaum requested a review from shrinedogg September 23, 2026 12:54

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: clean, no blocking findings. One non-blocking suggestion inline.

Verified locally:

  • Diff scope against fresh origin/main is exactly the four files (validate.yml, docs/dependencies.md, mise.toml, the rewritten test). No dangling references to the removed gate remain: the only --changed-from left in the repo is the air-gap image-digests test, which keeps its own fetch-depth: 0, untouched by this PR.
  • Dropping fetch-depth: 0 from the zarf-mise-pin job is safe: tests/test-mise-zarf-pin.py reads mise.toml and runs mise x -- zarf version; it uses no git history. The CI job of the same name passed on the shallow checkout at this head.
  • Evidence row in VALIDATED_PAIRINGS checks out: Actions run 35554802424 is the air-gapped workflow, event schedule, conclusion success, 2026-09-21. At the commit main pointed to at that time (d9582b8), bootstrap.toml pinned cert-manager 1.21.2 and mgmt/local-host/clusters/docker/cluster.yaml pinned v1.37.0, the exact pairing the row records.
  • Ran the new test with mise's Python 3.14: the current pairing exits 0 with the validated NOTICE. Failure-path probes in a sandbox copy: an unvalidated pairing exits 0 with the untested NOTICE (plus the ::notice annotation under GITHUB_ACTIONS=true); a missing charts.cert-manager pin, a missing cluster.yaml, and an unparseable k8s version line all exit 1.
  • mise exec -- ruff check and ruff format --check pass on the test; tests/test-cert-manager-version-consistency.py and tests/test-bootstrap-config.py pass. The rewritten docs/dependencies.md passage matches the implementation (never fails except pin-parse errors, VALIDATED_PAIRINGS procedure). The retained #322 passage describes the surviving version-consistency test, so docs describe the new behavior, not the old.
  • CI on head d446fb3: 10 checks success; Rendered Flux diff skipped (fork PR, by design).

Looks good:

  • The gate's removal is justified by the evidence above: the pairing it rejects passed the scheduled nightly, and the failure it was built on (chart/image drift, #322) is covered by the surviving version-consistency test.
  • The new non-blocking signal keeps visibility without blocking Renovate PRs, and mise.toml's validate task mirrors the CI step with the same fail-only-on-parse-error semantics.

Comment thread .github/workflows/validate.yml Resolved
@shrinedogg

Copy link
Copy Markdown
Contributor

Fix branch: polarsquad/krops:fix/458-gate-name @ 4e3e8cc

Addresses the review finding and resolves the conflict with origin/main.

Finding addressed: step rename

cert-manager/Kubernetes pairing is empirically validated (non-blocking, #384)
-> cert-manager/Kubernetes pairing notice (non-blocking, #384)

The rename is accurate against tests/test-cert-manager-kubernetes-support.py:
the untested-pairing branch (no VALIDATED_PAIRINGS match) prints
NOTICE: cert-manager X has not been empirically tested with Kubernetes vY. Verify compatibility before merging. and returns 0, so the step exits 0 with
only a NOTICE when the pairing is NOT validated. The old name asserted a state
the step does not guarantee; the new name describes the signal. The step's
run: command is unchanged.

One correction beyond the rename: the literal name (non-blocking, #384)
truncates under YAML parsing. A # preceded by a space starts a comment, so
pyyaml reads the name as cert-manager/Kubernetes pairing notice (non-blocking, (this also affects the original name in the PR head). The
name is now double-quoted in validate.yml so the full text, including #384,
is what GitHub renders.

Conflict resolution

origin/main (bcd0a56) merged into the PR
head (merge commit 26502eb, subject "merge origin/main into
ci/drop-cert-manager-k8s-support-gate"). Only .github/workflows/validate.yml
conflicted, and it was resolved by keeping both sides:

  • main's side kept: the airgap-image-existence job that main MOVED out of
    validate.yml into the new .github/workflows/airgap-image-existence.yml
    (so the old inline job in the PR head is gone), the permissions: contents: read block, the tool pin bumps (mise 2026.9.16, uv 0.12.20,
    renovate 44.115.13), and the new steps added after the merge base
    ("Verify every Renovate replacement template round-trips its match",
    "shell REGION_PLAN vs bootstrap.toml", the toolbox-run.sh parity step,
    "AWS run-tag standard (e2e: run/revision/expiry tagging standard for AWS test resources #381)").
  • the PR's side kept: the removal of fetch-depth: 0 from the zarf-mise-pin
    checkout, the removal of the old blocking gate step "Verify cert-manager
    supports the pinned Kubernetes version (air-gapped: scheduled workflow is failing #322)" with its
    --changed-from invocation, and the new non-blocking pairing step (now
    renamed and quoted as above).

The final validate.yml diff against origin/main is exactly the PR's intent
(those three hunks) and nothing else.

How to apply (author commands, fork remote assumed to be origin):

git fetch https://github.com/polarsquad/krops.git fix/458-gate-name
git checkout -B ci/drop-cert-manager-k8s-support-gate FETCH_HEAD
git push origin ci/drop-cert-manager-k8s-support-gate

Verified locally

  • uv run --locked python -c "import yaml; list(yaml.safe_load_all(open('.github/workflows/validate.yml')))"
    -> parses OK (1 doc). Run via the repo's uv-managed Python 3.14.7 because
    this host's system python3 lacks pyyaml and tomllib (CI runners are
    Python 3.12+, so the bare python3 invocation in the workflow is
    unaffected).
  • Same environment, structural check of the step: name resolves to the full
    quoted string cert-manager/Kubernetes pairing notice (non-blocking, #384)
    and run: is python3 tests/test-cert-manager-kubernetes-support.py -> OK.
    A probe of the unquoted form confirms the old name would have parsed as
    ... (non-blocking, (comment truncation).
  • Diff of the rename commit (26502eb -> 4e3e8cc) on validate.yml -> only the
    step's name: line changed; run: identical.
  • python3 tests/test-cert-manager-kubernetes-support.py (the workflow's
    exact invocation, via the uv-managed interpreter) -> exit 0, prints
    NOTICE: cert-manager 1.21.2 with Kubernetes v1.37.0 is empirically validated (air-gapped nightly run 35554802424, 2026-09-21).
  • Untested-pairing branch (VALIDATED_PAIRINGS emptied, same interpreter) ->
    exit 0, prints NOTICE: cert-manager 1.21.2 has not been empirically tested with Kubernetes v1.37.0. Verify compatibility before merging.,
    confirming the rename describes the signal rather than asserting it.

mise run validate was not run locally (the host's mise self-version pin
2026.9.16 is not installed here); the workflow file's correctness is covered
by the parse and structural checks above, and CI will run the full job.

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving the fix. Verified the fix branch (polarsquad/krops:fix/458-gate-name @ 4e3e8cc) against the review finding and the conflict with main; no regressions found. This review is anchored to the PR head d446fb3 for thread resolution; the apply instructions live in the comment from 2026-09-28 and on the fix branch.

Finding 1 (step name overstated the signal): RESOLVED.

  • Merge commit 26502eb merges origin/main (bcd0a56) into the PR head and resolves the validate.yml conflict by keeping both sides. Fix commit 4e3e8cc on top renames the step to exactly "cert-manager/Kubernetes pairing notice (non-blocking, #384)" and touches nothing else (git show 4e3e8cc: 1 file changed, 1 insertion, 1 deletion).
  • Evidence for the rename target: parsing the fix branch's validate.yml with pyyaml, the step's name string equals "cert-manager/Kubernetes pairing notice (non-blocking, #384)" exactly, and the step's run command is unchanged (python3 tests/test-cert-manager-kubernetes-support.py).
  • Evidence the name is now accurate: I ran the script's untested branch directly (perturbed the Kubernetes pin to v9.99.9 in a scratch checkout of fixreview-458) and it printed "NOTICE: cert-manager 1.21.2 has not been empirically tested with Kubernetes v9.99.9. Verify compatibility before merging." and exited 0. The as-is pairing also exits 0 with a "is empirically validated" notice. The old name asserted a state the step does not guarantee; the new name describes the signal.
  • The implementer's extra fix is also real: an unquoted name 'cert-manager/Kubernetes pairing notice (non-blocking, #384)' truncates at '(non-blocking,' under YAML comment parsing (a # preceded by whitespace starts a comment), and the PR head's original name had the same flaw (parses to 'cert-manager/Kubernetes pairing is empirically validated (non-blocking,'). The double-quoted name parses to the full string.

Conflict resolution (validate.yml): VERIFIED.

  • git merge-tree --write-tree pr-458 origin/main reports validate.yml as the only conflicted path. The actual merge tree differs from the would-be auto-merge tree in exactly that one file, so no silent resolution leaked into other files.
  • Within validate.yml the resolution keeps both sides: main's side (the airgap-image-existence job extraction to .github/workflows/airgap-image-existence.yml, present on the fix branch; permissions: contents: read; mise 2026.9.16, uv 0.12.20, renovate 44.115.13 pin bumps; the new replacement-template, region-plan, toolbox-run parity, and AWS run-tag steps) and the PR's side (fetch-depth: 0 removal from the zarf-mise-pin checkout, removal of the old blocking gate step and its --changed-from invocation, the new non-blocking pairing step).
  • The PR's original intent is intact: git diff origin/main...fixreview-458 --stat matches the PR's own stat exactly (4 files, 62 insertions, 172 deletions), and the merge adds nothing unrelated.

No stray references: the only live references to the pairing check are the renamed step (validate.yml:185), the mise run validate wrapper (mise.toml:132, which labels it a non-blocking warning and only fails on pin-parse errors), and docs/dependencies.md:124 (accurately describes notice-not-fail behavior). No conflict markers in the tree; yamllint and all other CI checks pass on both the PR head and the fix branch.

Verified locally:

  • git merge-tree --write-tree pr-458 origin/main: validate.yml is the only conflict; merge tree diff vs auto-merge touches only validate.yml
  • git show 4e3e8cc: rename-only commit (1 file, 1+/1-), run command unchanged
  • pyyaml parse of fix branch validate.yml: step name equals "cert-manager/Kubernetes pairing notice (non-blocking, #384)" exactly
  • Ran tests/test-cert-manager-kubernetes-support.py at fixreview-458 via uv: as-is pins exit 0 with "is empirically validated" notice; perturbed k8s pin (v9.99.9) exits 0 with "has not been empirically tested" NOTICE (the previously untested branch)
  • Reproduced the YAML truncation: unquoted name parses to 'cert-manager/Kubernetes pairing notice (non-blocking,'; the PR-head name truncates the same way; the quoted fix parses to the full string
  • CI: all checks pass on PR head d446fb3 and on fix/458-gate-name @ 4e3e8cc; "Rendered Flux diff" is skipped by design for fork PRs (konflate.yml guards on head.repo.fork == false)

@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch 2 times, most recently from 3bd5b50 to 2be8d37 Compare September 30, 2026 05:30
@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch from 2be8d37 to 4793177 Compare October 3, 2026 03:42

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES. The gate removal itself is correct and well documented, but this rewrite resurrects a CI job that main deliberately moved out of validate.yml (#465), and it drops the step-name fix agreed in the previous review round.

Verified locally:

  • git diff origin/main...pr-458: 4 files, +71/-171. The content beyond the gate removal is the non-blocking pairing test, its docs passage, the mise validate wrapper task, and the airgap-image-existence job block (finding 1).
  • tests/test-cert-manager-kubernetes-support.py at head 4793177, run with the repo's mise python: current pins print "NOTICE: cert-manager 1.21.2 with Kubernetes v1.37.0 is empirically validated (air-gapped nightly run 35554802424, 2026-09-21)" and exit 0; a perturbed Kubernetes pin (v9.99.9) and a perturbed cert-manager pin (9.99.9) each print the "has not been empirically tested" NOTICE and exit 0; with the pin files missing it exits 1 (the documented parse-error-only failure path). py_compile clean.
  • Evidence row checked via the API: run 35554802424 is the air-gapped workflow, event schedule, conclusion success, 2026-09-21, and at that run's head sha (d9582b8) bootstrap.toml pins cert-manager 1.21.2 and mgmt/local-host/clusters/docker/cluster.yaml pins version v1.37.0.
  • The fetch-depth: 0 removal from the zarf-mise-pin checkout is safe: tests/test-mise-zarf-pin.py uses no git history (mise GitHub asset check only).
  • No dangling references to the removed gate: git grep across the PR tree finds no surviving --changed-from invocation, supported-releases URL, or support-window step. docs/dependencies.md:111 keeps the #322 chart/image mismatch history as retained context, and the new passage at docs/dependencies.md:118-124 accurately describes the notice-not-fail behavior.
  • pyyaml parse of head validate.yml (evidence for finding 2).
  • git merge-tree --write-tree origin/main pr-458 (tree 0c3fc64): merges clean, and the result contains BOTH .github/workflows/airgap-image-existence.yml and the validate.yml job (evidence for finding 1).
  • CI at head 4793177 (run 37094132323, 2026-10-03): all 10 checks pass; "Rendered Flux diff" is skipping by design for fork PRs.

Blocking findings:

  1. .github/workflows/validate.yml:68 (job airgap-image-existence, new lines 68-76). This adds the job to validate.yml while main already runs the same script through the path-filtered .github/workflows/airgap-image-existence.yml, which #465 (5cc45dd) created when it removed the job from validate.yml "to prevent unconditional Docker Hub rate-limit exposure on every PR". The merge tree keeps both, so after merge the 45-digest registry check runs unconditionally on every PR and push, regressing #465. The previous approved head had resolved this conflict the other way (main's side kept). Fix: delete the job block from this PR's validate.yml; the dedicated workflow covers it.

  2. .github/workflows/validate.yml:195. The step name is an unquoted scalar, so YAML ends it at the " #" and the workflow UI shows "cert-manager/Kubernetes pairing is empirically validated (non-blocking," (parsed with pyyaml on this tree). This is the same defect flagged in the 2026-09-28 review; fix/458-gate-name @ 4e3e8cc renamed and quoted the step, but this rewrite did not carry the fix. Fix: double-quote the name, for example "cert-manager/Kubernetes pairing notice (non-blocking, #384)", or drop the ", #384" suffix.

Looks good:

  • Removing the gate is the right call and the rationale is fully documented: the PR body's Why section and the new docs/dependencies.md passage record the #322 misdiagnosis, the #332 fix, the consistency check that stays, and the green 2026-09-21 air-gapped run on the exact pairing the gate rejected (Fixes #384).
  • The replacement test fails only on unparseable pins, exactly as its docstring and the docs passage claim, and the VALIDATED_PAIRINGS row cites a real, successful run whose head sha carries both pinned versions.
  • mise.toml's validate wrapper labels the step non-blocking and fails only on pin-parse errors, matching the test contract.

Supersedes my APPROVE at 3bd5b50: that approval was anchored to the pre-rewrite head d446fb3; the force-pushed history reviewed here reintroduced both issues above.

Comment thread .github/workflows/validate.yml Resolved
Comment thread .github/workflows/validate.yml Resolved
@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch 3 times, most recently from db73acd to 78fd54c Compare October 12, 2026 04:56

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Batch review (2026-10-12): the gate removal is complete and the non-blocking replacement is sound (verified by execution against five simulated pin sets; no dangling references to the removed gate; the fetch-depth removal is safe). Two findings block the auto-merge:

  1. .github/workflows/validate.yml:70-78 - the commit adds a blocking 'airgap-image-existence' job that the title and body never mention. It duplicates the existing path-filtered .github/workflows/airgap-image-existence.yml (identical job name and steps; that workflow is deliberately scoped to airgap/images.txt, the test, and itself). In validate.yml it would run on every PR and every push to main, pulling every pinned digest on each run. Drop the job from this PR (the dedicated workflow already covers it) or split it into its own PR with rationale.
  2. tests/test-cert-manager-kubernetes-support.py:4-5 and docs/dependencies.md:126-127 - both claim "the repo pins Kubernetes v1.37.0 in mgmt/local-host/clusters/docker/cluster.yaml", but at this head the pin is v1.36.4 (restored by #538; the v1.37.0 bump is #591, which lands after this PR). Consequence, demonstrated by running the test against the head's real pins: VALIDATED_PAIRINGS contains only (1.21.2, 1.37.0, ...), so until #591 lands every validate run (including main pushes) prints "NOTICE: cert-manager 1.21.2 has not been empirically tested with Kubernetes v1.36.4" plus a GHA ::notice annotation - flagging the in-window pairing (cert-manager 1.21's published window is 1.33-1.36) that #538 intentionally restored. Seed VALIDATED_PAIRINGS with the current (1.21.2, 1.36.4) pairing plus nightly evidence, or correct the docstring/docs claim.

Both addressed, this is approvable: the rationale is sound (#322 was a chart/image mismatch, still guarded by the kept consistency test; the 2026-09-21 air-gapped nightly empirically validated the 1.37.0 pairing; the future-pairing procedure is documented at docs/dependencies.md:131-135).

@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch 2 times, most recently from b87770c to d5508ad Compare October 13, 2026 06:08

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES on head d5508ad (the branch moved since my 2026-10-12 review of 78fd54c: rebased onto current main f190b55, plus commit d5508ad, which only renames WARNING to NOTICE). Both blocking findings from the previous round are still present and re-verified on this head.

Blocking findings:

  1. .github/workflows/validate.yml:70-78: the PR re-adds an unconditional 'airgap-image-existence' job to validate.yml while main already runs the same script through the path-filtered .github/workflows/airgap-image-existence.yml (paths: airgap/images.txt, the test, the workflow itself). Re-verified on this head: main's validate.yml has no such job, the dedicated workflow file exists on both main and this branch, and git merge-tree --write-tree origin/main d5508ad merges clean with the merged tree containing both. After merge, the 45-digest live-registry check runs on every PR and every push to main, regressing #465 (5cc45dd moved it out of validate.yml specifically to prevent unconditional Docker Hub rate-limit exposure on every PR). The title and body never mention this addition. Fix: drop the job block from this PR; the dedicated workflow already covers it.

  2. tests/test-cert-manager-kubernetes-support.py:4-5 and docs/dependencies.md:125-127 both state the repo pins Kubernetes v1.37.0, but at this head (and on main) the pin is v1.36.4, restored by #538; the v1.37.0 bump is #591, still open, and this PR does not touch the pin. Verified by running the test with the repo's mise Python against the head's real pins: with VALIDATED_PAIRINGS containing only ("1.21.2", "1.37.0", ...), every validate run until #591 lands prints "NOTICE: cert-manager 1.21.2 has not been empirically tested with Kubernetes v1.36.4" and emits a ::notice annotation, flagging the pairing inside cert-manager 1.21's published 1.33-1.36 window that #538 intentionally restored. Commit d5508ad renamed WARNING to NOTICE but did not address this. Fix: seed VALIDATED_PAIRINGS with the current (1.21.2, 1.36.4) pairing plus nightly evidence, or correct the docstring and docs claim to describe the actual pin.

Non-blocking:

  1. .github/workflows/validate.yml:202 (renovate-digest-pinning job): the step name is an unquoted scalar, so YAML ends it at the ' #': parsing this head's validate.yml with pyyaml yields 'cert-manager/Kubernetes pairing is empirically validated (non-blocking,'. The name also asserts a state the step does not guarantee, since it exits 0 with a NOTICE on unvalidated pairings. Same defect flagged on 2026-09-28 and in the 2026-10-06 round; the agreed fix (double-quoted "cert-manager/Kubernetes pairing notice (non-blocking, #384)") was dropped in the rewrite.

Blast radius (context for the maintainer, not part of the verdict): merging this removes the only CI gate that rejects cert-manager/Kubernetes pairings outside the upstream published window. PR #591 (kubernetes-version bump to 1.37) is currently blocked by this gate; once this merges, nothing automated blocks #591 on pairing grounds, and the replacement test would report (1.21.2, 1.37.0) as empirically validated from the seeded row. Accepting out-of-window pairings on the strength of the green 2026-09-21 nightly is a policy decision for the maintainer.

Verified locally (all on head d5508ad; merge-base with origin/main is current main f190b55):

  • Ran the rewritten test with the repo's mise Python against the head's real pins: exits 0 with the "has not been empirically tested with Kubernetes v1.36.4" NOTICE; under GITHUB_ACTIONS=true it additionally emits the ::notice annotation (evidence for finding 2).
  • pyyaml parse of this head's validate.yml: the pairing step name truncates at '(non-blocking,' (evidence for finding 3); the rest of the workflow parses clean.
  • git merge-tree --write-tree origin/main d5508ad: merges clean; merged tree contains both .github/workflows/airgap-image-existence.yml and validate.yml's airgap-image-existence job (evidence for finding 1).
  • Evidence row re-checked via the API: Actions run 35554802424 is the air-gapped workflow, event schedule, conclusion success, 2026-09-21, head d9582b8, where bootstrap.toml pins cert-manager 1.21.2 and mgmt/local-host/clusters/docker/cluster.yaml pins v1.37.0.
  • No dangling references to the removed gate: zero hits for SUPPORTED_RELEASES_URL, the cert-manager.io/docs/releases URL, or the upstream kubeVersion note across the tree; the only surviving --changed-from is the unrelated airgap image-digests test, whose job keeps its own fetch-depth: 0.
  • Dropping fetch-depth: 0 from the zarf-mise-pin checkout is safe: tests/test-mise-zarf-pin.py needs no git history, and the Zarf mise asset pin check passed at this head on the shallow checkout.
  • The mise.toml validate wrapper mirrors the CI step with the same contract: pairing mismatch exits 0 with a notice, only unparseable pins fail.
  • CI at this head: all checks pass; "Rendered Flux diff" skips by design for fork PRs.

Looks good:

  • Removing the blocking gate is the right call: the pairing it rejects passed the scheduled air-gapped deploy on 2026-09-21 (issue #384), and the original #322 failure was chart/image drift, still guarded by tests/test-cert-manager-version-consistency.py, which this PR keeps.
  • The replacement's fail-only-on-parse-errors contract is accurately documented in docs/dependencies.md (apart from the pin claim in finding 2) and mirrored in mise run validate.
  • The future-pairing procedure (run a green air-gapped nightly on the new pairing, then append a VALIDATED_PAIRINGS row) is documented and actionable.

@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch from d5508ad to f4969f4 Compare October 13, 2026 08:40

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES on head f4969f4. The branch moved since my 2026-10-13 review of d5508ad: rebased onto current main 07dd4ad, picking up the aws-orphan-report test step in validate.yml and the matching mise.toml wrapper; the four PR files are otherwise unchanged (git diff d5508ad..f4969f4 on the PR files shows only those rebase pickups). All three findings from the previous round are still present and re-verified on this head.

Blocking findings:

  1. .github/workflows/validate.yml:73-81: the PR re-adds an unconditional 'airgap-image-existence' job to validate.yml while main already runs the same script through the path-filtered .github/workflows/airgap-image-existence.yml (paths: airgap/images.txt, the test, the workflow itself). Re-verified on this head: main's validate.yml has no such job, the dedicated workflow exists on both main and this branch, and git merge-tree --write-tree origin/main f4969f4 merges clean (tree 3ee8703) with the merged tree containing both. After merge, the 45-digest live-registry check runs on every PR and every push to main, regressing #465 (5cc45dd moved it out of validate.yml specifically to prevent unconditional Docker Hub rate-limit exposure on every PR). Live evidence at this head: the duplicated job already executed inside this PR's validate run (run 38370893219, "Air-gap image existence" pass, alongside the validate jobs). The title and body never mention this addition. Fix: drop the job block from this PR; the dedicated workflow already covers it.

  2. tests/test-cert-manager-kubernetes-support.py:5 and docs/dependencies.md:126 both state the repo pins Kubernetes v1.37.0, but at this head (and on main) the pin is v1.36.4 (mgmt/local-host/clusters/docker/cluster.yaml:21, restored by #538; the v1.37.0 bump is #591, still open, and this PR does not touch the pin). Verified by running the test with the repo's mise Python against the head's real pins: with VALIDATED_PAIRINGS (line 23) containing only ("1.21.2", "1.37.0", ...), every validate run until #591 lands prints "NOTICE: cert-manager 1.21.2 has not been empirically tested with Kubernetes v1.36.4" and emits a ::notice annotation, flagging the pairing inside cert-manager 1.21's published 1.33-1.36 window that #538 intentionally restored. Fix: seed VALIDATED_PAIRINGS with the current (1.21.2, 1.36.4) pairing plus nightly evidence, or correct the docstring and docs claim to describe the actual pin.

Non-blocking:

  1. .github/workflows/validate.yml:205: the step name is an unquoted scalar, so YAML ends it at the " #": parsing this head's validate.yml yields 'cert-manager/Kubernetes pairing is empirically validated (non-blocking,'. The name also asserts a state the step does not guarantee, since the step exits 0 with a NOTICE on unvalidated pairings. Same defect flagged on 2026-09-28 and in both later rounds; the agreed fix (double-quoted "cert-manager/Kubernetes pairing notice (non-blocking, #384)") was again not carried over.

Blast radius (context for the maintainer, not part of the verdict): merging this removes the only CI gate that rejects cert-manager/Kubernetes pairings outside the upstream published window. PR #591 (kubernetes-version bump to 1.37) is currently blocked by this gate; once this merges, nothing automated blocks #591 on pairing grounds, and the replacement test reports (1.21.2, 1.37.0) as empirically validated from the seeded row. Accepting out-of-window pairings on the strength of the green 2026-09-21 nightly is a policy decision for the maintainer.

Verified locally (all on head f4969f4; merge-base with origin/main is current main 07dd4ad):

  • Ran the rewritten test with the repo's mise Python against the head's real pins: exits 0 with the "has not been empirically tested with Kubernetes v1.36.4" NOTICE; under GITHUB_ACTIONS=true it additionally emits the ::notice annotation (evidence for finding 2).
  • pyyaml parse of this head's validate.yml: the pairing step name truncates at '(non-blocking,' (evidence for finding 3); the rest of the workflow parses clean, and the zarf-mise-pin checkout carries no fetch-depth key.
  • git merge-tree --write-tree origin/main f4969f4: merges clean (tree 3ee8703); merged tree contains both .github/workflows/airgap-image-existence.yml and validate.yml's airgap-image-existence job (evidence for finding 1).
  • Evidence row re-checked via the API: Actions run 35554802424 is the air-gapped workflow, event schedule, conclusion success, 2026-09-21, head d9582b8, where bootstrap.toml pins cert-manager 1.21.2 and mgmt/local-host/clusters/docker/cluster.yaml pins v1.37.0. The row is real; only the current-pin claim in finding 2 is wrong.
  • No dangling references to the removed gate: zero hits for SUPPORTED_RELEASES_URL, the --changed-from invocation, or the cert-manager.io/docs/releases URL across the tree; the only surviving references to the test file are the new validate.yml step, the mise.toml wrapper, and the docs passage.
  • Dropping fetch-depth: 0 from the zarf-mise-pin checkout is safe: tests/test-mise-zarf-pin.py needs no git history, and the Zarf mise asset pin check passed at this head on the shallow checkout (run 38370893219).
  • The mise.toml pairing wrapper (mise.toml:151-155) mirrors the CI step with the same contract: pairing mismatch exits 0 with a notice, only unparseable pins fail.
  • CI at this head: all 10 checks pass; "Rendered Flux diff" skips by design for fork PRs. The "Renovate airgap digest pinning" red at capture time is green on the latest run.

Looks good:

  • Removing the blocking gate is the right call and well documented: the pairing it rejects passed the scheduled air-gapped deploy on 2026-09-21 (issue #384), and the original #322 failure was chart/image drift, still guarded by tests/test-cert-manager-version-consistency.py, which this PR keeps. The two stale docs passages (the scraped-docs workaround and the upstream kubeVersion note) correctly go with it.
  • The replacement's fail-only-on-parse-errors contract is accurately documented in docs/dependencies.md (apart from the pin claim in finding 2) and mirrored in mise run validate.
  • The future-pairing procedure (green air-gapped nightly on the new pairing, then append a VALIDATED_PAIRINGS row) is documented and actionable.

Comment thread .github/workflows/validate.yml Resolved
Comment thread .github/workflows/validate.yml Resolved
@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch from f4969f4 to 3ab1079 Compare October 13, 2026 14:15
)

Remove the CI gate that rejected cert-manager/Kubernetes pairings outside
the upstream published window. The gate was originally added to catch
chart/image drift (polarsquad#322), but that failure mode is already covered by the
surviving tests/test-cert-manager-version-consistency.py. The gate then
blocked PR polarsquad#591 (v1.37.0 bump) even though the exact pairing passed the
scheduled air-gapped nightly on 2026-09-21 (issue polarsquad#384).

Replace with a non-blocking notice: tests/test-cert-manager-kubernetes-support.py
prints a NOTICE when the pinned pairing is not in VALIDATED_PAIRINGS and
exits 0 in all cases; it only exits 1 on unparseable pins. The dedicated
.github/workflows/airgap-image-existence.yml (path-filtered, polarsquad#465) already
covers the unconditional digest checks removed here from validate.yml.

Seed VALIDATED_PAIRINGS with both the current pin (1.21.2, 1.36.4, within
the cert-manager 1.21 published window 1.33-1.36) and the air-gapped
nightly result (1.21.2, 1.37.0, run 35554802424, 2026-09-21).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@lmilbaum
lmilbaum force-pushed the ci/drop-cert-manager-k8s-support-gate branch from 3ab1079 to e6979fb Compare October 13, 2026 14:58
@lmilbaum
lmilbaum requested a review from shrinedogg October 13, 2026 15:01

@shrinedogg shrinedogg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict: no blocking findings on head e6979fb (unchanged since the wave-4 capture). All three findings from my 2026-10-13 review of f4969f4 are fixed and re-verified on this head. One non-blocking observation remains (inline on the new VALIDATED_PAIRINGS row). Posting as COMMENT rather than APPROVE only to carry that observation; it does not block merge.

The branch moved since the last review: rebased onto current main febb3d0 (the head is a direct descendant of the main tip; the PR diff is again exactly the four files: validate.yml, docs/dependencies.md, mise.toml, the rewritten test). The delta since f4969f4 outside those files is rebase pickup of main's docs rework only.

Prior findings, re-verified on this head:

  1. Duplicate airgap-image-existence job (was validate.yml:73-81): FIXED. pyyaml parse of this head's validate.yml lists six jobs (airgap-image-pins, kustomize-build, python-lint, renovate-digest-pinning, yaml-lint, zarf-mise-pin); the job is gone and the zarf-mise-pin checkout carries no fetch-depth. Remaining airgap-image-existence references in the tree are AGENTS.md/CONTRIBUTING/docs descriptions of the path-filtered dedicated workflow, which are accurate.

  2. Wrong pin claim (was test docstring + docs/dependencies.md): FIXED. mgmt/local-host/clusters/docker/cluster.yaml:21 pins v1.36.4; the docstring and docs now say v1.36.4 within the 1.33-1.36 window, and VALIDATED_PAIRINGS gains a (1.21.2, 1.36.4) row. Ran the test with the repo's mise Python against the real pins: exit 0 with the positive "empirically validated" notice and no ::notice annotation under GITHUB_ACTIONS=true. Out-of-window probe (copy of the test against a fake v1.39.0 pin): plain NOTICE plus ::notice, exit 0, so the signal still fires where it should.

  3. Step name (was validate.yml:205): FIXED. Parses as the full double-quoted "cert-manager/Kubernetes pairing notice (non-blocking, #384)" (the step now lives in the renovate-digest-pinning job after main's restructure; this PR's rename carried over), and the name no longer claims a passing state.

Correction to my prior evidence: the air-gapped nightly is not currently green; it has failed every scheduled run since 2026-09-22, spanning both the v1.37.0 pin (09-22 to 10-05) and the current v1.36.4 pin (10-06 onward; latest run 38338471903, "Deploy without external egress" exits 1 deep in the offline deploy, not a pairing failure). The last green deploy remains run 35554802424 (2026-09-21) on the v1.37.0 pairing, so the row citing it is accurate. Consequence: no green deploy exists yet on the (1.21.2, v1.36.4) pairing; see the inline observation.

Blast radius (maintainer context, unchanged): merging removes the only CI gate that rejects out-of-window cert-manager/Kubernetes pairings. #591 (kubernetes-version 1.37) stays blocked on its own review findings; after this lands nothing automated blocks a pairing bump, and the replacement test notices (non-blocking) on unvalidated pairs. Accepting out-of-window pairings on the strength of the 2026-09-21 nightly remains a policy decision for the maintainer.

Verified locally (all on head e6979fb; repo's mise Python / uv-run pyyaml):

  • pyyaml parse of validate.yml: job list as above; pairing step name parses in full; zarf-mise-pin checkout has no fetch-depth key.
  • tests/test-cert-manager-kubernetes-support.py against the real pins: exit 0, positive validated notice, no ::notice; against a fake v1.39.0 pin: NOTICE + ::notice, exit 0.
  • mise.toml wrapper (mise.toml:151-155) mirrors the CI step with the same parse-errors-only failure contract.
  • No dangling references to the removed gate: zero hits for SUPPORTED_RELEASES_URL; --changed-from survives only in the retained airgap digests test; the test file is referenced only by its CI step, the mise wrapper, and the docs passage.
  • No stale "repo pins v1.37.0" claims remain in docs (remaining 1.37.0 hits are unrelated: kind node images in the wiremock docs, the Talos cluster pin, the mise kubectl pin, and the historical VALIDATED_PAIRINGS row).
  • CI at this head: all checks pass; "Rendered Flux diff" skips by design for fork PRs.

Looks good:

  • Removing the blocking gate is right and well documented; the original #322 failure was chart/image drift and stays guarded by tests/test-cert-manager-version-consistency.py.
  • The docs rewrite (docs/dependencies.md:125-136) now matches the real pin and accurately describes the notice-only contract and the future-pairing procedure.
  • Dropping fetch-depth: 0 is safe: the zarf pin test needs no git history and the Zarf mise asset pin check passed at this head.

# (cert-manager, Kubernetes, evidence). Append a row only after a green
# air-gapped deploy with that exact pairing.
VALIDATED_PAIRINGS = [
("1.21.2", "1.36.4", "cert-manager 1.21 published support window covers Kubernetes 1.33-1.36"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: this row's evidence is the published support window, not a green air-gapped deploy on that exact pairing, which contradicts the append rule two lines above ("Append a row only after a green air-gapped deploy with that exact pairing") and docs/dependencies.md:129 + 134-135 ("empirically tested combinations", green-nightly-then-add-row). No green nightly exists on (1.21.2, v1.36.4): the nightly has been red since 2026-09-22 (across both pins) and the last green run, 35554802424 on 2026-09-21, was the v1.37.0 pairing. The row is honest about its basis and the behavior it produces is right (the current in-window pairing stays silent), so this does not block. Suggestion: either soften the list comment and the docs to accept published-window evidence for in-window pairings, or drop the row and accept the notice until a green 1.36.4 nightly exists.

@shrinedogg
shrinedogg merged commit 184272a into polarsquad:main Oct 14, 2026
10 checks passed
@lmilbaum
lmilbaum deleted the ci/drop-cert-manager-k8s-support-gate branch October 14, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cert-manager 1.21.2 pin does not support the pinned Kubernetes v1.37.0; support-window gate fails any bootstrap.toml PR

2 participants