Submit an issue View all issues Source
MIR-1072

Type the node-id boundary so we stop string-slicing across controllers

Done public
phinze phinze Opened Apr 27, 2026 Updated Jul 21, 2026

Surfaced during MIR-1030 review — Evan flagged the myNodeId helper added in #770 as a localized fix for a much bigger pattern. A quick git grep finds 40+ call sites across components/diskio, controllers/disk, controllers/sandbox, and controllers/integration that all do some variant of entity.Id("node/" + raw), sometimes with a strings.TrimPrefix to defend against double-prefixing, sometimes without. A handful of them are inconsistent with each other — controllers/disk/disk_lease_controller.go:251 blindly prepends node/, while the new helper in disk_controller.go:91 defensively trims first. The original cause of MIR-1030 (a runner creating a disk_volume with the wrong NodeId) is exactly the kind of bug this fuzziness invites.

The actual problem is that we have two forms in flight — a raw node identifier (UUID or "miren") and a prefixed entity ID (node/<raw>) — and no type discipline about which one any given variable holds. Controllers store nodeId string and patch in the prefix at every point of use; tests construct entity IDs by hand. The compiler never tells anyone they got it wrong.

The shape of the cleanup is probably a real type — something like entity.NodeId (alias of entity.Id) with one constructor entity.NewNodeId(raw string) NodeId that handles trimming and prefixing once, and is the only sanctioned way to construct one. Then push the typed value through:

  • Controller fields become nodeId entity.NodeId instead of nodeId string.
  • Constructors like NewDiskController, NewDiskVolumeController, NewDiskMountController take entity.NodeId.
  • Comparisons against entity.NodeId fields on entities (existingVolume.NodeId) become direct equality without re-prefixing.
  • Test fixtures construct via the helper instead of hand-rolling the prefix.

Worth doing as one focused PR rather than scattered cleanups — the goal is that after the refactor, the string "node/" appears in exactly one place in the code, and nowhere else in the runtime can a node id be misshapen. Should also catch a few latent inconsistencies (the lease controller's missing TrimPrefix, the inline-prefix path in diskio controllers).

Not urgent — the MIR-1030 fix shipped in #770 with its localized helper. This is the architectural follow-up Evan called out.