diff --git a/client/iface/iface_test.go b/client/iface/iface_test.go index 89c8cd16e..e0ee696a9 100644 --- a/client/iface/iface_test.go +++ b/client/iface/iface_test.go @@ -44,10 +44,7 @@ func TestWGIface_UpdateAddr(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4) addr := "100.64.0.1/8" wgPort := 33100 - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, @@ -127,10 +124,7 @@ func getIfaceAddrs(ifaceName string) ([]net.Addr, error) { func Test_CreateInterface(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+1) wgIP := "10.99.99.1/32" - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, Address: wgaddr.MustParseWGAddress(wgIP), @@ -170,10 +164,7 @@ func Test_Close(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+2) wgIP := "10.99.99.2/32" wgPort := 33100 - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, @@ -215,10 +206,7 @@ func TestRecreation(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+2) wgIP := "10.99.99.2/32" wgPort := 33100 - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, @@ -288,10 +276,7 @@ func Test_ConfigureInterface(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+3) wgIP := "10.99.99.5/30" wgPort := 33100 - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, Address: wgaddr.MustParseWGAddress(wgIP), @@ -343,10 +328,7 @@ func Test_ConfigureInterface(t *testing.T) { func Test_UpdatePeer(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4) wgIP := "10.99.99.9/30" - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, @@ -413,10 +395,7 @@ func Test_UpdatePeer(t *testing.T) { func Test_RemovePeer(t *testing.T) { ifaceName := fmt.Sprintf("utun%d", WgIntNumber+4) wgIP := "10.99.99.13/30" - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := WGIFaceOpts{ IFaceName: ifaceName, @@ -477,10 +456,7 @@ func Test_ConnectPeers(t *testing.T) { peer2wgPort := 33200 keepAlive := 1 * time.Second - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) guid := fmt.Sprintf("{%s}", uuid.New().String()) device.CustomWindowsGUIDString = strings.ToLower(guid) @@ -516,10 +492,7 @@ func Test_ConnectPeers(t *testing.T) { guid = fmt.Sprintf("{%s}", uuid.New().String()) device.CustomWindowsGUIDString = strings.ToLower(guid) - newNet, err = stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet = stdnet.NewNet(context.Background(), nil) optsPeer2 := WGIFaceOpts{ IFaceName: peer2ifaceName, diff --git a/client/iface/udpmux/mux.go b/client/iface/udpmux/mux.go index c5d2de4a5..68cecc953 100644 --- a/client/iface/udpmux/mux.go +++ b/client/iface/udpmux/mux.go @@ -200,10 +200,7 @@ func (m *SingleSocketUDPMux) updateLocalAddresses() { } if len(networks) > 0 { if m.params.Net == nil { - var err error - if m.params.Net, err = stdnet.NewNet(context.Background(), nil); err != nil { - m.params.Logger.Errorf("failed to get create network: %v", err) - } + m.params.Net = stdnet.NewNet(context.Background(), nil) } ips, err := localInterfaces(m.params.Net, m.params.InterfaceFilter, nil, networks, true) diff --git a/client/internal/dns/server_privileged_test.go b/client/internal/dns/server_privileged_test.go index a17044cf5..69d74fec3 100644 --- a/client/internal/dns/server_privileged_test.go +++ b/client/internal/dns/server_privileged_test.go @@ -243,10 +243,7 @@ func TestUpdateDNSServer(t *testing.T) { for n, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { privKey, _ := wgtypes.GenerateKey() - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := iface.WGIFaceOpts{ IFaceName: fmt.Sprintf("utun230%d", n), @@ -348,11 +345,7 @@ func TestDNSFakeResolverHandleUpdates(t *testing.T) { defer t.Setenv("NB_WG_KERNEL_DISABLED", ov) t.Setenv("NB_WG_KERNEL_DISABLED", "true") - newNet, err := stdnet.NewNet(context.Background(), []string{"utun2301"}) - if err != nil { - t.Errorf("create stdnet: %v", err) - return - } + newNet := stdnet.NewNet(context.Background(), []string{"utun2301"}) privKey, _ := wgtypes.GeneratePrivateKey() opts := iface.WGIFaceOpts{ diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go index 96e55a354..d52dcc287 100644 --- a/client/internal/dns/server_test.go +++ b/client/internal/dns/server_test.go @@ -394,11 +394,7 @@ func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) { defer t.Setenv("NB_WG_KERNEL_DISABLED", ov) t.Setenv("NB_WG_KERNEL_DISABLED", "true") - newNet, err := stdnet.NewNet(context.Background(), []string{"utun2301"}) - if err != nil { - t.Fatalf("create stdnet: %v", err) - return nil, err - } + newNet := stdnet.NewNet(context.Background(), []string{"utun2301"}) privKey, _ := wgtypes.GeneratePrivateKey() diff --git a/client/internal/engine.go b/client/internal/engine.go index fd2ac1d80..452d481bd 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -2110,10 +2110,7 @@ func (e *Engine) close() { } func (e *Engine) newWgIface() (*iface.WGIface, error) { - transportNet, err := e.newStdNet() - if err != nil { - log.Errorf("failed to create pion's stdnet: %s", err) - } + transportNet := e.newStdNet() opts := iface.WGIFaceOpts{ IFaceName: e.config.WgIfaceName, diff --git a/client/internal/engine_stdnet.go b/client/internal/engine_stdnet.go index 1ebb5779c..86f6d297a 100644 --- a/client/internal/engine_stdnet.go +++ b/client/internal/engine_stdnet.go @@ -6,6 +6,6 @@ import ( "github.com/netbirdio/netbird/client/internal/stdnet" ) -func (e *Engine) newStdNet() (*stdnet.Net, error) { +func (e *Engine) newStdNet() *stdnet.Net { return stdnet.NewNet(e.clientCtx, e.config.IFaceBlackList) } diff --git a/client/internal/engine_stdnet_android.go b/client/internal/engine_stdnet_android.go index de3c80bcf..b14deeadf 100644 --- a/client/internal/engine_stdnet_android.go +++ b/client/internal/engine_stdnet_android.go @@ -2,6 +2,6 @@ package internal import "github.com/netbirdio/netbird/client/internal/stdnet" -func (e *Engine) newStdNet() (*stdnet.Net, error) { +func (e *Engine) newStdNet() *stdnet.Net { return stdnet.NewNetWithDiscover(e.clientCtx, e.mobileDep.IFaceDiscover, e.config.IFaceBlackList) } diff --git a/client/internal/engine_test.go b/client/internal/engine_test.go index 4e9faa437..56efc786b 100644 --- a/client/internal/engine_test.go +++ b/client/internal/engine_test.go @@ -582,10 +582,7 @@ func TestEngine_UpdateNetworkMapWithRoutes(t *testing.T) { StatusRecorder: peer.NewRecorder("https://mgm"), }, MobileDependency{}) engine.ctx = ctx - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := iface.WGIFaceOpts{ IFaceName: wgIfaceName, @@ -790,10 +787,7 @@ func TestEngine_UpdateNetworkMapWithDNSUpdate(t *testing.T) { }, MobileDependency{}) engine.ctx = ctx - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := iface.WGIFaceOpts{ IFaceName: wgIfaceName, Address: wgaddr.MustParseWGAddress(wgAddr), diff --git a/client/internal/peer/ice/agent.go b/client/internal/peer/ice/agent.go index c74b46d10..6cd8c48de 100644 --- a/client/internal/peer/ice/agent.go +++ b/client/internal/peer/ice/agent.go @@ -39,10 +39,7 @@ func NewAgent(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, c iceFailedTimeout := iceFailedTimeout() iceRelayAcceptanceMinWait := iceRelayAcceptanceMinWait() - transportNet, err := newStdNet(ctx, iFaceDiscover, config.InterfaceBlackList) - if err != nil { - log.Errorf("failed to create pion's stdnet: %s", err) - } + transportNet := newStdNet(ctx, iFaceDiscover, config.InterfaceBlackList) fac := logging.NewDefaultLoggerFactory() diff --git a/client/internal/peer/ice/stdnet.go b/client/internal/peer/ice/stdnet.go index 685ed0363..0c819ff66 100644 --- a/client/internal/peer/ice/stdnet.go +++ b/client/internal/peer/ice/stdnet.go @@ -8,6 +8,6 @@ import ( "github.com/netbirdio/netbird/client/internal/stdnet" ) -func newStdNet(ctx context.Context, _ stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) (*stdnet.Net, error) { +func newStdNet(ctx context.Context, _ stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) *stdnet.Net { return stdnet.NewNet(ctx, ifaceBlacklist) } diff --git a/client/internal/peer/ice/stdnet_android.go b/client/internal/peer/ice/stdnet_android.go index 5033ec1b9..2962ecf66 100644 --- a/client/internal/peer/ice/stdnet_android.go +++ b/client/internal/peer/ice/stdnet_android.go @@ -6,6 +6,6 @@ import ( "github.com/netbirdio/netbird/client/internal/stdnet" ) -func newStdNet(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) (*stdnet.Net, error) { +func newStdNet(ctx context.Context, iFaceDiscover stdnet.ExternalIFaceDiscover, ifaceBlacklist []string) *stdnet.Net { return stdnet.NewNetWithDiscover(ctx, iFaceDiscover, ifaceBlacklist) } diff --git a/client/internal/relay/relay.go b/client/internal/relay/relay.go index 051717608..f0c65301e 100644 --- a/client/internal/relay/relay.go +++ b/client/internal/relay/relay.go @@ -201,11 +201,7 @@ func (p *StunTurnProbe) probeSTUN(ctx context.Context, uri *stun.URI) (addr stri } }() - net, err := stdnet.NewNet(ctx, nil) - if err != nil { - probeErr = fmt.Errorf("new net: %w", err) - return - } + net := stdnet.NewNet(ctx, nil) client, err := stun.DialURI(uri, &stun.DialConfig{ Net: net, @@ -290,11 +286,7 @@ func (p *StunTurnProbe) probeTURN(ctx context.Context, uri *stun.URI) (addr stri } }() - net, err := stdnet.NewNet(ctx, nil) - if err != nil { - probeErr = fmt.Errorf("new net: %w", err) - return - } + net := stdnet.NewNet(ctx, nil) cfg := &turn.ClientConfig{ STUNServerAddr: turnServerAddr, TURNServerAddr: turnServerAddr, diff --git a/client/internal/routemanager/manager_test.go b/client/internal/routemanager/manager_test.go index 18b44820a..d97f532ba 100644 --- a/client/internal/routemanager/manager_test.go +++ b/client/internal/routemanager/manager_test.go @@ -406,10 +406,7 @@ func TestManagerUpdateRoutes(t *testing.T) { for n, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { peerPrivateKey, _ := wgtypes.GeneratePrivateKey() - newNet, err := stdnet.NewNet(context.Background(), nil) - if err != nil { - t.Fatal(err) - } + newNet := stdnet.NewNet(context.Background(), nil) opts := iface.WGIFaceOpts{ IFaceName: fmt.Sprintf("utun43%d", n), Address: wgaddr.MustParseWGAddress("100.65.65.2/24"), diff --git a/client/internal/routemanager/systemops/systemops_generic_test.go b/client/internal/routemanager/systemops/systemops_generic_test.go index c4f739c30..95b17f5c0 100644 --- a/client/internal/routemanager/systemops/systemops_generic_test.go +++ b/client/internal/routemanager/systemops/systemops_generic_test.go @@ -436,8 +436,7 @@ func createWGInterface(t *testing.T, interfaceName, ipAddressCIDR string, listen peerPrivateKey, err := wgtypes.GeneratePrivateKey() require.NoError(t, err) - newNet, err := stdnet.NewNet(context.Background(), nil) - require.NoError(t, err) + newNet := stdnet.NewNet(context.Background(), nil) opts := iface.WGIFaceOpts{ IFaceName: interfaceName, diff --git a/client/internal/stdnet/stdnet.go b/client/internal/stdnet/stdnet.go index 381886ac6..c3a9d3d97 100644 --- a/client/internal/stdnet/stdnet.go +++ b/client/internal/stdnet/stdnet.go @@ -45,7 +45,7 @@ type Net struct { } // NewNetWithDiscover creates a new StdNet instance. -func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover, disallowList []string) (*Net, error) { +func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover, disallowList []string) *Net { if ctx == nil { ctx = context.Background() } @@ -60,20 +60,19 @@ func NewNetWithDiscover(ctx context.Context, iFaceDiscover ExternalIFaceDiscover } else { n.iFaceDiscover = newMobileIFaceDiscover(iFaceDiscover) } - return n, n.UpdateInterfaces() + return n } // NewNet creates a new StdNet instance. -func NewNet(ctx context.Context, disallowList []string) (*Net, error) { +func NewNet(ctx context.Context, disallowList []string) *Net { if ctx == nil { ctx = context.Background() } - n := &Net{ + return &Net{ iFaceDiscover: pionDiscover{}, interfaceFilter: InterfaceFilter(disallowList), ctx: ctx, } - return n, n.UpdateInterfaces() } // resolveAddr performs DNS resolution with context support and timeout. @@ -122,45 +121,18 @@ func (n *Net) resolveAddr(network, address string) (netip.AddrPort, error) { return netip.AddrPortFrom(addrs[0], uint16(port)), nil } -// UpdateInterfaces updates the internal list of network interfaces -// and associated addresses filtering them by name. -// The interfaces are discovered by an external iFaceDiscover function or by a default discoverer if the external one -// wasn't specified. -func (n *Net) UpdateInterfaces() (err error) { - n.mu.Lock() - defer n.mu.Unlock() - - return n.updateInterfaces() -} - -func (n *Net) updateInterfaces() (err error) { - allIfaces, err := n.iFaceDiscover.iFaces() - if err != nil { - return err - } - - n.interfaces = n.filterInterfaces(allIfaces) - - n.lastUpdate = time.Now() - - return nil -} - // Interfaces returns a slice of interfaces which are available on the // system func (n *Net) Interfaces() ([]*transport.Interface, error) { n.mu.Lock() defer n.mu.Unlock() - if time.Since(n.lastUpdate) < updateInterval { - return slices.Clone(n.interfaces), nil + iFaces, err := n.freshInterfacesLocked() + if err != nil { + return nil, err } - if err := n.updateInterfaces(); err != nil { - return nil, fmt.Errorf("update interfaces: %w", err) - } - - return slices.Clone(n.interfaces), nil + return slices.Clone(iFaces), nil } // InterfaceByIndex returns the interface specified by index. @@ -171,7 +143,13 @@ func (n *Net) Interfaces() ([]*transport.Interface, error) { func (n *Net) InterfaceByIndex(index int) (*transport.Interface, error) { n.mu.Lock() defer n.mu.Unlock() - for _, ifc := range n.interfaces { + + iFaces, err := n.freshInterfacesLocked() + if err != nil { + return nil, err + } + + for _, ifc := range iFaces { if ifc.Index == index { return ifc, nil } @@ -184,7 +162,13 @@ func (n *Net) InterfaceByIndex(index int) (*transport.Interface, error) { func (n *Net) InterfaceByName(name string) (*transport.Interface, error) { n.mu.Lock() defer n.mu.Unlock() - for _, ifc := range n.interfaces { + + iFaces, err := n.freshInterfacesLocked() + if err != nil { + return nil, err + } + + for _, ifc := range iFaces { if ifc.Name == name { return ifc, nil } @@ -193,6 +177,31 @@ func (n *Net) InterfaceByName(name string) (*transport.Interface, error) { return nil, fmt.Errorf("%w: %s", transport.ErrInterfaceNotFound, name) } +func (n *Net) freshInterfacesLocked() ([]*transport.Interface, error) { + if time.Since(n.lastUpdate) < updateInterval { + return n.interfaces, nil + } + + if err := n.updateInterfacesLocked(); err != nil { + return nil, fmt.Errorf("update interfaces: %w", err) + } + + return n.interfaces, nil +} + +func (n *Net) updateInterfacesLocked() error { + allIFaces, err := n.iFaceDiscover.iFaces() + if err != nil { + return err + } + + n.interfaces = n.filterInterfaces(allIFaces) + + n.lastUpdate = time.Now() + + return nil +} + func (n *Net) filterInterfaces(interfaces []*transport.Interface) []*transport.Interface { if n.interfaceFilter == nil { return interfaces diff --git a/client/internal/stdnet/stdnet_test.go b/client/internal/stdnet/stdnet_test.go new file mode 100644 index 000000000..991f973fb --- /dev/null +++ b/client/internal/stdnet/stdnet_test.go @@ -0,0 +1,124 @@ +package stdnet + +import ( + "context" + "errors" + "net" + "testing" + + "github.com/pion/transport/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type countingDiscover struct { + calls int + list []*transport.Interface + err error +} + +func (d *countingDiscover) iFaces() ([]*transport.Interface, error) { + d.calls++ + if d.err != nil { + return nil, d.err + } + return d.list, nil +} + +func newTestNet(t *testing.T, d iFaceDiscover) *Net { + t.Helper() + return &Net{ + iFaceDiscover: d, + ctx: context.Background(), + } +} + +func testIFace(index int, name string) *transport.Interface { + return transport.NewInterface(net.Interface{Index: index, Name: name}) +} + +func TestNet_InterfacesDiscoversLazilyAndCaches(t *testing.T) { + d := &countingDiscover{list: []*transport.Interface{testIFace(1, "eth0")}} + n := newTestNet(t, d) + + require.Zero(t, d.calls, "construction must not discover interfaces") + + iFaces, err := n.Interfaces() + require.NoError(t, err) + require.Len(t, iFaces, 1) + assert.Equal(t, 1, d.calls) + + _, err = n.Interfaces() + require.NoError(t, err) + assert.Equal(t, 1, d.calls) +} + +func TestNet_InterfacesRetryAfterDiscoveryFailure(t *testing.T) { + discoverErr := errors.New("discover failed") + d := &countingDiscover{err: discoverErr} + n := newTestNet(t, d) + + _, err := n.Interfaces() + require.ErrorIs(t, err, discoverErr) + + d.err = nil + d.list = []*transport.Interface{testIFace(1, "eth0")} + + iFaces, err := n.Interfaces() + require.NoError(t, err) + require.Len(t, iFaces, 1) + assert.Equal(t, 2, d.calls) +} + +func TestNet_InterfaceByNameRefreshes(t *testing.T) { + d := &countingDiscover{list: []*transport.Interface{testIFace(3, "eth0")}} + n := newTestNet(t, d) + + ifc, err := n.InterfaceByName("eth0") + require.NoError(t, err) + assert.Equal(t, "eth0", ifc.Name) + assert.Equal(t, 1, d.calls) + + _, err = n.InterfaceByName("nope") + require.ErrorIs(t, err, transport.ErrInterfaceNotFound) +} + +func TestNet_InterfaceByIndexRefreshes(t *testing.T) { + d := &countingDiscover{list: []*transport.Interface{testIFace(3, "eth0")}} + n := newTestNet(t, d) + + ifc, err := n.InterfaceByIndex(3) + require.NoError(t, err) + assert.Equal(t, "eth0", ifc.Name) + assert.Equal(t, 1, d.calls) + + _, err = n.InterfaceByIndex(99) + require.ErrorIs(t, err, transport.ErrInterfaceNotFound) +} + +func TestNet_InterfaceLookupPropagatesDiscoveryError(t *testing.T) { + discoverErr := errors.New("discover failed") + n := newTestNet(t, &countingDiscover{err: discoverErr}) + + _, err := n.InterfaceByName("eth0") + require.ErrorIs(t, err, discoverErr) + + _, err = n.InterfaceByIndex(1) + require.ErrorIs(t, err, discoverErr) +} + +func TestNet_InterfacesReturnsCopy(t *testing.T) { + d := &countingDiscover{list: []*transport.Interface{testIFace(1, "eth0")}} + n := newTestNet(t, d) + + iFaces, err := n.Interfaces() + require.NoError(t, err) + require.Len(t, iFaces, 1) + + iFaces[0] = testIFace(2, "tampered") + + iFaces, err = n.Interfaces() + require.NoError(t, err) + require.Len(t, iFaces, 1) + assert.Equal(t, "eth0", iFaces[0].Name) +}