diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24da9a1..93f7de8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,6 +118,16 @@ jobs: - name: Excluded alert states are somebody's job run: ./scripts/check-alert-coverage.py + # A rule labelled to page is asking for a human to be woken. Grafana keeps + # that promise by matching the label against a notification policy; with + # no matching route the alert falls to the tree's root, and with no tree at + # all to the workspace default policy and its empty default contact point. + # The rule and the policy never name each other, so nothing else in the + # tree can see the gap — and the signal arrives at the moment nobody is + # watching a dashboard. + - name: Every alert severity routes to a declared contact point + run: ./scripts/check-alert-severity-routes.py + # ── Kyverno policy unit tests ──────────────────────────────────────── # `kyverno test` proves each policy rule PASSES a compliant resource and FAILS # a violating one (policies/kyverno/tests). Without this, a policy could be diff --git a/Taskfile.yaml b/Taskfile.yaml index cde71ce..08555e2 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -121,6 +121,11 @@ tasks: cmds: - ./scripts/tests/controls.py + validate:alert-severity-routes: + desc: "Alert routing gate — every severity a rule claims resolves to a declared contact point" + cmds: + - ./scripts/check-alert-severity-routes.py + validate:empty-corpus: desc: "Vacuity gate — no gate reports success over a corpus that is not there" cmds: @@ -217,6 +222,7 @@ tasks: - validate:policy-validity - validate:gate-tests - validate:gate-controls + - validate:alert-severity-routes - validate:empty-corpus - validate:reverify-tests - validate:named-things diff --git a/applicationsets/dashboards.yaml b/applicationsets/dashboards.yaml index 44f7084..e1642de 100644 --- a/applicationsets/dashboards.yaml +++ b/applicationsets/dashboards.yaml @@ -85,6 +85,13 @@ spec: - op: replace path: /spec/dataFrom/0/extract/key value: '{{ index .metadata.labels "cluster_name" }}-grafana-token' + - target: + kind: ExternalSecret + name: alert-destinations + patch: |- + - op: replace + path: /spec/dataFrom/0/extract/key + value: '{{ index .metadata.labels "cluster_name" }}-alert-destinations' - target: kind: CronJob name: grafana-token-rotator diff --git a/dashboards/base/alerting/alert-destinations.yaml b/dashboards/base/alerting/alert-destinations.yaml new file mode 100644 index 0000000..fb265fb --- /dev/null +++ b/dashboards/base/alerting/alert-destinations.yaml @@ -0,0 +1,35 @@ +# The credentials the alert destinations authenticate with, pulled from AWS +# Secrets Manager through the same aws-secrets-manager ClusterSecretStore the +# Grafana service-account token uses. +# +# The key committed here is a placeholder. The real secret is cluster-scoped — +# `-alert-destinations` — so co-located sibling clusters in one account +# do not collide, and the dashboards ApplicationSet patches the name per cluster +# from the cluster_name label, as it does for the Grafana token and the +# monitoring endpoints. +# +# The secret itself is created and seeded by the landing-zone managed-monitoring +# component, as `-grafana-token` is — it is not hand-made here. Until it +# carries a value the contact points reconcile with no integration key and +# Grafana delivers nothing, which is the state this file exists to end rather +# than one it introduces. +# +# extract, not a data list, for the reason monitoring-endpoints.yaml records: +# the payload's field names are already the key names the contact points read, +# so enumerating them here would restate the producer's shape and name one +# cluster-scoped secret once per field. +apiVersion: external-secrets.io/v1 +kind: ExternalSecret +metadata: + name: alert-destinations +spec: + refreshInterval: 1h + secretStoreRef: + name: aws-secrets-manager + kind: ClusterSecretStore + target: + name: alert-destinations + creationPolicy: Owner + dataFrom: + - extract: + key: eks-alert-destinations diff --git a/dashboards/base/alerting/contact-points.yaml b/dashboards/base/alerting/contact-points.yaml new file mode 100644 index 0000000..dd5dea4 --- /dev/null +++ b/dashboards/base/alerting/contact-points.yaml @@ -0,0 +1,60 @@ +# Where a firing alert is delivered. One contact point per urgency, because the +# urgency is what the rules label themselves with and a route can only send to a +# receiver that exists. +# +# The integration credentials are never written here. Each receiver takes its +# secret through valuesFrom, from the Secret alert-destinations.yaml +# materialises — so the value lives in Secrets Manager and this file names the +# key rather than carrying it. +# +# Neither is marked optional. An optional missing secret produces a contact +# point with no integration key: Grafana accepts it, the route resolves, and +# nothing is delivered — which is indistinguishable from a working destination +# by every signal except an incident nobody answered. Required, the contact +# point reports NotReady and says so. +apiVersion: grafana.integreatly.org/v1beta1 +kind: GrafanaContactPoint +metadata: + name: platform-page +spec: + instanceSelector: + matchLabels: + dashboards: external + name: platform-page + receivers: + - uid: platform-page-pagerduty + type: pagerduty + settings: + severity: critical + # The alert's own labels, so the incident names the system it is about + # rather than only the rule that fired. + class: '{{ .CommonLabels.service }}' + component: '{{ .CommonLabels.component }}' + valuesFrom: + - targetPath: integrationKey + valueFrom: + secretKeyRef: + name: alert-destinations + key: PAGERDUTY_INTEGRATION_KEY +--- +apiVersion: grafana.integreatly.org/v1beta1 +kind: GrafanaContactPoint +metadata: + name: platform-ticket +spec: + instanceSelector: + matchLabels: + dashboards: external + name: platform-ticket + receivers: + - uid: platform-ticket-slack + type: slack + settings: + title: '{{ .CommonLabels.alertname }} — {{ .CommonLabels.service }}' + text: '{{ range .Alerts }}{{ .Annotations.summary }}{{ "\n" }}{{ end }}' + valuesFrom: + - targetPath: url + valueFrom: + secretKeyRef: + name: alert-destinations + key: SLACK_WEBHOOK_URL diff --git a/dashboards/base/alerting/notification-policy.yaml b/dashboards/base/alerting/notification-policy.yaml new file mode 100644 index 0000000..ce25d1c --- /dev/null +++ b/dashboards/base/alerting/notification-policy.yaml @@ -0,0 +1,53 @@ +# The routing tree every alert rule under this directory is written against. +# +# A rule labels itself with the urgency it is asking for. Grafana matches those +# labels against this tree and delivers to the receiver the matching route +# names; with no tree the workspace default applies, and the workspace default +# contact point is empty. A `severity: page` label with no route is a promise +# that a human is woken, kept by nobody, in the one moment it matters. +# +# The root receiver is the low-urgency one rather than a dedicated fallback. An +# alert whose severity matches no route is a rule this tree has not been taught +# about, and the two answers to that are "deliver it somewhere a person reads" +# and "drop it silently". scripts/check-alert-severity-routes.py makes the case +# impossible to arrive at unnoticed; this makes it survivable if it does. +apiVersion: grafana.integreatly.org/v1beta1 +kind: GrafanaNotificationPolicy +metadata: + name: platform-severity-routes +spec: + instanceSelector: + matchLabels: + dashboards: external + route: + receiver: platform-ticket + # Grouped by the system rather than by the rule, so a fleet-wide failure + # raising six rules against one service is one notification and not six. + group_by: + - alertname + - service + - component + routes: + # Wakes a human. Repeated hourly while firing, because an unanswered page + # is the failure this whole tree exists to prevent. + - receiver: platform-page + matchers: + - name: severity + value: page + isEqual: true + isRegex: false + group_wait: 30s + group_interval: 5m + repeat_interval: 1h + # Read in working hours. Grouped and repeated slowly on purpose: a ticket + # that re-notifies like a page teaches people to mute the channel both + # arrive in. + - receiver: platform-ticket + matchers: + - name: severity + value: ticket + isEqual: true + isRegex: false + group_wait: 5m + group_interval: 30m + repeat_interval: 12h diff --git a/dashboards/base/kustomization.yaml b/dashboards/base/kustomization.yaml index 03238e9..50acce5 100644 --- a/dashboards/base/kustomization.yaml +++ b/dashboards/base/kustomization.yaml @@ -22,6 +22,12 @@ resources: # Grafana-managed SLO / burn-rate alert rules (folder + per-system rule groups), # reconciled onto the external Amazon Managed Grafana. - alerting/folder.yaml + # Where a firing rule is delivered. Without these the rules' severity labels + # select the workspace default policy and its empty default contact point, + # so every alert changes state in the Grafana list and reaches no human. + - alerting/alert-destinations.yaml + - alerting/contact-points.yaml + - alerting/notification-policy.yaml - alerting/portal.yaml - alerting/agent-operator.yaml - alerting/fleet-vend.yaml diff --git a/scripts/check-alert-severity-routes.py b/scripts/check-alert-severity-routes.py new file mode 100755 index 0000000..5ef3bfe --- /dev/null +++ b/scripts/check-alert-severity-routes.py @@ -0,0 +1,326 @@ +#!/usr/bin/env python3 +"""Every label a rule routes on selects a route that exists. + +WHY THIS EXISTS + +A rule labelled `severity: page` is asking for a human to be woken. Grafana +keeps that promise by matching the label against a notification policy tree and +delivering to the receiver the matching route names. With no matching route the +alert falls to the tree's root, and with no tree at all it falls to the +workspace default policy and its empty default contact point. + +Nothing else in this repository can see that. kubeconform SKIPs the +grafana.integreatly.org CRDs, the rules and the policy are separate documents +that never reference each other by name, and an alert that reaches nobody looks +exactly like an alert that has not fired. The signal arrives at the moment +nobody is watching a dashboard, which is the moment the delivery path is the +only thing there is. + +WHAT IT CHECKS + +The keys come from the POLICY, not from a list here. Whatever label keys the +routes match on are the keys that make a claim about delivery, so: + + * every value a rule carries for one of those keys matches a route for that + key — an unrouted value falls to the root receiver, which is the bucket + this gate exists to keep things out of; + * every rule carries every key the policy routes on, because a rule missing + one lands in the same bucket by a different path; + * every receiver a route names is a contact point this catalog declares, in + both directions — a route to a receiver nobody declares delivers nothing, + and a contact point no route names is a destination that will not be + reached by anything and rots the way an unread exemption does. + + * every alerting object the directory holds is one the kustomization + renders, and every one it renders is one this gate read. `resources` is an + explicit list, so a routing tree can stop shipping without moving: the + files still describe complete delivery and the cluster gets rule groups + labelled for a pager and nothing to match them against. + +Derived rather than listed, so a new severity value, a new routing key or a +renamed contact point is caught by this gate rather than by an incident. + +WHAT IT DOES NOT CHECK + +Two facts about systems outside this repository, both of which must also hold +before a page is delivered, and neither of which is readable from this tree: + + * whether Amazon Managed Grafana ACCEPTS the contact point. A workspace + enumerates the destination types it will create in its own configuration, + and that configuration is the landing-zone managed-monitoring component's. + * whether the credential each receiver reads is seeded. The contact points + take theirs from a Secret an ExternalSecret materialises out of Secrets + Manager, and what is in Secrets Manager is not in this repository. + +So this asserts that the CATALOG routes every urgency it claims, which is the +half that lives here. A green run is not an assertion that a page arrived. +""" + +from __future__ import annotations + +import importlib.util +import pathlib +import subprocess +import sys + +import yaml + +_gl = pathlib.Path(__file__).resolve().parent / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +ROOT = pathlib.Path(__file__).resolve().parent.parent +ALERTING = ROOT / "dashboards" / "base" / "alerting" +# The kustomization whose `resources` list decides which of those files a +# cluster receives. Derived from the alerting directory rather than named +# again, so the two readings below cannot come to describe different trees. +KUSTOMIZE_ROOT = ALERTING.parent + +RULE_GROUP = "GrafanaAlertRuleGroup" +POLICY = "GrafanaNotificationPolicy" +CONTACT_POINT = "GrafanaContactPoint" +ALERTING_KINDS = (RULE_GROUP, POLICY, CONTACT_POINT) + + +def under_root(path: pathlib.Path) -> str: + """`path` relative to the repository when it is inside it, else as given. + + Not `relative_to` alone: it RAISES for a path outside the root, and the one + place this is called from is a refusal about a directory that is missing. + A refusal that crashes while composing its own message exits with a + traceback and the status of a rejection, which is the reading this gate's + exit codes exist to keep apart. + """ + try: + return str(path.relative_to(ROOT)) + except ValueError: + return str(path) + + +def documents(directory: pathlib.Path) -> list[tuple[pathlib.Path, dict]]: + """Every YAML document under `directory`, with the file it came from. + + The file is carried because a finding has to name one: these are eight + files of near-identical shape and "a rule is unrouted" is not actionable + without saying which. + """ + if not directory.is_dir(): + print(f"Cannot run: {under_root(directory)} does not exist, so the " + f"alert rules whose routing this gate checks are not there at all.") + print("This gate examined nothing, which is not the same as finding nothing.") + sys.exit(gatelib.CANNOT_RUN) + out = [] + for path in sorted(directory.glob("*.yaml")): + for doc in gatelib.read_yaml_all(path): + if isinstance(doc, dict): + out.append((path, doc)) + return out + + +def shipped(root: pathlib.Path) -> set[tuple[str, str]]: + """(kind, metadata.name) for every alerting object `root` renders. + + The reading that decides. A file under the alerting directory is a file; + what a cluster receives is what the kustomization's `resources` list names, + and that is an explicit list a line can leave. Drop `notification-policy` + from it and the render carries the rule groups and no routing tree — the + state this gate exists to refuse — while the file the gate read is still on + disk and still describes perfect routing. + + Reading only the render would decide correctly and report uselessly: a + rendered document no longer carries the file it came from, and "a rule is + unrouted" across near-identical groups is not actionable without one. So + both readings are taken and disagreement between them is itself a finding. + """ + gatelib.require("kustomize") + proc = subprocess.run(["kustomize", "build", str(root)], + capture_output=True, text=True) + if proc.returncode != 0: + print(f"Cannot run: kustomize build {under_root(root)} failed, so what " + f"this catalog delivers could not be read. The alerting files on " + f"disk say nothing about that on their own.") + print(proc.stderr.strip()[:500]) + sys.exit(gatelib.CANNOT_RUN) + out: set[tuple[str, str]] = set() + for doc in yaml.safe_load_all(proc.stdout): + if not isinstance(doc, dict): + continue + kind = str(doc.get("kind")) + if kind not in ALERTING_KINDS: + continue + name = str((doc.get("metadata") or {}).get("name")) + out.add((kind, name)) + return out + + +def routes_of(route: dict): + """Every route in the tree, depth first, including the root. + + Recursive because `routes` nests, and a nested route is as much a delivery + decision as a top-level one. + """ + yield route + for child in route.get("routes") or []: + if isinstance(child, dict): + yield from routes_of(child) + + +def matcher_pairs(route: dict) -> list[tuple[str, str]]: + """(label key, value) for every EXACT matcher on one route. + + Regex matchers are excluded rather than guessed at: deciding which rule + labels a pattern admits means running the pattern, and a gate that reports + a value as routed because it looked like it might match would be asserting + the thing it exists to check. A regex route still counts as a route for its + key — it just cannot vouch for a particular value. + """ + pairs = [] + for m in route.get("matchers") or []: + if not isinstance(m, dict) or m.get("isRegex"): + continue + name, value = m.get("name"), m.get("value") + if isinstance(name, str) and isinstance(value, str): + pairs.append((name, value)) + return pairs + + +def routing_keys(route: dict) -> set[str]: + """The label keys the tree makes decisions on, exact or regex.""" + keys = set() + for node in routes_of(route): + for m in node.get("matchers") or []: + if isinstance(m, dict) and isinstance(m.get("name"), str): + keys.add(m["name"]) + return keys + + +def rule_labels(doc: dict): + """(rule title, labels) for every rule in a GrafanaAlertRuleGroup.""" + for rule in (doc.get("spec") or {}).get("rules") or []: + if not isinstance(rule, dict): + continue + labels = rule.get("labels") + yield str(rule.get("title") or rule.get("uid") or ""), \ + labels if isinstance(labels, dict) else {} + + +def main() -> int: + docs = documents(ALERTING) + failures: list[str] = [] + + policies = [(p, d) for p, d in docs if d.get("kind") == POLICY] + contact_points = {str((d.get("spec") or {}).get("name") + or (d.get("metadata") or {}).get("name")): p + for p, d in docs if d.get("kind") == CONTACT_POINT} + rule_groups = [(p, d) for p, d in docs if d.get("kind") == RULE_GROUP] + + # What the directory holds against what the kustomization delivers. Neither + # reading can stand in for the other: the files carry the routing this gate + # reasons about and the render carries whether any of it reaches a cluster. + on_disk = {(str(d.get("kind")), str((d.get("metadata") or {}).get("name"))): p + for p, d in docs if str(d.get("kind")) in ALERTING_KINDS} + delivered = shipped(KUSTOMIZE_ROOT) + for kind, name in sorted(set(on_disk) - delivered): + failures.append( + f"{on_disk[(kind, name)].name}: {kind}/{name} is under " + f"{under_root(ALERTING)} and {under_root(KUSTOMIZE_ROOT)} does not " + f"render it, so this gate is reading a document no cluster receives. " + f"The routing it describes is checked here and delivered nowhere.") + for kind, name in sorted(delivered - set(on_disk)): + failures.append( + f"{under_root(KUSTOMIZE_ROOT)} renders {kind}/{name} and no file " + f"under {under_root(ALERTING)} declares it — an alerting object this " + f"gate never examined is one whose routing nothing here has checked.") + + if not rule_groups: + print(f"FAIL no {RULE_GROUP} found under " + f"{under_root(ALERTING)} — this gate examined no rule, which " + f"reports the same as a catalog whose rules are all routed.") + return gatelib.CANNOT_RUN + if not policies: + print(f"FAIL {len(rule_groups)} rule group(s) are delivered by no " + f"{POLICY}. Every label they carry selects the workspace default " + f"policy and its empty default contact point, so a rule asking to " + f"page reaches nobody.") + return 1 + if len(policies) > 1: + print(f"FAIL {len(policies)} {POLICY} resources: " + f"{', '.join(sorted(str(p.name) for p, _ in policies))}. Grafana " + f"keeps one routing tree per instance, so which of these delivers " + f"an alert is decided by whichever reconciled last.") + return 1 + + _, policy = policies[0] + root = (policy.get("spec") or {}).get("route") or {} + keys = routing_keys(root) + if not keys: + print(f"FAIL the {POLICY} matches on no label at all, so every alert " + f"takes the root route and the severities the rules carry decide " + f"nothing.") + return 1 + + # Which values each key can be routed to, and which receivers are reachable. + routed: dict[str, set[str]] = {k: set() for k in keys} + receivers = {str(node.get("receiver")) for node in routes_of(root) + if node.get("receiver")} + reached: set[str] = set() + for node in routes_of(root): + pairs = matcher_pairs(node) + for key, value in pairs: + routed.setdefault(key, set()).add(value) + if pairs and node.get("receiver"): + reached.add(str(node["receiver"])) + + for receiver in sorted(receivers): + if receiver not in contact_points: + failures.append( + f"the notification policy routes to receiver '{receiver}' and no " + f"{CONTACT_POINT} in this catalog declares it — Grafana has " + f"nowhere to deliver what that route matches.") + for name in sorted(contact_points): + if name not in receivers: + failures.append( + f"{contact_points[name].name} declares the contact point '{name}' " + f"and no route in the notification policy names it — a " + f"destination nothing reaches is one nobody re-reads.") + + for path, doc in rule_groups: + group = (doc.get("metadata") or {}).get("name", path.name) + for title, labels in rule_labels(doc): + for key in sorted(keys): + value = labels.get(key) + if value is None: + failures.append( + f"{path.name}: {group}/{title} carries no '{key}' label and " + f"the notification policy routes on it, so this rule takes " + f"the root route whatever it is asking for.") + elif str(value) not in routed.get(key, set()): + failures.append( + f"{path.name}: {group}/{title} is labelled {key}={value} and " + f"no route matches that value — it falls to the root " + f"receiver, which is where an unrouted alert goes to be " + f"read late or not at all.") + + if failures: + print(f"{len(failures)} alert-routing problem(s):\n") + for f in failures: + print(f" {f}") + return 1 + + covered = sum(len(v) for v in routed.values()) + print(f"✓ every alert rule routes to a declared contact point: " + f"{sum(len(list(rule_labels(d))) for _, d in rule_groups)} rule(s) across " + f"{len(rule_groups)} group(s), matched on " + f"{', '.join(sorted(keys))} against {covered} routed value(s) reaching " + f"{len(contact_points)} contact point(s)") + print(f" {len(delivered)} of them rendered by " + f"{under_root(KUSTOMIZE_ROOT)}; what AMG accepts and what Secrets " + f"Manager holds are outside this repository and outside this claim") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index ed1d26d..f4ea12b 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -489,6 +489,21 @@ def m_alert_coverage(root): marker='lastRunAt\\"} == 0') +def m_alert_severity_routes(root): + """Relabel one rule with a severity the notification policy does not route. + + The exact defect: the rule still parses, still evaluates, still changes + state in the Grafana alert list — and its label selects no route, so it + falls to the tree's root receiver instead of the destination it asked for. + Nothing else in the tree can see that, because the rule and the policy never + name each other. + """ + return _sub(root, "dashboards/base/alerting/agent-operator.yaml", + " severity: page\n", + f" severity: {MARKER}-urgent\n", + marker=f"severity: {MARKER}-urgent") + + def m_chart_deprecation(root): """A recorded chart that nothing pins, which the offline gate must reject.""" import json @@ -562,6 +577,8 @@ def m_env_coverage(root): "check-policy-validity.py": ("a structurally invalid ClusterPolicy", m_policy_validity), "check-serviceaccount-bindings.py": ("a pod naming an absent ServiceAccount", m_serviceaccount_bindings), "check-alert-coverage.py": ("an alert on an unexported KSM field", m_alert_coverage), + "check-alert-severity-routes.py": ("a severity label that routes nowhere", + m_alert_severity_routes), } diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index 17d0a77..04fcc63 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -156,6 +156,7 @@ run 0 "image-pins: a helm reporting OCI pulls on stdout" \ ./scripts/check-image-pins.py run 0 "check-renovate-coverage.py" ./scripts/check-renovate-coverage.py run 0 "check-ai-config.py" ./scripts/check-ai-config.py +run 0 "check-alert-severity-routes.py" ./scripts/check-alert-severity-routes.py run 0 "check-env-coverage.py" ./scripts/check-env-coverage.py run 0 "check-named-things.py" ./scripts/check-named-things.py run 0 "check-policy-validity.py" ./scripts/check-policy-validity.py @@ -352,6 +353,56 @@ PY run nonzero "ai-config: global. geo prefix" ./scripts/check-ai-config.py res $F +# A rule labelled to page is asking for a human to be woken, and Grafana keeps +# that promise by matching the label against a route. Relabelled to a severity +# the tree does not route, the rule still parses, still evaluates and still +# changes state in the alert list — and falls to the root receiver instead of +# the destination it asked for. The rule and the policy never name each other, +# so nothing else in this tree can see it. +F=dashboards/base/alerting/agent-operator.yaml; mut $F +python3 - "$F" <<'PY' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace(" severity: page\n", " severity: urgent\n", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" relabelled one rule severity: page -> urgent") +PY +run nonzero "alert-severity-routes: a severity the tree does not route" \ + ./scripts/check-alert-severity-routes.py +res $F + +# The other direction, and the one that rots: a destination nothing reaches. +F=dashboards/base/alerting/notification-policy.yaml; mut $F +python3 - "$F" <<'PY' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace(" - receiver: platform-page\n", " - receiver: platform-paige\n", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" misspelled the receiver one route delivers to") +PY +run nonzero "alert-severity-routes: a route to a contact point nobody declares" \ + ./scripts/check-alert-severity-routes.py +res $F + +# The routing tree stops being delivered without moving, being edited, or +# failing to render. Every earlier check here still passes on this tree: the +# rules are labelled, the routes match, the contact points are declared. What a +# cluster receives is the rule groups and nothing to match them against. +F=dashboards/base/kustomization.yaml; mut $F +python3 - "$F" <<'PYX' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace(" - alerting/notification-policy.yaml\n", "", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" dropped the notification policy from the kustomization's resources") +PYX +run nonzero "alert-severity-routes: the routing tree stops shipping" \ + ./scripts/check-alert-severity-routes.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -464,7 +515,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=37 +MIN_CHECKS=41 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index eeebf8c..c950e81 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -37,6 +37,7 @@ "test_kyverno_corpus", "test_named_things", "test_falco_rule_floor", + "test_alert_severity_routes", "test_gatelib", # The harness layer. controls.py excuses the harnesses from carrying a # positive control because each asserts its own outcome when it runs, and @@ -135,7 +136,7 @@ # largest in the tree, and among them are the gates on the paths testing-rubric # calls security-critical. So this figure being low is not offset by behavioural # coverage for precisely the files where that offset was being claimed. -COMBINED_FLOOR = 40 +COMBINED_FLOOR = 42 # A ceiling on gate scripts carrying NO unit coverage at all, complementing the # floors below. The floors stop a covered file regressing; nothing stopped a NEW @@ -169,6 +170,7 @@ "scripts/render-addons.py": 40, "scripts/check-log-volume-budget.py": 58, "scripts/check-falco-rule-floor.py": 28, + "scripts/check-alert-severity-routes.py": 95, "scripts/check-image-vulnerabilities.py": 41, "scripts/check-image-pins.py": 88, } diff --git a/scripts/tests/test_alert_severity_routes.py b/scripts/tests/test_alert_severity_routes.py new file mode 100644 index 0000000..81b6e9b --- /dev/null +++ b/scripts/tests/test_alert_severity_routes.py @@ -0,0 +1,384 @@ +"""Unit tests for the alert-routing gate. + +The gate's own failure mode is a FALSE PASS: reporting a rule as routed when +nothing delivers it. That failure is silent in the one moment it matters, +because an alert reaching nobody looks exactly like an alert that has not +fired — so these concentrate on the cases where a route looks present and is +not. + +The keys the gate routes on come from the policy, so the fixtures vary the +POLICY as well as the rules. A gate that read a hardcoded `severity` would pass +every case here that uses that key and none of the ones that do not, which is +what separates deriving the keys from listing them. +""" + +from __future__ import annotations + +import contextlib +import io +import pathlib +import shutil +import tempfile +import unittest + +import yaml +from gateloader import load + +gate = load("check-alert-severity-routes") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + +# The gate renders the kustomization to learn what a cluster receives, so the +# planted fixtures are rendered too rather than compared against a stub. The +# job running these installs it; a checkout without it skips rather than +# aborting the runner on the gate's exit-2 refusal. +HAS_KUSTOMIZE = shutil.which("kustomize") is not None + + +def rule_group(name, *rules): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaAlertRuleGroup", + "metadata": {"name": name}, + "spec": {"rules": [{"title": t, "labels": labels} + for t, labels in rules]}} + + +def contact_point(name): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaContactPoint", + "metadata": {"name": name}, + "spec": {"name": name, "receivers": []}} + + +def policy(route, name="routes"): + """`name` is a parameter because the render rejects two objects sharing one: + the two-policy case has to plant two distinct resources to reach the gate.""" + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaNotificationPolicy", + "metadata": {"name": name}, + "spec": {"route": route}} + + +def exact(key, value): + return {"name": key, "value": value, "isEqual": True, "isRegex": False} + + +@unittest.skipUnless(HAS_KUSTOMIZE, "kustomize is not on PATH") +class TheVerdict(unittest.TestCase): + """main() over a planted alerting directory.""" + + def verdict(self, *docs, unshipped=()): + """main() over a planted tree, rendered the way the real one is. + + `unshipped` names documents to write to the alerting directory and + leave out of the kustomization's `resources`, which is what a routing + tree that stops being delivered looks like from disk. + """ + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + alerting = base / "alerting" + alerting.mkdir(parents=True) + resources = [] + for i, doc in enumerate(docs): + name = f"{i:02d}-{doc['kind']}.yaml" + (alerting / name).write_text(yaml.safe_dump(doc)) + if doc not in unshipped: + resources.append(f" - alerting/{name}") + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n" + "\n".join(resources) + "\n") + saved = (gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT) + gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT = root, alerting, base + try: + with contextlib.redirect_stdout(io.StringIO()) as out: + rc = gate.main() + finally: + gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT = saved + return rc, out.getvalue() + + ROUTED = policy({"receiver": "low", + "routes": [{"receiver": "urgent", "matchers": [exact("severity", "page")]}, + {"receiver": "low", "matchers": [exact("severity", "ticket")]}]}) + + def healthy(self): + return (self.ROUTED, contact_point("urgent"), contact_point("low"), + rule_group("g", ("Paging", {"severity": "page"}), + ("Ticketing", {"severity": "ticket"}))) + + def test_a_routed_catalog_passes(self): + """The control. Without it every case below could be failing for a + reason the case did not plant.""" + rc, out = self.verdict(*self.healthy()) + self.assertEqual(rc, 0, out) + + def test_a_severity_no_route_matches_is_reported(self): + """The defect, exactly: the rule parses, evaluates and changes state, + and its label selects no route.""" + rc, out = self.verdict( + self.ROUTED, contact_point("urgent"), contact_point("low"), + rule_group("g", ("Loud", {"severity": "critical"}))) + self.assertEqual(rc, 1) + self.assertIn("severity=critical", out) + self.assertIn("root receiver", out) + + def test_a_rule_missing_a_routed_key_is_reported(self): + """Same bucket by a different path: no label at all takes the root + route as surely as an unrouted value does.""" + rc, out = self.verdict( + self.ROUTED, contact_point("urgent"), contact_point("low"), + rule_group("g", ("Silent", {"service": "portal"}))) + self.assertEqual(rc, 1) + self.assertIn("carries no 'severity' label", out) + + def test_no_policy_at_all_is_reported(self): + """The state this gate was written against — every rule delivered by the + workspace default policy and its empty default contact point.""" + rc, out = self.verdict(rule_group("g", ("Paging", {"severity": "page"}))) + self.assertEqual(rc, 1) + self.assertIn("delivered by no GrafanaNotificationPolicy", out) + + def test_two_policies_are_reported(self): + """Grafana keeps one tree per instance, so a second is not additional + routing — it is whichever reconciled last.""" + rc, out = self.verdict(self.ROUTED, policy({"receiver": "low"}, "second"), + contact_point("low"), + rule_group("g", ("Paging", {"severity": "page"}))) + self.assertEqual(rc, 1) + self.assertIn("2 GrafanaNotificationPolicy", out) + + def test_a_policy_matching_on_nothing_is_reported(self): + """A tree with no matcher routes everything to the root, so the + severities the rules carry decide nothing.""" + rc, out = self.verdict(policy({"receiver": "low"}), contact_point("low"), + rule_group("g", ("Paging", {"severity": "page"}))) + self.assertEqual(rc, 1) + self.assertIn("matches on no label at all", out) + + def test_a_route_to_an_undeclared_receiver_is_reported(self): + rc, out = self.verdict( + self.ROUTED, contact_point("low"), + rule_group("g", ("Paging", {"severity": "page"}), + ("Ticketing", {"severity": "ticket"}))) + self.assertEqual(rc, 1) + self.assertIn("receiver 'urgent'", out) + self.assertIn("nowhere to deliver", out) + + def test_a_contact_point_no_route_names_is_reported(self): + """The direction that rots. A destination nothing reaches is a + declaration nobody re-reads, and it widens the same way an unused + exemption does.""" + rc, out = self.verdict( + self.ROUTED, contact_point("urgent"), contact_point("low"), + contact_point("orphan"), + rule_group("g", ("Paging", {"severity": "page"}), + ("Ticketing", {"severity": "ticket"}))) + self.assertEqual(rc, 1) + self.assertIn("'orphan'", out) + + def test_no_rule_group_at_all_cannot_run(self): + """Exit 2. A directory with no rules is a gate that examined nothing, + which reports the same as a catalog whose rules are all routed.""" + rc, out = self.verdict(self.ROUTED, contact_point("urgent"), + contact_point("low")) + self.assertEqual(rc, gate.gatelib.CANNOT_RUN) + self.assertIn("examined no rule", out) + + def test_a_missing_directory_cannot_run(self): + saved = gate.ALERTING + gate.ALERTING = pathlib.Path(tempfile.mkdtemp()) / "gone" + try: + with self.assertRaises(SystemExit) as caught, \ + contextlib.redirect_stdout(io.StringIO()): + gate.documents(gate.ALERTING) + self.assertEqual(caught.exception.code, gate.gatelib.CANNOT_RUN) + finally: + gate.ALERTING = saved + + +@unittest.skipUnless(HAS_KUSTOMIZE, "kustomize is not on PATH") +class WhatShipsDecides(unittest.TestCase): + """The routing this gate reads has to be routing a cluster receives. + + `resources` is an explicit list, so every object here can stop being + delivered without moving, being edited, or failing to render. The files + then describe complete delivery and the cluster holds rule groups labelled + for a pager with nothing to match them against — a green gate over the + exact state it exists to refuse. + """ + + verdict = TheVerdict.verdict + ROUTED = TheVerdict.ROUTED + healthy = TheVerdict.healthy + + def test_a_routing_tree_left_out_of_the_kustomization_is_reported(self): + docs = self.healthy() + rc, out = self.verdict(*docs, unshipped=(self.ROUTED,)) + self.assertEqual(rc, 1, out) + self.assertIn("GrafanaNotificationPolicy/routes", out) + self.assertIn("does not render it", out) + + def test_a_contact_point_left_out_of_the_kustomization_is_reported(self): + docs = self.healthy() + rc, out = self.verdict(*docs, unshipped=(contact_point("urgent"),)) + self.assertEqual(rc, 1, out) + self.assertIn("GrafanaContactPoint/urgent", out) + + def test_a_rule_group_left_out_of_the_kustomization_is_reported(self): + """Not only the delivery objects. A rule group nothing renders is a + promise this gate certified and no cluster carries.""" + docs = self.healthy() + rc, out = self.verdict(*docs, unshipped=(docs[3],)) + self.assertEqual(rc, 1, out) + self.assertIn("GrafanaAlertRuleGroup/g", out) + + def test_the_finding_names_the_file(self): + """Eight files of near-identical shape; a verdict that does not name one + is a verdict nobody can act on.""" + rc, out = self.verdict(*self.healthy(), unshipped=(self.ROUTED,)) + self.assertEqual(rc, 1, out) + self.assertIn("GrafanaNotificationPolicy.yaml:", out) + + def test_an_alerting_object_rendered_from_outside_the_directory_is_reported(self): + """The other direction, and the one that decides which tree Grafana + obeys: a second notification policy the gate never opened still + reconciles, and whichever lands last wins.""" + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + alerting = base / "alerting" + alerting.mkdir(parents=True) + resources = [] + for i, doc in enumerate(TheVerdict.healthy(self)): + name = f"{i:02d}-{doc['kind']}.yaml" + (alerting / name).write_text(yaml.safe_dump(doc)) + resources.append(f" - alerting/{name}") + (base / "elsewhere.yaml").write_text( + yaml.safe_dump(policy({"receiver": "low"}, "second-tree"))) + resources.append(" - elsewhere.yaml") + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n" + "\n".join(resources) + "\n") + saved = (gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT) + gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT = root, alerting, base + try: + with contextlib.redirect_stdout(io.StringIO()) as out: + rc = gate.main() + finally: + gate.ROOT, gate.ALERTING, gate.KUSTOMIZE_ROOT = saved + self.assertEqual(rc, 1, out.getvalue()) + self.assertIn("GrafanaNotificationPolicy/second-tree", out.getvalue()) + + def test_a_render_that_fails_cannot_run(self): + """Exit 2. A kustomization that does not build says nothing about what + a cluster receives, and the files on disk say nothing about it either.""" + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + base.mkdir(parents=True) + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n - alerting/gone.yaml\n") + with self.assertRaises(SystemExit) as caught, \ + contextlib.redirect_stdout(io.StringIO()) as out: + gate.shipped(base) + self.assertEqual(caught.exception.code, gate.gatelib.CANNOT_RUN) + self.assertIn("could not be read", out.getvalue()) + + def test_the_render_is_read_rather_than_the_directory_listed_again(self): + """The gate would pass every case above by globbing the same directory + twice. What separates the two readings is that one shells out.""" + source = (ROOT / "scripts" / "check-alert-severity-routes.py").read_text() + self.assertIn('subprocess.run(["kustomize", "build"', source) + + +class TheShippedRootIsTheOneDelivered(unittest.TestCase): + """Over the tree: the root the gate renders is the root an ApplicationSet + hands ArgoCD. A gate rendering a root nothing delivers reports on a tree no + cluster has.""" + + def test_an_applicationset_delivers_the_rendered_root(self): + want = str(gate.KUSTOMIZE_ROOT.relative_to(ROOT)) + delivered = set() + for path in sorted((ROOT / "applicationsets").rglob("*.y*ml")): + doc = yaml.safe_load(path.read_text()) + if not isinstance(doc, dict) or doc.get("kind") != "ApplicationSet": + continue + spec = doc.get("spec") or {} + template = ((spec.get("template") or {}).get("spec") or {}).get("source") or {} + source_path = template.get("path") + if not isinstance(source_path, str): + continue + for el in gate.gatelib.list_elements(doc): + if el.get("path"): + delivered.add( + source_path.replace("{{ .path }}", str(el["path"])).strip("/")) + self.assertIn(want, delivered, + f"{want} is the root this gate renders and no ApplicationSet " + f"delivers it, so the gate is asserting over a tree that " + f"reaches no cluster") + + +class TheKeysComeFromThePolicy(unittest.TestCase): + """Not from a list here, so a routing decision added later is covered.""" + + def test_a_key_other_than_severity_is_required_of_every_rule(self): + keys = gate.routing_keys({"receiver": "r", "routes": [ + {"receiver": "r", "matchers": [exact("team", "platform")]}]}) + self.assertEqual(keys, {"team"}) + + def test_every_key_the_tree_matches_on_is_collected(self): + keys = gate.routing_keys({"receiver": "r", "routes": [ + {"receiver": "a", "matchers": [exact("severity", "page")]}, + {"receiver": "b", "matchers": [exact("team", "platform")], + "routes": [{"receiver": "c", "matchers": [exact("region", "us-west-2")]}]}]}) + self.assertEqual(keys, {"severity", "team", "region"}) + + def test_a_nested_route_is_a_delivery_decision_too(self): + """`routes` nests, and a child route delivers as much as a top-level + one — walking only the first level reports a nested destination as + absent.""" + root = {"receiver": "r", "routes": [ + {"receiver": "a", "matchers": [exact("severity", "page")], + "routes": [{"receiver": "deep", "matchers": [exact("severity", "urgent")]}]}]} + self.assertEqual(len(list(gate.routes_of(root))), 3) + self.assertIn("deep", {n.get("receiver") for n in gate.routes_of(root)}) + + def test_a_regex_matcher_routes_its_key_but_vouches_for_no_value(self): + """Deciding which values a pattern admits means running the pattern, and + a gate reporting a value as routed because it looked like it might match + would be asserting the thing it exists to check.""" + node = {"receiver": "r", "matchers": [ + {"name": "severity", "value": "page|ticket", "isRegex": True}]} + self.assertEqual(gate.routing_keys(node), {"severity"}) + self.assertEqual(gate.matcher_pairs(node), []) + + +class TheShippedCatalogRoutes(unittest.TestCase): + """Over the tree, so a rule added with a new severity fails here.""" + + def test_every_severity_the_rules_use_is_routed(self): + with contextlib.redirect_stdout(io.StringIO()): + self.assertEqual(gate.main(), 0) + + def test_the_rules_carry_at_least_two_severities(self): + """A catalog using one severity would pass a gate that routed only that + one, so the corpus this runs over is asserted rather than assumed.""" + docs = gate.documents(gate.ALERTING) + severities = {str(labels.get("severity")) + for _, d in docs if d.get("kind") == gate.RULE_GROUP + for _, labels in gate.rule_labels(d)} + self.assertGreaterEqual(len(severities), 2, severities) + + def test_every_declared_contact_point_takes_its_secret_from_a_reference(self): + """A credential in git is the other way to make a route resolve, and it + is not one this catalog takes.""" + for path, doc in gate.documents(gate.ALERTING): + if doc.get("kind") != gate.CONTACT_POINT: + continue + for receiver in (doc.get("spec") or {}).get("receivers") or []: + with self.subTest(contact_point=path.name, type=receiver.get("type")): + self.assertTrue(receiver.get("valuesFrom"), + "this receiver carries no valuesFrom, so whatever " + "it authenticates with is committed here") + + +if __name__ == "__main__": + unittest.main()