diff --git a/kubernetes/apps/monitoring-rules/kustomization.yaml b/kubernetes/apps/monitoring-rules/kustomization.yaml index cd1c17a1..e2418631 100644 --- a/kubernetes/apps/monitoring-rules/kustomization.yaml +++ b/kubernetes/apps/monitoring-rules/kustomization.yaml @@ -55,6 +55,10 @@ resources: # infrastructure/network-policies/external-secrets-ns.yaml in the same change, # or it scrapes nothing. See the file. - external-secrets-podmonitor.yaml + # The alert built on what that PodMonitor collects. Split from it on purpose: + # the `for:` window had to be chosen against real series, and Prometheus held + # none for this metric until the scrape landed. + - prometheusrule-external-secrets.yaml # Flux's own control plane. The controllers were never scraped, so nothing # about Flux's behaviour was queryable. Scrape only: per-object readiness is # not on this endpoint at the pinned controller versions and needs diff --git a/kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml b/kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml new file mode 100644 index 00000000..ee21aa8b --- /dev/null +++ b/kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml @@ -0,0 +1,227 @@ +# Alert when an ExternalSecret stops syncing. +# +# This is the second half of REN-103. The first half (#994) added the PodMonitor +# and the `monitoring` ingress grant that make the series exist at all; before +# that, `externalsecret_status_condition` held 0 series and a key that silently +# stopped refreshing was invisible to Prometheus, to Alertmanager and to anyone +# looking at a dashboard. The alert is split out because its `for:` window had to +# be chosen against real data rather than guessed. +# +# `exported_namespace`, NOT `namespace`. The metric carries its own `namespace` +# label (the ExternalSecret's), and the PodMonitor attaches a target label of the +# same name (the controller pod's, always `external-secrets`). Prometheus resolves +# that collision by renaming the metric's label to `exported_namespace`, so on +# every one of the ~105 live series `namespace="external-secrets"`. Grouping by +# `namespace` would therefore still fire correctly and still be useless: every +# failing ExternalSecret in the cluster would be reported as living in +# `external-secrets`. ESO's own shipped Grafana dashboard does group by +# `namespace` — that is right for the chart's ServiceMonitor, which scrapes ESO +# through its own Service and injects no conflicting target label, and wrong here. +# This is a general trap for the PodMonitor-instead-of-ServiceMonitor trade this +# repo keeps making: any metric carrying its own `namespace`, `pod` or `container` +# label gets it shadowed. +# +# `honorLabels: true` on the PodMonitor endpoint would keep the clean label name +# instead, and is deliberately not used: it drops the target labels, so nothing +# records which controller replica reported, and it would make this the only +# monitor in the directory doing so. +# +# A consequence worth knowing rather than fixing: aggregating by +# `exported_namespace` leaves these alerts with no `namespace` label at all, and +# Alertmanager's route groups on ['alertname', 'cluster', 'namespace']. So every +# failing ExternalSecret arrives in ONE email listing all of them, each named by +# its own annotation, instead of one email per namespace. For a fleet this size +# that is the better shape. +# +# The obvious worry about that shape — that a SECOND ExternalSecret failing while +# the first alert is already firing gets swallowed by the open group — does not +# hold, and the setting that decides it is `group_interval: 5m` on the root route +# in apps/monitoring/helmrelease-prometheus.yaml. `group_interval` governs +# notification of a group whose alert SET has changed, which is a different thing +# from `repeat_interval: 12h`, which governs re-sending an unchanged group. A +# second failure therefore produces a fresh email naming both within 5 minutes. +# This rule routes to the root `email` receiver: it is `severity: warning` and +# carries no `source = "flux"`, so it matches none of the four child routes. +# +# The inhibit rules are also checked and do not touch these alerts. All three key +# on `equal: ['namespace', 'alertname']` (or `['namespace']`), and no critical or +# warning alert in the cluster shares this `alertname`. In particular +# OpenBaoQuorumLost does NOT inhibit this rule — different alertname — so an +# OpenBao outage can surface both. That is the intended order rather than a +# problem: OpenBaoQuorumLost is critical at 2m and arrives first, well before +# this rule's 15m. +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + name: external-secrets + namespace: monitoring + labels: + # kube-prometheus-stack's ruleSelector matches on this label; without it the + # rule is created but never loaded into Prometheus. + release: kube-prometheus-stack +spec: + groups: + - name: external-secrets.rules + rules: + # Measured against the live Prometheus before this rule was proposed for + # merge, through the apiserver services/proxy read path: + # + # count(externalsecret_status_condition) = 105 + # count(count by (exported_namespace,name) (...)) = 53 + # count(count by (exported_namespace,name) (...{condition="Ready"})) = 52 + # count(count by (exported_namespace) (...)) = 20 + # count(...{condition="Ready",status="False"}) = 52 + # the ExternalSecretSyncFailing expression below = EMPTY + # + # So 105 = 52 ExternalSecrets x 2 Ready polarities + 1 Deleted tombstone, + # across 20 namespaces, and THIS RULE ARRIVES SILENT: it matches nothing + # in the cluster as it stands, which is what makes it safe to load. The + # 52 on the line below is a count, not an estimate. + # + # `== 1` IS LOAD-BEARING, not defensive. ESO writes BOTH polarities of the + # Ready condition on every reconcile — the true one at 1 and the opposite + # at 0 (esmetrics.UpdateExternalSecretCondition) — so + # `{condition="Ready",status="False"}` is present with value 0 for every + # healthy ExternalSecret. A bare selector would match all 52 of them. + # + # `max`, not the `sum` the vendor dashboard uses. Both give 1 today, + # because ESO runs a single controller replica. During a Deployment + # rollout Prometheus can briefly scrape the old and the new pod at once, + # and `sum(...) == 1` would then evaluate to 2 and silently STOP matching + # — the alert would resolve itself in the middle of a failure. `max` is 0 + # or 1 whatever the replica count. + # + # The gauge, not `externalsecret_sync_calls_error`. The counter was the + # other candidate, on the theory that a counter cannot flap the way a + # condition gauge might. Reading the controller says the opposite on both + # halves: the counter increments on strictly MORE paths than the condition + # does — a failed Get of the ExternalSecret itself, a failed Get or label + # Patch of the target Secret, an unresolvable SecretStore — and none of + # those call markAsFailed(), so none of them set Ready=False. A cache blip + # or an apiserver hiccup moves the counter and nothing else. It is the + # noisier signal, not the quieter one. And the gauge does + # not flap, because it is set to 1 only by markAsFailed() and cleared by + # the next successful reconcile. There is no unset state in between. + # + # The live numbers say the same thing, measured after #994 landed: + # `externalsecret_sync_calls_error` has exactly one series in the whole + # cluster, `ai/wiki-anthropic`, at 8 errors over 47 calls — and that + # ExternalSecret is Ready=True and has never been otherwise in the + # observation window. So on today's data the counter carries 8 events that + # correspond to no sync failure at all, and the gauge carries none. + # + # That observation window is now long enough to mean something, and it is + # the answer to the only open question this rule was held back for — how + # much the gauge flaps in ordinary operation. Measured 2026-09-26T08:02Z, + # over the 464 consecutive scrapes since #994 started collecting at + # 00:18Z: + # + # min_over_time(up{job="monitoring/external-secrets"}[8h]) = 1 + # sum(increase(externalsecret_sync_calls_total[8h])) = 959 + # count(increase(externalsecret_sync_calls_total[8h]) > 0) = 52 + # count(max_over_time( + # externalsecret_status_condition{ + # condition="Ready",status="False"}[8h]) == 1) = EMPTY + # sum(increase(externalsecret_sync_calls_error[8h])) = 0 + # + # Every one of the 52 ExternalSecrets reconciled at least once in that + # window — 959 syncs, so the 1h and 10m refreshInterval cycles both turned + # over repeatedly — and across all of them the Ready gauge never reached 1 + # in a single scrape. Zero flap amplitude, against real reconcile traffic + # rather than against silence. The counter was flat over the same window + # too, which is why 8 stays 8 above. `for: 15m` is therefore not damping + # anything measurable; it is derived from the failure modes below, and the + # data says no shorter window would have been noisy either. + # + # The 2026-09-20 `ai/wiki-embed-basic-auth` incident is evidence FOR this + # rule, not against it, though it reads the other way at first. That key + # sat in ESO retry backoff through ~10 failures and NEEDED A MANUAL + # force-sync (the incident is written up in + # clusters/homelab/uptime-monitor.yaml, as the in-repo proof for REN-100). + # Because it never self-healed, Ready=False was continuous from the first + # failure until a human intervened — hours, not minutes. `for: 15m` catches + # that with a wide margin. It was a stuck condition, not a flap; the flap + # reading would only hold if the retries had eventually succeeded on their + # own, and they did not. + - alert: ExternalSecretSyncFailing + expr: >- + max by (exported_namespace, name) ( + externalsecret_status_condition{condition="Ready",status="False"} + ) == 1 + # 15m, and the reason it can be this relaxed is that a failed sync is + # not an outage: consumers keep using the last Secret ESO wrote, so what + # degrades is freshness, not availability. Nothing here needs catching + # inside a minute. + # + # It needs to be at least this long because the provider-error path + # returns the error to controller-runtime, which retries with + # exponential backoff rather than waiting out `refreshInterval`. A + # momentary OpenBao blip is therefore cleared within seconds and never + # reaches 15m, while a rolling restart of the 3-replica OpenBao + # StatefulSet is ridden out entirely — and that outage has its own + # alert (OpenBaoQuorumLost, critical at 2m), so nothing is hidden by + # waiting. 15m of continuous Ready=False means repeated failure. + for: 15m + labels: + severity: warning + annotations: + summary: "ExternalSecret {{ $labels.exported_namespace }}/{{ $labels.name }} is not syncing" + description: >- + ESO has been unable to sync this ExternalSecret for 15 minutes. The + Secret it manages still holds its last successfully synced value, so + consumers keep working — what breaks is that a rotation at the + provider no longer propagates, which surfaces much later as a + credential that stopped being accepted. Get the reason with + `kubectl -n {{ $labels.exported_namespace }} get externalsecret + {{ $labels.name }} -o jsonpath='{.status.conditions}'`; the common + causes are a key that was renamed or removed in OpenBao, a + ClusterSecretStore that cannot authenticate, and a target Secret + owned by something else. + + # Dead-man's-switch, mirroring ExternalDNSMetricsMissing and + # OpenBaoMetricsMissing. The alert above can only fire on series that + # exist, so deleting the PodMonitor, reverting the `monitoring` ingress + # grant in infrastructure/network-policies/external-secrets-ns.yaml, or + # bumping the ESO chart onto a metrics port that no longer matches would + # all disarm it silently — which is the exact failure mode REN-103 exists + # to end. TargetDown does not cover this: it reports a target that exists + # and is failing, not the absence of a target. + - alert: ExternalSecretMetricsMissing + expr: absent(externalsecret_status_condition) + for: 15m + labels: + severity: warning + annotations: + summary: "No ExternalSecret metrics are reaching Prometheus" + description: >- + `externalsecret_status_condition` has no series. Either the ESO + controller is gone or its scrape stopped being collected, and + ExternalSecretSyncFailing is blind for as long as this is true. + Check the PodMonitor `monitoring/external-secrets` still selects the + controller pods, and that external-secrets-ns.yaml still admits + `monitoring` on TCP/8080. + + # NOT covered here, deliberately, so nobody assumes it is: + # + # * `condition="Deleted"`. In chart 2.11.0 the only writer of that + # condition is the NotFound branch of Reconcile, which fabricates + # `Deleted=False` purely to make UpdateExternalSecretCondition drop + # the object's Ready series. It is a tombstone for an ExternalSecret + # that no longer exists, not a health state, and `Deleted=True` is + # never emitted at all — a rule on it would be dead code. The one such + # series in the cluster today (`ai/wiki-index-basic-auth`) is leftover + # process state for an ExternalSecret that moved namespace; it clears + # when the controller next restarts. + # * An ExternalSecret that exists in the cluster but was never + # reconciled emits no series at all, so nothing above can see it. That + # needs object-existence data — kube-state-metrics custom resource + # state — rather than ESO's own metrics, and is not in this change. + # * A sync that fails and then succeeds on its own inside 15m. This is a + # choice, not an oversight: the retry left the Secret holding a freshly + # synced value, so nothing degraded and there is nothing for a human to + # do. Alerting on it would mean paging for a self-healed event. The case + # that would justify revisiting is the same failure recurring for days + # without any single occurrence lasting 15m, which would show up as a + # climbing `externalsecret_sync_calls_error` on an ExternalSecret that + # is Ready=True. Nothing in the cluster shows that shape today, and the + # rule for it needs a measured recurrence rate rather than a guess. diff --git a/kubernetes/apps/monitoring-rules/tests/README.md b/kubernetes/apps/monitoring-rules/tests/README.md index ece03100..207486de 100644 --- a/kubernetes/apps/monitoring-rules/tests/README.md +++ b/kubernetes/apps/monitoring-rules/tests/README.md @@ -27,6 +27,7 @@ there was nowhere in the repo to put it. That rule is now | --- | --- | | `cnpg-backup_test.yaml` | The suite. **Generated** — see below. | | `generate-cnpg-backup-test.py` | Regenerates it from the REN-129 measurement table. | +| `external-secrets_test.yaml` | Hand-written suite for `prometheusrule-external-secrets.yaml` (REN-103). | | `mutations.txt` | Negative controls. Each must make the suite fail. | ## Adding a suite diff --git a/kubernetes/apps/monitoring-rules/tests/external-secrets_test.yaml b/kubernetes/apps/monitoring-rules/tests/external-secrets_test.yaml new file mode 100644 index 00000000..fb90bb1c --- /dev/null +++ b/kubernetes/apps/monitoring-rules/tests/external-secrets_test.yaml @@ -0,0 +1,115 @@ +# promtool test rules for prometheusrule-external-secrets.yaml. +# +# Run by the `promtool` step of .woodpecker/ci-manifests-test.yml through +# scripts/ci/promtool-rules.sh, which strips `spec:` from the manifest into a +# mirrored tree — hence `../` below, resolved from this file's directory. See +# README.md here. The negative controls for this suite are the `eso-*` entries +# in mutations.txt. +# +# WHAT IT IS FOR. Every claim in the rule file that a reviewer would otherwise +# have to take on trust is asserted here instead: that the tombstone series does +# not alert, that `for: 15m` really is 15m, that the annotation names the +# ExternalSecret's own namespace and not the controller's, and that `max` +# survives a two-pod scrape where `sum` does not. +# +# THE REJECTED FORM, for the record. REN-100 and ESO's shipped dashboard both +# use `sum(...) by (namespace, name) == 1`. Run against the identical input +# series below, that expression produces exactly one alert, +# {namespace="external-secrets", name="gitea-db"} — the wrong namespace on the +# one it catches, and nothing at all for monitoring/loki-basic-auth, whose two +# scraped pods sum to 2 and stop matching `== 1`. Both defects, one expression. +rule_files: + - ../prometheusrule-external-secrets.yaml + +evaluation_interval: 1m + +tests: + # Series shapes copied from the live cluster after #994 landed: the metric's + # own `namespace` has already been shadowed to `exported_namespace` by the + # PodMonitor's target label, and `namespace` is the controller pod's. + - interval: 1m + input_series: + # A healthy ExternalSecret. ESO writes BOTH polarities of the Ready + # condition on every reconcile, the inactive one at 0 — which is why the + # rule needs `== 1` and not a bare selector. + - series: 'externalsecret_status_condition{condition="Ready",status="True",name="uptime-robot-api-key",exported_namespace="uptime-monitor",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '1+0x60' + - series: 'externalsecret_status_condition{condition="Ready",status="False",name="uptime-robot-api-key",exported_namespace="uptime-monitor",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '0+0x60' + + # Starts failing at t=10m and stays failed. + - series: 'externalsecret_status_condition{condition="Ready",status="False",name="gitea-db",exported_namespace="gitea",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '0+0x9 1+0x50' + - series: 'externalsecret_status_condition{condition="Ready",status="True",name="gitea-db",exported_namespace="gitea",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '1+0x9 0+0x50' + + # Tombstone left behind for an ExternalSecret that no longer exists. Held + # at 1 forever until the controller restarts, so a rule that matched + # `condition!="Ready"` or ignored `status` would page on it every day. + - series: 'externalsecret_status_condition{condition="Deleted",status="False",name="wiki-index-basic-auth",exported_namespace="ai",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '1+0x60' + + # The same failing ExternalSecret reported by TWO controller pods, as + # happens for the seconds of a Deployment rollout when Prometheus still + # has the old target and already has the new one. + - series: 'externalsecret_status_condition{condition="Ready",status="False",name="loki-basic-auth",exported_namespace="monitoring",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-56c649799b-t65gf"}' + values: '0+0x9 1+0x50' + - series: 'externalsecret_status_condition{condition="Ready",status="False",name="loki-basic-auth",exported_namespace="monitoring",namespace="external-secrets",job="monitoring/external-secrets",pod="external-secrets-7d4f9c6b88-qq2xv"}' + values: '0+0x9 1+0x50' + + alert_rule_test: + # 5m into the failure: pending, not firing. Asserts the window is real — + # a `for:` accidentally set to 0 passes every other case here and fails + # only this one. + - eval_time: 15m + alertname: ExternalSecretSyncFailing + exp_alerts: [] + + # 20m into the failure. Both failing ExternalSecrets fire, ONCE each; the + # healthy one and the tombstone stay silent. The annotations are asserted + # in full because the whole point of REN-103's label finding is that the + # text names `gitea`, not `external-secrets`. + - eval_time: 30m + alertname: ExternalSecretSyncFailing + exp_alerts: + - exp_labels: + severity: warning + exported_namespace: gitea + name: gitea-db + exp_annotations: + summary: "ExternalSecret gitea/gitea-db is not syncing" + description: "ESO has been unable to sync this ExternalSecret for 15 minutes. The Secret it manages still holds its last successfully synced value, so consumers keep working — what breaks is that a rotation at the provider no longer propagates, which surfaces much later as a credential that stopped being accepted. Get the reason with `kubectl -n gitea get externalsecret gitea-db -o jsonpath='{.status.conditions}'`; the common causes are a key that was renamed or removed in OpenBao, a ClusterSecretStore that cannot authenticate, and a target Secret owned by something else." + - exp_labels: + severity: warning + exported_namespace: monitoring + name: loki-basic-auth + exp_annotations: + summary: "ExternalSecret monitoring/loki-basic-auth is not syncing" + description: "ESO has been unable to sync this ExternalSecret for 15 minutes. The Secret it manages still holds its last successfully synced value, so consumers keep working — what breaks is that a rotation at the provider no longer propagates, which surfaces much later as a credential that stopped being accepted. Get the reason with `kubectl -n monitoring get externalsecret loki-basic-auth -o jsonpath='{.status.conditions}'`; the common causes are a key that was renamed or removed in OpenBao, a ClusterSecretStore that cannot authenticate, and a target Secret owned by something else." + + # Metrics are present, so the dead-man's-switch stays quiet. + - eval_time: 30m + alertname: ExternalSecretMetricsMissing + exp_alerts: [] + + # The scrape disappears entirely: PodMonitor deleted, selector broken by a + # chart bump, or the `monitoring` ingress grant reverted out of + # external-secrets-ns.yaml. ExternalSecretSyncFailing cannot fire on series + # that do not exist — this is the case that made REN-103 necessary, so it is + # the one the dead-man's-switch has to cover. + - interval: 1m + input_series: + - series: 'up{job="monitoring/external-secrets"}' + values: '0+0x60' + alert_rule_test: + - eval_time: 30m + alertname: ExternalSecretSyncFailing + exp_alerts: [] + - eval_time: 30m + alertname: ExternalSecretMetricsMissing + exp_alerts: + - exp_labels: + severity: warning + exp_annotations: + summary: "No ExternalSecret metrics are reaching Prometheus" + description: "`externalsecret_status_condition` has no series. Either the ESO controller is gone or its scrape stopped being collected, and ExternalSecretSyncFailing is blind for as long as this is true. Check the PodMonitor `monitoring/external-secrets` still selects the controller pods, and that external-secrets-ns.yaml still admits `monitoring` on TCP/8080." diff --git a/kubernetes/apps/monitoring-rules/tests/mutations.txt b/kubernetes/apps/monitoring-rules/tests/mutations.txt index dcf44134..e94e1043 100644 --- a/kubernetes/apps/monitoring-rules/tests/mutations.txt +++ b/kubernetes/apps/monitoring-rules/tests/mutations.txt @@ -70,3 +70,33 @@ no-value-selector|kubernetes/apps/monitoring-rules/prometheusrule-cnpg-backup.ya # on a backlog sitting at exactly five segments, which is 80 MiB and normal. # Detected by: case 11, pinned at exactly 5. backlog-ge-5|kubernetes/apps/monitoring-rules/prometheusrule-cnpg-backup.yaml|/^[[:space:]]*expr:/s/> 5$/>= 5/ + +# --- prometheusrule-external-secrets.yaml (REN-103) ------------------------- +# Scoped to the ExternalSecretSyncFailing expression with /max by/ or /== 1/ +# where it matters: the manifest's comments quote the rejected forms, and +# rewriting those would mutate the documentation and nothing else. + +# The window. At 5m the failure that starts at t=10m is already firing at the +# t=15m assertion, which expects pending. +# Detected by: the eval_time 15m case. +eso-for-5m|kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml|s/^\([[:space:]]*\)for: 15m$/\1for: 5m/ + +# REN-100's aggregation. `sum` over the two-pod rollout case is 2, fails +# `== 1`, and loki-basic-auth never fires. +# Detected by: the eval_time 30m case, loki-basic-auth. +eso-sum-not-max|kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml|s/^\([[:space:]]*\)max by (exported_namespace, name) ($/\1sum by (exported_namespace, name) (/ + +# The label Argus found shadowed on #994. Grouping by `namespace` names the +# controller's namespace on every alert. +# Detected by: the eval_time 30m case, exp_labels and the rendered summary. +eso-by-namespace|kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml|s/^\([[:space:]]*\)max by (exported_namespace, name) ($/\1max by (namespace, name) (/ + +# Drop `== 1`. ESO writes the inactive Ready polarity at 0, so every healthy +# ExternalSecret matches the bare selector and fires. +# Detected by: the eval_time 30m case, uptime-robot-api-key. +eso-no-eq-1|kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml|s/^\([[:space:]]*\)) == 1$/\1)/ + +# Drop the condition selector. The Deleted tombstone is status="False" at 1 +# forever, so it pages until the controller restarts. +# Detected by: the eval_time 30m case, wiki-index-basic-auth. +eso-any-condition|kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml|s/^\([[:space:]]*\)externalsecret_status_condition{condition="Ready",status="False"}$/\1externalsecret_status_condition{status="False"}/