Skip to content

🐛 eventing: five manifest assertions skip in CI, so the Secret-vs-ConfigMap rule is unenforced there #894

Description

@mrsabath

Raised in the #884 and #883 reviews. Five manifest assertions are @needs_kubectl and
skip in CI, so they are promises that only hold on a laptop.

The problem

tests/test_manifests.py's render() shells out to kubectl kustomize:

def render(overlay: str) -> str:
    r = _run(["kubectl", "kustomize", str(OVERLAYS[overlay])])

kubectl is not on the CI runner, so every test marked @needs_kubectl skips — currently
reported as 5 skipped on every run, in both CI and most sandboxes. The assertions that
do not run include:

  • test_no_phase3_secret_is_inlined_into_a_manifest — the §8.3 Secret-vs-ConfigMap rule,
    which is the whole of T22
  • test_the_test_overlay_mounts_no_credential_so_mock_mode_is_automatic — fixed in feat(eventing): Phase 2 — make event signing deployable on a cluster #884
    precisely because it had stopped meaning what it documented, and still not exercised
    automatically
  • test_no_credential_is_ever_rendered_into_a_manifest
  • the replicas invariants for both Deployments

So a manifest change that violates any of these merges green. #884 found one case of a
manifest assertion silently ceasing to mean anything; nothing automated would have caught
the next.

Why this is cheap to fix

kustomize is a separate binary from kubectl, and it is present in environments
where kubectl is not — including the sandbox where both of those reviews were run.
kubectl kustomize and kustomize build produce the same output for these overlays.

So render() can fall back:

def _kustomize_argv(path: str) -> list[str] | None:
    if shutil.which("kubectl"):
        return ["kubectl", "kustomize", path]
    if shutil.which("kustomize"):
        return ["kustomize", "build", path]
    return None

and needs_kubectl becomes needs_kustomize, skipping only when neither is available.
The tests that genuinely need a cluster (needs_cluster, for API-server-defaulted JSON)
are unaffected and should keep skipping.

Worth checking before doing it

  • Does CI have either binary? If neither, this needs a setup step in the workflow —
    still worth it, since kustomize is a single static binary, but it changes the shape
    of the fix from "a fallback" to "a fallback plus a CI change".
  • Do the two renderers agree byte for byte on all four overlays? They should for
    these manifests, but the assertions are string-matching against indentation, so it is
    worth diffing rather than assuming. If they differ, the assertions may need \s*
    loosening — test_manifests.py already does that in several places for related reasons.

Why it was not done in #883 or #884

Both reviews flagged it and both concluded it is bigger than a docs or feature PR should
carry: it is test infrastructure affecting five assertions across two phases, and it
deserves its own verification that the fallback renders identically. Filing it so it is
not rediscovered the next time a manifest assertion stops meaning anything.

Phases 1–3 (the assertions span all three). Not blocking anything.

Activity

  1. mrsabath commented on Oct 7, 2026

    @mrsabath
    MemberAuthor

    Went to fix this and checked the premise first — it does not hold: the five assertions
    named above do not skip in CI, they pass.

    From the CI run on main at 8ab8b30 — run
    37501858445, job
    112400597006 (eventing-test):

    tests/test_manifests.py::test_no_phase3_secret_is_inlined_into_a_manifest PASSED
    tests/test_manifests.py::test_the_test_overlay_mounts_no_credential_so_mock_mode_is_automatic PASSED
    tests/test_manifests.py::test_no_credential_is_ever_rendered_into_a_manifest PASSED
    tests/test_manifests.py::test_the_eventrunner_deployment_declares_no_spec_replicas PASSED
    tests/test_manifests.py::test_the_eventbridge_deployment_does_pin_one_replica PASSED
    

    The reason is that ubuntu-latest ships kubectl 1.37.1 and kustomize 5.8.1
    preinstalled
    (actions/runner-images, Ubuntu 24.04 manifest, image
    20260927.320.1), so _HAVE_KUBECTL is true on the runner and all 21
    @needs_kubectl tests execute. The Secret-vs-ConfigMap rule from T22 is enforced in CI
    today.

    The run ends 896 passed, 8 skipped. The five test_manifests.py skips are the
    @needs_cluster ones:

    • test_every_object_is_accepted_by_the_api_server
    • test_the_topics_are_accepted_by_strimzi
    • test_applying_the_overlay_never_overwrites_the_replica_count_keda_owns
    • test_the_server_keeps_eventbridge_at_one_replica
    • test_the_scaledobject_survives_the_keda_admission_webhook

    — which this issue itself says should keep skipping, since they need a live API server.
    So the "5 skipped" count is real, but it is the cluster-dependent five, not the
    kubectl-dependent five. It looks like the observation came from a sandbox with no
    kubectl on PATH and was read as a CI symptom.

    The kustomize fallback proposed here is still a mild robustness win for sandboxes
    without kubectl, and needs_kustomize would be a more accurate name than
    needs_kubectl. But it changes nothing in CI and fixes no unenforced assertion, so I
    don't think it earns a PR on its own — happy to be told otherwise if you'd rather have
    the local-dev ergonomics.

    For context, this came out of a pass over the other issues filed alongside it — #889,
    #886 and #891 did turn out to be real and now have PRs (#901, #902, #903).

    Closing as already satisfied. Reopen if you see these skipping on a run I've missed.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions