Ingest the RBAC graph in batches, not as a single CREATE

[marcinc]

Work in progress. Closes #525.

The problem

Rbac::Ingest#index_rbac sent the whole cluster graph as one statement:

@graph.query(%Q(CREATE #{graph.body}))

FalkorDB parses that into a single tree and applies it in one transaction, and the transient cost is far out of proportion to the data it produces.

Why it was not a one-line change

Edge clauses reference nodes as variables bound in the same statement:

CREATE (n1:Namespace {...}),(n2:Role {...}),(n1)<-[:SCOPE]-(n2)

Split that across statements and the second one has no n1. Sending nodes and edges separately needs a stable key on each node, and an index to match nodes back through.

What this does

• Node#to_s now carries the node's label as an _id property instead of binding it as a variable, so nodes are created anonymously: (:Role {..., _id:'n5'}).

• Builder#node_statements batches nodes into CREATE statements of at most 500.

• Builder#edge_statements groups edges by everything one pattern fixes — the kind of node at either end, the relation, and its direction — and emits one UNWIND per batch:

  UNWIND [['n5','n1'],...] AS pair
  MATCH (s:Role {_id: pair[0]}), (d:Namespace {_id: pair[1]})
  CREATE (s)-[:SCOPE]->(d)
  

  GRAPH.EXPLAIN confirms both ends resolve by index scan.

• Edge#to_s becomes Edge#directions. A bidirectional edge still yields a relationship each way.

• Ingest#create_graph runs nodes, then the _id indexes, then edges. Batching gives up the single statement's atomicity, so if any statement fails it discards the graph and re-raises, rather than leaving a partial graph for the report to read as a complete picture of the cluster.

• Builder#body is removed, since nothing calls it any more.

Testing

Unit suite: 292 examples, 0 failures.

End to end against FalkorDB v4.20.3 — the version the chart and the compose file both pin — configured the way they configure it (RESULTSET_SIZE -1, BROWSER 0). Cache: 600 Roles, 510 ClusterRoles, 600 RoleBindings, 510 ClusterRoleBindings across 40 namespaces.

	master	this branch
largest single statement	3.5 MB	86 KB
peak container memory	373 MiB	165 MiB
ingest wall time	6.1 s	2.2 s
graph nodes / relationships	2,488 / 130,260	2,488 / 130,260

Peak is read from the cgroup's memory.peak with no limit applied, so nothing is capped or reclaimed. The chart and the compose file both cap FalkorDB at 200Mi: master's peak is over that, this branch's fits under it.

To be clear about what that does and does not show: master is not reliably killed at 200Mi. Much of its peak is page cache from writing the tree files, which the kernel can reclaim, so it often survives while sitting right on the limit. It is killed when the bundled browser is left enabled, which raises the idle footprint. The defensible claim is the peak demand, not the kill.

Output is unchanged. The report JSON is identical (3,484 normalised lines; 15 danger, 20 warning, 2 info, 18 success), as are all 2,821 generated dashboard files apart from the tree manifest's timestamp. Also checked against cache/default, which exercises PodSecurityPolicy nodes and :SECURITY edges and contains a dangling reference — all 100 files identical there too.

Worth a look in review

• _id adds a property to every node. The resident graph grew from 8.6 MB to 9.3 MB on this cluster.
• The batch size of 500 is a first guess. It keeps the largest statement at 86 KB here. It has not been tuned across a range.
• The graph client's 1 s read timeout is untouched. On master, at this cluster size, the single CREATE takes 6 s and the report fails on that timeout before memory even becomes the question. Batching happens to avoid it, because no single statement is slow enough to matter, but the default itself is still fragile and is left for a separate change.

Edges naming an entity that is never defined

Worth its own note, as it is a real behaviour change.

An edge can name a node the builder never created. In a single CREATE, an undeclared variable is simply a fresh node, so the old code silently made an unlabelled, propertyless one for it. Batching cannot do that, and the MATCH finds nothing instead.

It is not hypothetical. Three of the seven cached clusters here have one: a ClusterRole granting use on a PodSecurityPolicy named kindnet that the cluster does not define. That is a dangling reference in the cluster's own RBAC, not a fault in the builder.

Dropping the edge is the better behaviour — every Cypher query in the codebase binds its nodes by label, so the node the old code created could never match one, which is why the reports come out identical. Measured on cache/default:

	master	this branch
nodes	802	801
relationships	5,540	5,538
unlabelled nodes	1	0
report and dashboard output	—	identical

What was wrong was doing it quietly. The edges are now collected and reported:

WARN: Left 1 graph edge(s) out, as they refer to an entity the cluster never defines (SECURITY x1).

Surfacing this as a report finding, alongside the existing "Dangling roles", would be a reasonable follow up. Out of scope here.

The failure path, exercised for real

No longer only unit tested. A copy of cache/default was doctored so a ClusterRole name contains a single quote, which the unescaped value interpolation turns into a Cypher syntax error part way through ingest:

	graph left behind
with the discard removed	rbac-failtest, 500 nodes — one node batch
as implemented	none

That partial graph is exactly what the report would otherwise have read as a complete picture of the cluster.

Doing this also turned up a fault in the tidy up. discard_graph only rescued DeleteError, so if the tidy up failed any other way — the graph having gone with the connection, say — that error replaced the one that caused it, losing the only useful diagnostic. It now swallows everything and lets the caller re-raise the original. Covered by a test that fails without the fix.

Note the unescaped interpolation used to provoke this is a pre-existing weakness, untouched here.

Related

The two secondary notes in #525 are split out as #532 and #533.