refactor(cni): split galactic-cni into veth/tap master plugins

[privateip]

Stack (merge bottom to top):

• refactor(cni): split galactic-cni into veth/tap master plugins #303 refactor(cni): split galactic-cni into veth/tap master plugins (this PR)
• refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin #304 refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin
• refactor(cni): extract galactic-bgp as its own CNI chain plugin #305 refactor(cni): extract galactic-bgp as its own CNI chain plugin
• refactor(cni): extract galactic-route as its own CNI chain plugin #306 refactor(cni): extract galactic-route as its own CNI chain plugin
• docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code #307 docs+fixes(cni): finish the CNI plugin-chain split - docs, containerlab, dead code

---

Summary

galactic-cni is now veth-only. A new galactic-tap-cni binary owns tap workloads (Kata, Firecracker, kraftlet/Unikraft). The interface_type config field and its runtime switch are gone from both binaries.

First branch in a stack: refactor/cni-chain-0-veth-tap-split → -1-galactic-ipam → -2-galactic-bgp → -3-galactic-route → -4-installer-docs. Each branch bases on the previous.

New shared packages

Extracted so internal/cnitap doesn't duplicate or reach into internal/cni's unexported internals:

• internal/cni/hostconf — HostConf/Load/node-name API fallback. Match on plugin type is now caller-supplied, not hardcoded to "galactic-cni".
• internal/cni/crdnames — BGP CRD naming and annotation-key helpers.
• internal/cni/nadpatch — NAD annotation logic, identical for both interface types.
• internal/cniipam — IPAM allocation/deallocation. Shared library for now; becomes its own delegated plugin in step 1.
• internal/cnibgp — BGP/SRv6/eBPF publish logic. Same treatment; becomes its own chain-invoked plugin in step 2.

Verification

• task lint ✅
• task build ✅ — builds and stages galactic-tap-cni too
• task test:unit ✅
• task test:e2e — not run. Needs a Kind cluster bring-up (sudo modprobe vrf, kind create cluster) not attempted here. installer.Bootstrap's static per-node conflist is unchanged, so existing veth-path e2e coverage should hold, but that's unverified — flagging for the reviewer.

Test coverage moved with the code it tests, not dropped: bgp_test.go/bgp_ebpf_test.go → internal/cnibgp, ipam_ops_test.go → internal/cniipam, nad_test.go → internal/cni/nadpatch, tap-mode cases from cni_test.go → internal/cnitap.

🤖 Generated with Claude Code

[mattdjenkinson]

Went through this one closely. The split looks solid overall: the shared-package extraction (hostconf, crdnames, nadpatch, cniipam, cnibgp) does what the description says, and tests moved with the code they cover. A few things worth looking at before this lands, roughly in order of how much I'd worry about them.

First, cmd/galactic-cni/main.go's isTapMode() / CNI_NETNS_OVERRIDE path is dead now, but not in a harmless way. internal/cni doesn't handle tap at all anymore (PluginConf.InterfaceType and the veth/tap switch are gone), yet this main.go still checks for interface_type: "tap" and flips off the netns-mismatch safety check when it sees it. If the external operator that templates per-attachment conflists hasn't been updated yet to route tap workloads to galactic-tap-cni, a config still tagged interface_type: tap will land on galactic-cni, skip the safety check, and get processed as veth anyway, misconfiguring the workload instead of erroring out. I noticed #307 deletes this exact block later as dead code, so you're clearly aware of it, just sequenced for a later branch. Might be worth pulling that deletion forward into this PR, or at least flagging the rollout-ordering risk in the description since this branch ships first.

Second, three files got copy-pasted instead of extracted, which cuts against the stated point of this refactor. The whole reason for pulling out hostconf, crdnames, nadpatch, cniipam, and cnibgp was so cnitap wouldn't duplicate or reach into cni's internals. But internal/cnitap/config.go reimplements parseConf and everything around it (log setup, status parsing, prevResult validation, base62/error sanitizing) as a near copy of internal/cni/config.go. internal/cnitap/resource.go duplicates newK8sClient() and the tracker/cleanup rollback sequence from internal/cni/resource.go, swapping in tap.Delete for veth.Delete and otherwise identical. And internal/cnitap/ops_check.go copies the CHECK/STATUS logic (checkNodeLevelState, checkTerminationRoutes, validateHostInterface, probeAPIServer) verbatim, none of which is interface-type specific.

A future validation fix or rollback-ordering fix on the veth side won't propagate to any of those tap copies. Given five other packages already went through this same extraction, it seems worth doing the same here rather than letting two copies drift apart.

A couple of smaller things:

• internal/cni/ops_add.go's veth path calls cnibgp.PublishBGPState without the nil-tracker.k8s guard that the tap path still has before its equivalent call. Not reachable today since newK8sClient() can't return a nil client here, but since cnibgp is meant to be a shared library future chain binaries call into directly, might be worth keeping the guard for defense in depth.
• internal/cni/doc.go still says the package creates "a veth or tap interface" and lists a tap subpackage, both stale now that cni is veth-only.
• hostconf/crdnames/nadpatch ended up nested under internal/cni/, while cnibgp/cniipam (doing the same cross-binary-sharing job) got promoted to top-level internal/. Just a placement inconsistency, but it makes hostconf etc. look like cni-owned internals that cnitap reaches into, which is the exact thing this PR is trying to get away from.

Given this is a stack, happy to have the duplication cleanup land in a later branch if that's cleaner, but the isTapMode one seems worth addressing here since this branch ships first.

[privateip]

Thanks for the thorough review, @mattdjenkinson — went through all five points against where this stack has landed since. Two are already handled without further changes needed here:

isTapMode()/CNI_NETNS_OVERRIDE dead code — already deleted, in 725ef87 (this stack's own step 4/#307). Confirmed cmd/galactic-cni/main.go no longer has the stdin-peek dance or the isTapMode() helper; the comment now in its place explains why it's gone (interface kind is which binary you invoke now, not a config field either binary branches on).

Missing nil-tracker.k8s guard before cnibgp.PublishBGPState — now moot rather than fixed. e33092c (step 2/#305, extracting galactic-bgp) moved BGP publish entirely out of internal/cni/ops_add.go into its own chain-invoked binary. There's now exactly one call site (internal/cnibgp/ops_add.go), which constructs its own k8sClient immediately before use with an error return on failure — the guard-ordering concern doesn't apply to that shape anymore.

The other three are real and I've landed fixes for them as follow-ups on top of the stack rather than pushing more commits onto this branch, so your review here stays scoped to what you already looked at:

• docs+refactor(cni): fix stale package doc, promote hostconf/crdnames/nadpatch #315 — fixes the stale internal/cni/doc.go package comment (still described the package as veth-or-tap, still listed tap as its own subpackage) and promotes hostconf/crdnames/nadpatch from internal/cni/* to top-level internal/* to match cnibgp/cniipam/cnitap/cniroute's placement — closing the exact "looks like cnitap reaches into cni's internals" gap you flagged.
• refactor(cni): dedupe internal/cni vs internal/cnitap into internal/cnimaster #316 (stacked on docs+refactor(cni): fix stale package doc, promote hostconf/crdnames/nadpatch #315) — the internal/cnitap vs internal/cni duplication in config.go/resource.go/ops_check.go. Extracted the interface-agnostic pieces (parseConf's validation, log setup, k8s client construction, CHECK/STATUS node-level checks, prevResult validation) into a new internal/cnimaster package that both master plugins now share, leaving only the genuinely interface-specific bits (which kernel primitive to call, tap's lack of a guest netns to check) in each package. Test coverage moved with the code, including catching that cnitap's own tests had been implicitly relying on cni's tests to cover logic it silently duplicated — exactly the risk you called out.

Both are green on task lint/task build/task test:unit.