diff options
| author | James O'Doherty <james@theodohertyfamily.com> | 2026-06-13 11:51:04 -0400 |
|---|---|---|
| committer | James O'Doherty <james@theodohertyfamily.com> | 2026-06-13 11:51:04 -0400 |
| commit | 29621ecbd1e77e6e1a70b6b3ea8fbe3a56e47df3 (patch) | |
| tree | fa54976bbb0c4e9db59c983e7cb4e60c5119d18b /internal/network | |
| parent | f8afb7d5889f5c8b6ea256fd078fa8426d21c7be (diff) | |
refactor: implement dependency injection and enable parallel testing
This commit refactors the core system operations to use a manager-based
dependency injection pattern, eliminating global state and resolving
data races in the test suite.
Architecture:
- Introduced NetworkManager and NetworkOps interface in internal/network
to abstract netlink calls.
- Introduced MountOps and FileSystem interfaces in internal/namespace
to abstract mount and filesystem operations.
- Introduced TunnelManager in internal/wireguard to coordinate tunnel
lifecycle using the new abstractions.
- Updated internal/cli and internal/manager to use these managers.
Testing:
- Restored t.Parallel() to unit tests in internal/network and
internal/wireguard.
- Implemented setupParallelEnv and an enhanced mockFS in
wireguard_unit_test.go to ensure complete test isolation.
- Added bootstrap_test.go to verify launcher preparation logic in
internal/namespace without requiring syscall.Exec.
- Resolved data races in internal/network tests.
CLI:
- Added support for -h, --help, and -help flags for the main command.
Verification:
- Passed all tests (unit, integration, E2E).
- Verified zero data races with 'go test -race'.
- Passed golangci-lint and go vet.
Diffstat (limited to 'internal/network')
| -rw-r--r-- | internal/network/network.go | 74 | ||||
| -rw-r--r-- | internal/network/network_test.go | 163 |
2 files changed, 228 insertions, 9 deletions
diff --git a/internal/network/network.go b/internal/network/network.go index 6afcf5e..e9dce77 100644 --- a/internal/network/network.go +++ b/internal/network/network.go @@ -16,9 +16,54 @@ type InterfaceInfo struct { Index int } +// NetworkOps abstracts the low-level netlink operations. +type NetworkOps interface { + LinkList() ([]netlink.Link, error) + LinkByName(name string) (netlink.Link, error) + LinkSetMTU(link netlink.Link, mtu int) error + LinkSetUp(link netlink.Link) error + AddrAdd(link netlink.Link, addr *netlink.Addr) error + RouteAdd(route *netlink.Route) error + RouteReplace(route *netlink.Route) error +} + +// realNetworkOps is the production implementation using netlink. +type realNetworkOps struct{} + +func (r *realNetworkOps) LinkList() ([]netlink.Link, error) { return netlink.LinkList() } +func (r *realNetworkOps) LinkByName(name string) (netlink.Link, error) { + return netlink.LinkByName(name) +} +func (r *realNetworkOps) LinkSetMTU(link netlink.Link, mtu int) error { + return netlink.LinkSetMTU(link, mtu) +} +func (r *realNetworkOps) LinkSetUp(link netlink.Link) error { return netlink.LinkSetUp(link) } +func (r *realNetworkOps) AddrAdd(link netlink.Link, addr *netlink.Addr) error { + return netlink.AddrAdd(link, addr) +} + +func (r *realNetworkOps) RouteAdd(route *netlink.Route) error { return netlink.RouteAdd(route) } +func (r *realNetworkOps) RouteReplace(route *netlink.Route) error { return netlink.RouteReplace(route) } + +// DefaultNetworkOps is the global instance used by the package functions. +// It can be replaced during tests. +var DefaultNetworkOps NetworkOps = &realNetworkOps{} + +// NetworkManager coordinates network configuration within a namespace. +type NetworkManager struct { + Ops NetworkOps +} + +// NewNetworkManager creates a new NetworkManager with production defaults. +func NewNetworkManager() *NetworkManager { + return &NetworkManager{ + Ops: DefaultNetworkOps, + } +} + // ListInterfaces returns a list of all network interfaces present in the current namespace. -func ListInterfaces() ([]InterfaceInfo, error) { - links, err := netlink.LinkList() +func (nm *NetworkManager) ListInterfaces() ([]InterfaceInfo, error) { + links, err := nm.Ops.LinkList() if err != nil { return nil, fmt.Errorf("failed to list interfaces: %w", err) } @@ -35,17 +80,17 @@ func ListInterfaces() ([]InterfaceInfo, error) { // ConfigureInterface sets the MTU, brings the interface up, assigns an IP address, // and configures the default route. -func ConfigureInterface(name, address string, mtu int) error { - link, err := netlink.LinkByName(name) +func (nm *NetworkManager) ConfigureInterface(name, address string, mtu int) error { + link, err := nm.Ops.LinkByName(name) if err != nil { return fmt.Errorf("failed to find link %s: %w", name, err) } - if err := netlink.LinkSetMTU(link, mtu); err != nil { + if err := nm.Ops.LinkSetMTU(link, mtu); err != nil { return fmt.Errorf("failed to set MTU %d on link %s: %w", mtu, name, err) } - if err := netlink.LinkSetUp(link); err != nil { + if err := nm.Ops.LinkSetUp(link); err != nil { return fmt.Errorf("failed to bring up link %s: %w", name, err) } @@ -53,7 +98,7 @@ func ConfigureInterface(name, address string, mtu int) error { if err != nil { return fmt.Errorf("invalid IP address %s: %w", address, err) } - if err := netlink.AddrAdd(link, addr); err != nil { + if err := nm.Ops.AddrAdd(link, addr); err != nil { if !strings.Contains(err.Error(), "file exists") { return fmt.Errorf("failed to add address %s to link %s: %w", address, name, err) } @@ -72,11 +117,22 @@ func ConfigureInterface(name, address string, mtu int) error { Dst: dst, } - if err := netlink.RouteAdd(route); err != nil { - if err := netlink.RouteReplace(route); err != nil { + if err := nm.Ops.RouteAdd(route); err != nil { + if err := nm.Ops.RouteReplace(route); err != nil { return fmt.Errorf("failed to configure default route via %s: %w", name, err) } } return nil } + +// ListInterfaces returns a list of all network interfaces present in the current namespace. +func ListInterfaces() ([]InterfaceInfo, error) { + return NewNetworkManager().ListInterfaces() +} + +// ConfigureInterface sets the MTU, brings the interface up, assigns an IP address, +// and configures the default route. +func ConfigureInterface(name, address string, mtu int) error { + return NewNetworkManager().ConfigureInterface(name, address, mtu) +} diff --git a/internal/network/network_test.go b/internal/network/network_test.go new file mode 100644 index 0000000..b598484 --- /dev/null +++ b/internal/network/network_test.go @@ -0,0 +1,163 @@ +//go:build linux + +package network + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/vishvananda/netlink" +) + +// mockNetworkOps allows us to control the behavior of netlink calls. +type mockNetworkOps struct { + linkByNameFunc func(name string) (netlink.Link, error) + linkSetMTUFunc func(link netlink.Link, mtu int) error + linkSetUpFunc func(link netlink.Link) error + addrAddFunc func(link netlink.Link, addr *netlink.Addr) error + routeAddFunc func(route *netlink.Route) error + routeReplaceFunc func(route *netlink.Route) error +} + +func (m *mockNetworkOps) LinkList() ([]netlink.Link, error) { return nil, nil } +func (m *mockNetworkOps) LinkByName(name string) (netlink.Link, error) { + if m.linkByNameFunc != nil { + return m.linkByNameFunc(name) + } + return nil, fmt.Errorf("not implemented") +} +func (m *mockNetworkOps) LinkSetMTU(link netlink.Link, mtu int) error { + if m.linkSetMTUFunc != nil { + return m.linkSetMTUFunc(link, mtu) + } + return nil +} +func (m *mockNetworkOps) LinkSetUp(link netlink.Link) error { + if m.linkSetUpFunc != nil { + return m.linkSetUpFunc(link) + } + return nil +} +func (m *mockNetworkOps) AddrAdd(link netlink.Link, addr *netlink.Addr) error { + if m.addrAddFunc != nil { + return m.addrAddFunc(link, addr) + } + return nil +} +func (m *mockNetworkOps) RouteAdd(route *netlink.Route) error { + if m.routeAddFunc != nil { + return m.routeAddFunc(route) + } + return nil +} +func (m *mockNetworkOps) RouteReplace(route *netlink.Route) error { + if m.routeReplaceFunc != nil { + return m.routeReplaceFunc(route) + } + return nil +} + +// mockLink implements netlink.Link. +type mockLink struct { + name string + idx int +} + +func (m *mockLink) Type() string { + return "mock" +} + +func (m *mockLink) Attrs() *netlink.LinkAttrs { + return &netlink.LinkAttrs{Name: m.name, Index: m.idx} +} + +func TestConfigureInterface_Success(t *testing.T) { + t.Parallel() + mock := &mockNetworkOps{ + linkByNameFunc: func(name string) (netlink.Link, error) { + return &mockLink{name: name, idx: 1}, nil + }, + } + nm := &NetworkManager{Ops: mock} + + err := nm.ConfigureInterface("tun0", "10.0.0.1/24", 1420) + if err != nil { + t.Errorf("expected success, got %v", err) + } +} + +func TestConfigureInterface_RouteFallback(t *testing.T) { + t.Parallel() + routeAddCalled := false + routeReplaceCalled := false + + mock := &mockNetworkOps{ + linkByNameFunc: func(name string) (netlink.Link, error) { + return &mockLink{name: name, idx: 1}, nil + }, + routeAddFunc: func(route *netlink.Route) error { + routeAddCalled = true + return errors.New("file exists") // Simulate EEXIST + }, + routeReplaceFunc: func(route *netlink.Route) error { + routeReplaceCalled = true + return nil + }, + } + nm := &NetworkManager{Ops: mock} + + err := nm.ConfigureInterface("tun0", "10.0.0.1/24", 1420) + if err != nil { + t.Errorf("expected success after fallback, got %v", err) + } + if !routeAddCalled { + t.Error("expected RouteAdd to be called first") + } + if !routeReplaceCalled { + t.Error("expected RouteReplace to be called after RouteAdd fails with 'file exists'") + } +} + +func TestConfigureInterface_RouteFailure(t *testing.T) { + t.Parallel() + mock := &mockNetworkOps{ + linkByNameFunc: func(name string) (netlink.Link, error) { + return &mockLink{name: name, idx: 1}, nil + }, + routeAddFunc: func(route *netlink.Route) error { + return errors.New("critical network failure") + }, + routeReplaceFunc: func(route *netlink.Route) error { + return errors.New("critical network failure") + }, + } + nm := &NetworkManager{Ops: mock} + + err := nm.ConfigureInterface("tun0", "10.0.0.1/24", 1420) + if err == nil { + t.Error("expected error when both RouteAdd and RouteReplace fail, got nil") + } + if !strings.Contains(err.Error(), "failed to configure default route") { + t.Errorf("expected route error, got: %v", err) + } +} + +func TestConfigureInterface_LinkNotFound(t *testing.T) { + t.Parallel() + mock := &mockNetworkOps{ + linkByNameFunc: func(name string) (netlink.Link, error) { + return nil, errors.New("no such device") + }, + } + nm := &NetworkManager{Ops: mock} + + err := nm.ConfigureInterface("nonexistent", "10.0.0.1/24", 1420) + if err == nil { + t.Error("expected error when link is not found, got nil") + } + if !strings.Contains(err.Error(), "failed to find link") { + t.Errorf("expected link not found error, got: %v", err) + } +} |
