Add Infrastructure Dependency Graph and Blast Radius Analysis#112
Add Infrastructure Dependency Graph and Blast Radius Analysis#112NitinKumar004 wants to merge 2 commits into
Conversation
Extends the topology/ package with dependency graph capabilities: - BuildDependencyGraph() — scans all compute, networking, and DNS resources to build a full resource relationship graph - BlastRadius(resourceID) — returns all directly and transitively affected resources if a resource is deleted - DependsOn(resourceID) — upstream dependencies - DependedBy(resourceID) — downstream dependents - ExportDOT() — Graphviz DOT format export - ExportMermaid() — Mermaid diagram format export Tracks 10 resource types and their relationships: VPC, Subnet, SecurityGroup, Instance, RouteTable, NATGateway, InternetGateway, PeeringConnection, NetworkACL, DNS Zone/Record Closes #103
Extend topology engine with optional drivers (LB, serverless, message queue, monitoring, IAM) via functional options. Add graph builders for volumes, load balancer chain, functions/ESMs, queues, alarms, channels, and instance profiles. Implement WhatIf (delete/stop/disconnect), orphaned resource detection, GetDependencyGraph alias, and engine-level ExportDOT/ExportMermaid. Add 15 unit tests and 3 cross-service integration tests covering all providers.
61a1964 to
e84ba8b
Compare
NitinKumar004
left a comment
There was a problem hiding this comment.
Review: Infrastructure Dependency Graph + Blast Radius
Rebased onto current development (one trivial test-comment conflict resolved) — now mergeable/clean, builds, and the full go test ./... tree is green. Nice, genuinely useful feature (issue #103) and the API is backward-compatible (topology.New stays variadic, no existing caller breaks). The BFS correctly guards a visited set so peering cycles can't recurse infinitely, and nil optional drivers are skipped. Findings below are marked inline.
Correctness
The most important is whatIfStop dropping transitive impact — combined with DNS node-ID collisions, orphan-detection edge-type logic, and dangling edges, the blast-radius output can be wrong in ways the tests don't catch.
Tests are too shallow to protect this
TestOrphanedResources never inspects report.OrphanedResources; TestWhatIfStop/TestWhatIfDisconnect assert only that BrokenConnections/Summary are non-empty; TestBlastRadius merges direct+transitive and only checks Contains, so it can't tell a direct hit from a transitive one. Every correctness finding above passes green. Please pin expected blast-radius/orphan contents.
Design / reuse
- Inventory walking duplicates
resourcediscovery/(inline) — the two will drift; and the per-service adders are near-identical copy-paste that aresourcediscovery-backed node set + an edge-derivation table would collapse. - Dependency-type vocabulary (
member-of/secured-by/peers-with/…) is ad-hoc string literals spread acrossgraph_builder.gowith no documented enumeration — worth defining as typed constants. - The feature (and the pre-existing
CanConnect/Resolve) isn't documented indocs/services.md.
Solid foundation — I'd fix the correctness findings + strengthen the tests before merge. Happy to take the transitive-impact and DOT-escaping fixes directly if useful.
| return nil, cerrors.Newf(cerrors.NotFound, "resource %s not found in graph", resourceID) | ||
| } | ||
|
|
||
| direct := findDirectDependents(graph, resourceID) |
There was a problem hiding this comment.
whatIfStop reports only direct dependents — transitive impact is dropped. It calls findDirectDependents and never findTransitiveDependents, so TransitiveImpact is always empty. Stop an instance that's a target-group member routed to by a listener: the target group shows as affected, but the listener/LB that transitively depend on it are never surfaced, so a caller concludes the LB is unaffected.
| } | ||
|
|
||
| for _, rec := range records { | ||
| recRef := ResourceRef{ID: rec.Name, Type: "dns-record", Name: rec.Type} |
There was a problem hiding this comment.
DNS record nodes key on bare rec.Name, causing node collisions. A record's identity is (zoneID, name, type). An A and AAAA record both named api.example.com collapse to one node, and the plain-string ID can collide with any other resource whose ID is a bare name (roles/alarms/instance-profiles). BlastRadius("api.example.com") then conflates unrelated resources. Key the node on name+type (+zone), like resolve.go does.
| continue | ||
| } | ||
|
|
||
| if dep.Type != "member-of" && dep.Type != "attached-to" && dep.Type != "belongs-to" { |
There was a problem hiding this comment.
isOrphaned only inspects member-of/attached-to/belongs-to edges. A resource whose real dependencies are secured-by/peers-with/routes-to is never orphaned even when all of them are destroyed; conversely a multi-parent resource is judged orphaned as soon as its member-of parent is affected even if a secured-by parent survives. The orphan set both under- and over-reports depending on edge-type mix.
|
|
||
| for _, th := range targets { | ||
| g.Dependencies = append(g.Dependencies, Dependency{ | ||
| From: ResourceRef{ID: th.Target.ID, Type: "instance"}, |
There was a problem hiding this comment.
Dangling edges to nodes that were never added. Target-health (and ESM at ~503) edges reference an instance/queue node by ID that only exists if independently enumerated. An IP/cross-account/filtered target i-123 yields an edge whose From node has no ResourceRef; ExportDOT/ExportMermaid emit an edge from an undeclared node and BlastRadius(tg) reports i-123 even though findResource("i-123") is not-found.
| b.WriteString("digraph CloudEmu {\n") | ||
|
|
||
| for _, r := range g.Resources { | ||
| label := fmt.Sprintf("%s\\n%s", r.Type, r.ID) |
There was a problem hiding this comment.
DOT label double-escapes the newline. fmt.Sprintf("%s\\n%s", ...) builds a literal backslash-n, then %q escapes the backslash again, so Graphviz shows the literal text vpc\n10.0.0.0/16 on one line instead of two. Build the label without the pre-escaped \n (let %q produce the escape, or write the label unquoted with a real \n).
| } | ||
|
|
||
| // GetDependencyGraph is an alias for BuildDependencyGraph. | ||
| func (e *Engine) GetDependencyGraph(ctx context.Context) (*DependencyGraph, error) { |
There was a problem hiding this comment.
GetDependencyGraph is a pure alias for BuildDependencyGraph. Both call buildGraph(ctx, e). Two public methods doing the same thing is inconsistent with the rest of topology.Engine (CanConnect/TraceRoute/Resolve are single verbs) and only the alias's own test uses it. Drop one.
| ) | ||
|
|
||
| // blastRadius computes the impact of removing or modifying the given resource. | ||
| func blastRadius(ctx context.Context, e *Engine, resourceID string) (*ImpactReport, error) { |
There was a problem hiding this comment.
The full graph is rebuilt from every driver on every query. blastRadius/dependsOn/dependedBy/whatIfStop/whatIfDisconnect each call BuildDependencyGraph (re-issuing Describe calls to all drivers) then do linear scans. Iterating impacts over many resources is quadratic-plus driver traffic. Consider exposing build-once/query-many — e.g. Impact(id) methods on *DependencyGraph (which already owns ExportDOT/ExportMermaid).
| } | ||
| } | ||
|
|
||
| func addVPCs(ctx context.Context, e *Engine, g *DependencyGraph) error { |
There was a problem hiding this comment.
Resource-inventory walking duplicates resourcediscovery/. addVPCs/addSubnets/addInstances/addFunctions/DNS walking re-read the same drivers resourcediscovery already walks (and it already handles e.g. the bucket-tag NotFound race the graph builder doesn't). Two inventory walkers over the same drivers will drift. Consider consuming resourcediscovery.Engine.ListAll for the node set and having the graph builder own only edge derivation.
Summary
Extends the
topology/package with dependency graph and blast radius analysis — answering "what breaks if I delete this resource?"Closes #103
Problem
Cloud infrastructure has complex resource dependencies (VPC -> Subnet -> Instance -> Volume). Deleting a parent resource can cascade failures. There's no way to visualize or analyze these dependencies in mock testing today.
Solution
A graph engine that scans all resources, builds a dependency graph, and enables impact analysis.
Architecture
BlastRadius Flow
API
Tracked Relationships
Files
topology/depgraph.gotopology/graph_builder.gotopology/blast_radius.gotopology/export.gotopology/topology_test.gocloudemu_test.goTests
Verification
go build ./...— compilesgolangci-lint— 0 issues