Repository navigation
CP-43292: make the webhook server optional - #847
Merged
Merged
Conversation
evan-cz
force-pushed
the
CP-43292/webhook-server-optional
branch
from
June 17, 2026 14:29
f7c9ee7 to
6cf4945
Compare
evan-cz
marked this pull request as draft
June 17, 2026 16:06
evan-cz
force-pushed
the
CP-43292/webhook-server-optional
branch
2 times, most recently
from
June 17, 2026 17:33
bc85a93 to
1840acc
Compare
evan-cz
marked this pull request as ready for review
June 17, 2026 17:38
evan-cz
enabled auto-merge
June 17, 2026 17:38
evan-cz
force-pushed
the
CP-43292/webhook-server-optional
branch
2 times, most recently
from
June 17, 2026 18:38
0f2530a to
b4b1010
Compare
roberthocking
approved these changes
Jun 17, 2026
auto-merge was automatically disabled
June 17, 2026 19:16
Head branch was modified
evan-cz
force-pushed
the
CP-43292/webhook-server-optional
branch
from
June 17, 2026 19:16
b4b1010 to
465cb0b
Compare
…nds: Command-based KUTTL steps across the pre-existing tests/kuttl suites silently no-op for two independent reasons, so their e2e assertions have never actually run (the suites report PASS without executing kubectl/helm/curl): 1. `command:` + `args:` — kuttl's Command has no `args` field and `command` is a single string, so only the bare first word ran. 2. `spec:` wrapper — kuttl's TestStep has a top-level `commands` field and no `spec`, so nesting `commands:` under `spec:` made the whole step a no-op. (Confirmed empirically with a marker probe against the pinned kuttl v0.25.0.) This converts the 11 affected step files in alloy-clustered-test, collector-test, collector-comprehensive-test, webhook-test, and webhook-comprehensive-test to valid kuttl, preserving each step's intent: - Drop `spec:`; put `commands:` at the top level. Use `script:` / single-string `command:` instead of `command:`+`args:`. - Reference shared agent components by label (app.kubernetes.io/name= aggregator|webhook-server|ksm) and discover the install namespace, instead of the stale hardcoded `cz-agent-*` names that don't exist in the current chart. Exclude the alloy co-install (instance!=cz-alloy-ct). - Background `kubectl port-forward` (foreground would hang the step) and land the curl body to a file before `head` so a failed fetch fails the step. - Walk up to the repo root before the alloy helm calls (kuttl runs scripts with CWD = the step dir); pre-poll selector-based `kubectl wait`s. - Set explicit per-command `timeout:` on long-running commands (sleeps, helm --wait, and every `kubectl wait`): these suites' kuttl-test.yaml put `timeout` under `spec:`, which kuttl ignores, so the 30s per-command default otherwise applied and clamped them. - Consolidate webhook-comprehensive/03 into one `script:` so $WEBHOOK_METRICS survives across the curl/grep/count chain (kuttl runs each entry in its own shell). Once the suites actually run, they need a working agent image, which exposed a pre-existing gap: the test-matrix-k8s CI path installs an unpullable image (kind-overrides pins an intentionally-invalid tag and the freshly-built untested image was never wired in), so agent pods never reached Ready. It was false-green only because the suites were hollow. Fix CI to make the built image usable: - Add a `kind-load` Makefile target. - In both test-matrix-k8s jobs, log in to ghcr, pull the freshly-built untested image for the matrix arch, retag it as the tag the chart installs, kind-load it into the node, and run the kind phases explicitly (up -> load -> install/wait -> helm-test-kuttl -> teardown) so the image is present before install. All five suites verified to genuinely execute and PASS against a local kind cluster using this exact sequence, with a negative control confirming the now-live assertions go red on a real regression. The CP-43292 webhookserver-* suites are handled on that branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A customer cannot run the CloudZero Agent in production with the admission
webhook server enabled, and needs the informer/KubeState-based path without it.
The only existing toggle, insightsController.enabled, did not cleanly disable the
webhook server: the config-loader Job stuck in ContainerCreating because it
mounted a webhook ConfigMap that was never created when disabled, and many
webhook resources rendered even when disabled, leaving dangling objects and a
perpetually-firing critical "webhook down" alert.
Implementation Approach:
Introduce a single ternary toggle, components.webhookServer.enabled
(true | false | "auto", default "auto" which currently maps to true), resolved by
one helper, cloudzero-agent.webhookServer.enabled. Every webhook resource, the
config-loader, and the validator consult that helper, so enablement is decided in
exactly one place and disabling removes the entire webhook surface with no
dangling objects. The legacy insightsController.enabled (default changed from true
to null) takes precedence when explicitly set, preserving backward compatibility,
and is marked deprecated in the schema. "auto" maps to enabled today behind a
commented seam to later follow the KubeState configuration. Default behavior is
unchanged.
Functional Requirements:
1. Operators must be able to disable the webhook server with a single value, with
the entire webhook surface removed.
Added the resolver helper in helm/templates/_webhook_helpers.tpl and gated every
webhook-component resource on it: the webhook Deployment, Service, ConfigMap,
HPA, and PDB; the ValidatingWebhookConfiguration; the init-cert Job, ClusterRole,
ClusterRoleBinding, and ServiceAccount; the TLS certificate, issuer, and secret;
the backfill job; and the agent and alloy webhook scrape-config blocks
(agent-cm.yaml, _alloy_helpers.tpl).
2. Disabling must not break the agent (the original ContainerCreating/CrashLoop bug).
In config-loader-job.yaml the webhook config volume and volumeMount are gated on
the resolver, and the --config-webhook argument is passed as an empty string when
disabled (NewSettings("") is benign), so the loader no longer mounts an absent
ConfigMap. No Go change is required.
3. The validator must not probe a webhook that is not running.
validator-cm.yaml forces the post-start webhook_server_reachable check to
"disabled" when the resolver is off, using a deepCopy of the checks map so
.Values is never mutated in place.
4. A disabled but monitored install must not page operators about the absent webhook.
The four webhook alerts in prometheusrule.yaml (CloudZeroWebhookDown, NoEvents,
ServerHighLatency, RemoteWriteFailures) are individually gated on the resolver,
leaving the non-webhook alerts in the shared rule group intact.
5. The new field must be schema-valid and the legacy field marked deprecated.
Added components.webhookServer.enabled (null | boolean | "auto") and changed
insightsController.enabled to (null | boolean) with deprecated: true in the source
helm/values.schema.yaml, then regenerated helm/values.schema.json. Regenerated the
helmless embedded defaults (app/functions/helmless/default-values.yaml) and the
helm template snapshots.
Validation:
- helm-unittest: a resolver truth-table (components.webhookServer.enabled crossed
with the legacy insightsController.enabled) plus per-resource presence/absence
assertions across the full webhook surface; the full suite passes (72 suites,
626 tests).
- Schema: added pass/fail fixtures for components.webhookServer.enabled
(true/false/auto/null pass, invalid-type fail); make helm-test-schema passes.
- KUTTL e2e (new suites): these run under `make kind-test`, which installs the
full default release first and then runs every suite against the same Kind node.
Each new suite installs its own differently-configured release alongside that
default one, which surfaced two CI-only failures, both addressed by
tests/kuttl/webhookserver-min-footprint.yaml without changing the behavior under
test. (a) Two full-size stacks exceeded the Kind node's allocatable CPU, so the
new pods stayed Pending; the overlay shrinks each test release to one replica
with tiny CPU/memory requests. (b) The validator's api_key_valid pre-start check
retries HTTP calls against the deliberately-invalid test key, adding minutes of
latency to the server's init container; the overlay disables that single check
(orthogonal to webhook gating) so init completes promptly. The per-step kuttl
timeouts are 600s and helm --wait is 8m to absorb a busy CI node. Both suites
were run to completion co-resident with the full default release: the disabled
fresh-install suite passes -- webhook Deployment, Service, and
ValidatingWebhookConfiguration absent, config-loader Job reaches Complete, agent
pod Ready with zero container restarts; the enabled-to-disabled upgrade suite
passes -- the ValidatingWebhookConfiguration and init-cert
ClusterRole/ClusterRoleBinding are present after the enabled install and pruned
after the upgrade.
- Lint, format, and generate checks pass in CI; default-on behavior verified
unchanged via a render diff.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
evan-cz
force-pushed
the
CP-43292/webhook-server-optional
branch
from
June 17, 2026 19:35
465cb0b to
f6b69d2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A customer cannot run the CloudZero Agent in production with the admission webhook server enabled, and needs the informer/KubeState-based path without it. The only existing toggle, insightsController.enabled, did not cleanly disable the webhook server: the config-loader Job stuck in ContainerCreating because it mounted a webhook ConfigMap that was never created when disabled, and many webhook resources rendered even when disabled, leaving dangling objects and a perpetually-firing critical "webhook down" alert.
Implementation Approach:
Introduce a single ternary toggle, components.webhookServer.enabled (true | false | "auto", default "auto" which currently maps to true), resolved by one helper, cloudzero-agent.webhookServer.enabled. Every webhook resource, the config-loader, and the validator consult that helper, so enablement is decided in exactly one place and disabling removes the entire webhook surface with no dangling objects. The legacy insightsController.enabled (default changed from true to null) takes precedence when explicitly set, preserving backward compatibility, and is marked deprecated in the schema. "auto" maps to enabled today behind a commented seam to later follow the KubeState configuration. Default behavior is unchanged.
Functional Requirements:
Operators must be able to disable the webhook server with a single value, with the entire webhook surface removed.
Added the resolver helper in helm/templates/_webhook_helpers.tpl and gated every webhook-component resource on it: the webhook Deployment, Service, ConfigMap, HPA, and PDB; the ValidatingWebhookConfiguration; the init-cert Job, ClusterRole, ClusterRoleBinding, and ServiceAccount; the TLS certificate, issuer, and secret; the backfill job; and the agent and alloy webhook scrape-config blocks (agent-cm.yaml, _alloy_helpers.tpl).
Disabling must not break the agent (the original ContainerCreating/CrashLoop bug).
In config-loader-job.yaml the webhook config volume and volumeMount are gated on the resolver, and the --config-webhook argument is passed as an empty string when disabled (NewSettings("") is benign), so the loader no longer mounts an absent ConfigMap. No Go change is required.
The validator must not probe a webhook that is not running.
validator-cm.yaml forces the post-start webhook_server_reachable check to "disabled" when the resolver is off, using a deepCopy of the checks map so .Values is never mutated in place.
A disabled but monitored install must not page operators about the absent webhook.
The four webhook alerts in prometheusrule.yaml (CloudZeroWebhookDown, NoEvents, ServerHighLatency, RemoteWriteFailures) are individually gated on the resolver, leaving the non-webhook alerts in the shared rule group intact.
The new field must be schema-valid and the legacy field marked deprecated.
Added components.webhookServer.enabled (null | boolean | "auto") and changed insightsController.enabled to (null | boolean) with deprecated: true in the source helm/values.schema.yaml, then regenerated helm/values.schema.json. Regenerated the helmless embedded defaults (app/functions/helmless/default-values.yaml) and the helm template snapshots.
Validation: