2026-09-27 06:34:41 +00:00
5 changed files with 377 additions and 0 deletions
@@ -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
@@ -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.
@@ -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
@@ -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."
@@ -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"}/