From 2c5ed7ab01392e1f7cd02bbf49722bfa14d6d74d Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 08:21:52 +0200 Subject: [PATCH 1/6] :sparkles: feat(lab): allowlist lab substrates so Kind-only scenarios run on Talos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lab harness gated on `[[ "${ctx}" == kind-* ]]` and refused everything else, which left P-06 (pprof under load) and U-08 (webhook rejection) unrunnable on the bare-metal Talos lab. Widening that check — or defaulting `--allow-non-kind` — would have turned a safety gate into a hole: these scripts port-forward, install Helm values and apply load, and a maintainer's ambient context is routinely a production cluster. Replace it with a default-deny ALLOWLIST in hack/lab/substrates.conf, enforced by lib/substrate.sh: `kind-*` and the kumulus lab (`kumulus-lab`, cluster `kumulus`) are permitted, everything else is refused with exit 2. Patterns are validated wherever they come from — the checked-in file, KOLLECT_LAB_SUBSTRATES_FILE, or KOLLECT_LAB_ALLOWED_CONTEXTS — so `*`, `*-prod` and `k*` fail the load closed instead of admitting every cluster. An entry's expected cluster name can only refuse a mismatch, never admit. `--allow-non-kind` survives as an explicit maintainer override but is no longer the mechanism by which the lab works. Substrate also decides image delivery. `kind load docker-image` has no Talos equivalent, so a non-Kind substrate must name a pinned registry reference; a local-only or mutable tag (`:dev`, `:latest`, untagged) is refused before helm is called rather than silently reusing whatever the nodes cached — the stale-image trap that blocked U-02's re-test. For U-08, `kollect_e2e_select_context` separates "create a kind cluster" from "run the assertions": with KOLLECT_E2E_EXISTING_CLUSTER=1 the webhook scenario asserts against the current allowlisted context using server-side dry runs only, skips the cert-manager Certificate wait when the release does not use one, and reports a missing webhook configuration as a precondition (exit 4) instead of a 300s timeout. Without that env var the Kind/CI path is unchanged. perf-kind.sh gains --release so the port-forward targets deploy/-controller-manager (the lab runs kollect-op1, not kollect), and perf-kind:quick becomes an alias for the substrate-neutral perf-lab:quick. --- Taskfile.yml | 19 +- hack/e2e/webhook-smoke.sh | 99 ++++++- hack/kind/README.md | 36 +++ hack/kind/common.sh | 85 +++++- hack/lab/README.md | 71 ++++- hack/lab/lib/substrate.sh | 274 ++++++++++++++++++ hack/lab/perf-kind.sh | 94 ++++-- hack/lab/substrates.conf | 18 ++ .../test/e2e_webhook_existing_cluster_test.sh | 157 ++++++++++ hack/test/lab_image_delivery_meta_test.sh | 111 +++++++ hack/test/lab_perf_kind_meta_test.sh | 56 +++- hack/test/lab_substrate_meta_test.sh | 169 +++++++++++ 12 files changed, 1131 insertions(+), 58 deletions(-) create mode 100644 hack/lab/lib/substrate.sh create mode 100644 hack/lab/substrates.conf create mode 100755 hack/test/e2e_webhook_existing_cluster_test.sh create mode 100755 hack/test/lab_image_delivery_meta_test.sh create mode 100755 hack/test/lab_substrate_meta_test.sh diff --git a/Taskfile.yml b/Taskfile.yml index b0a4720c..a265c837 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -241,11 +241,16 @@ tasks: cmds: - bash hack/perf-report.sh - perf-kind:quick: - desc: LAB-H10 quick Kind pprof path (offline --dry-run; live Kind maintainer opt-in) + perf-lab:quick: + desc: LAB-H10 quick pprof path (offline --dry-run; live capture on any allowlisted substrate) cmds: - bash hack/lab/perf-kind.sh --dry-run --run-id perf-quick --objects 100 --seed 42 + perf-kind:quick: + desc: Deprecated alias for perf-lab:quick (the path is no longer Kind-only) + cmds: + - task: perf-lab:quick + test-integration: desc: Run integration-tagged tests (testcontainers; requires Docker) env: @@ -467,3 +472,13 @@ tasks: desc: Delete kollect-e2e kind cluster cmds: - bash hack/kind/e2e/teardown.sh + + lab:webhook-smoke: + desc: >- + U-08 webhook rejection assertions against the EXISTING allowlisted cluster + (read-only: server dry-run only, never creates a cluster). + Override the target with KOLLECT_RELEASE / KOLLECT_NAMESPACE. + env: + KOLLECT_E2E_EXISTING_CLUSTER: "1" + cmds: + - bash hack/e2e/webhook-smoke.sh diff --git a/hack/e2e/webhook-smoke.sh b/hack/e2e/webhook-smoke.sh index 3130f8d9..e6d86f63 100755 --- a/hack/e2e/webhook-smoke.sh +++ b/hack/e2e/webhook-smoke.sh @@ -1,39 +1,106 @@ #!/usr/bin/env bash # Tier 1 webhook e2e: assert serving cert + validating webhook rejects invalid family sink CRs. +# +# Two modes (LAB-DEKIND / U-08) — "create a cluster" is separable from "assert against one": +# default CI/Kind. Switches to the kind-${CLUSTER_NAME} context and +# applies the valid sample for real. Unchanged behaviour. +# KOLLECT_E2E_EXISTING_CLUSTER=1 Run the same assertions against the cluster the CURRENT +# context points at — provided that context is on the lab +# substrate allowlist (hack/lab/substrates.conf). Never +# creates a cluster and never mutates: every apply is a +# server-side dry run, so it is safe against a live release +# that is holding evidence. +# +# Existing-cluster example (kumulus Talos lab, Helm release kollect-op1): +# KUBECONFIG=... KOLLECT_E2E_EXISTING_CLUSTER=1 \ +# KOLLECT_RELEASE=kollect-op1 KOLLECT_NAMESPACE=kollect-op1 bash hack/e2e/webhook-smoke.sh +# +# Exit codes: +# 0 assertions passed +# 1 assertion failed +# 2 kube context refused (not on the lab substrate allowlist) +# 4 webhook stack not installed on the target cluster (precondition, not a product bug) set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # shellcheck source=../kind/common.sh source "${SCRIPT_DIR}/../kind/common.sh" +# shellcheck source=../lab/lib/substrate.sh +source "${SCRIPT_DIR}/../lab/lib/substrate.sh" readonly CLUSTER_NAME="${CLUSTER_NAME:-kollect-e2e}" readonly WAIT_TIMEOUT="${WAIT_TIMEOUT:-300s}" +readonly EXISTING_CLUSTER="${KOLLECT_E2E_EXISTING_CLUSTER:-0}" +readonly TEST_NAMESPACE="${KOLLECT_E2E_TEST_NAMESPACE:-default}" _kind_require kubectl -kind_use_context "$CLUSTER_NAME" _log() { echo "[webhook-smoke] $*"; } -_log "Waiting for webhook serving Certificate Ready..." -kubectl wait --for=condition=Ready "certificate/${KOLLECT_RELEASE}-serving-cert" \ - -n "$KOLLECT_NAMESPACE" \ - --timeout="$WAIT_TIMEOUT" +# Mutating applies are the Kind-only half of this scenario. Against an existing lab cluster +# the same admission decision is observable with a server-side dry run, so the accept +# assertion is downgraded to --dry-run=server there (explicit --dry-run=none on Kind keeps +# the CI behaviour visible rather than implied). +APPLY_MODE="none" -_log "Asserting ValidatingWebhookConfiguration registered..." -if ! kubectl get validatingwebhookconfiguration "${KOLLECT_RELEASE}-validating-webhook-configuration" \ - >/dev/null 2>&1; then - kubectl get validatingwebhookconfiguration - exit 1 +if ! kollect_e2e_select_context "$CLUSTER_NAME"; then + _log "existing-cluster mode requires an allowlisted lab context; refusing to assert" + exit 2 +fi +if [[ "${EXISTING_CLUSTER}" == "1" ]]; then + APPLY_MODE="server" + _log "existing-cluster mode: read-only assertions (release ${KOLLECT_RELEASE}, namespace ${KOLLECT_NAMESPACE})" +fi + +_webhook_stack_missing() { + _log "FAIL: the validating webhook stack is not installed on this cluster." + _log "Release '${KOLLECT_RELEASE}' in namespace '${KOLLECT_NAMESPACE}' has no" + _log "ValidatingWebhookConfiguration '${KOLLECT_RELEASE}-validating-webhook-configuration'." + _log "Install/upgrade the release with webhooks enabled (and cert-manager present), then re-run." + kubectl get validatingwebhookconfiguration || true +} + +if [[ "${EXISTING_CLUSTER}" == "1" ]]; then + # cert-manager is a Kind-stack assumption: an existing lab release may serve its webhook + # cert another way. Only wait for the Certificate when cert-manager actually manages one. + if kubectl get crd certificates.cert-manager.io >/dev/null 2>&1 \ + && kubectl get certificate "${KOLLECT_RELEASE}-serving-cert" -n "$KOLLECT_NAMESPACE" >/dev/null 2>&1; then + _log "Waiting for webhook serving Certificate Ready..." + kubectl wait --for=condition=Ready "certificate/${KOLLECT_RELEASE}-serving-cert" \ + -n "$KOLLECT_NAMESPACE" \ + --timeout="$WAIT_TIMEOUT" + else + _log "No cert-manager Certificate for this release; asserting the webhook itself instead." + fi + + _log "Asserting ValidatingWebhookConfiguration registered..." + if ! kubectl get validatingwebhookconfiguration "${KOLLECT_RELEASE}-validating-webhook-configuration" \ + >/dev/null 2>&1; then + _webhook_stack_missing + exit 4 + fi +else + _log "Waiting for webhook serving Certificate Ready..." + kubectl wait --for=condition=Ready "certificate/${KOLLECT_RELEASE}-serving-cert" \ + -n "$KOLLECT_NAMESPACE" \ + --timeout="$WAIT_TIMEOUT" + + _log "Asserting ValidatingWebhookConfiguration registered..." + if ! kubectl get validatingwebhookconfiguration "${KOLLECT_RELEASE}-validating-webhook-configuration" \ + >/dev/null 2>&1; then + kubectl get validatingwebhookconfiguration + exit 1 + fi fi _log "Expect validating webhook to reject git snapshot sink without git block..." set +e -reject_out="$(kubectl apply --dry-run=server -f - 2>&1 <<'EOF' +reject_out="$(kubectl apply --dry-run=server -f - 2>&1 <`) because there is no +`kind load` equivalent — a local-only or `:latest`/`:dev` tag is rejected before install. + +The webhook scenario (`hack/e2e/webhook-smoke.sh`) can assert against an existing cluster +without creating anything, using server-side dry runs only: + +```sh +KOLLECT_E2E_EXISTING_CLUSTER=1 KOLLECT_RELEASE=kollect-op1 KOLLECT_NAMESPACE=kollect-op1 \ + task lab:webhook-smoke +``` + +CI is unaffected: with no extra environment set, the Kind path behaves exactly as before. + +`kollect_e2e_select_context` in `common.sh` is the seam: it switches to `kind-` by +default and, in existing-cluster mode, validates the current context against the allowlist +instead. Other scenario scripts can adopt it one line at a time. + +### Still Kind-only (deliberate) + +| Assumption | Where | Why it was left | +| --- | --- | --- | +| Switches to the `kind-kollect-e2e` context, then creates/deletes CRs, namespaces and sinks | `hack/kind/e2e/smoke.sh`, `bootstrap-samples.sh`, `pipeline-cli-smoke.sh`, `hack/e2e/{cert-manager,tenant-mode,multitenant,finalizer-cleanup-assert,git-export-assert}.sh` | These are *mutating* scenarios. Read-only server dry runs cannot express them, so pointing them at a lab cluster that is holding evidence is unsafe by construction. They can adopt `kollect_e2e_select_context` when a disposable lab cluster exists. | +| Single-node `cluster.yaml`, `kindest/node` version resolution, dev NodePorts 30080/30443 | `hack/kind/e2e/cluster.yaml`, `hack/kind/dev/`, `common.sh` | Only used while *creating* a kind cluster; unreachable on an existing-cluster run. | +| cert-manager `Certificate` gate for the webhook serving cert | `hack/e2e/webhook-smoke.sh` | Fixed for existing clusters — the wait is skipped when cert-manager does not manage the release's cert, and the webhook itself is asserted instead. | +| `kind load docker-image` | `common.sh` | Fixed — substrate decides delivery; non-Kind requires a pinned registry reference. | + +No storage-class, hostPath or LoadBalancer assumptions exist in the e2e path (the e2e chart +values request none), so nothing there blocks a bare-metal lab. + ## Prerequisites (dev) | Tool | Required for | diff --git a/hack/kind/common.sh b/hack/kind/common.sh index 22a02100..75d84e23 100755 --- a/hack/kind/common.sh +++ b/hack/kind/common.sh @@ -5,6 +5,11 @@ set -euo pipefail KIND_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" REPO_ROOT="$(cd "${KIND_DIR}/../.." && pwd)" +# Substrate allowlist + image-delivery policy (LAB-DEKIND). Sourced here so the install path +# has ONE auditable place deciding "which cluster" and "how images get there". +# shellcheck source=../lab/lib/substrate.sh +source "${KIND_DIR}/../lab/lib/substrate.sh" + # Pin kind CLI version (matches .github/workflows/e2e-nightly.yaml). readonly KIND_VERSION="${KIND_VERSION:-0.32.0}" @@ -117,6 +122,24 @@ kind_use_context() { kubectl config use-context "kind-${name}" >/dev/null } +# Decoupling seam (LAB-DEKIND): "create/select a kind cluster" vs "assert against whatever +# cluster we are pointed at". Scenario scripts call this instead of kind_use_context so they +# can run on an existing lab cluster with KOLLECT_E2E_EXISTING_CLUSTER=1. Default (CI) path +# is unchanged: switch to kind-. Returns 2 when the current context is off-allowlist. +kollect_e2e_select_context() { + local cluster="$1" + if [[ "${KOLLECT_E2E_EXISTING_CLUSTER:-0}" != "1" ]]; then + kind_use_context "$cluster" + return 0 + fi + local ctx + ctx="$(kubectl config current-context 2>/dev/null || true)" + if ! lab_substrate_assert_context "${ctx}"; then + return 2 + fi + return 0 +} + kind_create_cluster() { local name="$1" config="$2" if kind_cluster_exists "$name"; then @@ -200,13 +223,29 @@ kollect_helm_install() { local values_file="$1" shift || true - _kind_log "Installing kollect via Helm (values: ${values_file}, timeout ${KOLLECT_HELM_TIMEOUT})..." + # Split repo/tag on the LAST colon only when it follows the last slash, so a registry with + # a port (localhost:5000/kollect:v1) is not mangled. The chart renders repository:tag, so a + # digest-pinned reference is refused here rather than silently reinterpreted. + local image_repo image_tag + if [[ "$KOLLECT_IMAGE" == *"@"* ]]; then + _kind_log "KOLLECT_IMAGE '${KOLLECT_IMAGE}' is digest-pinned; the chart renders repository:tag — use a pinned v tag." + return 1 + fi + if [[ "${KOLLECT_IMAGE##*/}" == *:* ]]; then + image_repo="${KOLLECT_IMAGE%:*}" + image_tag="${KOLLECT_IMAGE##*:}" + else + image_repo="$KOLLECT_IMAGE" + image_tag="latest" + fi + + _kind_log "Installing kollect via Helm (values: ${values_file}, image ${image_repo}:${image_tag}, timeout ${KOLLECT_HELM_TIMEOUT})..." if ! helm upgrade --install "$KOLLECT_RELEASE" "$KOLLECT_HELM_CHART" \ --namespace "$KOLLECT_NAMESPACE" \ --create-namespace \ -f "$values_file" \ - --set "image.repository=${KOLLECT_IMAGE%%:*}" \ - --set "image.tag=${KOLLECT_IMAGE##*:}" \ + --set "image.repository=${image_repo}" \ + --set "image.tag=${image_tag}" \ --set image.pullPolicy=IfNotPresent \ "$@" \ --wait --timeout "$KOLLECT_HELM_TIMEOUT"; then @@ -283,12 +322,48 @@ kollect_wait_manager_ready() { --timeout="$timeout" } +# Resolve the substrate of the cluster the CURRENT context points at (default-deny). +# Prints the substrate kind; returns 2 when the context is not on the lab allowlist. +kollect_current_substrate() { + local ctx + ctx="$(kubectl config current-context 2>/dev/null || true)" + local kind_out + if ! kind_out="$(lab_substrate_resolve "${ctx}")"; then + lab_substrate_err "refusing kube context '${ctx}': not on the lab substrate allowlist [$(lab_substrate_allowlist_summary)]" + lab_substrate_err "default-deny — installs only run on a Kind cluster or an allowlisted lab cluster" + return 2 + fi + printf '%s' "${kind_out}" +} + +# Deliver the operator image to the target cluster. +# Kind → build locally and side-load (`kind load docker-image`), as CI has always done. +# other → there is NO side-load equivalent (Talos runs containerd on bare metal, no +# docker socket to import into), so the image MUST already exist in a registry at +# an immutable reference. Anything else is refused loudly here rather than +# silently running whatever the nodes cached — a stale image invalidates the run. +kollect_deliver_image() { + local cluster="$1" substrate="$2" + if [[ "$(lab_substrate_image_delivery "$substrate")" == "sideload" ]]; then + kollect_build_image + kollect_load_image "$cluster" + return 0 + fi + if ! lab_substrate_require_registry_image "$KOLLECT_IMAGE" "$substrate"; then + _kind_log "Substrate '${substrate}' cannot side-load images; set KOLLECT_IMAGE to a pushed, pinned reference." + return 1 + fi + _kind_log "Substrate ${substrate}: using pinned registry image ${KOLLECT_IMAGE} (no side-load, no rebuild)." + return 0 +} + kollect_install_base() { local cluster="$1" values_file="$2" shift 2 || true - kollect_build_image - kollect_load_image "$cluster" + local substrate + substrate="$(kollect_current_substrate)" || return 1 + kollect_deliver_image "$cluster" "$substrate" || return 1 kollect_wait_kube_system_ready kollect_helm_install "$values_file" "$@" kollect_wait_crds_established diff --git a/hack/lab/README.md b/hack/lab/README.md index 7cc8100d..523fed26 100644 --- a/hack/lab/README.md +++ b/hack/lab/README.md @@ -6,6 +6,30 @@ Operator walkthrough: [Local lab runbook](../../docs/operator-manual/local-lab-r Non-Kind kubeconfig is first-class. Scripts **never** create or destroy a cluster. +## Substrate allowlist (LAB-DEKIND) + +Lab tooling port-forwards, installs Helm values and applies load, so it refuses to run +against a cluster it does not recognise. The permitted kube contexts are enumerated in +[`substrates.conf`](substrates.conf) and enforced by [`lib/substrate.sh`](lib/substrate.sh): + +| Context pattern | Substrate | Image delivery | +| --- | --- | --- | +| `kind-*` | `kind` | build + `kind load docker-image` | +| `kumulus-lab` (cluster `kumulus`) | `talos` | pinned registry reference only | + +**Default-deny**: an unlisted context — including a maintainer's ambient production +context — is refused with exit **2**. Widen the list only via +`KOLLECT_LAB_ALLOWED_CONTEXTS` (`pattern[=substrate[=cluster]]`, comma separated) or +`KOLLECT_LAB_SUBSTRATES_FILE`; both are validated the same way and a wildcard-only or +under-specific pattern (`*`, `*-prod`, `k*`) fails the load closed. When an entry names an +expected cluster, a cluster-name mismatch can only **refuse** — it never admits a context +the pattern did not already match. + +Non-Kind substrates have **no** `kind load` equivalent, so the operator image must come from +a registry at an immutable reference (`ghcr.io/platformrelay/kollect:v`). A local-only +or mutable tag (`:dev`, `:latest`, untagged) is rejected before anything is installed rather +than silently reusing whatever the nodes already cached. + ## Preflight (LAB-H01) ```sh @@ -58,17 +82,27 @@ until live scenario bodies exist; dry-run does not create or delete cluster reso Minimal labeled batch/churn helper: `bash hack/lab/workload.sh --run-id --dry-run --out-dir ` (always labels `kollect.dev/lab-run=`; not required in default `quick`/`quick+sinks`). -## Perf Kind pprof (LAB-H10 / PERF-LAB-01) +## Perf pprof quick path (LAB-H10 / PERF-LAB-01) -Kind-oriented quick pprof path. CI verifies the offline `--dry-run` machine-encoded quick path; -live Kind capture uses localhost port-forward + `curl`/`go tool pprof` (maintainer opt-in). +Quick pprof path for **any allowlisted substrate** — Kind or the kumulus Talos lab. CI verifies +the offline `--dry-run` machine-encoded quick path; live capture uses localhost port-forward + +`curl`/`go tool pprof` (maintainer opt-in). ```sh -task perf-kind:quick +task perf-lab:quick # alias: task perf-kind:quick (unchanged) bash hack/lab/perf-kind.sh --dry-run --run-id perf-demo --objects 500 --seed 42 --duration 60s -bash hack/lab/perf-kind.sh --run-id perf-live --objects 500 # requires kind-* context + pprof.enabled + +# Live on Kind (release "kollect" in kollect-system) +bash hack/lab/perf-kind.sh --run-id perf-live --objects 500 + +# Live on the kumulus Talos lab (release kollect-op1 in namespace kollect-op1) +KUBECONFIG= bash hack/lab/perf-kind.sh --run-id perf-live \ + --release kollect-op1 --namespace kollect-op1 --objects 500 ``` +Both live paths need `pprof.enabled: true` on the release — the manager does not serve +`:6060` otherwise and the run exits **BLOCKED** rather than emitting placeholder profiles. + Live port-forward (never a public Service): ```sh @@ -81,10 +115,14 @@ kubectl -n kollect-system port-forward deploy/kollect-controller-manager 16060:6 | `--objects` | `100` \| `500` \| `2000` for live converge/churn (metadata-only in `--dry-run`) | | `--duration` | Phase dwell hint (default `60s`); CPU profile sample capped at `30s` | | `--namespace` | Manager namespace for port-forward (default: `kollect-system`) | +| `--release` | Helm release name → `deploy/-controller-manager` (default: `kollect`) | | `--seed` | Deterministic fixture metadata in `--dry-run` | -| `--allow-non-kind` | Skip Kind context gate (never creates/destroys cluster) | +| `--allow-non-kind` | Maintainer override for a context that is **not** on the substrate allowlist. Allowlisted lab clusters (including kumulus) need no flag | | `--keep-lab` | Hint: retain `kollect.dev/lab-run=` labeled workload | +Context fixtures for offline meta-tests: `--fixture=context-kind | context-kumulus | +context-non-kind | context-ambiguous | context-kumulus-lookalike | context-prod-lookalike`. + Phases: **idle → converge(N) → churn → recover**. pprof is off in product Helm by default; enable `pprof.enabled: true` on the release. Access via **localhost port-forward only** — the Service spec does not expose `:6060`; forward the deployment container port (matches load-test-runbook). @@ -93,4 +131,23 @@ On interrupt: tear down port-forward; delete only resources labeled `kollect.dev keep partial `profiles/` artifacts. Live runs that cannot reach the pprof endpoint exit **BLOCKED** with reason (no silent `.stub` placeholders). -Meta-tests: `hack/test/lab_perf_kind_meta_test.sh` (offline only). +Meta-tests: `hack/test/lab_perf_kind_meta_test.sh`, `hack/test/lab_substrate_meta_test.sh`, +`hack/test/lab_image_delivery_meta_test.sh` (offline only; run together via +`bash hack/test/lab_harness_meta_suite.sh`). + +## Webhook rejection against an existing cluster (U-08) + +The webhook scenario no longer needs a freshly created Kind stack. With +`KOLLECT_E2E_EXISTING_CLUSTER=1`, `hack/e2e/webhook-smoke.sh` asserts against whatever +allowlisted cluster the current context points at, **read-only** (every apply is a +server-side dry run), so it is safe to run against a live release that is holding evidence: + +```sh +KUBECONFIG= KOLLECT_E2E_EXISTING_CLUSTER=1 \ + KOLLECT_RELEASE=kollect-op1 KOLLECT_NAMESPACE=kollect-op1 \ + task lab:webhook-smoke +``` + +Exit **2** = context not on the allowlist; exit **4** = the release has no validating webhook +configuration (install/upgrade it with webhooks enabled first — the operator's +`--validating-webhooks-enabled=false` is a precondition failure, not a product bug). diff --git a/hack/lab/lib/substrate.sh b/hack/lab/lib/substrate.sh new file mode 100644 index 00000000..2ad5c33c --- /dev/null +++ b/hack/lab/lib/substrate.sh @@ -0,0 +1,274 @@ +#!/usr/bin/env bash +# Lab substrate allowlist + image-delivery policy (LAB-DEKIND). Source this file; do not execute. +# SPDX-License-Identifier: MIT +# shellcheck shell=bash +# +# WHY THIS EXISTS +# Lab tooling port-forwards, installs Helm values and applies load. Pointing it at the +# wrong cluster is destructive, and a maintainer's ambient kube context is frequently a +# production cluster. The gate is therefore an ALLOWLIST with DEFAULT-DENY semantics: +# a context is refused unless hack/lab/substrates.conf (or an explicitly validated +# KOLLECT_LAB_ALLOWED_CONTEXTS addition) names it. There is no "allow everything" value. +# +# Exit/return contract: +# lab_substrate_assert_context 0 = allowed, 2 = refused (callers exit 2) +# lab_substrate_resolve 0 = allowed (prints substrate kind), 1 = refused +# lab_substrate_load 0 = allowlist parsed, 1 = unusable/unsafe allowlist (fail closed) + +_LAB_SUBSTRATE_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +: "${LAB_SUBSTRATE_LOG_PREFIX:=[lab-substrate]}" + +lab_substrate_log() { printf '%s %s\n' "${LAB_SUBSTRATE_LOG_PREFIX}" "$*"; } +lab_substrate_warn() { printf '%s WARN: %s\n' "${LAB_SUBSTRATE_LOG_PREFIX}" "$*" >&2; } +lab_substrate_err() { printf '%s FAIL: %s\n' "${LAB_SUBSTRATE_LOG_PREFIX}" "$*" >&2; } + +LAB_SUBSTRATE_PATTERNS=() +LAB_SUBSTRATE_KINDS=() +LAB_SUBSTRATE_CLUSTERS=() +LAB_SUBSTRATE_LOADED=0 +LAB_SUBSTRATE_MATCHED_KIND="" +LAB_SUBSTRATE_MATCHED_CLUSTER="" + +lab_substrate_config_path() { + if [[ -n "${KOLLECT_LAB_SUBSTRATES_FILE:-}" ]]; then + printf '%s' "${KOLLECT_LAB_SUBSTRATES_FILE}" + return 0 + fi + printf '%s' "${_LAB_SUBSTRATE_LIB_DIR}/../substrates.conf" +} + +# A pattern is safe only if it is an exact context name or a single TRAILING '*' with at +# least 4 literal characters. '*', '**', '*-prod' and 'k*' are all rejected: an over-broad +# pattern turns the gate into a hole. +lab_substrate_valid_pattern() { + local pat="${1:-}" + [[ -n "${pat}" ]] || return 1 + local stars="${pat//[^*]/}" + ((${#stars} <= 1)) || return 1 + if [[ "${pat}" == *'*'* && "${pat}" != *'*' ]]; then + return 1 + fi + local literal="${pat%\*}" + ((${#literal} >= 4)) || return 1 + [[ "${literal}" =~ ^[A-Za-z0-9][A-Za-z0-9._:@/-]*$ ]] || return 1 + return 0 +} + +lab_substrate_valid_kind() { + case "${1:-}" in + kind | talos | generic) return 0 ;; + *) return 1 ;; + esac +} + +_lab_substrate_add() { + local pat="$1" kind="$2" cluster="$3" src="$4" + if ! lab_substrate_valid_pattern "${pat}"; then + lab_substrate_err "refusing unsafe context pattern '${pat}' from ${src}: want an exact context name or one trailing '*' after >= 4 literal characters" + return 1 + fi + if ! lab_substrate_valid_kind "${kind}"; then + lab_substrate_err "invalid substrate '${kind}' for pattern '${pat}' from ${src} (want kind|talos|generic)" + return 1 + fi + LAB_SUBSTRATE_PATTERNS+=("${pat}") + LAB_SUBSTRATE_KINDS+=("${kind}") + LAB_SUBSTRATE_CLUSTERS+=("${cluster}") + return 0 +} + +# Parse the checked-in allowlist plus KOLLECT_LAB_ALLOWED_CONTEXTS. Any unsafe or malformed +# entry fails the whole load — a partially-parsed allowlist is not a safety boundary. +lab_substrate_load() { + LAB_SUBSTRATE_PATTERNS=() + LAB_SUBSTRATE_KINDS=() + LAB_SUBSTRATE_CLUSTERS=() + LAB_SUBSTRATE_LOADED=0 + + local file + file="$(lab_substrate_config_path)" + if [[ ! -f "${file}" ]]; then + lab_substrate_err "substrate allowlist not found: ${file} (refusing every context)" + return 1 + fi + + local line pat kind cluster rest + while IFS= read -r line || [[ -n "${line}" ]]; do + line="${line%%#*}" + pat="" + kind="" + cluster="" + rest="" + read -r pat kind cluster rest <<<"${line}" || true + [[ -n "${pat}" ]] || continue + if [[ -n "${rest}" ]]; then + lab_substrate_err "malformed allowlist entry in ${file}: '${line}' (want ' [cluster]')" + return 1 + fi + _lab_substrate_add "${pat}" "${kind:-generic}" "${cluster}" "${file}" || return 1 + done <"${file}" + + local extra="${KOLLECT_LAB_ALLOWED_CONTEXTS:-}" + if [[ -n "${extra}" ]]; then + local item + for item in ${extra//,/ }; do + [[ -n "${item}" ]] || continue + IFS='=' read -r pat kind cluster rest <<<"${item}" + if [[ -n "${rest:-}" ]]; then + lab_substrate_err "malformed KOLLECT_LAB_ALLOWED_CONTEXTS entry '${item}' (want 'pattern[=substrate[=cluster]]')" + return 1 + fi + _lab_substrate_add "${pat}" "${kind:-generic}" "${cluster:-}" "KOLLECT_LAB_ALLOWED_CONTEXTS" || return 1 + done + fi + + LAB_SUBSTRATE_LOADED=1 + return 0 +} + +lab_substrate_allowlist_summary() { + if [[ "${LAB_SUBSTRATE_LOADED}" -ne 1 ]]; then + lab_substrate_load >/dev/null 2>&1 || true + fi + local i out="" + for ((i = 0; i < ${#LAB_SUBSTRATE_PATTERNS[@]}; i++)); do + out+="${out:+, }${LAB_SUBSTRATE_PATTERNS[i]}(${LAB_SUBSTRATE_KINDS[i]})" + done + printf '%s' "${out:-}" +} + +# Print the substrate kind for an allowlisted context; return 1 when the context is refused. +lab_substrate_resolve() { + local ctx="${1:-}" + LAB_SUBSTRATE_MATCHED_KIND="" + LAB_SUBSTRATE_MATCHED_CLUSTER="" + if [[ "${LAB_SUBSTRATE_LOADED}" -ne 1 ]]; then + lab_substrate_load || return 1 + fi + [[ -n "${ctx}" ]] || return 1 + + local i pat + for ((i = 0; i < ${#LAB_SUBSTRATE_PATTERNS[@]}; i++)); do + pat="${LAB_SUBSTRATE_PATTERNS[i]}" + # The allowlist entry IS the glob, so the right-hand side must stay unquoted. Quoting it + # would make 'kind-*' a literal context name and refuse every real Kind cluster. The + # pattern is validated above (exact name, or one trailing '*' after >= 4 literal chars), + # so this cannot widen into a match-everything wildcard. + # shellcheck disable=SC2053 + if [[ "${ctx}" == $pat ]]; then + LAB_SUBSTRATE_MATCHED_KIND="${LAB_SUBSTRATE_KINDS[i]}" + LAB_SUBSTRATE_MATCHED_CLUSTER="${LAB_SUBSTRATE_CLUSTERS[i]}" + printf '%s' "${LAB_SUBSTRATE_MATCHED_KIND}" + return 0 + fi + done + return 1 +} + +# Optional confirmation only: a cluster-name mismatch REFUSES, it never admits. +_lab_substrate_confirm_cluster() { + local ctx="$1" expected="$2" + [[ -n "${expected}" ]] || return 0 + command -v kubectl >/dev/null 2>&1 || { + lab_substrate_warn "kubectl unavailable; cannot confirm cluster '${expected}' for context ${ctx}" + return 0 + } + local actual + actual="$(kubectl config view -o "jsonpath={.contexts[?(@.name==\"${ctx}\")].context.cluster}" 2>/dev/null || true)" + if [[ -z "${actual}" ]]; then + lab_substrate_warn "could not read the cluster of context ${ctx}; proceeding on the context-name match" + return 0 + fi + if [[ "${actual}" != "${expected}" ]]; then + lab_substrate_err "context '${ctx}' points at cluster '${actual}', not the expected lab cluster '${expected}' — refusing" + return 1 + fi + return 0 +} + +# Gate entry point. Returns 0 when allowed, 2 when refused. +# Pass --offline to skip the live kubectl cluster confirmation (fixtures / meta-tests). +lab_substrate_assert_context() { + local ctx="${1:-}" + local offline=0 + [[ "${2:-}" == "--offline" ]] && offline=1 + + if [[ -z "${ctx}" ]]; then + lab_substrate_err "ambiguous kube context: no current context (refusing; set one explicitly)" + return 2 + fi + + local kind + if ! kind="$(lab_substrate_resolve "${ctx}")"; then + lab_substrate_err "refusing kube context '${ctx}': not on the lab substrate allowlist [$(lab_substrate_allowlist_summary)]" + lab_substrate_err "default-deny — add the context to $(lab_substrate_config_path) or KOLLECT_LAB_ALLOWED_CONTEXTS if it really is a lab cluster" + return 2 + fi + + if [[ "${offline}" -eq 0 ]] && ! _lab_substrate_confirm_cluster "${ctx}" "${LAB_SUBSTRATE_MATCHED_CLUSTER}"; then + return 2 + fi + + lab_substrate_log "kube context ok: ${ctx} (substrate=${kind})" + return 0 +} + +# How images reach the nodes on a given substrate. +lab_substrate_image_delivery() { + case "${1:-}" in + kind) printf 'sideload' ;; + *) printf 'registry' ;; + esac + return 0 +} + +# Non-Kind substrates have no `kind load docker-image` equivalent: the image MUST come from +# a registry at an immutable reference. Refusing loudly here is the whole point — a silent +# fallback to whatever tag the nodes already cached is how a stale-image run gets published +# as evidence. +lab_substrate_require_registry_image() { + local image="${1:-}" substrate="${2:-generic}" + local hint="set KOLLECT_IMAGE=ghcr.io/platformrelay/kollect:v (or @sha256:) and push it before running" + + if [[ -z "${image}" ]]; then + lab_substrate_err "no image configured for a ${substrate} substrate; ${hint}" + return 1 + fi + + local repo tag="" digest="" + if [[ "${image}" == *"@"* ]]; then + repo="${image%%@*}" + digest="${image#*@}" + elif [[ "${image##*/}" == *:* ]]; then + repo="${image%:*}" + tag="${image##*:}" + else + repo="${image}" + fi + + local host="${repo%%/*}" + if [[ "${repo}" != */* ]] || { [[ "${host}" != *.* ]] && [[ "${host}" != *:* ]] && [[ "${host}" != "localhost" ]]; }; then + lab_substrate_err "image '${image}' is not registry-qualified: a ${substrate} cluster cannot side-load a local image (no 'kind load' equivalent); ${hint}" + return 1 + fi + + if [[ -n "${digest}" ]]; then + if [[ "${digest}" =~ ^sha256:[0-9a-f]{64}$ ]]; then + return 0 + fi + lab_substrate_err "image '${image}' has a malformed digest; ${hint}" + return 1 + fi + + if [[ -z "${tag}" ]]; then + lab_substrate_err "image '${image}' has no tag: an untagged image resolves to a mutable 'latest' on a ${substrate} cluster; ${hint}" + return 1 + fi + + if [[ ! "${tag}" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+([-+.][0-9A-Za-z.-]+)?$ ]]; then + lab_substrate_err "image tag '${tag}' is not an immutable release tag (want a pinned v or a @sha256 digest); ${hint}" + return 1 + fi + return 0 +} diff --git a/hack/lab/perf-kind.sh b/hack/lab/perf-kind.sh index ba1c27e0..05c4e2b3 100755 --- a/hack/lab/perf-kind.sh +++ b/hack/lab/perf-kind.sh @@ -1,18 +1,22 @@ #!/usr/bin/env bash -# LAB-H10 / PERF-LAB-01 — Kind-oriented quick pprof workflow entrypoint. +# LAB-H10 / PERF-LAB-01 — quick pprof workflow entrypoint (Kind and bare-metal lab substrates). # SPDX-License-Identifier: MIT # # Phases: idle → converge(N objects) → churn → recover # pprof is disabled in product by default; lab enables via Helm values (pprof.enabled: true). # Access via localhost port-forward only — never create a public Service for pprof. # -# Live path: kubectl -n kollect-system port-forward deploy/kollect-controller-manager 16060:6060 +# Live path: kubectl -n port-forward deploy/-controller-manager 16060:6060 # then curl/go tool pprof against http://127.0.0.1:16060/debug/pprof/... # +# Substrate gate (LAB-DEKIND): the kube context must be on the lab allowlist +# (hack/lab/substrates.conf — kind-* and the kumulus Talos lab). DEFAULT-DENY: any other +# context, including an ambient production one, is refused with exit 2. +# # Exit codes: # 0 OK (dry-run fixture or live run completed) # 1 usage / invalid args -# 2 kube context refused (non-kind / ambiguous without --allow-non-kind) +# 2 kube context refused (not on the substrate allowlist / ambiguous) # 3 preflight / evidence / capture failure (live pprof BLOCKED when unreachable) set -euo pipefail @@ -23,14 +27,16 @@ ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" source "${SCRIPT_DIR}/lib/evidence.sh" # shellcheck source=lib/pprof-capture.sh source "${SCRIPT_DIR}/lib/pprof-capture.sh" +# shellcheck source=lib/substrate.sh +source "${SCRIPT_DIR}/lib/substrate.sh" : "${LAB_PERF_KIND_LOG_PREFIX:=[perf-kind]}" -# Kind/dev defaults (override with --namespace). +# Kind/dev defaults (override with --namespace / --release). readonly LAB_PERF_KIND_DEFAULT_NAMESPACE="${KOLLECT_NAMESPACE:-kollect-system}" +readonly LAB_PERF_KIND_DEFAULT_RELEASE="${KOLLECT_RELEASE:-kollect}" readonly LAB_PERF_KIND_PF_LOCAL_PORT=16060 readonly LAB_PERF_KIND_PF_REMOTE_PORT=6060 -readonly LAB_PERF_KIND_PF_RESOURCE="deploy/kollect-controller-manager" lab_perf_kind_log() { printf '%s %s\n' "${LAB_PERF_KIND_LOG_PREFIX}" "$*"; } lab_perf_kind_err() { printf '%s FAIL: %s\n' "${LAB_PERF_KIND_LOG_PREFIX}" "$*" >&2; } @@ -39,16 +45,17 @@ lab_perf_kind_usage() { cat < [options] -Kind-oriented quick pprof workflow (LAB-H10 / PERF-LAB-01). Runs phased capture: - idle → converge(N objects) → churn → recover +Quick pprof workflow (LAB-H10 / PERF-LAB-01) for Kind AND bare-metal lab substrates. +Runs phased capture: idle → converge(N objects) → churn → recover -Never creates or destroys a cluster. Refuses ambiguous or non-Kind kube contexts unless ---allow-non-kind is set. +Never creates or destroys a cluster. The kube context must be on the lab substrate +allowlist (${SCRIPT_DIR}/substrates.conf: kind-* plus the kumulus Talos lab); DEFAULT-DENY +means any other context — notably an ambient production one — is refused with exit 2. pprof is off in product Helm by default; enable \`pprof.enabled: true\` on the release. Live capture uses localhost port-forward only (never a public Service): - kubectl -n kollect-system port-forward deploy/kollect-controller-manager 16060:6060 + kubectl -n ${NAMESPACE} port-forward ${PF_RESOURCE} 16060:6060 Options: --run-id Required. Stable lab run id → kollect.dev/lab-run=. @@ -59,9 +66,16 @@ Options: --seed Deterministic seed for fixture metadata (default: 1). --artifacts-root Evidence root (default: artifacts/lab under repo). --namespace Manager namespace for port-forward (default: kollect-system). + --release Helm release name; the port-forward target is + deploy/-controller-manager (default: kollect). + The kumulus lab runs release kollect-op1 in namespace kollect-op1. --keep-lab Hint: retain lab namespaces/resources (default: cleanup labeled workload). - --allow-non-kind Skip Kind context gate (maintainer override; still no cluster create/destroy). - --fixture= Offline context fixtures: context-non-kind | context-ambiguous | context-kind + --allow-non-kind Maintainer override for a context that is NOT on the substrate + allowlist. This is an escape hatch, not the way the lab runs — + allowlisted lab contexts need no flag. Still no cluster create/destroy. + --fixture= Offline context fixtures: context-kind | context-kumulus | + context-non-kind | context-ambiguous | context-kumulus-lookalike | + context-prod-lookalike --simulate-interrupt= Dry-run only: stop after named phase; preserve partial profiles. --exercise-cleanup Dry-run only: emit cleanup log for kollect.dev/lab-run label. -h, --help Show this help. @@ -69,8 +83,12 @@ Options: Env: KOLLECT_LAB_PERF_KIND_CONTEXT_FIXTURE Same as --fixture=context-* for meta-tests. KOLLECT_NAMESPACE Default --namespace (kollect-system). + KOLLECT_RELEASE Default --release (kollect). + KOLLECT_LAB_ALLOWED_CONTEXTS Extra allowlist entries (validated; never a wildcard). + KOLLECT_LAB_SUBSTRATES_FILE Alternate allowlist file (validated the same way). -CI verifies the offline --dry-run quick path only. Live Kind capture is maintainer opt-in. +CI verifies the offline --dry-run quick path only. Live capture is maintainer opt-in and +runs on any allowlisted substrate (Kind or the kumulus Talos lab). EOF } @@ -81,6 +99,8 @@ DURATION="60s" SEED=1 ARTIFACTS_ROOT="${ROOT}/artifacts/lab" NAMESPACE="${LAB_PERF_KIND_DEFAULT_NAMESPACE}" +RELEASE="${LAB_PERF_KIND_DEFAULT_RELEASE}" +PF_RESOURCE="deploy/${RELEASE}-controller-manager" KEEP_LAB=0 ALLOW_NON_KIND=0 CONTEXT_FIXTURE="${KOLLECT_LAB_PERF_KIND_CONTEXT_FIXTURE:-}" @@ -114,6 +134,20 @@ lab_perf_kind_current_context() { printf 'kind-kollect-dev' return 0 ;; + context-kumulus | kumulus) + printf 'kumulus-lab' + return 0 + ;; + context-kumulus-lookalike | kumulus-lookalike) + # Near-miss on the lab name: must be refused (the allowlist is exact, not a prefix). + printf 'kumulus-lab-prod' + return 0 + ;; + context-prod-lookalike | prod-lookalike) + # Shape of a real ambient production context — the regression guard for the gate. + printf 'gke_acme-platform-4711_europe-west1_shared-cluster' + return 0 + ;; context-non-kind | non-kind) printf 'gke-prod-example' return 0 @@ -131,6 +165,10 @@ lab_perf_kind_current_context() { kubectl config current-context 2>/dev/null || printf '' } +# LAB-DEKIND: allowlist gate, not a kind-prefix test. Substrates permitted for lab work are +# enumerated in hack/lab/substrates.conf (kind-* and the kumulus Talos lab). Anything else — +# including the maintainer's ambient production context — is refused. --allow-non-kind stays +# as an explicit maintainer escape hatch; it is NOT how allowlisted lab clusters are reached. lab_perf_kind_check_context() { local allow_non_kind="$1" local dry_run="$2" @@ -151,22 +189,20 @@ lab_perf_kind_check_context() { ;; esac - if [[ -z "${ctx}" ]]; then - lab_perf_kind_err "ambiguous kube context: no current context" - return 2 - fi + local assert_args=("${ctx}") + # Fixtures must never reach a live cluster: skip the kubectl cluster-name confirmation. + [[ -n "${fixture}" ]] && assert_args+=(--offline) - if [[ "${ctx}" == kind-* ]]; then - lab_perf_kind_log "kube context ok: ${ctx}" + if lab_substrate_assert_context "${assert_args[@]}"; then return 0 fi if [[ "${allow_non_kind}" -eq 1 ]]; then - lab_perf_kind_log "WARN: non-Kind context ${ctx} allowed via --allow-non-kind" + lab_perf_kind_log "WARN: off-allowlist context '${ctx}' forced via --allow-non-kind (maintainer override)" return 0 fi - lab_perf_kind_err "refusing non-Kind context '${ctx}' (expected kind-*); use --allow-non-kind to override" + lab_perf_kind_err "refusing kube context '${ctx}': add it to the lab substrate allowlist, or pass --allow-non-kind to override deliberately" return 2 } @@ -237,7 +273,8 @@ Run \`${RUN_ID}\` — LAB-H10 / PERF-LAB-01 quick path (capture: ${PERF_KIND_CAP | phases | idle → converge → churn → recover | | profiles index | profiles/index.md | | findings | summary/performance-findings.md | -| port-forward | kubectl -n ${NAMESPACE} port-forward ${LAB_PERF_KIND_PF_RESOURCE} ${LAB_PERF_KIND_PF_LOCAL_PORT}:${LAB_PERF_KIND_PF_REMOTE_PORT} | +| helm release | ${RELEASE} (namespace ${NAMESPACE}) | +| port-forward | kubectl -n ${NAMESPACE} port-forward ${PF_RESOURCE} ${LAB_PERF_KIND_PF_LOCAL_PORT}:${LAB_PERF_KIND_PF_REMOTE_PORT} | pprof: **localhost port-forward only** — never expose via public Service. EOF @@ -274,6 +311,10 @@ lab_perf_kind_main() { NAMESPACE="$2" shift 2 ;; + --release) + RELEASE="$2" + shift 2 + ;; --keep-lab) KEEP_LAB=1 shift @@ -321,6 +362,11 @@ lab_perf_kind_main() { lab_perf_kind_err "invalid --objects (100|500|2000): ${OBJECTS}" exit 1 fi + if ! lab_perf_kind_valid_run_id "${RELEASE}"; then + lab_perf_kind_err "invalid --release (DNS1123 label): ${RELEASE}" + exit 1 + fi + PF_RESOURCE="deploy/${RELEASE}-controller-manager" lab_perf_kind_check_context "${ALLOW_NON_KIND}" "${DRY_RUN}" || exit 2 @@ -349,11 +395,11 @@ lab_perf_kind_main() { if [[ "${DRY_RUN}" -eq 1 ]]; then lab_pprof_portforward_start "${run_dir}" "${NAMESPACE}" \ "${LAB_PERF_KIND_PF_LOCAL_PORT}" "${LAB_PERF_KIND_PF_REMOTE_PORT}" \ - "${LAB_PERF_KIND_PF_RESOURCE}" --fixture + "${PF_RESOURCE}" --fixture else lab_pprof_portforward_start "${run_dir}" "${NAMESPACE}" \ "${LAB_PERF_KIND_PF_LOCAL_PORT}" "${LAB_PERF_KIND_PF_REMOTE_PORT}" \ - "${LAB_PERF_KIND_PF_RESOURCE}" || { + "${PF_RESOURCE}" || { lab_perf_kind_err "port-forward failed; is kollect installed with pprof.enabled in ${NAMESPACE}?" lab_pprof_write_findings_blocked "${run_dir}" "${RUN_ID}" "port-forward to ${LAB_PERF_KIND_PF_RESOURCE} failed" exit 3 diff --git a/hack/lab/substrates.conf b/hack/lab/substrates.conf new file mode 100644 index 00000000..a3d2db07 --- /dev/null +++ b/hack/lab/substrates.conf @@ -0,0 +1,18 @@ +# kollect lab substrate allowlist (LAB-DEKIND). +# +# Lab tooling (pprof capture, load, webhook assertions) refuses ANY kube context that is +# not matched here. This is a DEFAULT-DENY list: an unknown context is refused, never +# allowed. The operator's ambient context is a production cluster — treat every widening +# of this file as a security change. +# +# Format: [expected-cluster] +# context-pattern exact context name, or a single trailing '*' (>= 4 literal chars) +# substrate kind | talos | generic — drives image delivery (see lib/substrate.sh) +# expected-cluster optional; when set and readable it can only REFUSE a mismatch, +# it never admits a context that the pattern did not already match +# +# Additions for a one-off substrate go in KOLLECT_LAB_ALLOWED_CONTEXTS +# ("pattern[=substrate[=cluster]]", comma or space separated) — same validation applies. + +kind-* kind +kumulus-lab talos kumulus diff --git a/hack/test/e2e_webhook_existing_cluster_test.sh b/hack/test/e2e_webhook_existing_cluster_test.sh new file mode 100755 index 00000000..13c1baf4 --- /dev/null +++ b/hack/test/e2e_webhook_existing_cluster_test.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# LAB-DEKIND / U-08: webhook rejection assertions must run against an EXISTING cluster, +# not only a freshly created Kind stack — without changing the CI (Kind) path by one byte +# and without mutating the target cluster. +# Fully offline: kubectl/kind/helm are stubs; any real invocation fails the test. +# SPDX-License-Identifier: MIT +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +SCRIPT="${ROOT}/hack/e2e/webhook-smoke.sh" +TMP="$(mktemp -d "${TMPDIR:-/tmp}/e2e-webhook-existing.XXXXXX")" +trap 'rm -rf "${TMP}"' EXIT + +fail() { + printf 'e2e webhook existing-cluster: %s\n' "$*" >&2 + exit 1 +} +pass() { printf 'ok - %s\n' "$*"; } + +[[ -f "${SCRIPT}" ]] || fail "missing script: ${SCRIPT}" + +PROD_CTX='gke_acme-platform-4711_europe-west1_shared-cluster' +BIN="${TMP}/bin" +mkdir -p "${BIN}" + +# Stub kubectl: records every call, answers from FAKE_* env. +cat >"${BIN}/kubectl" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"${CALLS}" +case "$*" in + *"config current-context"*) + printf '%s\n' "${FAKE_CTX:-}" + ;; + *"config view"*) + printf '%s\n' "${FAKE_CLUSTER:-}" + ;; + *"config use-context"*) ;; + *"get validatingwebhookconfiguration "*) + [[ "${FAKE_VWC:-0}" == "1" ]] || exit 1 + printf 'validatingwebhookconfiguration.admissionregistration.k8s.io/stub\n' + ;; + *"get validatingwebhookconfiguration"*) + printf 'NAME\n' + ;; + *"get crd certificates.cert-manager.io"*) + [[ "${FAKE_CERT_MANAGER:-0}" == "1" ]] || exit 1 + printf 'customresourcedefinition.apiextensions.k8s.io/certificates.cert-manager.io\n' + ;; + *"get certificate"*) + [[ "${FAKE_CERT_MANAGER:-0}" == "1" ]] || exit 1 + ;; + *wait*) + [[ "${FAKE_CERT_MANAGER:-0}" == "1" ]] || exit 1 + ;; + *apply*) + # The invalid sink arrives on stdin; drain it so the heredoc does not SIGPIPE. + case "$*" in + *" -f -"*) cat >/dev/null ;; + esac + if [[ "$*" == *"-f -"* ]]; then + printf 'Error from server (Forbidden): admission webhook denied the request: spec.git is required\n' >&2 + exit 1 + fi + printf 'kollectsnapshotsink.kollect.dev/e2e-snapshot-sink configured\n' + ;; + *) + printf 'e2e webhook existing-cluster: unexpected kubectl: %s\n' "$*" >&2 + exit 97 + ;; +esac +EOF + +# kind/helm must never be touched by the assertion path. +for forbidden in kind helm docker; do + cat >"${BIN}/${forbidden}" <>"\${CALLS}" +exit 96 +EOF +done +chmod +x "${BIN}"/* + +run_smoke() { + local calls="$1" + shift + : >"${calls}" + env -u KUBECONFIG PATH="${BIN}:${PATH}" CALLS="${calls}" "$@" \ + bash "${SCRIPT}" 2>&1 +} + +# --- 1. CI path unchanged: no new env ⇒ kind context switch + mutating apply --- +CALLS_CI="${TMP}/calls-ci" +rc=0 +out="$(run_smoke "${CALLS_CI}" FAKE_CTX=kind-kollect-e2e FAKE_VWC=1 FAKE_CERT_MANAGER=1)" || rc=$? +[[ "${rc}" -eq 0 ]] || fail "default (Kind/CI) path must still pass, rc=${rc}: ${out}" +grep -q 'config use-context kind-kollect-e2e' "${CALLS_CI}" || + fail "default path must still switch to the kind-kollect-e2e context" +grep -Eq '^apply --dry-run=none -f .*snapshot-sink.yaml$' "${CALLS_CI}" || + fail "default path must still apply the valid sample for real (CI parity)" +grep -Eq '^apply .*--dry-run=server -f .*snapshot-sink.yaml' "${CALLS_CI}" && + fail "default path must not downgrade the sample apply to a server dry run" +grep -q 'forbidden' "${CALLS_CI}" && fail "assertion path must not shell out to kind/helm/docker" +pass "default path unchanged: kind context switch + real apply" + +# --- 2. existing-cluster mode refuses a non-allowlisted (production) context --- +CALLS_PROD="${TMP}/calls-prod" +rc=0 +out="$(run_smoke "${CALLS_PROD}" KOLLECT_E2E_EXISTING_CLUSTER=1 FAKE_CTX="${PROD_CTX}" FAKE_VWC=1)" || rc=$? +[[ "${rc}" -eq 2 ]] || + fail "existing-cluster mode MUST refuse a production context with exit 2, got ${rc}: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'allowlist|refus' || + fail "refusal must mention the allowlist: ${out}" +grep -q 'use-context' "${CALLS_PROD}" && + fail "refused run must never switch kube context" +grep -q 'apply' "${CALLS_PROD}" && + fail "refused run must never apply anything (not even a server dry-run)" +pass "existing-cluster mode refuses a production context before touching it" + +# --- 3. existing-cluster mode fails loudly and early when webhooks are not installed --- +CALLS_NOWH="${TMP}/calls-nowh" +rc=0 +out="$(run_smoke "${CALLS_NOWH}" KOLLECT_E2E_EXISTING_CLUSTER=1 FAKE_CTX=kumulus-lab \ + FAKE_CLUSTER=kumulus FAKE_VWC=0 FAKE_CERT_MANAGER=0)" || rc=$? +[[ "${rc}" -eq 4 ]] || + fail "missing webhook stack must exit 4 (precondition), got ${rc}: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'webhook.*(not installed|not present|disabled)' || + fail "missing webhook stack must say so explicitly: ${out}" +grep -q 'apply' "${CALLS_NOWH}" && + fail "must not attempt assertions when the webhook stack is absent" +pass "existing-cluster mode reports a missing webhook stack as a precondition (exit 4)" + +# --- 4. existing-cluster mode asserts read-only against a live release --- +CALLS_OK="${TMP}/calls-ok" +rc=0 +out="$(run_smoke "${CALLS_OK}" KOLLECT_E2E_EXISTING_CLUSTER=1 FAKE_CTX=kumulus-lab \ + FAKE_CLUSTER=kumulus FAKE_VWC=1 FAKE_CERT_MANAGER=0 \ + KOLLECT_RELEASE=kollect-op1 KOLLECT_NAMESPACE=kollect-op1)" || rc=$? +[[ "${rc}" -eq 0 ]] || fail "existing-cluster assertions must pass on an allowlisted lab, rc=${rc}: ${out}" +grep -q 'use-context' "${CALLS_OK}" && fail "existing-cluster mode must not switch kube context" +while IFS= read -r call; do + [[ "${call}" == apply* ]] || continue + [[ "${call}" == *"--dry-run=server"* ]] || + fail "existing-cluster mode must only server-dry-run; mutating call: ${call}" +done <"${CALLS_OK}" +grep -q 'apply --dry-run=server' "${CALLS_OK}" || + fail "existing-cluster mode must still exercise the reject/accept assertions" +grep -q 'kollect-op1' "${CALLS_OK}" || + fail "existing-cluster mode must honour KOLLECT_RELEASE/KOLLECT_NAMESPACE" +grep -q 'forbidden' "${CALLS_OK}" && fail "existing-cluster mode must not shell out to kind/helm/docker" +pass "existing-cluster mode is read-only (server dry-run) and release-parameterised" + +# --- 5. the runbook documents the existing-cluster invocation --- +grep -rq 'KOLLECT_E2E_EXISTING_CLUSTER' "${ROOT}/hack/kind/README.md" "${ROOT}/hack/lab/README.md" || + fail "existing-cluster mode must be documented in hack/kind/README.md or hack/lab/README.md" +pass "existing-cluster mode documented" + +echo "All e2e webhook existing-cluster tests passed." diff --git a/hack/test/lab_image_delivery_meta_test.sh b/hack/test/lab_image_delivery_meta_test.sh new file mode 100755 index 00000000..b7455e04 --- /dev/null +++ b/hack/test/lab_image_delivery_meta_test.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# LAB-DEKIND: image delivery must follow the substrate (offline). +# `kind load docker-image` has no Talos equivalent, so on a non-Kind substrate the operator +# image MUST come from a pinned registry reference. A silent fallback to whatever the nodes +# already cached is how a stale-image run gets published as evidence — fail loud and early. +# SPDX-License-Identifier: MIT +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +COMMON="${ROOT}/hack/kind/common.sh" +TMP="$(mktemp -d "${TMPDIR:-/tmp}/lab-image-delivery.XXXXXX")" +trap 'rm -rf "${TMP}"' EXIT + +fail() { + printf 'lab image delivery meta: %s\n' "$*" >&2 + exit 1 +} +pass() { printf 'ok - %s\n' "$*"; } + +[[ -f "${COMMON}" ]] || fail "missing ${COMMON}" + +BIN="${TMP}/bin" +mkdir -p "${BIN}" + +cat >"${BIN}/kubectl" <<'EOF' +#!/usr/bin/env bash +printf 'kubectl %s\n' "$*" >>"${CALLS}" +case "$*" in + *"config current-context"*) printf '%s\n' "${FAKE_CTX:-}" ;; + *"config view"*) printf '%s\n' "${FAKE_CLUSTER:-}" ;; + *"get deployment"*) printf 'kollect-controller-manager\n' ;; + *logs*) printf 'Starting Controller kollecttarget\n' ;; + *) : ;; +esac +EOF +for tool in kind helm docker task; do + cat >"${BIN}/${tool}" <>"\${CALLS}" +exit 0 +EOF +done +chmod +x "${BIN}"/* + +VALUES="${TMP}/values.yaml" +printf '{}\n' >"${VALUES}" + +# Exercise kollect_install_base with the real library and stubbed tools. +install_base() { + local calls="$1" + shift + : >"${calls}" + env -u KUBECONFIG PATH="${BIN}:${PATH}" CALLS="${calls}" \ + KIND_CLUSTER_WAIT=5s KOLLECT_HELM_TIMEOUT=5s KOLLECT_MANAGER_WAIT=5s \ + KOLLECT_CONTROLLERS_WAIT=10s "$@" \ + bash -c ' + set -uo pipefail + source "$1" + kollect_install_base kollect-e2e "$2" + ' _ "${COMMON}" "${VALUES}" 2>&1 +} + +# --- non-Kind substrate + default local image ⇒ loud, early refusal --- +CALLS_BAD="${TMP}/calls-bad" +rc=0 +out="$(install_base "${CALLS_BAD}" FAKE_CTX=kumulus-lab FAKE_CLUSTER=kumulus)" || rc=$? +[[ "${rc}" -ne 0 ]] || + fail "non-Kind substrate with a local-only image must fail: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'registry|side-load|pin' || + fail "refusal must explain the registry requirement: ${out}" +grep -q '^kind load' "${CALLS_BAD}" && + fail "must never attempt 'kind load' on a non-Kind substrate" +grep -q '^helm' "${CALLS_BAD}" && + fail "must refuse BEFORE installing anything (no helm call)" +pass "non-Kind substrate refuses a local-only image before installing" + +# --- non-Kind substrate + pinned registry tag ⇒ install, never side-load --- +CALLS_OK="${TMP}/calls-ok" +rc=0 +out="$(install_base "${CALLS_OK}" FAKE_CTX=kumulus-lab FAKE_CLUSTER=kumulus \ + KOLLECT_IMAGE=ghcr.io/platformrelay/kollect:v0.17.0)" || rc=$? +[[ "${rc}" -eq 0 ]] || fail "pinned registry image should install on a non-Kind substrate: ${out}" +grep -q '^kind load' "${CALLS_OK}" && fail "must not side-load on a non-Kind substrate" +grep -q '^docker build' "${CALLS_OK}" && fail "must not rebuild the image for a registry install" +grep -q '^helm upgrade' "${CALLS_OK}" || fail "expected a helm install with the pinned image" +grep -q 'ghcr.io/platformrelay/kollect' "${CALLS_OK}" || + fail "helm install must carry the pinned registry repository" +pass "non-Kind substrate installs from the pinned registry reference only" + +# --- Kind substrate keeps building + side-loading (CI path unchanged) --- +CALLS_KIND="${TMP}/calls-kind" +rc=0 +out="$(install_base "${CALLS_KIND}" FAKE_CTX=kind-kollect-e2e)" || rc=$? +[[ "${rc}" -eq 0 ]] || fail "kind path must still work: ${out}" +grep -q '^kind load docker-image' "${CALLS_KIND}" || + fail "kind substrate must still side-load the freshly built image" +grep -q '^helm upgrade' "${CALLS_KIND}" || fail "kind substrate must still helm install" +pass "Kind substrate still builds and side-loads (CI parity)" + +# --- an off-allowlist context is refused outright, before any install work --- +CALLS_PROD="${TMP}/calls-prod" +rc=0 +out="$(install_base "${CALLS_PROD}" \ + FAKE_CTX='gke_acme-platform-4711_europe-west1_shared-cluster' \ + KOLLECT_IMAGE=ghcr.io/platformrelay/kollect:v0.17.0)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "off-allowlist context must not be installed onto: ${out}" +grep -q '^helm' "${CALLS_PROD}" && fail "off-allowlist context must never reach helm" +grep -q '^kind load' "${CALLS_PROD}" && fail "off-allowlist context must never reach kind load" +pass "off-allowlist context refused before any install work" + +echo "All lab image delivery meta tests passed." diff --git a/hack/test/lab_perf_kind_meta_test.sh b/hack/test/lab_perf_kind_meta_test.sh index 02ce7615..23cae0a0 100755 --- a/hack/test/lab_perf_kind_meta_test.sh +++ b/hack/test/lab_perf_kind_meta_test.sh @@ -114,24 +114,68 @@ out="$(run_perf --run-id "${RUN_ID}" --seed 1 \ [[ "${rc}" -ne 0 ]] || fail "ambiguous context must be refused: ${out}" pass "refuses ambiguous kube context" -# --- allow-non-kind bypasses check (still dry-run offline) --- +# --- LAB-DEKIND: the kumulus Talos lab is allowlisted WITHOUT --allow-non-kind --- +# A --dry-run with an explicit fixture still runs the full context gate (only a +# fixture-less --dry-run skips it), so this exercises the allowlist itself. +rc=0 +out="$(run_perf --dry-run --run-id "${RUN_ID}-kumulus" --seed 1 \ + --artifacts-root "${OUT_ROOT}" --fixture=context-kumulus 2>&1)" || rc=$? +[[ "${rc}" -eq 0 ]] || + fail "kumulus-lab context must be accepted without --allow-non-kind (rc=${rc}): ${out}" +printf '%s\n' "${out}" | grep -Eqi 'context ok|substrate=talos' || + fail "kumulus acceptance should log the substrate: ${out}" +pass "kumulus-lab accepted without --allow-non-kind (allowlist, not bypass)" + +# --- LAB-DEKIND regression guard: production-lookalike context is still REFUSED --- +rc=0 +out="$(run_perf --dry-run --run-id "${RUN_ID}-prod" --seed 1 \ + --artifacts-root "${OUT_ROOT}" --fixture=context-prod-lookalike 2>&1)" || rc=$? +[[ "${rc}" -eq 2 ]] || + fail "production-lookalike context MUST be refused with exit 2, got ${rc}: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'allowlist|refus' || + fail "production refusal must mention the allowlist: ${out}" +pass "production-lookalike context refused with exit 2 (safety gate held)" + +# Near-miss on the lab name must not slip through a prefix match. +rc=0 +out="$(run_perf --dry-run --run-id "${RUN_ID}-lookalike" --seed 1 \ + --artifacts-root "${OUT_ROOT}" --fixture=context-kumulus-lookalike 2>&1)" || rc=$? +[[ "${rc}" -eq 2 ]] || + fail "kumulus-lab-prod MUST be refused with exit 2, got ${rc}: ${out}" +pass "kumulus-lab-prod refused (allowlist is exact, not a prefix)" + +# --- allow-non-kind remains a maintainer override for an off-allowlist context --- if ! run_perf --dry-run --allow-non-kind --run-id "${RUN_ID}-nonkind" --seed 1 \ - --artifacts-root "${OUT_ROOT}" >/dev/null 2>&1; then - fail "--allow-non-kind + --dry-run should succeed offline" + --artifacts-root "${OUT_ROOT}" --fixture=context-prod-lookalike >/dev/null 2>&1; then + fail "--allow-non-kind + --dry-run should still override the gate for a maintainer" fi -pass "--allow-non-kind bypasses kind context gate in dry-run" +pass "--allow-non-kind remains an explicit maintainer override" # --- port-forward target: kollect-system + kollect-controller-manager (not kollect-dev-manager) --- grep -q 'kollect-system' "${PERF_KIND}" || fail "perf-kind.sh must reference namespace kollect-system" -grep -q 'kollect-controller-manager' "${PERF_KIND}" || - fail "perf-kind.sh must reference kollect-controller-manager" grep -q 'kollect-dev-manager' "${PERF_KIND}" && fail "perf-kind.sh must not reference broken kollect-dev-manager target" grep -q '16060:6060' "${PERF_KIND}" || fail "perf-kind.sh must use local:remote port-forward form 16060:6060" +grep -Eq 'deploy/.*kollect-system|port-forward' "${RUN_DIR}/summary.md" || + fail "summary.md must document the port-forward command" +grep -q 'deploy/kollect-controller-manager' "${RUN_DIR}/summary.md" || + fail "default port-forward target must stay deploy/kollect-controller-manager" pass "port-forward targets kollect-system/kollect-controller-manager" +# --- LAB-DEKIND: the manager Deployment is release-scoped, not hardcoded --- +# The kumulus lab runs Helm release kollect-op1 in namespace kollect-op1, so its manager is +# deploy/kollect-op1-controller-manager — a hardcoded name makes the live path unrunnable. +REL_OUT="${TMP}/release-test" +run_perf --dry-run --run-id rel-test --seed 1 --release kollect-op1 --namespace kollect-op1 \ + --artifacts-root "${REL_OUT}" >/dev/null 2>&1 || fail "--release dry-run failed" +grep -q 'deploy/kollect-op1-controller-manager' "${REL_OUT}/rel-test/summary.md" || + fail "--release must retarget the port-forward Deployment (kollect-op1)" +grep -q 'kollect-op1' "${REL_OUT}/rel-test/summary.md" || + fail "--namespace must be reflected in the summary port-forward command" +pass "--release/--namespace retarget the port-forward for a non-default install" + # --- live vs dry-run honesty: dry-run uses .stub; live path must not silently stub --- stub_count="$(find "${RUN_DIR}/profiles" -name '*.pb.gz.stub' 2>/dev/null | wc -l | tr -d ' ')" ((stub_count > 0)) || fail "dry-run must write .pb.gz.stub placeholders" diff --git a/hack/test/lab_substrate_meta_test.sh b/hack/test/lab_substrate_meta_test.sh new file mode 100755 index 00000000..f4843266 --- /dev/null +++ b/hack/test/lab_substrate_meta_test.sh @@ -0,0 +1,169 @@ +#!/usr/bin/env bash +# LAB-DEKIND: substrate allowlist + image-delivery policy meta-tests (offline). +# The allowlist is the safety gate that keeps lab tooling off production clusters. +# Every assertion here is a regression guard for that gate — default-deny must hold. +# SPDX-License-Identifier: MIT +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +LIB="${ROOT}/hack/lab/lib/substrate.sh" +CONF="${ROOT}/hack/lab/substrates.conf" +TMP="$(mktemp -d "${TMPDIR:-/tmp}/lab-substrate-meta.XXXXXX")" +trap 'rm -rf "${TMP}"' EXIT + +fail() { + printf 'lab substrate meta: %s\n' "$*" >&2 + exit 1 +} +pass() { printf 'ok - %s\n' "$*"; } + +[[ -f "${LIB}" ]] || fail "missing library: ${LIB}" +[[ -f "${CONF}" ]] || fail "missing checked-in allowlist: ${CONF}" + +# A production context that must NEVER be admitted (shape of the operator's ambient context). +PROD_CTX='gke_acme-platform-4711_europe-west1_shared-cluster' + +# Poison PATH: the allowlist must decide without talking to any cluster. +cat >"${TMP}/kubectl" <<'EOF' +#!/usr/bin/env bash +echo "lab substrate meta: unexpected live kubectl: $*" >&2 +exit 99 +EOF +chmod +x "${TMP}/kubectl" +export PATH="${TMP}:${PATH}" + +# Run one expression against a freshly sourced library (env overrides passed as VAR=VAL). +sub() { + local expr="$1" + shift + env -u KUBECONFIG "$@" bash -c ' + set -uo pipefail + source "$1" + shift + eval "$*" + ' _ "${LIB}" "${expr}" +} + +# --- checked-in allowlist admits the two known lab substrates --- +out="$(sub 'lab_substrate_resolve kind-kollect-e2e' 2>&1)" || + fail "kind-* context must be allowlisted: ${out}" +[[ "${out}" == "kind" ]] || fail "kind-* context must resolve to substrate 'kind', got: ${out}" +pass "kind-* context allowlisted as substrate kind" + +out="$(sub 'lab_substrate_resolve kumulus-lab' 2>&1)" || + fail "kumulus-lab must be allowlisted: ${out}" +[[ "${out}" == "talos" ]] || fail "kumulus-lab must resolve to substrate 'talos', got: ${out}" +pass "kumulus-lab context allowlisted as substrate talos" + +# --- default-deny: exact match, not prefix --- +rc=0 +out="$(sub "lab_substrate_resolve kumulus-lab-prod" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "kumulus-lab-prod must be REFUSED (allowlist is exact, not a prefix): ${out}" +pass "kumulus-lab-prod refused (exact match, not prefix)" + +rc=0 +out="$(sub "lab_substrate_resolve ${PROD_CTX}" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "production context must be REFUSED: ${out}" +pass "production-lookalike context refused" + +rc=0 +out="$(sub 'lab_substrate_resolve ""' 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "empty context must be refused: ${out}" +pass "empty context refused" + +# --- assert_context returns 2 (refusal) and names the context + default-deny --- +rc=0 +out="$(sub "lab_substrate_assert_context ${PROD_CTX}" 2>&1)" || rc=$? +[[ "${rc}" -eq 2 ]] || fail "assert_context must exit 2 on a non-allowlisted context, got ${rc}: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'allowlist|refus' || + fail "refusal must mention the allowlist: ${out}" +printf '%s\n' "${out}" | grep -Fq "${PROD_CTX}" || + fail "refusal must name the refused context: ${out}" +pass "assert_context refuses production context with exit 2 and a named reason" + +rc=0 +out="$(sub 'lab_substrate_assert_context kumulus-lab --offline' 2>&1)" || rc=$? +[[ "${rc}" -eq 0 ]] || fail "assert_context must admit kumulus-lab, got ${rc}: ${out}" +pass "assert_context admits kumulus-lab" + +# --- env additions: valid pattern extends, unsafe pattern fails closed --- +out="$(sub 'lab_substrate_resolve talos-scratch' KOLLECT_LAB_ALLOWED_CONTEXTS='talos-scratch=talos' 2>&1)" || + fail "valid env addition must extend the allowlist: ${out}" +[[ "${out}" == "talos" ]] || fail "env addition must carry its substrate kind, got: ${out}" +pass "KOLLECT_LAB_ALLOWED_CONTEXTS extends the allowlist" + +for bad in '*' '**' '*-prod' 'k*' 'a' 'ctx name'; do + rc=0 + out="$(sub "lab_substrate_resolve ${PROD_CTX}" KOLLECT_LAB_ALLOWED_CONTEXTS="${bad}" 2>&1)" || rc=$? + [[ "${rc}" -ne 0 ]] || + fail "unsafe env pattern '${bad}' must not admit ${PROD_CTX}: ${out}" +done +pass "unsafe wildcard/short env patterns never admit a production context" + +rc=0 +out="$(sub 'lab_substrate_load' KOLLECT_LAB_ALLOWED_CONTEXTS='*' 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "wildcard-only env pattern must fail the load closed: ${out}" +printf '%s\n' "${out}" | grep -Eqi 'pattern' || fail "load failure must name the bad pattern: ${out}" +pass "wildcard-only env pattern fails the load closed" + +# --- file override is validated too (it is itself a bypass vector) --- +printf '%s\n' '* generic' >"${TMP}/wide.conf" +rc=0 +out="$(sub "lab_substrate_resolve ${PROD_CTX}" KOLLECT_LAB_SUBSTRATES_FILE="${TMP}/wide.conf" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "wildcard-only file entry must not admit ${PROD_CTX}: ${out}" +pass "wildcard-only entry in an override file fails closed" + +rc=0 +out="$(sub "lab_substrate_resolve kind-x" KOLLECT_LAB_SUBSTRATES_FILE="${TMP}/missing.conf" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "missing allowlist file must fail closed: ${out}" +pass "missing allowlist file fails closed" + +printf '# comment only\n\n' >"${TMP}/empty.conf" +rc=0 +out="$(sub "lab_substrate_resolve kind-x" KOLLECT_LAB_SUBSTRATES_FILE="${TMP}/empty.conf" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "empty allowlist must admit nothing: ${out}" +pass "empty allowlist admits nothing" + +# --- checked-in allowlist itself carries no over-broad entry --- +while IFS= read -r line; do + line="${line%%#*}" + read -r pat _rest <<<"${line}" || true + [[ -n "${pat:-}" ]] || continue + out="$(sub "lab_substrate_valid_pattern '${pat}' && echo VALID" 2>&1)" || true + [[ "${out}" == "VALID" ]] || + fail "checked-in allowlist entry '${pat}' fails the pattern validator" +done <"${CONF}" +pass "every checked-in allowlist entry passes the pattern validator" + +# --- image delivery policy: no silent stale image on a non-Kind substrate --- +out="$(sub 'lab_substrate_require_registry_image ghcr.io/platformrelay/kollect:v0.17.0 talos' 2>&1)" || + fail "pinned registry tag must be accepted: ${out}" +pass "pinned registry tag accepted for a non-Kind substrate" + +out="$(sub 'lab_substrate_require_registry_image ghcr.io/platformrelay/kollect@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef talos' 2>&1)" || + fail "digest-pinned image must be accepted: ${out}" +pass "digest-pinned image accepted for a non-Kind substrate" + +for bad_image in \ + 'kollect-controller-manager:dev' \ + 'kollect-controller-manager' \ + 'ghcr.io/platformrelay/kollect:latest' \ + 'ghcr.io/platformrelay/kollect:dev' \ + 'ghcr.io/platformrelay/kollect'; do + rc=0 + out="$(sub "lab_substrate_require_registry_image ${bad_image} talos" 2>&1)" || rc=$? + [[ "${rc}" -ne 0 ]] || + fail "image '${bad_image}' must be refused on a non-Kind substrate" + printf '%s\n' "${out}" | grep -Eqi 'registry|tag|pin' || + fail "refusal of '${bad_image}' must explain the registry/tag requirement: ${out}" +done +pass "unqualified / mutable-tag images refused loudly on a non-Kind substrate" + +# Kind keeps side-loading: the policy must not break the CI path. +out="$(sub 'lab_substrate_image_delivery kind' 2>&1)" || fail "kind delivery lookup failed: ${out}" +[[ "${out}" == "sideload" ]] || fail "kind substrate must use sideload delivery, got: ${out}" +out="$(sub 'lab_substrate_image_delivery talos' 2>&1)" || fail "talos delivery lookup failed: ${out}" +[[ "${out}" == "registry" ]] || fail "non-kind substrate must use registry delivery, got: ${out}" +pass "image delivery mode is sideload on Kind and registry elsewhere" + +echo "All lab substrate meta tests passed." From f6e34d1837cc8188e72a0d988f73b6797b17503a Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 08:22:00 +0200 Subject: [PATCH 2/6] :construction_worker: ci: run the lab substrate allowlist guards in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hack/test/lab_*_meta_test.sh was never wired into a workflow, so the offline lab harness suite only ever ran on a maintainer's laptop. That suite now contains the regression guard for the substrate allowlist (a production-lookalike context must stay refused) and for the non-Kind image-delivery policy — a guard nobody runs is not a guard. Add it to the lint job, and add a webhook-existing-cluster meta job to E2E extended that proves both halves of U-08: the Kind path still switches context and applies for real, and the existing-cluster path refuses an off-allowlist context without touching it. --- .github/workflows/ci.yaml | 5 +++++ .github/workflows/e2e-extended.yaml | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 479bcbc5..4940ed2a 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -142,6 +142,11 @@ jobs: run: bash hack/test/core_events_rbac_test.sh - name: Verify changelog-sync release guard (fbb5196a3 regression lock) run: bash hack/test/changelog_sync_release_guard_test.sh + # LAB-DEKIND: this suite is the enforcement mechanism for the lab substrate allowlist + # (default-deny on kube contexts) and the non-Kind image-delivery policy. Unwired, the + # regression guard against pointing lab tooling at a production cluster never runs. + - name: Lab harness offline meta-tests (substrate allowlist, image delivery, pprof) + run: bash hack/test/lab_harness_meta_suite.sh - name: Run Sonar SECURITY remediation meta-tests run: | shopt -s nullglob diff --git a/.github/workflows/e2e-extended.yaml b/.github/workflows/e2e-extended.yaml index 4de21192..27b8a5e3 100644 --- a/.github/workflows/e2e-extended.yaml +++ b/.github/workflows/e2e-extended.yaml @@ -38,6 +38,19 @@ jobs: - name: Verify multitenant assert diagnostics + collecting wait run: bash hack/test/e2e_mt_repro_harden_test.sh + webhook-existing-cluster-meta: + name: webhook-existing-cluster-meta + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + # LAB-DEKIND / U-08: proves the Kind path is byte-for-byte unchanged AND that the + # existing-cluster mode refuses a non-allowlisted context without touching it. + - name: Verify webhook smoke runs against an existing cluster read-only + run: bash hack/test/e2e_webhook_existing_cluster_test.sh + gate: name: gate runs-on: ubuntu-latest From 4c13c7d45e73e76aa14adf6e0f0e797d0680a40c Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 09:19:12 +0200 Subject: [PATCH 3/6] :bug: fix(lab): close a fail-open in the substrate allowlist (pathname expansion) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `for item in ${extra//,/ }` is subject to PATHNAME expansion, not just word splitting. Run from a directory containing files named after refused contexts, `KOLLECT_LAB_ALLOWED_CONTEXTS='*'` was rewritten into those filenames BEFORE lab_substrate_valid_pattern ever saw them — the allowlist loaded rc=0 with `gke-prod-example` and `kumulus-lab-prod` admitted, and perf-kind.sh walked straight past the gate. Two documented guarantees were false: "there is no 'allow everything' value" and "a wildcard-only pattern fails the load closed". I audited the glob on the matching side (the SC2053 suppression, which is bounded and correct) and missed the one on the input side. Split with `read -a`, which never globs, and iterate the array quoted. The parser additionally runs under `set -f`, so no future edit inside it can turn a pattern into a list of filenames; the two layers are independent and both tested. The regression test was worthless: it ran in the caller's CWD, so creating a three-character directory flipped it from red to green, and it passed in CI only because api/, bin/ and ui/ are shorter than the validator's 4-character minimum. It now runs from a decoy directory seeded with exactly the names that must stay refused (an EMPTY dir would hide the bug — an unmatched glob stays literal and is correctly rejected), asserts the REASON of each refusal rather than only its exit code, and calls the inner parser directly with globbing on so the noglob wrapper cannot mask a missing quote. Verified both ways: reintroducing the unquoted loop turns the suite red and names the leaked entry. Two smaller inconsistencies in the same file: - Digest references were accepted, and the remediation hint recommended them, while kollect_helm_install rejects any `@` because the chart renders repository:tag. Advice that cannot work is worse than no advice: refuse digests where the operator can act on it, and stop suggesting them. - lab_substrate_allowlist_summary swallowed a failed load and printed the partially-parsed list, which appears inside refusal messages. A load that failed closed admits nothing; say that instead. --- hack/lab/README.md | 4 + hack/lab/lib/substrate.sh | 46 ++++++++++-- hack/test/lab_substrate_meta_test.sh | 108 +++++++++++++++++++++++++-- 3 files changed, 143 insertions(+), 15 deletions(-) diff --git a/hack/lab/README.md b/hack/lab/README.md index 523fed26..fef14f1f 100644 --- a/hack/lab/README.md +++ b/hack/lab/README.md @@ -30,6 +30,10 @@ a registry at an immutable reference (`ghcr.io/platformrelay/kollect:v`) or mutable tag (`:dev`, `:latest`, untagged) is rejected before anything is installed rather than silently reusing whatever the nodes already cached. +Digest references (`@sha256:…`) are also rejected — not because they are weak pins, but +because the chart renders `repository:tag` (`charts/kollect/templates/_helpers.tpl`, +`kollect.image`) and cannot install one. The policy refuses what the install path refuses. + ## Preflight (LAB-H01) ```sh diff --git a/hack/lab/lib/substrate.sh b/hack/lab/lib/substrate.sh index 2ad5c33c..2f7cf7d1 100644 --- a/hack/lab/lib/substrate.sh +++ b/hack/lab/lib/substrate.sh @@ -80,7 +80,24 @@ _lab_substrate_add() { # Parse the checked-in allowlist plus KOLLECT_LAB_ALLOWED_CONTEXTS. Any unsafe or malformed # entry fails the whole load — a partially-parsed allowlist is not a safety boundary. +# +# The parser runs with pathname expansion DISABLED. Every individual expansion below is +# already quoted or read-split, but this makes the property structural: no future edit inside +# the parser can turn an allowlist pattern into a list of filenames from the caller's CWD. lab_substrate_load() { + local rc=0 restore=0 + if [[ "$-" != *f* ]]; then + set -f + restore=1 + fi + _lab_substrate_load_impl || rc=$? + if [[ "${restore}" -eq 1 ]]; then + set +f + fi + return "${rc}" +} + +_lab_substrate_load_impl() { LAB_SUBSTRATE_PATTERNS=() LAB_SUBSTRATE_KINDS=() LAB_SUBSTRATE_CLUSTERS=() @@ -111,8 +128,15 @@ lab_substrate_load() { local extra="${KOLLECT_LAB_ALLOWED_CONTEXTS:-}" if [[ -n "${extra}" ]]; then + # `for item in ${extra}` would be subject to PATHNAME EXPANSION, not just word + # splitting: from a directory containing a file named after a production context, the + # shell would rewrite `*` into that filename BEFORE the validator ever saw it and the + # allowlist would fail OPEN. Split with `read -a`, which never globs, and iterate the + # array quoted. Never reintroduce an unquoted expansion here. + local -a items=() + IFS=', ' read -r -a items <<<"${extra}" local item - for item in ${extra//,/ }; do + for item in "${items[@]}"; do [[ -n "${item}" ]] || continue IFS='=' read -r pat kind cluster rest <<<"${item}" if [[ -n "${rest:-}" ]]; then @@ -129,7 +153,12 @@ lab_substrate_load() { lab_substrate_allowlist_summary() { if [[ "${LAB_SUBSTRATE_LOADED}" -ne 1 ]]; then - lab_substrate_load >/dev/null 2>&1 || true + # Never present a partially-parsed list as if it were the allowlist: a load that failed + # closed admits NOTHING, and saying so is the honest answer inside a refusal message. + if ! lab_substrate_load >/dev/null 2>&1; then + printf '' + return 0 + fi fi local i out="" for ((i = 0; i < ${#LAB_SUBSTRATE_PATTERNS[@]}; i++)); do @@ -229,7 +258,11 @@ lab_substrate_image_delivery() { # as evidence. lab_substrate_require_registry_image() { local image="${1:-}" substrate="${2:-generic}" - local hint="set KOLLECT_IMAGE=ghcr.io/platformrelay/kollect:v (or @sha256:) and push it before running" + # Only recommend a form the install path can actually use. The chart renders + # `repository:tag` (charts/kollect/templates/_helpers.tpl "kollect.image"), so a digest + # reference is NOT installable — do not suggest one here and then reject it in + # kollect_helm_install two calls later. + local hint="set KOLLECT_IMAGE=ghcr.io/platformrelay/kollect:v and push it before running" if [[ -z "${image}" ]]; then lab_substrate_err "no image configured for a ${substrate} substrate; ${hint}" @@ -254,10 +287,9 @@ lab_substrate_require_registry_image() { fi if [[ -n "${digest}" ]]; then - if [[ "${digest}" =~ ^sha256:[0-9a-f]{64}$ ]]; then - return 0 - fi - lab_substrate_err "image '${image}' has a malformed digest; ${hint}" + # A digest is the strongest pin, but the kollect chart cannot render one, so accepting it + # here would only defer the failure to helm. Refuse it where the operator can act on it. + lab_substrate_err "image '${image}' is digest-pinned: the kollect chart renders repository:tag and cannot install a digest; ${hint}" return 1 fi diff --git a/hack/test/lab_substrate_meta_test.sh b/hack/test/lab_substrate_meta_test.sh index f4843266..509caaab 100755 --- a/hack/test/lab_substrate_meta_test.sh +++ b/hack/test/lab_substrate_meta_test.sh @@ -32,16 +32,34 @@ EOF chmod +x "${TMP}/kubectl" export PATH="${TMP}:${PATH}" +# DECOY CWD. The allowlist parser must never let the shell expand a pattern against the +# filesystem: `for item in ${extra//,/ }` performs PATHNAME expansion, so from a directory +# holding files named after refused contexts, `KOLLECT_LAB_ALLOWED_CONTEXTS='*'` would be +# rewritten into those names before validation ever ran — the allowlist fails OPEN. +# Every `sub` call therefore runs from a directory seeded with exactly the names that must +# stay refused, so a reintroduced glob turns this suite RED instead of green. (An *empty* +# CWD would hide the bug: an unmatched `*` stays literal and is correctly rejected.) +DECOY="${TMP}/decoy" +mkdir -p "${DECOY}" +: >"${DECOY}/${PROD_CTX}" +: >"${DECOY}/kumulus-lab-prod" +: >"${DECOY}/gke-prod-example" +: >"${DECOY}/alpha" +: >"${DECOY}/bravo" +: >"${DECOY}/ninechars" +mkdir -p "${DECOY}/art" + # Run one expression against a freshly sourced library (env overrides passed as VAR=VAL). sub() { local expr="$1" shift env -u KUBECONFIG "$@" bash -c ' set -uo pipefail - source "$1" - shift + cd "$1" || exit 1 + source "$2" + shift 2 eval "$*" - ' _ "${LIB}" "${expr}" + ' _ "${DECOY}" "${LIB}" "${expr}" } # --- checked-in allowlist admits the two known lab substrates --- @@ -103,8 +121,68 @@ pass "unsafe wildcard/short env patterns never admit a production context" rc=0 out="$(sub 'lab_substrate_load' KOLLECT_LAB_ALLOWED_CONTEXTS='*' 2>&1)" || rc=$? [[ "${rc}" -ne 0 ]] || fail "wildcard-only env pattern must fail the load closed: ${out}" -printf '%s\n' "${out}" | grep -Eqi 'pattern' || fail "load failure must name the bad pattern: ${out}" -pass "wildcard-only env pattern fails the load closed" +# Assert the REASON, not just the exit code: a load that failed for an unrelated cause would +# otherwise look like a working safety gate. +printf '%s\n' "${out}" | grep -Eqi "refusing unsafe context pattern '\*'" || + fail "load failure must name the rejected pattern '*': ${out}" +pass "wildcard-only env pattern fails the load closed, naming the pattern" + +# --- the allowlist parser must not perform PATHNAME EXPANSION on its input --- +# Runs from a CWD seeded with files named after contexts that must stay refused. +for glob in '*' '?????????' '[a-z]*' '*prod*' '*-prod'; do + rc=0 + out="$(sub 'lab_substrate_load && lab_substrate_allowlist_summary' \ + KOLLECT_LAB_ALLOWED_CONTEXTS="${glob}" 2>&1)" || rc=$? + [[ "${rc}" -ne 0 ]] || + fail "glob '${glob}' must fail the load closed, not expand against the filesystem: ${out}" + for decoy in "${PROD_CTX}" kumulus-lab-prod gke-prod-example alpha bravo; do + printf '%s\n' "${out}" | grep -Fq "${decoy}(" && + fail "glob '${glob}' expanded to filesystem entry '${decoy}' — allowlist failed OPEN: ${out}" + done +done +pass "env patterns are never pathname-expanded against the working directory" + +# The check above passes if EITHER layer holds (the `set -f` wrapper, or quoted expansions in +# the parser). Exercise the inner parser DIRECTLY, with globbing left on, so removing one +# layer cannot silently leave the other untested. +for glob in '*' '?????????' '[a-z]*'; do + rc=0 + out="$(sub "set +f; _lab_substrate_load_impl && lab_substrate_allowlist_summary" \ + KOLLECT_LAB_ALLOWED_CONTEXTS="${glob}" 2>&1)" || rc=$? + [[ "${rc}" -ne 0 ]] || + fail "parser itself must not glob '${glob}' even with pathname expansion enabled: ${out}" + for decoy in "${PROD_CTX}" kumulus-lab-prod gke-prod-example alpha bravo; do + printf '%s\n' "${out}" | grep -Fq "${decoy}(" && + fail "parser expanded '${glob}' to '${decoy}' with globbing on — the quoted-expansion fix is missing: ${out}" + done +done +pass "the parser is glob-safe on its own, not only behind the noglob wrapper" + +# Structural backstop: the loop must iterate a read-split array, never a bare expansion. +grep -Eq 'for item in "\$\{items\[@\]\}"' "${LIB}" || + fail "the KOLLECT_LAB_ALLOWED_CONTEXTS loop must iterate a quoted array" +# Code only — the explanatory comment above the loop quotes the dangerous form on purpose. +grep -Eq '^[^#]*for [a-z]+ in \$\{extra' "${LIB}" && + fail "unquoted expansion of KOLLECT_LAB_ALLOWED_CONTEXTS reintroduced (pathname expansion)" +pass "no unquoted expansion of the allowlist input remains" + +for glob in '*' '?????????' '[a-z]*'; do + for decoy in "${PROD_CTX}" kumulus-lab-prod gke-prod-example; do + rc=0 + out="$(sub "lab_substrate_resolve ${decoy}" KOLLECT_LAB_ALLOWED_CONTEXTS="${glob}" 2>&1)" || rc=$? + [[ "${rc}" -ne 0 ]] || + fail "glob '${glob}' admitted '${decoy}' via pathname expansion — allowlist failed OPEN" + done +done +pass "no filesystem-derived context is ever admitted" + +# Same for the file-override parser (its fields are read, never re-expanded): '[a-z]*' would +# expand to the decoy filenames if the parser globbed, and fails validation if it does not. +printf '%s\n' '[a-z]* generic' >"${DECOY}/glob.conf" +rc=0 +out="$(sub "lab_substrate_resolve ${PROD_CTX}" KOLLECT_LAB_SUBSTRATES_FILE="${DECOY}/glob.conf" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "file-sourced entries must not expand into other filenames: ${out}" +pass "file-sourced patterns are not pathname-expanded either" # --- file override is validated too (it is itself a bypass vector) --- printf '%s\n' '* generic' >"${TMP}/wide.conf" @@ -140,9 +218,23 @@ out="$(sub 'lab_substrate_require_registry_image ghcr.io/platformrelay/kollect:v fail "pinned registry tag must be accepted: ${out}" pass "pinned registry tag accepted for a non-Kind substrate" -out="$(sub 'lab_substrate_require_registry_image ghcr.io/platformrelay/kollect@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef talos' 2>&1)" || - fail "digest-pinned image must be accepted: ${out}" -pass "digest-pinned image accepted for a non-Kind substrate" +# Digest references are refused, not recommended: the chart renders `repository:tag` +# (charts/kollect/templates/_helpers.tpl "kollect.image"), so kollect_helm_install cannot +# install one. Accepting a digest here while the install path rejects it two calls later +# would hand the operator advice that cannot work. +DIGEST='ghcr.io/platformrelay/kollect@sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef' +rc=0 +out="$(sub "lab_substrate_require_registry_image ${DIGEST} talos" 2>&1)" || rc=$? +[[ "${rc}" -ne 0 ]] || fail "digest-pinned image must be refused (the chart cannot render it): ${out}" +printf '%s\n' "${out}" | grep -Eqi 'digest' || + fail "digest refusal must say why: ${out}" +pass "digest-pinned image refused, matching what the chart can actually install" + +# No hint text may recommend a form the install path rejects. +out="$(sub 'lab_substrate_require_registry_image kollect:dev talos' 2>&1 || true)" +printf '%s\n' "${out}" | grep -Fq '@sha256' && + fail "guidance must not recommend a digest the chart cannot render: ${out}" +pass "refusal guidance recommends only an installable form" for bad_image in \ 'kollect-controller-manager:dev' \ From 3c0026b6a58df5fec402f58e1bc8151ea5ce0481 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 09:19:22 +0200 Subject: [PATCH 4/6] :bug: fix(lab): restore the exit-3 BLOCKED path and keep cleanup off unknown clusters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit perf-kind.sh still referenced LAB_PERF_KIND_PF_RESOURCE on the port-forward failure branch after the rename to PF_RESOURCE, so `set -u` aborted the script with exit 1 instead of the documented exit 3 — and lab_pprof_write_findings_blocked never ran, meaning the BLOCKED findings register designed to prevent a silent stale result was the one thing a failed run did not produce. `shellcheck --severity=warning` cannot catch this (an all-caps unset name is assumed environment-supplied), so the gap was a missing test, not a missing linter: the port-forward-start failure path is now exercised end to end and asserts both the exit code and the register contents. While in that path: the BLOCKED remediation text hardcoded `kollect-system` and `deploy/kollect-controller-manager`, so a run that passed --release/--namespace told the operator to look in the wrong place. It now reports the target that was actually used. Cleanup runs `kubectl delete all,cm,secret,sa,role,rolebinding -A -l kollect.dev/lab-run=` — a cluster-wide write. It was reachable through --allow-non-kind, so an override meant for profiling an unusual substrate also authorized deleting across every namespace of a cluster the allowlist does not recognise. Gate it on the allowlist instead: off-allowlist runs skip cleanup and print the label to remove by hand. --- hack/lab/lib/pprof-capture.sh | 5 +- hack/lab/perf-kind.sh | 34 ++++++++++-- hack/test/lab_perf_kind_meta_test.sh | 79 ++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 4 deletions(-) diff --git a/hack/lab/lib/pprof-capture.sh b/hack/lab/lib/pprof-capture.sh index dad19d74..a67b4c5b 100644 --- a/hack/lab/lib/pprof-capture.sh +++ b/hack/lab/lib/pprof-capture.sh @@ -439,6 +439,9 @@ lab_pprof_write_findings_blocked() { local run_dir="$1" local run_id="$2" local reason="$3" + # Remediation command for the target that was ACTUALLY used. Callers that know their + # release/namespace pass it; the default only fits a default-release install. + local pf_command="${4:-kubectl -n kollect-system port-forward deploy/kollect-controller-manager 16060:6060}" mkdir -p "${run_dir}/summary" cat >"${run_dir}/summary/performance-findings.md" </dev/null 2>&1; then lab_perf_kind_log "cleanup: deleting resources labeled kollect.dev/lab-run=${run_id}" kubectl delete all,cm,secret,sa,role,rolebinding -A \ @@ -401,13 +425,16 @@ lab_perf_kind_main() { "${LAB_PERF_KIND_PF_LOCAL_PORT}" "${LAB_PERF_KIND_PF_REMOTE_PORT}" \ "${PF_RESOURCE}" || { lab_perf_kind_err "port-forward failed; is kollect installed with pprof.enabled in ${NAMESPACE}?" - lab_pprof_write_findings_blocked "${run_dir}" "${RUN_ID}" "port-forward to ${LAB_PERF_KIND_PF_RESOURCE} failed" + lab_pprof_write_findings_blocked "${run_dir}" "${RUN_ID}" \ + "port-forward to ${PF_RESOURCE} in ${NAMESPACE} failed" \ + "$(lab_perf_kind_pf_command)" exit 3 } if ! lab_pprof_wait_ready "${LAB_PERF_KIND_PF_LOCAL_PORT}"; then lab_perf_kind_err "pprof endpoint unreachable at http://127.0.0.1:${LAB_PERF_KIND_PF_LOCAL_PORT}/debug/pprof/" lab_pprof_write_findings_blocked "${run_dir}" "${RUN_ID}" \ - "pprof endpoint unreachable after port-forward (enable pprof.enabled and check manager pod)" + "pprof endpoint unreachable after port-forward (enable pprof.enabled and check manager pod)" \ + "$(lab_perf_kind_pf_command)" lab_pprof_portforward_stop "${run_dir}" || true exit 3 fi @@ -420,7 +447,8 @@ lab_perf_kind_main() { if [[ "${fixture}" -eq 0 && "${phase_rc}" -eq 3 ]]; then lab_perf_kind_err "live pprof capture failed in phase ${phase}; partial tree preserved" lab_pprof_write_findings_blocked "${run_dir}" "${RUN_ID}" \ - "live capture failed in phase ${phase} (curl/go tool pprof)" + "live capture failed in phase ${phase} (curl/go tool pprof)" \ + "$(lab_perf_kind_pf_command)" lab_pprof_portforward_stop "${run_dir}" || true trap - INT TERM exit 3 diff --git a/hack/test/lab_perf_kind_meta_test.sh b/hack/test/lab_perf_kind_meta_test.sh index 23cae0a0..25a7b644 100755 --- a/hack/test/lab_perf_kind_meta_test.sh +++ b/hack/test/lab_perf_kind_meta_test.sh @@ -215,6 +215,85 @@ if [[ -d "${LIVE_DIR}/profiles" ]]; then fi pass "live path refuses silent stub capture (BLOCKED on unreachable pprof)" +# --- port-forward FAILURE path: documented exit 3 + BLOCKED findings actually written --- +# Distinct from "endpoint unreachable": here the port-forward never starts (no kubectl on +# PATH). This branch shipped broken — it referenced a variable the CLI no longer defines, so +# `set -u` aborted with exit 1 and the BLOCKED findings file was never written. shellcheck +# structurally cannot catch that (an all-caps unset name is assumed environment-supplied), +# so it needs a behavioural test. +PF_FAIL_ROOT="${TMP}/pf-start-failure" +rc=0 +pf_out="$(cd "${TMP}" && env -u KUBECONFIG PATH=/usr/bin:/bin bash "${PERF_KIND}" \ + --run-id pf-start-failure --seed 1 --artifacts-root "${PF_FAIL_ROOT}" \ + --fixture=context-kind 2>&1)" || rc=$? +[[ "${rc}" -eq 3 ]] || + fail "port-forward start failure must exit 3 (documented), got ${rc}: ${pf_out}" +printf '%s\n' "${pf_out}" | grep -Eqi 'unbound variable|command not found' && + fail "port-forward failure path must not abort on a shell error: ${pf_out}" +PF_FINDINGS="${PF_FAIL_ROOT}/pf-start-failure/summary/performance-findings.md" +[[ -f "${PF_FINDINGS}" ]] || + fail "port-forward failure must still write the BLOCKED findings register" +grep -q 'BLOCKED' "${PF_FINDINGS}" || + fail "findings register must record BLOCKED for a failed port-forward" +grep -Eqi 'port-forward' "${PF_FINDINGS}" || + fail "findings register must record the port-forward reason" +pass "port-forward start failure exits 3 and writes the BLOCKED findings register" + +# --- BLOCKED remediation text must name the release/namespace that was actually targeted --- +PF_REL_ROOT="${TMP}/pf-release-hint" +rc=0 +pf_out="$(cd "${TMP}" && env -u KUBECONFIG PATH=/usr/bin:/bin bash "${PERF_KIND}" \ + --run-id pf-release-hint --seed 1 --artifacts-root "${PF_REL_ROOT}" \ + --release kollect-op1 --namespace kollect-op1 --fixture=context-kind 2>&1)" || rc=$? +[[ "${rc}" -eq 3 ]] || fail "release-scoped port-forward failure must exit 3, got ${rc}" +REL_FINDINGS="${PF_REL_ROOT}/pf-release-hint/summary/performance-findings.md" +grep -q 'kollect-op1-controller-manager' "${REL_FINDINGS}" || + fail "BLOCKED remediation must name the targeted deployment, not a hardcoded default" +grep -q 'kollect-system' "${REL_FINDINGS}" && + fail "BLOCKED remediation must not tell the operator to look in the wrong namespace" +pass "BLOCKED remediation reflects --release/--namespace" + +# --- cluster-wide cleanup is NOT unlocked by --allow-non-kind --- +# The override exists to profile an unusual substrate. Cleanup runs +# `kubectl delete all,cm,secret,sa,role,rolebinding -A -l kollect.dev/lab-run=`, which is +# a cluster-wide write: it must stay gated on the ALLOWLIST, not on the override flag. +CLEAN_BIN="${TMP}/cleanup-bin" +mkdir -p "${CLEAN_BIN}" +cat >"${CLEAN_BIN}/kubectl" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"${DELETE_LOG}" +EOF +chmod +x "${CLEAN_BIN}/kubectl" + +cleanup_probe() { + local allowlisted="$1" log="$2" + : >"${log}" + env -u KUBECONFIG PATH="${CLEAN_BIN}:${PATH}" DELETE_LOG="${log}" bash -c ' + set -uo pipefail + source "$1" + CONTEXT_ALLOWLISTED="$2" + lab_perf_kind_cleanup "$3" cleanup-probe 0 + ' _ "${PERF_KIND}" "${allowlisted}" "${TMP}/cleanup-run" 2>&1 +} + +DENY_LOG="${TMP}/delete-denied.log" +out="$(cleanup_probe 0 "${DENY_LOG}")" +grep -q 'delete' "${DENY_LOG}" && + fail "cleanup must NOT issue a cluster-wide delete on a non-allowlisted context: $(cat "${DENY_LOG}")" +printf '%s\n' "${out}" | grep -Eqi 'skip|refus|not.*allowlist' || + fail "skipped cleanup must say so (and name the label to clean by hand): ${out}" +printf '%s\n' "${out}" | grep -Eq 'kollect.dev/lab-run' || + fail "skipped cleanup must tell the operator which label to remove manually: ${out}" +pass "--allow-non-kind does not authorize cluster-wide cleanup deletes" + +ALLOW_LOG="${TMP}/delete-allowed.log" +cleanup_probe 1 "${ALLOW_LOG}" >/dev/null +grep -q 'delete' "${ALLOW_LOG}" || + fail "cleanup must still run on an allowlisted context: $(cat "${ALLOW_LOG}")" +grep -q 'kollect.dev/lab-run=cleanup-probe' "${ALLOW_LOG}" || + fail "cleanup must stay scoped to the lab-run label" +pass "cleanup still runs, label-scoped, on an allowlisted context" + # --- --duration wired into index / CPU note --- DUR_OUT="${TMP}/duration-test" run_perf --dry-run --run-id dur-test --seed 7 --duration 45s \ From 8163cf72d8b03d885a50fe0327c8ec07b35862fd Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 09:19:28 +0200 Subject: [PATCH 5/6] :lock: fix(e2e): keep operator input out of the piped webhook manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KOLLECT_E2E_TEST_NAMESPACE was interpolated into a heredoc that is piped straight to the API server, so an operator-supplied value could carry YAML into the manifest. Restore the quoted heredoc, pass the namespace via `kubectl -n`, and validate it as a DNS-1123 label before anything is built — with a test that an injection-shaped value is refused before any apply. Also correct the CI comment added with this suite: the lab harness meta-tests were not completely unwired — hack/docs/verify.sh runs them — but that workflow is path-filtered and hack/lab/** is not in the filter, so a change to the harness itself never triggered them. --- .github/workflows/ci.yaml | 6 +++-- hack/e2e/webhook-smoke.sh | 11 +++++++-- .../test/e2e_webhook_existing_cluster_test.sh | 24 ++++++++++++++++++- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4940ed2a..a1e0ea42 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -143,8 +143,10 @@ jobs: - name: Verify changelog-sync release guard (fbb5196a3 regression lock) run: bash hack/test/changelog_sync_release_guard_test.sh # LAB-DEKIND: this suite is the enforcement mechanism for the lab substrate allowlist - # (default-deny on kube contexts) and the non-Kind image-delivery policy. Unwired, the - # regression guard against pointing lab tooling at a production cluster never runs. + # (default-deny on kube contexts) and the non-Kind image-delivery policy. It was only + # reachable via hack/docs/verify.sh in the path-filtered docs workflow, whose filter + # does not include hack/lab/** — so a change to the harness itself never triggered it. + # Run it here, unconditionally, where the guard actually protects something. - name: Lab harness offline meta-tests (substrate allowlist, image delivery, pprof) run: bash hack/test/lab_harness_meta_suite.sh - name: Run Sonar SECURITY remediation meta-tests diff --git a/hack/e2e/webhook-smoke.sh b/hack/e2e/webhook-smoke.sh index e6d86f63..76827c8f 100755 --- a/hack/e2e/webhook-smoke.sh +++ b/hack/e2e/webhook-smoke.sh @@ -37,6 +37,14 @@ _kind_require kubectl _log() { echo "[webhook-smoke] $*"; } +# KOLLECT_E2E_TEST_NAMESPACE reaches a manifest that is piped to the API server. Validate it +# as a DNS-1123 label so it can never carry YAML — the manifest itself stays a quoted +# heredoc and the namespace is supplied via `kubectl -n`, so nothing is interpolated into it. +if [[ ! "${TEST_NAMESPACE}" =~ ^[a-z0-9]([-a-z0-9]{0,61}[a-z0-9])?$ ]]; then + echo "invalid KOLLECT_E2E_TEST_NAMESPACE '${TEST_NAMESPACE}' (want a DNS-1123 label)" >&2 + exit 1 +fi + # Mutating applies are the Kind-only half of this scenario. Against an existing lab cluster # the same admission decision is observable with a server-side dry run, so the accept # assertion is downgraded to --dry-run=server there (explicit --dry-run=none on Kind keeps @@ -95,12 +103,11 @@ fi _log "Expect validating webhook to reject git snapshot sink without git block..." set +e -reject_out="$(kubectl apply --dry-run=server -f - 2>&1 <&1 <<'EOF' apiVersion: kollect.dev/v1alpha1 kind: KollectSnapshotSink metadata: name: webhook-reject-test - namespace: ${TEST_NAMESPACE} spec: type: git endpoint: https://example.com/repo.git diff --git a/hack/test/e2e_webhook_existing_cluster_test.sh b/hack/test/e2e_webhook_existing_cluster_test.sh index 13c1baf4..a7dca8b2 100755 --- a/hack/test/e2e_webhook_existing_cluster_test.sh +++ b/hack/test/e2e_webhook_existing_cluster_test.sh @@ -149,7 +149,29 @@ grep -q 'kollect-op1' "${CALLS_OK}" || grep -q 'forbidden' "${CALLS_OK}" && fail "existing-cluster mode must not shell out to kind/helm/docker" pass "existing-cluster mode is read-only (server dry-run) and release-parameterised" -# --- 5. the runbook documents the existing-cluster invocation --- +# --- 5. the test namespace never reaches the manifest as raw text --- +# KOLLECT_E2E_TEST_NAMESPACE is operator-supplied and the manifest is piped to the API +# server; a value carrying YAML must be refused, not templated in. +CALLS_INJ="${TMP}/calls-inj" +for bad_ns in 'default +spec: + type: git' 'default"; kubectl delete ns default #' 'Default' '-leading-dash'; do + rc=0 + out="$(run_smoke "${CALLS_INJ}" KOLLECT_E2E_EXISTING_CLUSTER=1 FAKE_CTX=kumulus-lab \ + FAKE_CLUSTER=kumulus FAKE_VWC=1 KOLLECT_E2E_TEST_NAMESPACE="${bad_ns}")" || rc=$? + [[ "${rc}" -eq 1 ]] || + fail "invalid KOLLECT_E2E_TEST_NAMESPACE must be refused with exit 1, got ${rc}: ${out}" + grep -q 'apply' "${CALLS_INJ}" && + fail "invalid namespace must be refused before any apply: $(cat "${CALLS_INJ}")" +done +pass "invalid KOLLECT_E2E_TEST_NAMESPACE is refused before the manifest is built" + +# The manifest itself must stay a NON-interpolating heredoc. +grep -q "<<'EOF'" "${SCRIPT}" || + fail "the piped manifest must use a quoted heredoc (no shell interpolation into YAML)" +pass "piped manifest uses a non-interpolating heredoc" + +# --- 6. the runbook documents the existing-cluster invocation --- grep -rq 'KOLLECT_E2E_EXISTING_CLUSTER' "${ROOT}/hack/kind/README.md" "${ROOT}/hack/lab/README.md" || fail "existing-cluster mode must be documented in hack/kind/README.md or hack/lab/README.md" pass "existing-cluster mode documented" From 898dce28ff041000cd7311116eb9a020502db709 Mon Sep 17 00:00:00 2001 From: Konrad Heimel Date: Sun, 16 Aug 2026 09:28:40 +0200 Subject: [PATCH 6/6] :test_tube: test(lab): cover live-interrupt cleanup on an allowlisted context Gating cleanup on the substrate allowlist could have disarmed Ctrl-C cleanup on a legitimate live lab run. --simulate-interrupt is dry-run only, so nothing covered the live path: lab_perf_kind_on_interrupt with DRY_RUN=0 and an allowlisted context must still issue the label-scoped delete. It does; now it stays that way. --- hack/test/lab_perf_kind_meta_test.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/hack/test/lab_perf_kind_meta_test.sh b/hack/test/lab_perf_kind_meta_test.sh index 25a7b644..d94f3f87 100755 --- a/hack/test/lab_perf_kind_meta_test.sh +++ b/hack/test/lab_perf_kind_meta_test.sh @@ -294,6 +294,24 @@ grep -q 'kollect.dev/lab-run=cleanup-probe' "${ALLOW_LOG}" || fail "cleanup must stay scoped to the lab-run label" pass "cleanup still runs, label-scoped, on an allowlisted context" +# The allowlist gate must not silently disarm Ctrl-C cleanup on a legitimate live lab run. +# (--simulate-interrupt is dry-run only, so it does not cover this path.) +INT_LOG="${TMP}/delete-interrupt.log" +: >"${INT_LOG}" +env -u KUBECONFIG PATH="${CLEAN_BIN}:${PATH}" DELETE_LOG="${INT_LOG}" bash -c ' + set -uo pipefail + source "$1" + CONTEXT_ALLOWLISTED=1 + DRY_RUN=0 + KEEP_LAB=0 + RUN_ID=interrupt-probe + PERF_KIND_RUN_DIR="$2" + lab_perf_kind_on_interrupt +' _ "${PERF_KIND}" "${TMP}/cleanup-run" >/dev/null 2>&1 +grep -q 'kollect.dev/lab-run=interrupt-probe' "${INT_LOG}" || + fail "interrupting a live allowlisted run must still clean up: $(cat "${INT_LOG}")" +pass "live interrupt still cleans up on an allowlisted context" + # --- --duration wired into index / CPU note --- DUR_OUT="${TMP}/duration-test" run_perf --dry-run --run-id dur-test --seed 7 --duration 45s \