From e6794d87ddfc4b5009aced1a44772af5592de315 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Fri, 2 Oct 2026 15:21:05 +1000 Subject: [PATCH] refactor: move host privilege escalation to runtime --- spinifex/bootstrap/preflight/preflight.go | 8 +-- .../bootstrap/preflight/preflight_test.go | 8 +-- spinifex/network/host/firewall.go | 6 +- spinifex/network/host/firewall_test.go | 4 +- spinifex/network/host/gateway_claim.go | 24 ++++---- spinifex/network/host/health.go | 2 +- spinifex/network/host/health_test.go | 4 +- spinifex/network/host/imds_attach_test.go | 6 +- spinifex/network/host/imds_datapath.go | 4 +- spinifex/network/host/imds_datapath_test.go | 10 ++-- spinifex/network/host/ipsec.go | 10 ++-- spinifex/network/host/ipsec_enable.go | 10 ++-- spinifex/network/host/ipsec_enable_test.go | 58 +++++++++---------- spinifex/network/host/ovn_raft.go | 4 +- spinifex/network/host/plumber.go | 16 ++--- spinifex/network/host/plumber_test.go | 20 +++---- spinifex/network/host/run.go | 6 +- spinifex/runtime/host/command/command.go | 2 +- .../host/command/privilege.go} | 2 +- .../command/privilege_invariants_test.go} | 12 ++-- .../host/command/privilege_test.go} | 2 +- spinifex/vpcd/vpcd.go | 5 +- 22 files changed, 112 insertions(+), 111 deletions(-) rename spinifex/{utils/sudo.go => runtime/host/command/privilege.go} (99%) rename spinifex/{utils/sudo_invariants_test.go => runtime/host/command/privilege_invariants_test.go} (92%) rename spinifex/{utils/sudo_test.go => runtime/host/command/privilege_test.go} (99%) diff --git a/spinifex/bootstrap/preflight/preflight.go b/spinifex/bootstrap/preflight/preflight.go index a3bd8f983..e277ea9ff 100644 --- a/spinifex/bootstrap/preflight/preflight.go +++ b/spinifex/bootstrap/preflight/preflight.go @@ -12,7 +12,7 @@ import ( "slices" "strings" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) //go:generate ../../../scripts/gen-asset-manifest.sh @@ -59,7 +59,7 @@ type Result struct { // keyed by installed path. An explicit list, not a directory scan, so adding // an asset here is a reviewed change and never accidental. var managedHelpers = []string{ - utils.EndpointSysctlHelper, + hostcommand.EndpointSysctlHelper, "/usr/local/lib/spinifex/ovs-socket-perms.sh", } @@ -128,7 +128,7 @@ func checkSudoersGrant(root string) Result { if strings.HasPrefix(trimmed, "#") { continue } - if strings.Contains(trimmed, "NOPASSWD") && strings.Contains(trimmed, utils.EndpointSysctlHelper) { + if strings.Contains(trimmed, "NOPASSWD") && strings.Contains(trimmed, hostcommand.EndpointSysctlHelper) { return Result{Path: sudoersGrantFile, Kind: "sudoers-grant", Status: OK} } } @@ -136,7 +136,7 @@ func checkSudoersGrant(root string) Result { Path: sudoersGrantFile, Kind: "sudoers-grant", Status: Ungranted, - Detail: "no NOPASSWD grant line for " + utils.EndpointSysctlHelper, + Detail: "no NOPASSWD grant line for " + hostcommand.EndpointSysctlHelper, } } diff --git a/spinifex/bootstrap/preflight/preflight_test.go b/spinifex/bootstrap/preflight/preflight_test.go index 49eddf633..217991a44 100644 --- a/spinifex/bootstrap/preflight/preflight_test.go +++ b/spinifex/bootstrap/preflight/preflight_test.go @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // writeHelper writes the given content at the managed path under root, @@ -23,7 +23,7 @@ func writeHelper(t *testing.T, root, path string, content []byte) { } func TestCheckHostAt_Helper(t *testing.T) { - path := utils.EndpointSysctlHelper + path := hostcommand.EndpointSysctlHelper want, ok := canonicalHashes[path] if !ok { t.Fatalf("no canonical hash for %s — is manifest_gen.go generated?", path) @@ -87,7 +87,7 @@ func TestCheckHostAt_SudoersGrant(t *testing.T) { root := t.TempDir() writeHelper(t, root, sudoersGrantFile, []byte( "spinifex-daemon ALL=(root) NOPASSWD: /sbin/ip, /usr/sbin/ip\n"+ - "spinifex-daemon ALL=(root) NOPASSWD: "+utils.EndpointSysctlHelper+"\n")) + "spinifex-daemon ALL=(root) NOPASSWD: "+hostcommand.EndpointSysctlHelper+"\n")) r := findResult(t, CheckHostAt(root), sudoersGrantFile) if r.Status != OK { t.Errorf("status = %v, want OK (detail: %s)", r.Status, r.Detail) @@ -134,7 +134,7 @@ func canonicalHelperBody(t *testing.T, path string) []byte { var file, anchor string switch path { - case utils.EndpointSysctlHelper: + case hostcommand.EndpointSysctlHelper: file = filepath.Join(root, "scripts", "setup.sh") anchor = "install_endpoint_sysctl_helper()" case "/usr/local/lib/spinifex/ovs-socket-perms.sh": diff --git a/spinifex/network/host/firewall.go b/spinifex/network/host/firewall.go index a9040c55b..1a05a06cd 100644 --- a/spinifex/network/host/firewall.go +++ b/spinifex/network/host/firewall.go @@ -17,7 +17,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/bootstrap/config" "github.com/mulgadc/spinifex/spinifex/foundation/telemetry" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // firewallApplyHelper is the fixed-verb root helper installed by setup.sh. The @@ -168,7 +168,7 @@ func ReconcileFirewall(configPath string, clusterConfig *config.ClusterConfig) e return nil } - cmd := utils.SudoCommand(firewallApplyHelper, "set-peers") + cmd := hostcommand.SudoCommand(firewallApplyHelper, "set-peers") cmd.Stdin = strings.NewReader(strings.Join(peers, ",") + "\n" + strings.Join(encap, ",") + "\n") if out, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("set firewall peers: %w: %s", err, strings.TrimSpace(string(out))) @@ -232,7 +232,7 @@ func disableFirewall() error { if _, statErr := os.Stat(firewallPeersPath); errors.Is(statErr, os.ErrNotExist) { return nil } - out, err := utils.SudoCommand(firewallApplyHelper, "disable").CombinedOutput() + out, err := hostcommand.SudoCommand(firewallApplyHelper, "disable").CombinedOutput() if err != nil { return fmt.Errorf("disable firewall: %w: %s", err, strings.TrimSpace(string(out))) } diff --git a/spinifex/network/host/firewall_test.go b/spinifex/network/host/firewall_test.go index c2d920d25..d4b2102e2 100644 --- a/spinifex/network/host/firewall_test.go +++ b/spinifex/network/host/firewall_test.go @@ -12,7 +12,7 @@ import ( "time" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -71,7 +71,7 @@ func newFirewallTestEnv(t *testing.T, encap string) *firewallTestEnv { firewallModePath, firewallTableCheck = origMode, origTable }) - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { env.record(name, args...) if len(args) > 0 && args[0] == "set-peers" { return exec.Command("sh", "-c", env.setPeersScript()) diff --git a/spinifex/network/host/gateway_claim.go b/spinifex/network/host/gateway_claim.go index c87353573..d97d66fc0 100644 --- a/spinifex/network/host/gateway_claim.go +++ b/spinifex/network/host/gateway_claim.go @@ -8,7 +8,7 @@ import ( "os/exec" "strings" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // GatewayClaimProber checks the SB chassis claim for OVN gateway router ports. @@ -82,7 +82,7 @@ func (p *GatewayClaimProber) GatewayPortElsewhere(_ context.Context, crPortName // unbound. Output() not CombinedOutput(): sudo PAM noise on stderr would be // misread as a non-empty chassis value. func (p *GatewayClaimProber) boundChassis(crPortName string) (string, error) { - out, err := utils.SudoCommand("ovn-sbctl", p.sbArgs("--bare", "--columns=chassis", "find", "Port_Binding", "logical_port="+crPortName)...).Output() + out, err := hostcommand.SudoCommand("ovn-sbctl", p.sbArgs("--bare", "--columns=chassis", "find", "Port_Binding", "logical_port="+crPortName)...).Output() if err != nil { return "", fmt.Errorf("ovn-sbctl find Port_Binding %s: %w", crPortName, err) } @@ -92,7 +92,7 @@ func (p *GatewayClaimProber) boundChassis(crPortName string) (string, error) { // localChassis returns this host's OVN chassis name, which ovn-controller takes // from the OVSDB external_ids:system-id it registered with. func (p *GatewayClaimProber) localChassis() (string, error) { - out, err := utils.SudoCommand("ovs-vsctl", "--if-exists", "get", "Open_vSwitch", ".", "external_ids:system-id").Output() + out, err := hostcommand.SudoCommand("ovs-vsctl", "--if-exists", "get", "Open_vSwitch", ".", "external_ids:system-id").Output() if err != nil { return "", fmt.Errorf("ovs-vsctl get system-id: %w", err) } @@ -101,7 +101,7 @@ func (p *GatewayClaimProber) localChassis() (string, error) { // chassisName resolves an SB Chassis row UUID to its name. func (p *GatewayClaimProber) chassisName(uuid string) (string, error) { - out, err := utils.SudoCommand("ovn-sbctl", p.sbArgs("--bare", "--columns=name", "list", "Chassis", uuid)...).Output() + out, err := hostcommand.SudoCommand("ovn-sbctl", p.sbArgs("--bare", "--columns=name", "list", "Chassis", uuid)...).Output() if err != nil { return "", fmt.Errorf("ovn-sbctl list Chassis %s: %w", uuid, err) } @@ -129,7 +129,7 @@ func (p *GatewayClaimProber) GuestPortUp(_ context.Context, lspName string) (boo args = append(args, "--db="+p.sbAddr) } args = append(args, "--bare", "--columns=up", "find", "Port_Binding", "logical_port="+lspName) - out, err := utils.SudoCommand("ovn-sbctl", args...).Output() + out, err := hostcommand.SudoCommand("ovn-sbctl", args...).Output() if err != nil { return false, fmt.Errorf("ovn-sbctl find Port_Binding %s: %w", lspName, err) } @@ -139,7 +139,7 @@ func (p *GatewayClaimProber) GuestPortUp(_ context.Context, lspName string) (boo // NudgeRecompute asks the local ovn-controller to re-evaluate logical flows via // the incremental engine, forcing a re-claim of unbound Port_Bindings. func (p *GatewayClaimProber) NudgeRecompute(_ context.Context) error { - out, err := utils.SudoCommand("ovn-appctl", "-t", "ovn-controller", "inc-engine/recompute").CombinedOutput() + out, err := hostcommand.SudoCommand("ovn-appctl", "-t", "ovn-controller", "inc-engine/recompute").CombinedOutput() if err != nil { return fmt.Errorf("ovn-appctl recompute: %s: %w", strings.TrimSpace(string(out)), err) } @@ -153,7 +153,7 @@ func (p *GatewayClaimProber) NudgeRecompute(_ context.Context) error { // realising new Port_Bindings. Output() not CombinedOutput(): sudo PAM noise on // stderr must not be misread as the status token. func (p *GatewayClaimProber) SBConnectionState(_ context.Context) (string, error) { - out, err := utils.SudoCommand("ovn-appctl", "-t", "ovn-controller", "connection-status").Output() + out, err := hostcommand.SudoCommand("ovn-appctl", "-t", "ovn-controller", "connection-status").Output() if err != nil { return "", fmt.Errorf("ovn-appctl connection-status: %w", err) } @@ -165,7 +165,7 @@ func (p *GatewayClaimProber) SBConnectionState(_ context.Context) (string, error // process restart or a flow wipe. Targeted equivalent of the manual // `systemctl restart ovn-controller` recovery. func (p *GatewayClaimProber) ResetSBClusterState(_ context.Context) error { - out, err := utils.SudoCommand("ovn-appctl", "-t", "ovn-controller", "sb-cluster-state-reset").CombinedOutput() + out, err := hostcommand.SudoCommand("ovn-appctl", "-t", "ovn-controller", "sb-cluster-state-reset").CombinedOutput() if err != nil { return fmt.Errorf("ovn-appctl sb-cluster-state-reset: %s: %w", strings.TrimSpace(string(out)), err) } @@ -186,7 +186,7 @@ func (p *GatewayClaimProber) FlushMACBinding(_ context.Context, ip string) error findArgs = append(findArgs, "--db="+p.sbAddr) } findArgs = append(findArgs, "--bare", "--columns=_uuid", "find", "MAC_Binding", "ip="+ip) - out, err := utils.SudoCommand("ovn-sbctl", findArgs...).Output() + out, err := hostcommand.SudoCommand("ovn-sbctl", findArgs...).Output() if err != nil { return fmt.Errorf("ovn-sbctl find MAC_Binding ip=%s: %w", ip, err) } @@ -196,7 +196,7 @@ func (p *GatewayClaimProber) FlushMACBinding(_ context.Context, ip string) error delArgs = append(delArgs, "--db="+p.sbAddr) } delArgs = append(delArgs, "--if-exists", "destroy", "MAC_Binding", uuid) - if dout, derr := utils.SudoCommand("ovn-sbctl", delArgs...).CombinedOutput(); derr != nil { + if dout, derr := hostcommand.SudoCommand("ovn-sbctl", delArgs...).CombinedOutput(); derr != nil { return fmt.Errorf("ovn-sbctl destroy MAC_Binding %s: %s: %w", uuid, strings.TrimSpace(string(dout)), derr) } } @@ -215,7 +215,7 @@ func (p *GatewayClaimProber) RepairDatapath(ctx context.Context) error { if !linkExists(dev) { continue } - if out, err := utils.SudoCommand("ip", "link", "set", dev, "up").CombinedOutput(); err != nil { + if out, err := hostcommand.SudoCommand("ip", "link", "set", dev, "up").CombinedOutput(); err != nil { slog.Warn("gateway claim: veth uplink admin-up failed", "dev", dev, "out", strings.TrimSpace(string(out)), "err", err) } } @@ -268,7 +268,7 @@ func (p *GatewayClaimProber) EIPReachable(ctx context.Context, eip string) (bool if err != nil { return false, err } - _ = utils.SudoCommand("ip", "neigh", "del", eip, "dev", dev).Run() + _ = hostcommand.SudoCommand("ip", "neigh", "del", eip, "dev", dev).Run() _ = exec.CommandContext(ctx, "ping", "-c", "1", "-W", "1", eip).Run() out, err := exec.CommandContext(ctx, "ip", "neigh", "show", eip, "dev", dev).Output() if err != nil { diff --git a/spinifex/network/host/health.go b/spinifex/network/host/health.go index 0756fc880..fb8453fb5 100644 --- a/spinifex/network/host/health.go +++ b/spinifex/network/host/health.go @@ -21,7 +21,7 @@ func HealthStatus() OVNHealth { // healthStatus is HealthStatus over an injected Runner. Every probe here is // read-only and runs unprivileged: ovs-vsctl and ovn-appctl are socket clients -// (utils.NeedsPrivilege), and db.sock plus the ovn-controller ctl socket are +// (hostcommand.NeedsPrivilege), and db.sock plus the ovn-controller ctl socket are // group-owned by `spinifex`. Escalating instead would mean granting the daemon a // root-equivalent ovn-appctl rule to read a status. func healthStatus(ctx context.Context, r Runner) OVNHealth { diff --git a/spinifex/network/host/health_test.go b/spinifex/network/host/health_test.go index f0c56f9c2..963eda27f 100644 --- a/spinifex/network/host/health_test.go +++ b/spinifex/network/host/health_test.go @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) const ( @@ -111,7 +111,7 @@ func TestHealthStatus_ProbesAreSocketClients(t *testing.T) { for _, c := range s.calls { tool := strings.Fields(c)[0] - if utils.NeedsPrivilege(tool) { + if hostcommand.NeedsPrivilege(tool) { t.Errorf("probe tool %q is classified as needing privilege; health must only use socket clients", tool) } } diff --git a/spinifex/network/host/imds_attach_test.go b/spinifex/network/host/imds_attach_test.go index e427aee33..a63651c5c 100644 --- a/spinifex/network/host/imds_attach_test.go +++ b/spinifex/network/host/imds_attach_test.go @@ -9,7 +9,7 @@ import ( networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) func TestIMDSDatapathSpec(t *testing.T) { @@ -60,7 +60,7 @@ func TestInstallIMDSDatapath(t *testing.T) { s := newStubRunner() s.expect("ovs-vsctl", nil, nil) s.expect("ip", nil, nil) - s.expect(utils.EndpointSysctlHelper, nil, nil) + s.expect(hostcommand.EndpointSysctlHelper, nil, nil) s.expect("ovs-ofctl", nil, nil) d := imdsDatapathSpec("eni-0abc1234", "02:00:00:00:01:05", "subnet-0fedcba9") @@ -147,7 +147,7 @@ func TestInstallIMDSDatapath_ServingFailureWrapped(t *testing.T) { s := newStubRunner() s.expect("ovs-vsctl", nil, nil) s.expect("ip", nil, nil) - s.expect(utils.EndpointSysctlHelper, nil, nil) + s.expect(hostcommand.EndpointSysctlHelper, nil, nil) s.expect("ovs-ofctl del-flows", nil, nil) // Forward flows (priority=100) are connectivity and must succeed; the demux/egress // flows (priority=200) are serving — fail those to land in the serving stage. diff --git a/spinifex/network/host/imds_datapath.go b/spinifex/network/host/imds_datapath.go index edcf2b13f..b1e279798 100644 --- a/spinifex/network/host/imds_datapath.go +++ b/spinifex/network/host/imds_datapath.go @@ -10,7 +10,7 @@ import ( "net" networkids "github.com/mulgadc/spinifex/spinifex/network/identifiers" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // IMDS / VPC-DNS service addresses captured at every tap. .254 is IMDS, served @@ -254,7 +254,7 @@ func ensureIMDSEndpoint(ctx context.Context, r Runner, d IMDSTapDatapath) error // wildcard the old `sysctl -qw net.ipv4.conf.*` rule relied on. func setEndpointSysctl(ctx context.Context, r Runner, endpoint, suffix, val string) error { key := "net.ipv4.conf." + endpoint + "." + suffix - if _, err := r.Run(ctx, utils.EndpointSysctlHelper, endpoint, suffix, val); err != nil { + if _, err := r.Run(ctx, hostcommand.EndpointSysctlHelper, endpoint, suffix, val); err != nil { return fmt.Errorf("set %s=%s: %w", key, val, err) } return nil diff --git a/spinifex/network/host/imds_datapath_test.go b/spinifex/network/host/imds_datapath_test.go index d676fabf2..d9b94f8f6 100644 --- a/spinifex/network/host/imds_datapath_test.go +++ b/spinifex/network/host/imds_datapath_test.go @@ -6,7 +6,7 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) func testDatapath() IMDSTapDatapath { @@ -116,7 +116,7 @@ func TestInstallTapDatapath(t *testing.T) { s := newStubRunner() s.expect("ovs-vsctl", nil, nil) s.expect("ip", nil, nil) - s.expect(utils.EndpointSysctlHelper, nil, nil) + s.expect(hostcommand.EndpointSysctlHelper, nil, nil) s.expect("ovs-ofctl", nil, nil) d := testDatapath() @@ -132,8 +132,8 @@ func TestInstallTapDatapath(t *testing.T) { "ip link set " + d.Endpoint + " up", "ip addr replace " + imdsMetaAddr + "/32 dev " + d.Endpoint, "ip addr replace " + imdsDNSAddr + "/32 dev " + d.Endpoint, - utils.EndpointSysctlHelper + " " + d.Endpoint + " rp_filter 0", - utils.EndpointSysctlHelper + " " + d.Endpoint + " accept_local 1", + hostcommand.EndpointSysctlHelper + " " + d.Endpoint + " rp_filter 0", + hostcommand.EndpointSysctlHelper + " " + d.Endpoint + " accept_local 1", // Flows are not cleared here: installIMDSDatapath clears the shared cookie // once up front so this install does not wipe the patch's forward flows. // ARP responder, one per captured addr, for guests that resolve the @@ -167,7 +167,7 @@ func TestInstallTapDatapathReattachIsIdempotent(t *testing.T) { s := newStubRunner() s.expect("ovs-vsctl", nil, nil) s.expect("ip", nil, nil) - s.expect(utils.EndpointSysctlHelper, nil, nil) + s.expect(hostcommand.EndpointSysctlHelper, nil, nil) s.expect("ovs-ofctl", nil, nil) d := testDatapath() diff --git a/spinifex/network/host/ipsec.go b/spinifex/network/host/ipsec.go index 4a61a254f..7b52c0604 100644 --- a/spinifex/network/host/ipsec.go +++ b/spinifex/network/host/ipsec.go @@ -7,14 +7,14 @@ import ( "slices" "strings" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // SetIPSecCertPaths writes the local IPsec peer cert pointers into the OVS // Open_vSwitch table. ovs-monitor-ipsec reads these to materialise strongSwan // configs for every Geneve tunnel ovn-controller programs. func SetIPSecCertPaths(certPath, keyPath, caCertPath string) error { - out, err := utils.SudoCommand("ovs-vsctl", "set", "Open_vSwitch", ".", + out, err := hostcommand.SudoCommand("ovs-vsctl", "set", "Open_vSwitch", ".", fmt.Sprintf("other_config:certificate=%s", certPath), fmt.Sprintf("other_config:private_key=%s", keyPath), fmt.Sprintf("other_config:ca_cert=%s", caCertPath), @@ -29,7 +29,7 @@ func SetIPSecCertPaths(certPath, keyPath, caCertPath string) error { // Open_vSwitch row. Caller must first verify ovs-monitor-ipsec is active — // flipping without a live daemon creates a silent-drop trap. func EnableIPSecEncapsulation() error { - out, err := utils.SudoCommand("ovs-vsctl", "set", "Open_vSwitch", ".", + out, err := hostcommand.SudoCommand("ovs-vsctl", "set", "Open_vSwitch", ".", "other_config:ipsec_encapsulation=true", ).CombinedOutput() if err != nil { @@ -72,7 +72,7 @@ func nbctlArgs(nbAddr string, leaderOnly bool, cmd ...string) []string { // Reads stdout alone. ovn-nbctl writes vlog lines to stderr on a successful run, // and folding those into the value parses a live "true" as false. func GetNBGlobalIPSec(nbAddr string) (bool, error) { - cmd := utils.SudoCommand("ovn-nbctl", nbctlArgs(nbAddr, false, "get", "NB_Global", ".", "ipsec")...) + cmd := hostcommand.SudoCommand("ovn-nbctl", nbctlArgs(nbAddr, false, "get", "NB_Global", ".", "ipsec")...) var stderr bytes.Buffer cmd.Stderr = &stderr out, err := cmd.Output() @@ -102,7 +102,7 @@ func SetNBGlobalIPSec(nbAddr string, enable bool) error { if enable { val = "true" } - out, err := utils.SudoCommand("ovn-nbctl", nbctlArgs(nbAddr, true, "set", "NB_Global", ".", "ipsec="+val)...).CombinedOutput() + out, err := hostcommand.SudoCommand("ovn-nbctl", nbctlArgs(nbAddr, true, "set", "NB_Global", ".", "ipsec="+val)...).CombinedOutput() if err != nil { return fmt.Errorf("set NB_Global ipsec=%s: %s: %w", val, strings.TrimSpace(string(out)), err) } diff --git a/spinifex/network/host/ipsec_enable.go b/spinifex/network/host/ipsec_enable.go index 7eba2954e..906aaeafa 100644 --- a/spinifex/network/host/ipsec_enable.go +++ b/spinifex/network/host/ipsec_enable.go @@ -15,7 +15,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/admin" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" "github.com/mulgadc/spinifex/spinifex/foundation/telemetry" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // systemctlActiveTimeout bounds the wait for openvswitch-ipsec.service to become active. @@ -199,7 +199,7 @@ func ensureIPSecServices(want bool) error { if want { state = "on" } - out, err := utils.SudoCommand(ipsecStateHelper, state).CombinedOutput() + out, err := hostcommand.SudoCommand(ipsecStateHelper, state).CombinedOutput() if err != nil { return fmt.Errorf("set ipsec state %s: %w: %s", state, err, strings.TrimSpace(string(out))) } @@ -218,13 +218,13 @@ func ipsecServicesMatch(want bool) bool { // covers masked, static and a unit the distro does not ship at all. The helper // masks rather than disables, so the off state reads back as "masked". func unitIsEnabled(unit string) bool { - out, _ := utils.SudoCommand("systemctl", "is-enabled", unit).CombinedOutput() + out, _ := hostcommand.SudoCommand("systemctl", "is-enabled", unit).CombinedOutput() state := strings.TrimSpace(string(out)) return state == "enabled" || state == "enabled-runtime" || state == "alias" } func unitIsActive(unit string) bool { - out, _ := utils.SudoCommand("systemctl", "is-active", unit).CombinedOutput() + out, _ := hostcommand.SudoCommand("systemctl", "is-active", unit).CombinedOutput() return strings.TrimSpace(string(out)) == "active" } @@ -394,7 +394,7 @@ func ensureOVSMonitorIPSecActive() error { deadline := time.Now().Add(systemctlActiveTimeout) var lastOut string for time.Now().Before(deadline) { - out, _ := utils.SudoCommand("systemctl", "is-active", ovsIPSecUnit).CombinedOutput() + out, _ := hostcommand.SudoCommand("systemctl", "is-active", ovsIPSecUnit).CombinedOutput() lastOut = strings.TrimSpace(string(out)) if lastOut == "active" { return nil diff --git a/spinifex/network/host/ipsec_enable_test.go b/spinifex/network/host/ipsec_enable_test.go index b9aaa7f36..e1b983a6b 100644 --- a/spinifex/network/host/ipsec_enable_test.go +++ b/spinifex/network/host/ipsec_enable_test.go @@ -13,7 +13,7 @@ import ( "time" "github.com/mulgadc/spinifex/spinifex/bootstrap/config" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -179,7 +179,7 @@ func (r *recordingSudo) ovsSets() []string { func TestEnableOVNIPSec(t *testing.T) { recorder := &recordingSudo{nbUnreachable: true} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configDir := ipsecTestConfigDir(t) configPath := filepath.Join(configDir, "spinifex.toml") @@ -199,7 +199,7 @@ func TestEnableOVNIPSec(t *testing.T) { func TestEnableOVNIPSec_Management(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -213,7 +213,7 @@ func TestEnableOVNIPSec_Management(t *testing.T) { // others have not finished, and every guest crossing chassis black-holes. func TestEnableOVNIPSec_HoldsFlagUntilEveryChassisIsReady(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") barrier := node2Pending() @@ -231,7 +231,7 @@ func TestEnableOVNIPSec_HoldsFlagUntilEveryChassisIsReady(t *testing.T) { // floor; plaintext is the state the cluster had before IPsec was asked for. func TestEnableOVNIPSec_RetractsFlagWhenAChassisRegresses(t *testing.T) { recorder := &recordingSudo{nbIPSec: "true"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -246,7 +246,7 @@ func TestEnableOVNIPSec_RetractsFlagWhenAChassisRegresses(t *testing.T) { // must never downgrade a working encrypted mesh to plaintext. func TestEnableOVNIPSec_BarrierErrorLeavesTheFlagAlone(t *testing.T) { recorder := &recordingSudo{nbIPSec: "true"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -260,7 +260,7 @@ func TestEnableOVNIPSec_BarrierErrorLeavesTheFlagAlone(t *testing.T) { // Already true and everyone ready: no write, so a steady-state pass is free. func TestEnableOVNIPSec_AlreadyAssertedIsANoOp(t *testing.T) { recorder := &recordingSudo{nbIPSec: "true"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -272,7 +272,7 @@ func TestEnableOVNIPSec_AlreadyAssertedIsANoOp(t *testing.T) { // dropping the record would make it pending forever to everyone else. func TestEnableOVNIPSec_PublishFailureFailsThePass(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -284,8 +284,8 @@ func TestEnableOVNIPSec_PublishFailureFailsThePass(t *testing.T) { } func TestEnableOVNIPSec_SingleNodeSkip(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { - t.Fatalf("utils.SudoCommand must not run on single-node short-circuit; got %s %v", name, args) + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Fatalf("hostcommand.SudoCommand must not run on single-node short-circuit; got %s %v", name, args) return exec.Command("true") })) @@ -302,7 +302,7 @@ func TestEnableOVNIPSec_SingleNodeSkip(t *testing.T) { func TestEnableOVNIPSec_MonitorIPSecInactive(t *testing.T) { recorder := &recordingSudo{activeOutput: "inactive\n"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) origTimeout := systemctlActiveTimeout systemctlActiveTimeout = 100 * time.Millisecond @@ -330,7 +330,7 @@ func TestEnableOVNIPSec_MonitorIPSecInactive(t *testing.T) { func TestEnableOVNIPSec_MissingCert(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configDir := t.TempDir() configPath := filepath.Join(configDir, "spinifex.toml") @@ -353,7 +353,7 @@ func TestEnableOVNIPSec_NoConfigPath(t *testing.T) { func TestReconcileOVNIPSec_DisabledStopsCharon(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady())) @@ -368,7 +368,7 @@ func TestReconcileOVNIPSec_DisabledStopsCharon(t *testing.T) { // directly: it would fail at the polkit prompt and leave charon listening. func TestReconcileOVNIPSec_NeverCallsSystemctlDirectly(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady())) @@ -385,7 +385,7 @@ func TestReconcileOVNIPSec_NeverCallsSystemctlDirectly(t *testing.T) { // listening even though ipsec_enabled defaults to true. func TestReconcileOVNIPSec_SingleNodeStopsCharon(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) cfg := &config.ClusterConfig{Node: "node1", Nodes: map[string]config.Config{"node1": {}}} cfg.Network.IPSecEnabled = true @@ -406,7 +406,7 @@ func TestReconcileOVNIPSec_EnabledTurnsTheServicesOn(t *testing.T) { strongswanStarterUnit: "inactive\n", }, } - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) recorder.nbUnreachable = true configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -430,7 +430,7 @@ func TestReconcileOVNIPSec_AlreadyInStateIsNoOp(t *testing.T) { enabledOutput: map[string]string{strongswanStarterUnit: "disabled\n", ovsIPSecUnit: "disabled\n"}, activePerUnit: map[string]string{strongswanStarterUnit: "inactive\n", ovsIPSecUnit: "inactive\n"}, } - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady())) assert.Empty(t, recorder.helperRuns()) @@ -447,7 +447,7 @@ func TestReconcileOVNIPSec_UnknownUnitIsNotAnError(t *testing.T) { ovsIPSecUnit: "inactive\n", }, } - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady())) assert.Empty(t, recorder.helperRuns()) @@ -456,8 +456,8 @@ func TestReconcileOVNIPSec_UnknownUnitIsNotAnError(t *testing.T) { // A nil cluster config means the intent is unknown; tearing down working // tunnels on a guess would be worse than leaving them. func TestReconcileOVNIPSec_NilConfigLeavesUnitsAlone(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { - t.Fatalf("utils.SudoCommand must not run with a nil cluster config; got %s %v", name, args) + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Fatalf("hostcommand.SudoCommand must not run with a nil cluster config; got %s %v", name, args) return exec.Command("true") })) @@ -468,7 +468,7 @@ func TestReconcileOVNIPSec_NilConfigLeavesUnitsAlone(t *testing.T) { // while a peer already required encryption; the pass has to be retried. func TestMaintainIPSec_RetriesAFailedPass(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) origRetry, origInterval := ipsecRetryDelay, ipsecReconcileInterval ipsecRetryDelay, ipsecReconcileInterval = time.Millisecond, time.Millisecond @@ -526,7 +526,7 @@ func (f *flakyBarrier) succeeded() bool { // strongSwan connection in the cluster. func TestEnableOVNIPSec_OnlyTheElectedWriterTouchesNBGlobal(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -558,7 +558,7 @@ func TestNBGlobalWriter_SkipsNodesWithoutANBDB(t *testing.T) { // freshness window, which is the black hole again with a slower fuse. func TestEnableOVNIPSec_PublishesUnreadyWhenTheLocalHalfFails(t *testing.T) { recorder := &recordingSudo{activeOutput: "inactive\n"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) origTimeout := systemctlActiveTimeout systemctlActiveTimeout = 100 * time.Millisecond @@ -583,7 +583,7 @@ func TestEnableOVNIPSec_PublishesUnreadyWhenTheLocalHalfFails(t *testing.T) { // the other direction. func TestReconcileOVNIPSec_DisabledRetractsNBGlobal(t *testing.T) { recorder := &recordingSudo{nbIPSec: "true"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) barrier := allReady() require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", @@ -598,7 +598,7 @@ func TestReconcileOVNIPSec_DisabledRetractsNBGlobal(t *testing.T) { // Already released: the off path must not rewrite a flag that is already false. func TestReconcileOVNIPSec_DisabledIsANoOpWhenAlreadyReleased(t *testing.T) { recorder := &recordingSudo{} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) require.NoError(t, ReconcileOVNIPSec(t.Context(), "/etc/spinifex/spinifex.toml", multiNodeIPSecConfig(false), allReady())) @@ -611,7 +611,7 @@ func TestReconcileOVNIPSec_DisabledIsANoOpWhenAlreadyReleased(t *testing.T) { // the flag could go blind and never retract. func TestEnableOVNIPSec_NBReadFailureFailsThePass(t *testing.T) { recorder := &recordingSudo{nbError: "ovn-nbctl: unix:/var/run/ovn/ovnnb_db.sock: permission denied"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") @@ -624,7 +624,7 @@ func TestEnableOVNIPSec_NBReadFailureFailsThePass(t *testing.T) { // No NB DB answering is skipped quietly, not a fault, and the node stands down. func TestEnableOVNIPSec_NoLocalNBDBIsNotAnError(t *testing.T) { recorder := &recordingSudo{nbUnreachable: true} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) configPath := filepath.Join(ipsecTestConfigDir(t), "spinifex.toml") barrier := allReady() @@ -641,7 +641,7 @@ func TestEnableOVNIPSec_NoLocalNBDBIsNotAnError(t *testing.T) { // the value parses a live "true" as false, which sends the reconcile into the // "holding encryption off" branch while the flag is in fact still asserted. func TestGetNBGlobalIPSec_IgnoresStderrChatter(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(string, ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(string, ...string) *exec.Cmd { return exec.Command("sh", "-c", `echo "ovsdb-idl|WARN|reconnecting" >&2; echo true`) })) @@ -680,7 +680,7 @@ func TestNBctlArgs_ReadsAcceptFollowersWritesStayLeaderOnly(t *testing.T) { func TestReconcileOVNIPSec_UsesTheConfiguredNBRemotes(t *testing.T) { const nb = "tcp:10.0.0.1:6641,tcp:10.0.0.2:6641" recorder := &recordingSudo{nbIPSec: "true"} - t.Cleanup(utils.SetSudoCommandForTest(recorder.stub)) + t.Cleanup(hostcommand.SetSudoCommandForTest(recorder.stub)) cfg := multiNodeIPSecConfig(false) cfg.Nodes["node1"] = config.Config{VPCD: config.VPCDConfig{OVNNBAddr: nb}} diff --git a/spinifex/network/host/ovn_raft.go b/spinifex/network/host/ovn_raft.go index 162d57167..62df9c375 100644 --- a/spinifex/network/host/ovn_raft.go +++ b/spinifex/network/host/ovn_raft.go @@ -6,7 +6,7 @@ import ( "strings" "time" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // OVN DB ctl targets and schemas for cluster/status probes. The short "-t" @@ -31,7 +31,7 @@ const ovnAppctlTimeout = 500 * time.Millisecond func OVNDBRole(target, schema string) string { ctx, cancel := context.WithTimeout(context.Background(), ovnAppctlTimeout) defer cancel() - out, err := utils.SudoCommandContext(ctx, "ovn-appctl", "-t", target, "cluster/status", schema).Output() + out, err := hostcommand.SudoCommandContext(ctx, "ovn-appctl", "-t", target, "cluster/status", schema).Output() if err != nil { slog.Debug("Failed to query OVN cluster/status", "schema", schema, "err", err) return "" diff --git a/spinifex/network/host/plumber.go b/spinifex/network/host/plumber.go index f2d6dcf4a..223de2319 100644 --- a/spinifex/network/host/plumber.go +++ b/spinifex/network/host/plumber.go @@ -10,8 +10,8 @@ import ( "strconv" "strings" - "github.com/mulgadc/spinifex/spinifex/utils" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) // OVSPlumber implements vm.NetworkPlumber using system commands @@ -20,7 +20,7 @@ type OVSPlumber struct{} var _ vm.NetworkPlumber = (*OVSPlumber)(nil) -// NewOVSPlumber returns the default plumber wired to utils.SudoCommand. +// NewOVSPlumber returns the default plumber wired to hostcommand.SudoCommand. func NewOVSPlumber() *OVSPlumber { return &OVSPlumber{} } // deleteTapLink removes a tap by name. `ip link del` rather than `ip tuntap @@ -28,14 +28,14 @@ func NewOVSPlumber() *OVSPlumber { return &OVSPlumber{} } // a multi_queue tap unless the caller repeats the exact creation flags, which // a delete-by-name path does not know. RTM_DELLINK is flag-agnostic. func deleteTapLink(name string) *exec.Cmd { - return utils.SudoCommand("ip", "link", "del", "dev", name) + return hostcommand.SudoCommand("ip", "link", "del", "dev", name) } // SetupTap creates the kernel tap, brings it up, and attaches it to spec.Bridge. // Pre-create del-port is unconditional: OVS conf.db survives reboot but kernel // taps don't, and --may-exist would silently keep stale external_ids. func (p *OVSPlumber) SetupTap(spec vm.TapSpec) error { - if err := utils.SudoCommand("ovs-vsctl", "--if-exists", "del-port", spec.Bridge, spec.Name).Run(); err != nil { + if err := hostcommand.SudoCommand("ovs-vsctl", "--if-exists", "del-port", spec.Bridge, spec.Name).Run(); err != nil { slog.Warn("Pre-create del-port failed (continuing)", "tap", spec.Name, "bridge", spec.Bridge, "err", err) } if _, err := os.Stat("/sys/class/net/" + spec.Name); err == nil { @@ -57,7 +57,7 @@ func (p *OVSPlumber) SetupTap(spec vm.TapSpec) error { if uid := os.Geteuid(); uid != 0 { addArgs = append(addArgs, "user", strconv.Itoa(uid), "group", strconv.Itoa(os.Getegid())) } - if out, err := utils.SudoCommand("ip", addArgs...).CombinedOutput(); err != nil { + if out, err := hostcommand.SudoCommand("ip", addArgs...).CombinedOutput(); err != nil { return fmt.Errorf("create tap %s: %s: %w", spec.Name, strings.TrimSpace(string(out)), err) } @@ -68,7 +68,7 @@ func (p *OVSPlumber) SetupTap(spec vm.TapSpec) error { if spec.MTU > 0 { upArgs = append(upArgs, "mtu", strconv.Itoa(spec.MTU)) } - if out, err := utils.SudoCommand("ip", upArgs...).CombinedOutput(); err != nil { + if out, err := hostcommand.SudoCommand("ip", upArgs...).CombinedOutput(); err != nil { if cleanErr := deleteTapLink(spec.Name).Run(); cleanErr != nil { slog.Warn("Failed to clean up tap after bring-up failure", "tap", spec.Name, "err", cleanErr) } @@ -84,7 +84,7 @@ func (p *OVSPlumber) SetupTap(spec vm.TapSpec) error { addPortArgs = append(addPortArgs, fmt.Sprintf("external_ids:%s=%s", k, spec.ExternalIDs[k])) } } - if out, err := utils.SudoCommand("ovs-vsctl", addPortArgs...).CombinedOutput(); err != nil { + if out, err := hostcommand.SudoCommand("ovs-vsctl", addPortArgs...).CombinedOutput(); err != nil { if cleanErr := deleteTapLink(spec.Name).Run(); cleanErr != nil { slog.Warn("Failed to clean up tap after OVS failure", "tap", spec.Name, "err", cleanErr) } @@ -99,7 +99,7 @@ func (p *OVSPlumber) SetupTap(spec vm.TapSpec) error { // Idempotent: callers may invoke for an instance that never reached SetupTap // (e.g. terminate that races mid-launch). func (p *OVSPlumber) CleanupTap(name string) error { - if out, err := utils.SudoCommand("ovs-vsctl", "--if-exists", "del-port", name).CombinedOutput(); err != nil { + if out, err := hostcommand.SudoCommand("ovs-vsctl", "--if-exists", "del-port", name).CombinedOutput(); err != nil { slog.Warn("Failed to remove tap from OVS", "tap", name, "err", err, "out", strings.TrimSpace(string(out))) } diff --git a/spinifex/network/host/plumber_test.go b/spinifex/network/host/plumber_test.go index e233b07ee..f1b8ce466 100644 --- a/spinifex/network/host/plumber_test.go +++ b/spinifex/network/host/plumber_test.go @@ -6,8 +6,8 @@ import ( "strings" "testing" - "github.com/mulgadc/spinifex/spinifex/utils" "github.com/mulgadc/spinifex/spinifex/runtime/compute/vm" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) func TestOVSPlumber_SetupTap_AddPortArgs(t *testing.T) { @@ -46,7 +46,7 @@ func TestOVSPlumber_SetupTap_AddPortArgs(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { var calls [][]string - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { call := append([]string{name}, args...) calls = append(calls, call) return exec.Command("/bin/true") @@ -91,7 +91,7 @@ func TestOVSPlumber_SetupTap_MultiQueue(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { var addArgs []string - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ip" && len(args) >= 2 && args[0] == "tuntap" && args[1] == "add" { addArgs = args } @@ -130,7 +130,7 @@ func TestOVSPlumber_SetupTap_MTU(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { var upArgs []string - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ip" && len(args) >= 2 && args[0] == "link" && args[1] == "set" && slices.Contains(args, "up") { upArgs = args } @@ -161,7 +161,7 @@ func TestOVSPlumber_SetupTap_MTU(t *testing.T) { // "lo" is always present so os.Stat hits the pre-create cleanup branch. func TestOVSPlumber_SetupTap_PreExistingKernelTap(t *testing.T) { failLinkDel := true - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ip" && len(args) >= 2 && args[0] == "link" && args[1] == "del" && failLinkDel { failLinkDel = false return exec.Command("/bin/false") @@ -213,7 +213,7 @@ func TestOVSPlumber_SetupTap_ErrorBranches(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if tc.failMatch(name, args) { return exec.Command("/bin/false") } @@ -237,7 +237,7 @@ func TestOVSPlumber_SetupTap_ErrorBranches(t *testing.T) { func TestOVSPlumber_CleanupTap_PresentKernelTap(t *testing.T) { var sawLinkDel bool - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ip" && len(args) >= 2 && args[0] == "link" && args[1] == "del" { sawLinkDel = true } @@ -258,7 +258,7 @@ func TestOVSPlumber_CleanupTap_PresentKernelTap(t *testing.T) { // does not care how the tap was created. func TestOVSPlumber_TapDeletionNeverUsesTuntapDel(t *testing.T) { var calls [][]string - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { calls = append(calls, append([]string{name}, args...)) return exec.Command("/bin/true") })) @@ -279,7 +279,7 @@ func TestOVSPlumber_TapDeletionNeverUsesTuntapDel(t *testing.T) { } func TestOVSPlumber_CleanupTap_DelPortLoggedWarn(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ovs-vsctl" { return exec.Command("/bin/false") } @@ -293,7 +293,7 @@ func TestOVSPlumber_CleanupTap_DelPortLoggedWarn(t *testing.T) { } func TestOVSPlumber_CleanupTap_LinkDelFailure(t *testing.T) { - t.Cleanup(utils.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { + t.Cleanup(hostcommand.SetSudoCommandForTest(func(name string, args ...string) *exec.Cmd { if name == "ip" && len(args) >= 2 && args[0] == "link" && args[1] == "del" { return exec.Command("/bin/false") } diff --git a/spinifex/network/host/run.go b/spinifex/network/host/run.go index 9fbd7a336..cca54a693 100644 --- a/spinifex/network/host/run.go +++ b/spinifex/network/host/run.go @@ -11,10 +11,10 @@ import ( "path/filepath" "strings" - "github.com/mulgadc/spinifex/spinifex/utils" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" ) -// execRunner escalates only the commands that need it — see utils.NeedsPrivilege. +// execRunner escalates only the commands that need it — see hostcommand.NeedsPrivilege. // The OVS/OVN socket clients reach their daemons over the group-owned control // sockets, and ip/iptables/arping run under the unit's ambient capabilities. type execRunner struct{} @@ -26,7 +26,7 @@ func NewExecRunner() Runner { return execRunner{} } func (execRunner) Run(ctx context.Context, name string, args ...string) ([]byte, error) { var cmd *exec.Cmd - if utils.NeedsPrivilege(name, args...) { + if hostcommand.NeedsPrivilege(name, args...) { cmd = exec.CommandContext(ctx, "sudo", append([]string{name}, args...)...) } else { cmd = exec.CommandContext(ctx, name, args...) diff --git a/spinifex/runtime/host/command/command.go b/spinifex/runtime/host/command/command.go index 72ee8c553..71c4fbc0b 100644 --- a/spinifex/runtime/host/command/command.go +++ b/spinifex/runtime/host/command/command.go @@ -1,4 +1,4 @@ -// Package command runs bounded host commands for node-local runtime work. +// Package command runs host commands and applies node-local privilege policy. package command import ( diff --git a/spinifex/utils/sudo.go b/spinifex/runtime/host/command/privilege.go similarity index 99% rename from spinifex/utils/sudo.go rename to spinifex/runtime/host/command/privilege.go index 0bbaafba1..b915304cd 100644 --- a/spinifex/utils/sudo.go +++ b/spinifex/runtime/host/command/privilege.go @@ -1,4 +1,4 @@ -package utils +package command import ( "context" diff --git a/spinifex/utils/sudo_invariants_test.go b/spinifex/runtime/host/command/privilege_invariants_test.go similarity index 92% rename from spinifex/utils/sudo_invariants_test.go rename to spinifex/runtime/host/command/privilege_invariants_test.go index 76cd07f17..63227296e 100644 --- a/spinifex/utils/sudo_invariants_test.go +++ b/spinifex/runtime/host/command/privilege_invariants_test.go @@ -1,4 +1,4 @@ -package utils_test +package command_test import ( "os" @@ -10,11 +10,11 @@ import ( ) // policyOwners are the only files allowed to build a sudo invocation. Everything -// else must route through utils.SudoCommand / host.NewExecRunner so the +// else must route through hostcommand.SudoCommand / host.NewExecRunner so the // escalation policy in NeedsPrivilege applies. var policyOwners = map[string]bool{ - filepath.Join("spinifex", "utils", "sudo.go"): true, - filepath.Join("spinifex", "network", "host", "run.go"): true, + filepath.Join("spinifex", "runtime", "host", "command", "privilege.go"): true, + filepath.Join("spinifex", "network", "host", "run.go"): true, } // repoRoot walks up from this test file to the directory holding go.mod. @@ -86,8 +86,8 @@ func TestRG10_SudoOnlyThroughThePolicy(t *testing.T) { } if len(offenders) > 0 { - t.Fatalf("RG-10: these build sudo invocations directly, bypassing utils.NeedsPrivilege:\n %s\n"+ - "Use utils.SudoCommand or host.NewExecRunner so the OVS/OVN socket clients stay unescalated.", + t.Fatalf("RG-10: these build sudo invocations directly, bypassing hostcommand.NeedsPrivilege:\n %s\n"+ + "Use hostcommand.SudoCommand or host.NewExecRunner so the OVS/OVN socket clients stay unescalated.", strings.Join(offenders, "\n ")) } } diff --git a/spinifex/utils/sudo_test.go b/spinifex/runtime/host/command/privilege_test.go similarity index 99% rename from spinifex/utils/sudo_test.go rename to spinifex/runtime/host/command/privilege_test.go index 8270d19db..3986f4b8a 100644 --- a/spinifex/utils/sudo_test.go +++ b/spinifex/runtime/host/command/privilege_test.go @@ -1,4 +1,4 @@ -package utils +package command import ( "os" diff --git a/spinifex/vpcd/vpcd.go b/spinifex/vpcd/vpcd.go index 45c3cf916..318bf4949 100644 --- a/spinifex/vpcd/vpcd.go +++ b/spinifex/vpcd/vpcd.go @@ -30,6 +30,7 @@ import ( "github.com/mulgadc/spinifex/spinifex/network/reconcile" "github.com/mulgadc/spinifex/spinifex/network/subscribers" "github.com/mulgadc/spinifex/spinifex/network/topology" + hostcommand "github.com/mulgadc/spinifex/spinifex/runtime/host/command" "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" @@ -78,11 +79,11 @@ var waitForFlowsHV = func(nbAddr string) error { return nil } -// sudoCommand is utils.SudoCommand, which escalates only what genuinely needs +// sudoCommand is hostcommand.SudoCommand, which escalates only what genuinely needs // it. Every caller here is an OVS/OVN socket client, so none of them escalate; // a local copy that always sudoed silently bypassed that policy and broke the // flows-ready barrier once the grants were removed. -var sudoCommand = utils.SudoCommand +var sudoCommand = hostcommand.SudoCommand var serviceName = "vpcd"