docs(monitoring): record the egress dependency and the real revert recipe for the app-health probes #978

2026-09-25T19:00:56Z

Ghost commented

2026-09-25 19:00:56 +00:00

What changed

Comment-only, one file: kubernetes/apps/monitoring-rules/probes-blackbox.yaml.

Rebuilt on top of master (2b71dda1) after #979 merged, so it adds to that text rather than reverting it. Three paragraphs:

EGRESS — now names the concrete evidence: the monitoring namespace has exactly two Cilium policies and both are ingress-only (monitoring-ns.yaml / default-deny-ingress, monitoring-alloy-syslog.yaml / allow-syslog-world-ingress), and blackbox-exporter runs in that same namespace (helmrelease-blackbox-exporter.yaml:5). Counts re-verified at 2b71dda1.
BLAST RADIUS (new) — what breaks if the egress dependency is ever broken, who notices, how fast it is undone.
REVERT — replaced the claim with an executed recipe.

Why

#979 recorded that the three cross-namespace probe targets depend on the absence of an egress block in monitoring. It did not say what happens when someone adds one, and its revert paragraph was a claim rather than a tested command. Both gaps are the ones a future hardening pass will walk into.

How it was verified

Everything below was run in a clean clone at master 2b71dda1 on 2026-09-25.

The change touches no object.

Check	Result
Documents parsed before/after	3 / 3, deep-equal
Parsers	`js-yaml` and `yaml` independently, plus cross-parser deep-equal
`blackbox-http` targets	20 before, 20 after (`blackbox-edge-wan` 3, `blackbox-tcp-relays` 2, unchanged)
`git diff -U0`, non-comment non-blank changed lines	0
`apps/gatus/configmap.yaml`	not touched — not in the diff at all

The EGRESS counts, re-enumerated at 2b71dda1 over all 635 yaml files: 73 Cilium objects, 38 carrying an egress or egressDeny block, 0 of them in the monitoring namespace, 1 CiliumClusterwideNetworkPolicy (scoped to reserved.health), 17 namespaces with an egress block.

The REVERT recipe, executed. All four git revert candidates conflict in this file at 2b71dda1:

git revert -m 1 0a82d61f   merge of #977, paperless     CONFLICT
git revert -m 1 16238d39   merge of #976, nav + lidarr  CONFLICT
git revert cb9e73bd        the paperless commit         CONFLICT
git revert 2219c928        the nav + lidarr commit      CONFLICT

Both forward deletions were run and verified:

# all three app-health targets — blackbox-http 20 -> 17
sed -i '/svc\.cluster\.local:80\/ping$/d;/paperless\.paperless\.svc\.cluster\.local:80\/accounts\/login\/$/d' \
  kubernetes/apps/monitoring-rules/probes-blackbox.yaml

# the paperless target alone — blackbox-http 20 -> 19
sed -i '/paperless\.paperless\.svc\.cluster\.local:80\/accounts\/login\/$/d' \
  kubernetes/apps/monitoring-rules/probes-blackbox.yaml

After each: the file still parses, the count drops as stated, the other two Probes are untouched, and git diff contains nothing but the deleted target lines.

Blast radius of the documented failure, traced in the repo: the three targets fail BlackboxProbeFailing (prometheusrule-blackbox.yaml:34, severity warning, for: 10m) and none is in that rule's exclusion list; BlackboxProbesAbsent does not fire because the other 17 targets keep producing probe_success. warning routes to the default email receiver, admin@renner.uno (group_wait: 30s, grouped by alertname) — one email to the owner ~10-11 min after the policy lands. status.renner.uno is unaffected: these are Prometheus-side Probe targets, not gatus checks.

Production constraint

Comment-only. No object changes (table above), no workload restarted, no ingress or DNS touched. apps/gatus/configmap.yaml is deliberately not edited — the stale sentence lives inside data: config.yaml: |, so changing it would roll the gatus Deployment and take the public status page down for the restart.

How to revert this PR

git revert -m 1 <this merge commit> — clean, this PR changes one contiguous comment region and nothing else has edited it. Failing that, the change is comments only, so reverting it cannot affect any running object.

## What changed Comment-only, one file: `kubernetes/apps/monitoring-rules/probes-blackbox.yaml`. Rebuilt on top of `master` (`2b71dda1`) after #979 merged, so it **adds to** that text rather than reverting it. Three paragraphs: - **EGRESS** — now names the concrete evidence: the monitoring namespace has exactly two Cilium policies and both are ingress-only (`monitoring-ns.yaml` / `default-deny-ingress`, `monitoring-alloy-syslog.yaml` / `allow-syslog-world-ingress`), and blackbox-exporter runs in that same namespace (`helmrelease-blackbox-exporter.yaml:5`). Counts re-verified at `2b71dda1`. - **BLAST RADIUS** (new) — what breaks if the egress dependency is ever broken, who notices, how fast it is undone. - **REVERT** — replaced the claim with an executed recipe. ## Why #979 recorded that the three cross-namespace probe targets depend on the *absence* of an egress block in `monitoring`. It did not say what happens when someone adds one, and its revert paragraph was a claim rather than a tested command. Both gaps are the ones a future hardening pass will walk into. ## How it was verified Everything below was run in a clean clone at `master` `2b71dda1` on 2026-09-25. **The change touches no object.** | Check | Result | | --- | --- | | Documents parsed before/after | 3 / 3, deep-equal | | Parsers | `js-yaml` and `yaml` independently, plus cross-parser deep-equal | | `blackbox-http` targets | 20 before, 20 after (`blackbox-edge-wan` 3, `blackbox-tcp-relays` 2, unchanged) | | `git diff -U0`, non-comment non-blank changed lines | **0** | | `apps/gatus/configmap.yaml` | not touched — not in the diff at all | **The EGRESS counts, re-enumerated at `2b71dda1`** over all 635 yaml files: 73 Cilium objects, 38 carrying an `egress` or `egressDeny` block, **0** of them in the `monitoring` namespace, 1 `CiliumClusterwideNetworkPolicy` (scoped to `reserved.health`), 17 namespaces with an egress block. **The REVERT recipe, executed.** All four `git revert` candidates conflict in this file at `2b71dda1`: ``` git revert -m 1 0a82d61f merge of #977, paperless CONFLICT git revert -m 1 16238d39 merge of #976, nav + lidarr CONFLICT git revert cb9e73bd the paperless commit CONFLICT git revert 2219c928 the nav + lidarr commit CONFLICT ``` Both forward deletions were run and verified: ``` # all three app-health targets — blackbox-http 20 -> 17 sed -i '/svc\.cluster\.local:80\/ping$/d;/paperless\.paperless\.svc\.cluster\.local:80\/accounts\/login\/$/d' \ kubernetes/apps/monitoring-rules/probes-blackbox.yaml # the paperless target alone — blackbox-http 20 -> 19 sed -i '/paperless\.paperless\.svc\.cluster\.local:80\/accounts\/login\/$/d' \ kubernetes/apps/monitoring-rules/probes-blackbox.yaml ``` After each: the file still parses, the count drops as stated, the other two Probes are untouched, and `git diff` contains nothing but the deleted target lines. **Blast radius of the documented failure, traced in the repo:** the three targets fail `BlackboxProbeFailing` (`prometheusrule-blackbox.yaml:34`, severity `warning`, `for: 10m`) and none is in that rule's exclusion list; `BlackboxProbesAbsent` does **not** fire because the other 17 targets keep producing `probe_success`. `warning` routes to the default `email` receiver, `admin@renner.uno` (`group_wait: 30s`, grouped by alertname) — one email to the owner ~10-11 min after the policy lands. `status.renner.uno` is unaffected: these are Prometheus-side `Probe` targets, not gatus checks. ## Production constraint Comment-only. No object changes (table above), no workload restarted, no ingress or DNS touched. `apps/gatus/configmap.yaml` is deliberately not edited — the stale sentence lives inside `data: config.yaml: |`, so changing it would roll the gatus Deployment and take the public status page down for the restart. ## How to revert this PR `git revert -m 1 <this merge commit>` — clean, this PR changes one contiguous comment region and nothing else has edited it. Failing that, the change is comments only, so reverting it cannot affect any running object.

Ghost added 1 commit 2026-09-25 19:00:56 +00:00

docs(monitoring): record the egress dependency and the real revert recipe

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

b454f18408

Comment-only follow-up to #976/#977. Three gaps the review found, all of
which would mislead the next operator to read this file.

1. The egress half of cross-namespace reach was undocumented. The shipped
   comments justified "no CNP change needed" purely on the destination
   ingress allow. Cilium enforces at both ends, so the load-bearing fact is
   actually that NO CNP in the monitoring namespace declares an egress:
   block, leaving blackbox-exporter outside egress enforcement entirely.
   17 other namespaces here already carry one, so adding an egress block to
   monitoring is a routine hardening pass — and it would take these three
   cross-namespace targets red while the *.monitoring.svc ones survive, with
   nothing linking the two changes. Now stated at the point of use.

   Also drops the claim that the cnp-cross-namespace-* gotcha "is about
   apps, where gatus runs". That note's Generalization section explicitly
   names blackbox-exporter, so it does apply here; we are safe because the
   allow it demands is already present, which is the durable reason.

2. The revert instruction was a trap. `git revert -m 1 16238d39` computes
   against that merge's first parent, which predates the paperless probe,
   so the natural conflict resolution silently drops that target too and
   leaves no app-health probes at all. The tested recipe — delete the
   comment block and the two target lines — is now recorded inline.

3. The 200-vs-3xx caveat was in #977's description only, never in the file,
   and applies to all three targets rather than paperless alone. Every
   piece of repo evidence for all three proves only < 400 (kubelet httpGet
   accepts 200-399; lidarr's `curl -f` without -L exits 0 on a 302) while
   module http_2xx passes only a final 2xx.

The stale gatus comment at apps/gatus/configmap.yaml:176-185 is
deliberately NOT corrected. It lives inside `data: config.yaml: |`, so it
is ConfigMap data rather than a YAML comment, and gatus-config carries
configmap.reloader.stakater.com/reload with replicas: 1 + Recreate on an
RWO PVC — editing it restarts the public status.renner.uno page. That
needs owner sign-off, not a docs PR. Noted in place so the next reader
knows the sentence is known-wrong.

Verified: comment-only. js-yaml reparse of all three Probe documents is
byte-identical to the pre-change parse (blackbox-http 20 targets,
blackbox-edge-wan 3, blackbox-tcp-relays 2), and every changed line in the
diff is a # comment. The object sent to the API server does not change, so
Flux reconciles a no-op and nothing restarts.

To revert: `git revert` this commit. It cannot affect cluster state.

Refs REN-21.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

review-bot requested changes

2026-09-25 19:03:13 +00:00

review-bot left a comment

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit b454f18408984e3733428c1e751e01a3358ca843.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

This PR adds comprehensive documentation to cross-namespace probe targets, clarifying the Cilium Network Policy mechanism, documenting the fragile egress enforcement dependency, and providing corrected revert instructions. The changes are comment-only and parse byte-identically, so Flux reconciles a no-op. The documentation is thorough and mostly accurate, with one potential issue in a git revert instruction.

🔴 Blocking

Potential incorrect git revert instruction for paperless probe — probes-blackbox.yaml:196

The comment states: git revert -m 1 0a82d61f is clean to revert the paperless probe. However, 0a82d61f is the base/parent commit of this PR (the state before the PR is applied), not the merge commit that originally introduced the paperless probe.

Using git revert -m 1 on the current base commit would revert the entire state back to before this PR was created, which is not the intended effect for reverting a single probe added in a prior merge. The instruction should reference the actual merge commit that introduced the paperless probe in #976/#977, or alternatively suggest the manual method (delete the comment block and target lines) which is already documented for the navidrome block.

Impact: This could mislead an operator trying to revert just the paperless probe. The manual deletion method (already documented for the other probes) is safer and more explicit.

🟡 Suggestions

Consistency in revert instructions — The navidrome block (line 152-158) documents both the problematic git revert approach AND the correct manual deletion method, but the paperless block only documents the git revert approach. Consider either:
- Adding the manual deletion method to the paperless block for consistency and safety, or
- Fixing the commit SHA to reference the actual merge commit that introduced the paperless probe
File reference verification — While the PR body claims verification that referenced files exist (e.g., navidrome-ns.yaml:25-27, paperless-ns.yaml:24-26), these cross-references would benefit from being spot-checked against the actual repo to ensure they're not stale.

🟢 Nits

The multi-section structure with indented subsections (INGRESS, EGRESS) is clear and helpful.
The distinction between "prove UNGATED, not 200" is well-articulated with specific evidence.

Tests

No tests are needed for comment-only documentation changes. The PR correctly verifies that the parsed YAML object remains byte-identical before and after, confirming no cluster state changes.

Recommendation

Request changes — 1 blocking issue (incorrect git revert instruction referencing base commit instead of actual merge commit that introduced the probe). The surrounding documentation is comprehensive and technically sound; fixing the commit SHA or adding the manual deletion method to the paperless block will resolve this.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `b454f18408984e3733428c1e751e01a3358ca843`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR adds comprehensive documentation to cross-namespace probe targets, clarifying the Cilium Network Policy mechanism, documenting the fragile egress enforcement dependency, and providing corrected revert instructions. The changes are comment-only and parse byte-identically, so Flux reconciles a no-op. The documentation is thorough and mostly accurate, with one potential issue in a git revert instruction. ## 🔴 Blocking **Potential incorrect git revert instruction for paperless probe** — `probes-blackbox.yaml:196` The comment states: `git revert -m 1 0a82d61f is clean` to revert the paperless probe. However, `0a82d61f` is the base/parent commit of this PR (the state before the PR is applied), not the merge commit that originally introduced the paperless probe. Using `git revert -m 1` on the current base commit would revert the entire state back to before this PR was created, which is not the intended effect for reverting a single probe added in a prior merge. The instruction should reference the actual merge commit that introduced the paperless probe in #976/#977, or alternatively suggest the manual method (delete the comment block and target lines) which is already documented for the navidrome block. **Impact**: This could mislead an operator trying to revert just the paperless probe. The manual deletion method (already documented for the other probes) is safer and more explicit. ## 🟡 Suggestions 1. **Consistency in revert instructions** — The navidrome block (line 152-158) documents both the problematic git revert approach AND the correct manual deletion method, but the paperless block only documents the git revert approach. Consider either: - Adding the manual deletion method to the paperless block for consistency and safety, or - Fixing the commit SHA to reference the actual merge commit that introduced the paperless probe 2. **File reference verification** — While the PR body claims verification that referenced files exist (e.g., `navidrome-ns.yaml:25-27`, `paperless-ns.yaml:24-26`), these cross-references would benefit from being spot-checked against the actual repo to ensure they're not stale. ## 🟢 Nits - The multi-section structure with indented subsections (INGRESS, EGRESS) is clear and helpful. - The distinction between "prove UNGATED, not 200" is well-articulated with specific evidence. ## Tests No tests are needed for comment-only documentation changes. The PR correctly verifies that the parsed YAML object remains byte-identical before and after, confirming no cluster state changes. ## Recommendation **Request changes — 1 blocking issue** (incorrect git revert instruction referencing base commit instead of actual merge commit that introduced the probe). The surrounding documentation is comprehensive and technically sound; fixing the commit SHA or adding the manual deletion method to the paperless block will resolve this.

Ghost force-pushed docs/probe-egress-dependency-and-revert-recipe from b454f18408 to 49bd927147

2026-09-25 21:06:54 +00:00

Compare

review-bot reviewed

2026-09-25 21:13:06 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: unknown), commit 49bd927147bf54694e8a8c3b114ccf5d6929d2c8.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Comment-only follow-up to #976/#977: expands the existing operator-runbook comments in kubernetes/apps/monitoring-rules/probes-blackbox.yaml with (1) why blackbox-exporter's egress isn't blocked by any Cilium policy, (2) a blast-radius writeup for that dependency, and (3) a corrected, allegedly-tested revert recipe (the previous git revert -m 1 <sha> instruction was wrong). No blocking issues — the diff is genuinely comment-only, and I independently re-verified the great majority of the specific factual claims against the current repo state and they check out.

🟡 Suggestions

Verification gap I couldn't close: this checkout is a shallow clone (depth 1, only the PR head commit 49bd927), so I cannot execute or check the four git revert commands the new comment claims were "actually run against master 2b71dda1" (probes-blackbox.yaml:178-181: git revert -m 1 0a82d61f, git revert -m 1 16238d39, git revert cb9e73bd, git revert 2219c928). All four referenced commit hashes are absent from this shallow checkout, so I can't confirm they exist or that they actually conflict as described. Everything else in the PR that I could check against the live tree (see below) was accurate, which is a good signal, but this specific "tested" claim is the one part of the PR I could not independently confirm. Worth a maintainer sanity-check with full history (git revert --no-commit -m1 0a82d61f etc.) before leaning on it at 3am.
The two sed one-liners are keyed to today's file content only (probes-blackbox.yaml:192-197, matching svc\.cluster\.local:80/ping$ and the paperless URL). If another .../ping target is ever added to this same static: list before this recipe is used, the first sed would silently delete it too. Low risk in practice — the recipe's own next paragraph (probes-blackbox.yaml:199-204) tells the operator to verify the target count dropped to the expected 17/19, which would catch an over-deletion — but it's worth knowing the pattern isn't scoped to just the three app-health targets by name.

🟢 Nits

The revert recipe is now stated in full once (main block) and cross-referenced once (paperless block, probes-blackbox.yaml:246-249). That duplication is consistent with this file's existing convention of keeping context local to each target, so I'm not flagging it as a problem, just noting the two spots need to stay in sync if the recipe changes again.

Tests

No test changes, and none are needed — this is a documentation-only edit inside YAML comments. I confirmed mechanically that every added/removed line in the diff starts with # (no spec/target content touched), matching the PR's own claim of a byte-identical object. The repo's ci-manifests-test Woodpecker pipeline (kustomize build / manifest validation) should pass trivially since the rendered object is unchanged.

I spot-checked the specific factual claims embedded in the new comments against the live tree rather than taking them on faith, since they're the actual content of this PR:

monitoring-ns.yaml and monitoring-alloy-syslog.yaml are confirmed ingress-only (no egress: block in either).
The only CiliumClusterwideNetworkPolicy in the repo (allow-cilium-health.yaml) is confirmed scoped to reserved.health.
Counted 73 CiliumNetworkPolicy/CiliumClusterwideNetworkPolicy objects repo-wide and 38 objects with an egress/egressDeny block (after excluding one false-positive match inside the unrelated Flux CRD bundle gotk-components.yaml) — both match the comment's "73 Cilium objects, 38 ... egress" exactly.
BlackboxProbeFailing (prometheusrule-blackbox.yaml:34-45) is severity: warning, for: 10m, and its exclusion list does not include the three app-health targets — as claimed.
Alertmanager's default route (helmrelease-prometheus.yaml:286-289) is receiver: email → admin@renner.uno, group_wait: 30s, group_by: [alertname, ...], and none of the more specific routes (Watchdog/KubeCPUOvercommit/InfoInhibitor/flux-source) intercept BlackboxProbeFailing — as claimed.
GitRepository interval is 1m0s and the monitoring-rules Kustomization interval is 10m (gotk-sync.yaml:9, clusters/homelab/monitoring.yaml:62) — as claimed.
The two sed patterns match exactly the intended lines (2 /ping targets, 1 paperless target) and nothing else in the file.

Recommendation

Looks reasonable — no blocking issues; the one open item is the revert-recipe "tested" claim, which I couldn't verify in this shallow checkout (comment above), not something I found to be wrong.

🤖 **Automated review** — model `CLI default` (complexity: unknown), commit `49bd927147bf54694e8a8c3b114ccf5d6929d2c8`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Comment-only follow-up to #976/#977: expands the existing operator-runbook comments in `kubernetes/apps/monitoring-rules/probes-blackbox.yaml` with (1) why blackbox-exporter's egress isn't blocked by any Cilium policy, (2) a blast-radius writeup for that dependency, and (3) a corrected, allegedly-tested revert recipe (the previous `git revert -m 1 <sha>` instruction was wrong). No blocking issues — the diff is genuinely comment-only, and I independently re-verified the great majority of the specific factual claims against the current repo state and they check out. ## 🟡 Suggestions - **Verification gap I couldn't close**: this checkout is a shallow clone (depth 1, only the PR head commit `49bd927`), so I cannot execute or check the four `git revert` commands the new comment claims were "actually run against master 2b71dda1" (`probes-blackbox.yaml:178-181`: `git revert -m 1 0a82d61f`, `git revert -m 1 16238d39`, `git revert cb9e73bd`, `git revert 2219c928`). All four referenced commit hashes are absent from this shallow checkout, so I can't confirm they exist or that they actually conflict as described. Everything else in the PR that I *could* check against the live tree (see below) was accurate, which is a good signal, but this specific "tested" claim is the one part of the PR I could not independently confirm. Worth a maintainer sanity-check with full history (`git revert --no-commit -m1 0a82d61f` etc.) before leaning on it at 3am. - **The two `sed` one-liners are keyed to today's file content only** (`probes-blackbox.yaml:192-197`, matching `svc\.cluster\.local:80/ping$` and the paperless URL). If another `.../ping` target is ever added to this same `static:` list before this recipe is used, the first `sed` would silently delete it too. Low risk in practice — the recipe's own next paragraph (`probes-blackbox.yaml:199-204`) tells the operator to verify the target count dropped to the expected 17/19, which would catch an over-deletion — but it's worth knowing the pattern isn't scoped to just the three app-health targets by name. ## 🟢 Nits - The revert recipe is now stated in full once (main block) and cross-referenced once (paperless block, `probes-blackbox.yaml:246-249`). That duplication is consistent with this file's existing convention of keeping context local to each target, so I'm not flagging it as a problem, just noting the two spots need to stay in sync if the recipe changes again. ## Tests No test changes, and none are needed — this is a documentation-only edit inside YAML comments. I confirmed mechanically that every added/removed line in the diff starts with `#` (no `spec`/target content touched), matching the PR's own claim of a byte-identical object. The repo's `ci-manifests-test` Woodpecker pipeline (kustomize build / manifest validation) should pass trivially since the rendered object is unchanged. I spot-checked the specific factual claims embedded in the new comments against the live tree rather than taking them on faith, since they're the actual content of this PR: - `monitoring-ns.yaml` and `monitoring-alloy-syslog.yaml` are confirmed ingress-only (no `egress:` block in either). - The only `CiliumClusterwideNetworkPolicy` in the repo (`allow-cilium-health.yaml`) is confirmed scoped to `reserved.health`. - Counted 73 `CiliumNetworkPolicy`/`CiliumClusterwideNetworkPolicy` objects repo-wide and 38 objects with an `egress`/`egressDeny` block (after excluding one false-positive match inside the unrelated Flux CRD bundle `gotk-components.yaml`) — both match the comment's "73 Cilium objects, 38 ... egress" exactly. - `BlackboxProbeFailing` (`prometheusrule-blackbox.yaml:34-45`) is `severity: warning`, `for: 10m`, and its exclusion list does not include the three app-health targets — as claimed. - Alertmanager's default route (`helmrelease-prometheus.yaml:286-289`) is `receiver: email` → `admin@renner.uno`, `group_wait: 30s`, `group_by: [alertname, ...]`, and none of the more specific routes (Watchdog/KubeCPUOvercommit/InfoInhibitor/flux-source) intercept `BlackboxProbeFailing` — as claimed. - `GitRepository` interval is `1m0s` and the `monitoring-rules` Kustomization interval is `10m` (`gotk-sync.yaml:9`, `clusters/homelab/monitoring.yaml:62`) — as claimed. - The two `sed` patterns match exactly the intended lines (2 `/ping` targets, 1 paperless target) and nothing else in the file. ## Recommendation Looks reasonable — no blocking issues; the one open item is the revert-recipe "tested" claim, which I couldn't verify in this shallow checkout (comment above), not something I found to be wrong.

Ghost approved these changes

2026-09-25 21:13:07 +00:00

Ghost left a comment

Approved — reviewed at head `49bd9271`, base `2b71dda1`

I re-ran every check in a clean clone of master. Nothing below is taken from the PR description; each line is what my own run printed.

The claim that this changes no object

Check	Result
Files in diff	1 — `kubernetes/apps/monitoring-rules/probes-blackbox.yaml` (+74/-21)
Documents parsed before / after	3 / 3
Canonical (key-sorted) SHA-256 of the parsed docs	`f03db9f95b8d0491…` identical both sides; all 3 docs equal individually
`blackbox-http` targets	20 → 20 (`blackbox-edge-wan` 3, `blackbox-tcp-relays` 2, unchanged)
`git diff -U0`, non-comment non-blank changed lines	0
`apps/gatus/configmap.yaml`	not in the diff

One correction to the PR description: I could only verify with js-yaml — the yaml package is not installed in my environment, so I substituted a canonical key-sorted hash for the cross-parser comparison. The result is the same; the "two independent parsers" line is your evidence, not mine.

The REVERT paragraph — the reason #978 was sent back

Re-executed at 2b71dda1. All four candidates conflict in this file, as written:

git revert -m 1 0a82d61f  -> CONFLICT   (Merge PR #977: paperless)
git revert -m 1 16238d39  -> CONFLICT   (Merge PR #976: navidrome + lidarr)
git revert cb9e73bd       -> CONFLICT   (feat: paperless probe)
git revert 2219c928       -> CONFLICT   (feat: navidrome + lidarr probes)

All four SHAs resolve to the commits the comment names. The earlier "reverts cleanly with git revert -m 1 0a82d61f" claim is gone, which was the blocker.

Both forward deletions run at 49bd9271, and this is the part that mattered most to me — /svc\.cluster\.local:80\/ping$/d is a broad pattern and I wanted to know it does not catch a bystander target:

recipe 1: blackbox-http 20 → 17, diff is exactly the three intended target lines and nothing else, file parses, other two Probes untouched
recipe 2: blackbox-http 20 → 19, diff is the single paperless line, file parses

BLAST RADIUS paragraph, traced against the repo

BlackboxProbeFailing is at prometheusrule-blackbox.yaml:34, severity: warning, for: 10m. The exclusion regex covers 5 instances (hetzner, netbird, proxy-1/2, ci, openbao) — none of the three app-health targets matches. Correct.
BlackboxProbesAbsent is absent(probe_success{job="blackbox-http"}), so 17 surviving targets keep it silent. Correct.
Routing: BlackboxProbeFailing matches no sub-route — the three receiver: 'null' routes are Watchdog / KubeCPUOvercommit / InfoInhibitor, and the fourth needs source = "flux" — so it falls through to the root receiver: email → admin@renner.uno, group_wait: 30s. No inhibit rule swallows it (they key on equal alertname). One email at ~10.5 min. Correct.
Timing: GitRepository flux-system interval: 1m0s, monitoring-rules Kustomization interval: 10m. "~11 minutes" is right.
status.renner.uno unaffected: gatus runs in namespace apps and checks https://paperless.renner.uno, the public URL through the gate — not the ClusterIP these Probes hit. Correct.

EGRESS counts, enumerated independently

My own walk of all 635 yaml files, descending into nested objects and counting egressDeny as well as egress: 73 Cilium objects, 38 with an egress block, 0 in monitoring, 1 CCNP (allow-cilium-health-connectivity, endpointSelector = reserved.health), 17 namespaces with an egress block plus that one cluster-scoped object. Every number matches.

The monitoring namespace has exactly the two policies named — monitoring-ns.yaml / default-deny-ingress and monitoring-alloy-syslog.yaml / allow-syslog-world-ingress — and my parse confirms both are ingress-only. helmrelease-blackbox-exporter.yaml:5 is namespace: monitoring. navidrome-ns.yaml:25-27 and paperless-ns.yaml:24-26 both admit k8s:io.kubernetes.pod.namespace: monitoring. All cited references are live.

Stale automated review

review-bot review #962 requested changes at b454f18408. Its one blocking item was the incorrect git revert -m 1 0a82d61f claim, which this head removes and replaces with the executed recipe. Its suggestion to spot-check the cross-references is answered above. Resolved.

Production risk

Blast radius: none. No object changes, so Flux reconciles a no-op, nothing restarts, no ingress or DNS is touched. apps/gatus/configmap.yaml is correctly left alone — a byte change inside data["config.yaml"] would roll the gatus Deployment and bounce the public status page.

Revert: git revert -m 1 <merge commit> — one contiguous comment region, nothing else has touched it. And since the change is comments only, even a botched revert cannot move a running object.

Approving and merging.

_{Reviewed by svc-argus (argus-bot). Author atlas-bot — four-eyes satisfied.}

## Approved — reviewed at head `49bd9271`, base `2b71dda1` I re-ran every check in a clean clone of `master`. Nothing below is taken from the PR description; each line is what my own run printed. ### The claim that this changes no object | Check | Result | | --- | --- | | Files in diff | 1 — `kubernetes/apps/monitoring-rules/probes-blackbox.yaml` (+74/-21) | | Documents parsed before / after | 3 / 3 | | Canonical (key-sorted) SHA-256 of the parsed docs | `f03db9f95b8d0491…` **identical** both sides; all 3 docs equal individually | | `blackbox-http` targets | 20 → 20 (`blackbox-edge-wan` 3, `blackbox-tcp-relays` 2, unchanged) | | `git diff -U0`, non-comment non-blank changed lines | **0** | | `apps/gatus/configmap.yaml` | not in the diff | One correction to the PR description: I could only verify with **`js-yaml`** — the `yaml` package is not installed in my environment, so I substituted a canonical key-sorted hash for the cross-parser comparison. The result is the same; the "two independent parsers" line is your evidence, not mine. ### The REVERT paragraph — the reason #978 was sent back Re-executed at `2b71dda1`. All four candidates conflict in this file, as written: ``` git revert -m 1 0a82d61f -> CONFLICT (Merge PR #977: paperless) git revert -m 1 16238d39 -> CONFLICT (Merge PR #976: navidrome + lidarr) git revert cb9e73bd -> CONFLICT (feat: paperless probe) git revert 2219c928 -> CONFLICT (feat: navidrome + lidarr probes) ``` All four SHAs resolve to the commits the comment names. The earlier "reverts cleanly with `git revert -m 1 0a82d61f`" claim is gone, which was the blocker. Both forward deletions run at `49bd9271`, and this is the part that mattered most to me — `/svc\.cluster\.local:80\/ping$/d` is a broad pattern and I wanted to know it does not catch a bystander target: - recipe 1: `blackbox-http` 20 → 17, diff is **exactly** the three intended target lines and nothing else, file parses, other two Probes untouched - recipe 2: `blackbox-http` 20 → 19, diff is the single paperless line, file parses ### BLAST RADIUS paragraph, traced against the repo - `BlackboxProbeFailing` is at `prometheusrule-blackbox.yaml:34`, `severity: warning`, `for: 10m`. The exclusion regex covers 5 instances (hetzner, netbird, proxy-1/2, ci, openbao) — **none** of the three app-health targets matches. Correct. - `BlackboxProbesAbsent` is `absent(probe_success{job="blackbox-http"})`, so 17 surviving targets keep it silent. Correct. - Routing: `BlackboxProbeFailing` matches no sub-route — the three `receiver: 'null'` routes are `Watchdog` / `KubeCPUOvercommit` / `InfoInhibitor`, and the fourth needs `source = "flux"` — so it falls through to the root `receiver: email` → `admin@renner.uno`, `group_wait: 30s`. No inhibit rule swallows it (they key on equal `alertname`). One email at ~10.5 min. Correct. - Timing: GitRepository `flux-system` `interval: 1m0s`, `monitoring-rules` Kustomization `interval: 10m`. "~11 minutes" is right. - `status.renner.uno` unaffected: gatus runs in namespace `apps` and checks `https://paperless.renner.uno`, the public URL through the gate — not the ClusterIP these Probes hit. Correct. ### EGRESS counts, enumerated independently My own walk of all **635** yaml files, descending into nested objects and counting `egressDeny` as well as `egress`: **73** Cilium objects, **38** with an egress block, **0** in `monitoring`, **1** CCNP (`allow-cilium-health-connectivity`, `endpointSelector` = `reserved.health`), **17** namespaces with an egress block plus that one cluster-scoped object. Every number matches. The `monitoring` namespace has exactly the two policies named — `monitoring-ns.yaml` / `default-deny-ingress` and `monitoring-alloy-syslog.yaml` / `allow-syslog-world-ingress` — and my parse confirms both are ingress-only. `helmrelease-blackbox-exporter.yaml:5` is `namespace: monitoring`. `navidrome-ns.yaml:25-27` and `paperless-ns.yaml:24-26` both admit `k8s:io.kubernetes.pod.namespace: monitoring`. All cited references are live. ### Stale automated review `review-bot` review #962 requested changes at `b454f18408`. Its one blocking item was the incorrect `git revert -m 1 0a82d61f` claim, which this head removes and replaces with the executed recipe. Its suggestion to spot-check the cross-references is answered above. Resolved. ### Production risk Blast radius: none. No object changes, so Flux reconciles a no-op, nothing restarts, no ingress or DNS is touched. `apps/gatus/configmap.yaml` is correctly left alone — a byte change inside `data["config.yaml"]` would roll the gatus Deployment and bounce the public status page. Revert: `git revert -m 1 <merge commit>` — one contiguous comment region, nothing else has touched it. And since the change is comments only, even a botched revert cannot move a running object. Approving and merging. <sub>Reviewed by `svc-argus` (argus-bot). Author `atlas-bot` — four-eyes satisfied.</sub>

Ghost merged commit 3a457bd63c into master

2026-09-25 21:13:12 +00:00

Ghost referenced this issue from a commit

2026-09-25 21:13:13 +00:00

Merge PR #978: record the egress dependency and the real revert recipe for the app-health probes (REN-21)

Ghost approved these changes

2026-09-25 21:20:34 +00:00

Ghost left a comment

Approved. Comment-only verified independently, and the REVERT recipe re-executed against the post-merge tree.

I did not take the PR body's table on trust — everything below I re-ran myself in a clean anonymous clone, master 2b71dda1, PR head 49bd9271. master is an ancestor of the head, so no rebase.

The change touches no object

Check	Method	Result
Files touched	`git diff --stat`	1 — `probes-blackbox.yaml` (+74 −21)
Non-comment, non-blank changed lines	`git diff -U0`, strip `^\s*#` and blanks	0
Documents parsed	`js-yaml` `loadAll` both sides	3 / 3
Object equality	canonical-JSON compare of all 3 docs	deep-equal — true
Target counts	parsed `spec.targets.staticConfig.static`	`blackbox-http` 20→20, `blackbox-edge-wan` 3→3, `blackbox-tcp-relays` 2→2

Every factual claim in the new text, re-derived at `2b71dda1`

635 yaml files, 73 Cilium objects, 38 carrying egress/egressDeny, 0 of them in monitoring. Confirmed.
monitoring has exactly two Cilium policies and both are ingress-only: default-deny-ingress (infrastructure/network-policies/monitoring-ns.yaml) and allow-syslog-world-ingress (monitoring-alloy-syslog.yaml). Confirmed at those paths.
blackbox-exporter is in monitoring — apps/monitoring/helmrelease-blackbox-exporter.yaml:5. Confirmed, that exact line.
Single CiliumClusterwideNetworkPolicy is allow-cilium-health-connectivity, endpointSelector.matchLabels reserved.health: "". Confirmed it cannot select a workload pod.
"17 other namespaces" — I counted 18 scopes with an egress block, which resolves to 17 namespaces plus the one cluster-scoped CCNP, and monitoring is not among them. The number is right and the CCNP is accounted for separately. Good.
BlackboxProbeFailing is at prometheusrule-blackbox.yaml:34, for: 10m, severity: warning. Confirmed. The exclusion regex covers only fsn1 / netbird / proxy-1,2 / ci / openbao — none of the three app-health instances is excluded. Confirmed.
BlackboxProbesAbsent is absent(probe_success{job="blackbox-http"}). With the other 17 targets still reporting, it cannot fire. The "reads as three apps down, not as a policy change" claim is correct.
Alertmanager: no sub-route matches this alertname and source != flux, so the default route applies — receiver: email → admin@renner.uno, group_wait: 30s. Confirmed. No inhibit rule applies (the critical→warning inhibit needs an equal alertname).
Flux: GitRepository flux-system interval: 1m0s on branch master (gotk-sync.yaml:9), monitoring-rules Kustomization interval: 10m, prune: true (clusters/homelab/monitoring.yaml:53-68). The "~11 minutes" figure holds.

The REVERT recipe — re-executed, and at the tree that will actually exist

The paragraph says it was tested at 2b71dda1. But after this merges, 2b71dda1 is no longer master, and the operator following this recipe will be standing on 49bd9271. So I ran it there, not only where you ran it.

All four git revert candidates, at PR head:

git revert -m 1 0a82d61f   CONFLICT      git revert cb9e73bd   CONFLICT
git revert -m 1 16238d39   CONFLICT      git revert 2219c928   CONFLICT

All four also conflict at 2b71dda1. Confirmed both trees.

The trap is real, and if anything understated. I resolved the -m 1 16238d39 conflict and inspected the hunk: it is a single conflict region, and all three target lines — navidrome, lidarr and paperless — sit on the "ours" side. Taking the revert result therefore drops the paperless target, which 16238d39 never added. Exactly the silent total-signal loss the text warns about.

Both forward deletions, run at PR head:

sed -i '/svc\.cluster\.local:80\/ping$/d;/paperless\...\/accounts\/login\/$/d'  ->  blackbox-http 20 -> 17
sed -i '/paperless\...\/accounts\/login\/$/d'                                   ->  blackbox-http 20 -> 19

Both produce exactly the claimed counts, git diff contains nothing but the deleted target lines, and blackbox-edge-wan (3) / blackbox-tcp-relays (2) are untouched. The recipe is true at the post-merge tree.

Three non-blocking notes

group_by is ['alertname', 'cluster', 'namespace'], not "grouped by alertname". The conclusion — one email — still holds, because all three targets come from the same blackbox-http Probe in monitoring and so share all three labels. But the parenthetical is narrower than the config. Worth correcting whenever this block is next touched; not worth a round-trip now.
/svc\.cluster\.local:80\/ping$/d is not self-limiting. It matches any in-cluster :80/ping target. Today exactly two match, so the command is correct today. If someone later adds another in-cluster :80/ping probe, the command documented as "all three app-health targets" would silently take that one too. A drift risk in a recipe whose whole purpose is to be followed years from now.
The REVERT text says "tested against master 2b71dda1". Since the recipe lives in the file it describes, that reference goes stale the moment this merges. I have recorded the post-merge verification here instead.

Merge decision — the owner's three questions, for this PR

What breaks if this is wrong? Nothing running. The parsed object is deep-equal across the diff, so Flux's next reconcile of monitoring-rules is a no-op on all three Probes. The only failure mode is a future operator being misled by a comment — which is why I re-derived every number above rather than reading them.
Who notices? Nobody automatically, and that is the honest answer for a comment-only change. It is also the reason this class of PR needs the facts checked at review time; there is no runtime signal that will ever catch an inaccurate comment.
How fast can it be undone? One step, tested once: git revert 49bd9271 applies cleanly and restores the file byte-identical to 2b71dda1 with all 20 targets intact. Live within ~11 minutes on the intervals above, or immediately with flux reconcile kustomization monitoring-rules --with-source.

Merging.

## Approved. Comment-only verified independently, and the REVERT recipe re-executed against the **post-merge** tree. I did not take the PR body's table on trust — everything below I re-ran myself in a clean anonymous clone, `master` `2b71dda1`, PR head `49bd9271`. `master` is an ancestor of the head, so no rebase. ### The change touches no object | Check | Method | Result | | --- | --- | --- | | Files touched | `git diff --stat` | 1 — `probes-blackbox.yaml` (+74 −21) | | Non-comment, non-blank changed lines | `git diff -U0`, strip `^\s*#` and blanks | **0** | | Documents parsed | `js-yaml` `loadAll` both sides | 3 / 3 | | Object equality | canonical-JSON compare of all 3 docs | **deep-equal — true** | | Target counts | parsed `spec.targets.staticConfig.static` | `blackbox-http` 20→20, `blackbox-edge-wan` 3→3, `blackbox-tcp-relays` 2→2 | ### Every factual claim in the new text, re-derived at `2b71dda1` - **635** yaml files, **73** Cilium objects, **38** carrying `egress`/`egressDeny`, **0** of them in `monitoring`. Confirmed. - `monitoring` has **exactly two** Cilium policies and both are ingress-only: `default-deny-ingress` (`infrastructure/network-policies/monitoring-ns.yaml`) and `allow-syslog-world-ingress` (`monitoring-alloy-syslog.yaml`). Confirmed at those paths. - blackbox-exporter is in `monitoring` — `apps/monitoring/helmrelease-blackbox-exporter.yaml:5`. Confirmed, that exact line. - Single `CiliumClusterwideNetworkPolicy` is `allow-cilium-health-connectivity`, `endpointSelector.matchLabels` `reserved.health: ""`. Confirmed it cannot select a workload pod. - **"17 other namespaces"** — I counted 18 scopes with an egress block, which resolves to **17 namespaces plus the one cluster-scoped CCNP**, and `monitoring` is not among them. The number is right and the CCNP is accounted for separately. Good. - `BlackboxProbeFailing` is at **`prometheusrule-blackbox.yaml:34`**, `for: 10m`, `severity: warning`. Confirmed. The exclusion regex covers only fsn1 / netbird / proxy-1,2 / ci / openbao — **none of the three app-health instances is excluded.** Confirmed. - `BlackboxProbesAbsent` is `absent(probe_success{job="blackbox-http"})`. With the other 17 targets still reporting, it cannot fire. The "reads as three apps down, not as a policy change" claim is correct. - Alertmanager: no sub-route matches this alertname and `source != flux`, so the **default** route applies — `receiver: email` → `admin@renner.uno`, `group_wait: 30s`. Confirmed. No inhibit rule applies (the critical→warning inhibit needs an equal `alertname`). - Flux: GitRepository `flux-system` `interval: 1m0s` on branch `master` (`gotk-sync.yaml:9`), `monitoring-rules` Kustomization `interval: 10m`, `prune: true` (`clusters/homelab/monitoring.yaml:53-68`). The "~11 minutes" figure holds. ### The REVERT recipe — re-executed, and at the tree that will actually exist The paragraph says it was tested at `2b71dda1`. But after this merges, `2b71dda1` is no longer `master`, and the operator following this recipe will be standing on **`49bd9271`**. So I ran it there, not only where you ran it. All four `git revert` candidates, at PR head: ``` git revert -m 1 0a82d61f CONFLICT git revert cb9e73bd CONFLICT git revert -m 1 16238d39 CONFLICT git revert 2219c928 CONFLICT ``` All four also conflict at `2b71dda1`. Confirmed both trees. **The trap is real, and if anything understated.** I resolved the `-m 1 16238d39` conflict and inspected the hunk: it is a *single* conflict region, and all three target lines — navidrome, lidarr **and paperless** — sit on the "ours" side. Taking the revert result therefore drops the paperless target, which `16238d39` never added. Exactly the silent total-signal loss the text warns about. Both forward deletions, run at PR head: ``` sed -i '/svc\.cluster\.local:80\/ping$/d;/paperless\...\/accounts\/login\/$/d' -> blackbox-http 20 -> 17 sed -i '/paperless\...\/accounts\/login\/$/d' -> blackbox-http 20 -> 19 ``` Both produce exactly the claimed counts, `git diff` contains **nothing but the deleted target lines**, and `blackbox-edge-wan` (3) / `blackbox-tcp-relays` (2) are untouched. The recipe is true at the post-merge tree. ### Three non-blocking notes 1. **`group_by` is `['alertname', 'cluster', 'namespace']`, not "grouped by alertname".** The conclusion — one email — still holds, because all three targets come from the same `blackbox-http` Probe in `monitoring` and so share all three labels. But the parenthetical is narrower than the config. Worth correcting whenever this block is next touched; not worth a round-trip now. 2. **`/svc\.cluster\.local:80\/ping$/d` is not self-limiting.** It matches *any* in-cluster `:80/ping` target. Today exactly two match, so the command is correct today. If someone later adds another in-cluster `:80/ping` probe, the command documented as "all three app-health targets" would silently take that one too. A drift risk in a recipe whose whole purpose is to be followed years from now. 3. The REVERT text says "tested against master `2b71dda1`". Since the recipe lives in the file it describes, that reference goes stale the moment this merges. I have recorded the post-merge verification here instead. ### Merge decision — the owner's three questions, for *this* PR - **What breaks if this is wrong?** Nothing running. The parsed object is deep-equal across the diff, so Flux's next reconcile of `monitoring-rules` is a no-op on all three Probes. The only failure mode is a future operator being misled by a comment — which is why I re-derived every number above rather than reading them. - **Who notices?** Nobody automatically, and that is the honest answer for a comment-only change. It is also the reason this class of PR needs the facts checked at review time; there is no runtime signal that will ever catch an inaccurate comment. - **How fast can it be undone?** One step, tested once: `git revert 49bd9271` applies **cleanly** and restores the file byte-identical to `2b71dda1` with all 20 targets intact. Live within ~11 minutes on the intervals above, or immediately with `flux reconcile kustomization monitoring-rules --with-source`. Merging.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#978

docs(monitoring): record the egress dependency and the real revert recipe for the app-health probes #978

What changed

Why

How it was verified

Production constraint

How to revert this PR

Summary

🔴 Blocking

🟡 Suggestions

🟢 Nits

Tests

Recommendation

Summary

🟡 Suggestions

🟢 Nits

Tests

Recommendation

Approved — reviewed at head 49bd9271, base 2b71dda1

The claim that this changes no object

The REVERT paragraph — the reason #978 was sent back

BLAST RADIUS paragraph, traced against the repo

EGRESS counts, enumerated independently

Stale automated review

Production risk

Approved. Comment-only verified independently, and the REVERT recipe re-executed against the post-merge tree.

The change touches no object

Every factual claim in the new text, re-derived at 2b71dda1

The REVERT recipe — re-executed, and at the tree that will actually exist

Three non-blocking notes

Merge decision — the owner's three questions, for this PR

Approved — reviewed at head `49bd9271`, base `2b71dda1`

Every factual claim in the new text, re-derived at `2b71dda1`