From c10f6920cdf23208bcaeb6956a23a50fe54f7d58 Mon Sep 17 00:00:00 2001 From: dtoro Date: Wed, 29 Jul 2026 09:44:17 +0200 Subject: [PATCH] fix: blast radius walks dependency direction, and reachability survives no ICMP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things the entity window redesign surfaced but deliberately left alone. **blast_radius answered the wrong question.** It walked source→target for every relationship type, but which end of an edge is the dependent differs per type: "machine hosts container" means the target breaks, while "service depends-on service" and "ingress routes-to service" mean the SOURCE breaks. Walking everything forwards was right for hosts/provides and backwards for everything else — and swept in 2,800+ documents/involves/targets edges of pure bookkeeping, so the result contained tasks and executions that cannot break. Direction is now declared per relationship type in seeds/ontology.yaml (blast_direction: forward | backward | none), the same shape as the entity types' monitoring: declaration, and defaults to none so an undeclared edge contributes nothing rather than a confidently wrong answer. It also needed a modelling fix: `routes-to` names an ingress's BACKEND, so nothing recorded that all 21 public hostnames are terminated by caddy. A `served-by` edge type now says so. pool:ludo-lvm 2 -> 23 (every container storing on it, then their services) lxc:caddy 4 -> 22 (service:caddy, then all 21 ingress routes) service:authentik 7 (what authenticates via it) **Every ping check was reporting down.** Not a host:strong false positive: all seven, including ws:mac-mini — the Docker host itself. The scheduler runs in Docker on macOS, whose VM does not route ICMP to the LAN; loopback pings succeed and every LAN ping fails. Under health aggregation each broken probe dragged its entity to down. The question the check exists to answer is "is it reachable", and ICMP is only one way to ask it. checkPing now falls back to a TCP connect before concluding anything, which restores an honest verdict for the four hosts that are genuinely up while leaving the genuinely unreachable ones down. TestBlastRadiusTerminatesOnCycles asserted the old direction (caddy=1, authentik=2 — the cycle walked the wrong way); it now asserts the corrected depths, and its exact-node-count check is relaxed because walking the right way also surfaces the seed's own real dependents, which are correct answers. Co-Authored-By: Claude --- internal/db/integration_test.go | 28 ++++++- internal/db/seed.go | 16 +++- internal/scheduler/reachability_test.go | 25 +++++++ internal/scheduler/scheduler.go | 40 +++++++++- .../028_relationship_blast_direction.up.sql | 75 +++++++++++++++++++ seeds/inventory.yaml | 24 ++++++ seeds/ontology.yaml | 34 +++++++++ 7 files changed, 233 insertions(+), 9 deletions(-) create mode 100644 internal/scheduler/reachability_test.go create mode 100644 migrations/028_relationship_blast_direction.up.sql diff --git a/internal/db/integration_test.go b/internal/db/integration_test.go index 5a4ca7b..e606696 100644 --- a/internal/db/integration_test.go +++ b/internal/db/integration_test.go @@ -306,7 +306,17 @@ func TestBlastRadiusTerminatesOnCycles(t *testing.T) { pool := newTestPool(t) seedAll(t, pool, seedsDir()) - // Build a dependency cycle: gitea → caddy → authentik → gitea + // Build a dependency cycle: gitea → caddy → authentik → gitea. + // + // `depends-on` is declared blast_direction: backward — "A depends-on B" + // means B failing breaks A — so the blast radius of gitea walks the edges + // BACKWARDS: whoever depends on gitea is affected first. That is authentik + // (1 hop), then caddy which depends on authentik (2 hops). + // + // This test previously asserted caddy=1, authentik=2, which is the same + // cycle walked the wrong way round: blast_radius used to follow every edge + // source→target regardless of what the edge means, so it answered "what + // does gitea depend on" while being named for the opposite question. cycle := []byte(` version: 1 relationships: @@ -342,14 +352,24 @@ relationships: } got[slug] = depth } - want := map[string]int{"service:gitea": 0, "service:caddy": 1, "service:authentik": 2} + want := map[string]int{"service:gitea": 0, "service:authentik": 1, "service:caddy": 2} for slug, depth := range want { if got[slug] != depth { t.Errorf("blast_radius[%s] = %d, want %d (full: %v)", slug, got[slug], depth, got) } } - if len(got) != len(want) { - t.Errorf("blast_radius returned %d nodes, want %d: %v", len(got), len(want), got) + // Deliberately not an exact node count. Walking the right way round also + // surfaces the real seed's own dependents of gitea (homelab-mcp and what + // depends on it), which are correct answers — the old exact-count + // assertion only held because the forward walk found nothing real. + // What matters here is that the cycle terminates rather than recursing. + if len(got) > 20 { + t.Errorf("blast_radius did not terminate sensibly: %d nodes: %v", len(got), got) + } + for slug, depth := range got { + if depth > 5 { + t.Errorf("blast_radius[%s] = %d, beyond the max_depth bound", slug, depth) + } } } diff --git a/internal/db/seed.go b/internal/db/seed.go index 20a2cbc..f4c8e4c 100644 --- a/internal/db/seed.go +++ b/internal/db/seed.go @@ -67,12 +67,20 @@ func IngestOntologySeed(ctx context.Context, tx pgx.Tx, data map[string]any) (*S targetType, _ := rtMap["target"].(string) cardinality, _ := rtMap["cardinality"].(string) desc, _ := rtMap["description"].(string) + // Which end of the edge depends on the other; drives blast_radius(). + // Absent means 'none' — an undeclared edge contributes nothing rather + // than silently producing a wrong dependency answer. + blastDirection, _ := rtMap["blast_direction"].(string) + if blastDirection == "" { + blastDirection = "none" + } _, err := tx.Exec(ctx, - `INSERT INTO relationship_types (name, inverse, source_type, target_type, cardinality, description) - VALUES ($1, $2, $3, $4, $5, $6) + `INSERT INTO relationship_types (name, inverse, source_type, target_type, cardinality, description, blast_direction) + VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (name) DO UPDATE SET inverse = $2, source_type = $3, - target_type = $4, cardinality = $5, description = $6`, - name, nullableStr(inverse), sourceType, targetType, cardinality, desc) + target_type = $4, cardinality = $5, description = $6, + blast_direction = $7`, + name, nullableStr(inverse), sourceType, targetType, cardinality, desc, blastDirection) if err != nil { return nil, fmt.Errorf("relationship_type %s: %w", name, err) } diff --git a/internal/scheduler/reachability_test.go b/internal/scheduler/reachability_test.go new file mode 100644 index 0000000..a23b869 --- /dev/null +++ b/internal/scheduler/reachability_test.go @@ -0,0 +1,25 @@ +package scheduler + +import ( + "context" + "testing" + "time" +) + +// The scheduler runs in Docker on macOS, whose VM does not route ICMP to the +// LAN — every ping check reported "down" for hosts that were demonstrably up. +// tcpReachable is the fallback that keeps "is it reachable" answerable. +func TestTCPReachableAnswersWhenICMPCannot(t *testing.T) { + ctx := context.Background() + // localhost:22 is open on this machine (sshd), and port 1 is not. + if !tcpReachable(ctx, "127.0.0.1", 22, 3*time.Second) { + t.Skip("no sshd on localhost — cannot exercise the positive case") + } + if tcpReachable(ctx, "127.0.0.1", 1, 1*time.Second) { + t.Error("port 1 should not be reachable") + } + // Defaults to 22 when unset, which is what checkdefaults' ping configs use. + if !tcpReachable(ctx, "127.0.0.1", 0, 3*time.Second) { + t.Error("port 0 should default to 22") + } +} diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 79a523b..2a59fbc 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -605,6 +605,9 @@ func checkPing(ctx context.Context, cd sqlcgen.ListEnabledCheckDefsRow) checkRes cfg := struct { Host string `json:"host"` Count int `json:"count"` + // Port for the TCP fallback below. Defaults to 22; set it for hosts + // that answer on something else (a Home Assistant VM has no sshd). + Port int `json:"port"` }{} if len(cd.Config) > 0 { _ = json.Unmarshal(cd.Config, &cfg) @@ -638,9 +641,26 @@ func checkPing(ctx context.Context, cd sqlcgen.ListEnabledCheckDefsRow) checkRes output, err := cmd.Output() if err != nil { + // ICMP failing does not mean the host is down — it may mean ICMP is + // simply unavailable from here. On this deployment the scheduler runs + // in Docker on macOS, whose VM network stack does not route ICMP to + // the LAN: loopback pings succeed, every LAN ping fails, and all seven + // ping checks reported "down" for hosts that were demonstrably up + // (including the Docker host itself). Under health aggregation that one + // broken probe was enough to drag each entity to down. + // + // The question this check exists to answer is "is it reachable", and + // ICMP is only one way to ask. Fall back to a TCP connect before + // concluding anything. + if tcpReachable(ctx, cfg.Host, cfg.Port, timeout) { + return checkResult{ + health: "healthy", + metrics: map[string]float64{}, + } + } return checkResult{ health: "down", signalKind: "ping", - evidence: fmt.Sprintf("ping %s: %v", cfg.Host, err), + evidence: fmt.Sprintf("no ICMP or TCP response from %s: %v", cfg.Host, err), err: err, } } @@ -654,6 +674,24 @@ func checkPing(ctx context.Context, cd sqlcgen.ListEnabledCheckDefsRow) checkRes return checkResult{health: "healthy", metrics: metrics} } +// tcpReachable reports whether a TCP handshake completes, used as the +// reachability answer when ICMP is unavailable rather than unanswered. +func tcpReachable(ctx context.Context, host string, port int, timeout time.Duration) bool { + if port == 0 { + port = 22 + } + if timeout <= 0 { + timeout = 5 * time.Second + } + d := net.Dialer{Timeout: timeout} + conn, err := d.DialContext(ctx, "tcp", net.JoinHostPort(host, strconv.Itoa(port))) + if err != nil { + return false + } + conn.Close() + return true +} + var pingRttRe = regexp.MustCompile(`(?:rtt\s+min\/avg\/max\/mdev|round-trip\s+min\/avg\/max\/stddev)\s*=\s*[\d.]+\/([\d.]+)\/`) func parsePingLatency(output []byte) float64 { diff --git a/migrations/028_relationship_blast_direction.up.sql b/migrations/028_relationship_blast_direction.up.sql new file mode 100644 index 0000000..2ea820f --- /dev/null +++ b/migrations/028_relationship_blast_direction.up.sql @@ -0,0 +1,75 @@ +-- 028_relationship_blast_direction.up.sql +-- Make blast_radius answer the question it is named after. +-- +-- blast_radius walked source_id -> target_id for every relationship type. But +-- which end of an edge is the DEPENDENT differs per type: +-- +-- machine --hosts--> container if the machine dies, the container dies +-- -> dependent is the TARGET (forward) +-- service --depends-on--> service if the target dies, the SOURCE breaks +-- -> dependent is the SOURCE (backward) +-- ingress --routes-to--> service if the service dies, the route 502s +-- -> dependent is the SOURCE (backward) +-- document --documents--> entity neither breaks the other +-- -> no runtime dependency at all +-- +-- Walking everything forwards meant the answer was right for `hosts` and +-- `provides` and wrong for every backward edge, while `documents`, `involves` +-- and `targets` (2,800+ edges of pure bookkeeping) polluted the result with +-- tasks and executions that cannot "break". +-- +-- Direction is therefore a property of the relationship type, declared in +-- seeds/ontology.yaml — the same shape as the `monitoring:` declaration on +-- entity types. +-- +-- forward : if the SOURCE fails, the TARGET is affected +-- backward : if the TARGET fails, the SOURCE is affected +-- none : no runtime dependency (default — bookkeeping and documentation) +-- +-- Defaulting to 'none' is deliberate: an undeclared edge contributes nothing +-- rather than silently producing a wrong answer, which is how the old +-- everything-is-forward behaviour went unnoticed. + +ALTER TABLE relationship_types + ADD COLUMN IF NOT EXISTS blast_direction TEXT NOT NULL DEFAULT 'none' + CHECK (blast_direction IN ('forward', 'backward', 'none')); + +COMMENT ON COLUMN relationship_types.blast_direction IS + 'Which end of this edge depends on the other. forward = target depends on source. backward = source depends on target. none = no runtime dependency. Drives blast_radius().'; + +-- Walk the dependency graph in the direction each edge type declares. +-- +-- Returns everything that is affected when start_id fails, with the number of +-- hops. Cycles are guarded by the path array, as before. +CREATE OR REPLACE FUNCTION blast_radius(start_id UUID, max_depth INT DEFAULT 3, + rel_types TEXT[] DEFAULT NULL) +RETURNS TABLE(entity_id UUID, depth INT) AS $$ + WITH RECURSIVE walk AS ( + SELECT start_id AS entity_id, 0 AS depth, ARRAY[start_id] AS path + UNION ALL + SELECT next_id, w.depth + 1, w.path || next_id + FROM walk w + JOIN LATERAL ( + -- forward: this entity is the source, so the target depends on it + SELECT r.target_id AS next_id + FROM relationships r + JOIN relationship_types rt ON rt.name = r.type + WHERE r.source_id = w.entity_id + AND r.valid_to IS NULL + AND rt.blast_direction = 'forward' + AND (rel_types IS NULL OR r.type = ANY(rel_types)) + UNION ALL + -- backward: this entity is the target, so the source depends on it + SELECT r.source_id AS next_id + FROM relationships r + JOIN relationship_types rt ON rt.name = r.type + WHERE r.target_id = w.entity_id + AND r.valid_to IS NULL + AND rt.blast_direction = 'backward' + AND (rel_types IS NULL OR r.type = ANY(rel_types)) + ) nxt ON TRUE + WHERE w.depth < LEAST(max_depth, 5) + AND NOT nxt.next_id = ANY(w.path) + ) + SELECT entity_id, MIN(depth) FROM walk GROUP BY entity_id; +$$ LANGUAGE sql STABLE; diff --git a/seeds/inventory.yaml b/seeds/inventory.yaml index 3198468..957594e 100644 --- a/seeds/inventory.yaml +++ b/seeds/inventory.yaml @@ -462,6 +462,30 @@ relationships: # ingress:mcp a cardinality violation. - {source: "ws:mac-mini", target: "service:oikos", type: provides} - {source: "ingress:mcp.hubris.network", target: "service:oikos", type: routes-to} + # Every public hostname is terminated by caddy. Without these the + # reverse proxy — the single widest point of failure in the lab — + # had a blast radius of one. + - {source: "ingress:proxmox.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:git.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:auth.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:media.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:cloud.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:paperless.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:matrix.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:photos.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:artifacto.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:trmnl.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:zimaos.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:teddy.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:mcp.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:secrets.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:house.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:books.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:seanime.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:roms.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:jellyseerr.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:qbit.hubris.network", target: "service:caddy", type: served-by} + - {source: "ingress:sab.hubris.network", target: "service:caddy", type: served-by} - {source: "ingress:secrets.hubris.network", target: "service:secrets-issuance", type: routes-to} - {source: "ingress:house.hubris.network", target: "service:house", type: routes-to} - {source: "ingress:books.hubris.network", target: "service:grimmory", type: routes-to} diff --git a/seeds/ontology.yaml b/seeds/ontology.yaml index 2fd506b..6ec9bb2 100644 --- a/seeds/ontology.yaml +++ b/seeds/ontology.yaml @@ -699,36 +699,42 @@ relationship_types: target: compute-entity cardinality: one-to-many description: Machine hosts a VM/container (hubris hosts lxc:apps). + blast_direction: forward runs-hypervisor: inverse: hypervisor-on source: machine target: hypervisor cardinality: one-to-one description: Machine runs hypervisor software. + blast_direction: forward member-of: inverse: has-member source: proxmox-host target: cluster cardinality: many-to-one description: PVE host belongs to a cluster. + blast_direction: backward part-of: inverse: comprises source: docker-container target: compose-stack cardinality: many-to-one description: Docker container belongs to a compose stack. + blast_direction: backward provides: inverse: provided-by source: compute-entity target: service cardinality: one-to-many description: Compute entity provides a service (lxc:gitea provides service:gitea). + blast_direction: forward runs: inverse: run-by source: service target: application cardinality: one-to-many description: Service runs an application. + blast_direction: backward configured-by: inverse: configures source: entity @@ -741,36 +747,52 @@ relationship_types: target: entity cardinality: many-to-one description: Pipeline deploys to a service/host. + blast_direction: forward routes-to: inverse: routed-via source: ingress-route target: service cardinality: many-to-one description: Public hostname routes to a service. + blast_direction: backward + served-by: + inverse: serves + source: ingress-route + target: service + cardinality: many-to-one + description: Ingress route is terminated by this reverse proxy. Distinct + from routes-to, which names the BACKEND the route forwards to — without + this edge the proxy's blast radius is invisible, and lxc:caddy reported + one affected entity despite terminating every *.hubris.network route. + blast_direction: backward secured-by: inverse: secures source: ingress-route target: identity-provider cardinality: many-to-one description: Route gated by forward-auth. + blast_direction: backward uses-certificate: inverse: certifies source: ingress-route target: certificate cardinality: many-to-one description: Route served with this certificate. + blast_direction: backward authenticates-via: inverse: authenticates-service source: service target: identity-provider cardinality: many-to-one description: Service uses native OIDC (jellyfin authenticates-via authentik). + blast_direction: backward in-zone: inverse: contains-record source: dns-record target: dns-zone cardinality: many-to-one description: Record belongs to a zone. + blast_direction: backward resolves-to: inverse: resolved-from source: dns-record @@ -783,24 +805,28 @@ relationship_types: target: service cardinality: many-to-many description: Runtime dependency (blast-radius edge). + blast_direction: backward connects-via: inverse: connects source: compute-entity target: network cardinality: many-to-many description: Coarse network membership (host on LAN / mesh). + blast_direction: backward has-interface: inverse: interface-of source: compute-entity target: network-interface cardinality: one-to-many description: Optional per-interface refinement. + blast_direction: backward interface-on: inverse: has-endpoint source: network-interface target: network cardinality: many-to-one description: Interface attaches to a network. + blast_direction: backward # Storage mounts: @@ -810,24 +836,28 @@ relationship_types: cardinality: many-to-many description: Compute entity mounts a volume. Edge attributes carry mount_point and options. + blast_direction: backward stores-on: inverse: stores-for source: compute-entity target: storage-pool cardinality: many-to-many description: Rootfs/data lives on a pool. + blast_direction: backward contains: inverse: contained-in source: storage-pool target: volume cardinality: one-to-many description: Pool contains a volume. + blast_direction: forward holds-dataset: inverse: dataset-on source: volume target: dataset cardinality: one-to-many description: Volume holds a tracked dataset. + blast_direction: backward backs-up-to: inverse: backup-of source: entity @@ -842,12 +872,14 @@ relationship_types: target: ups cardinality: many-to-one description: Machine on UPS power. + blast_direction: backward located-at: inverse: location-of source: machine target: site cardinality: many-to-one description: Machine's physical site. + blast_direction: backward registered-with: inverse: registrar-of source: domain-registration @@ -880,12 +912,14 @@ relationship_types: target: secret cardinality: many-to-one description: Grant covers a secret. + blast_direction: forward can-decrypt: inverse: readable-by source: compute-entity target: secret cardinality: many-to-many description: Host can decrypt a secret (legacy SOPS; Infisical grants later). + blast_direction: backward # Cognition checks: