Skip to content

Commit

Permalink
Feature/permanent dns (netbirdio#967)
Browse files Browse the repository at this point in the history
* Add DNS list argument for mobile client

* Write testable code

Many places are checked the wgInterface != nil condition.
It is doing it just because to avoid the real wgInterface creation for tests.
Instead of this involve a wgInterface interface what is moc-able.

* Refactor the DNS server internal code structure

With the fake resolver has been involved several
if-else statement and generated some unused
variables to distinguish the listener and fake
resolver solutions at running time. With this
commit the fake resolver and listener based
solution has been moved into two separated
structure. Name of this layer is the 'service'.
With this modification the unit test looks
simpler and open the option to add new logic for
the permanent DNS service usage for mobile
systems.



* Remove is running check in test

We can not ensure the state well so remove this
check. The test will fail if the server is not
running well.
  • Loading branch information
pappz authored Jul 14, 2023
1 parent 9c2c0e7 commit 7ebe58f
Show file tree
Hide file tree
Showing 30 changed files with 918 additions and 359 deletions.
23 changes: 21 additions & 2 deletions client/android/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/client/internal"
"github.com/netbirdio/netbird/client/internal/dns"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/internal/routemanager"
"github.com/netbirdio/netbird/client/internal/stdnet"
Expand Down Expand Up @@ -35,6 +36,11 @@ type RouteListener interface {
routemanager.RouteListener
}

// DnsReadyListener export internal dns ReadyListener for mobile
type DnsReadyListener interface {
dns.ReadyListener
}

func init() {
formatter.SetLogcatFormatter(log.StandardLogger())
}
Expand All @@ -49,6 +55,7 @@ type Client struct {
ctxCancelLock *sync.Mutex
deviceName string
routeListener routemanager.RouteListener
onHostDnsFn func([]string)
}

// NewClient instantiate a new Client
Expand All @@ -65,7 +72,7 @@ func NewClient(cfgFile, deviceName string, tunAdapter TunAdapter, iFaceDiscover
}

// Run start the internal client. It is a blocker function
func (c *Client) Run(urlOpener URLOpener) error {
func (c *Client) Run(urlOpener URLOpener, dns *DNSList, dnsReadyListener DnsReadyListener) error {
cfg, err := internal.UpdateOrCreateConfig(internal.ConfigInput{
ConfigPath: c.cfgFile,
})
Expand All @@ -90,7 +97,8 @@ func (c *Client) Run(urlOpener URLOpener) error {

// todo do not throw error in case of cancelled context
ctx = internal.CtxInitState(ctx)
return internal.RunClient(ctx, cfg, c.recorder, c.tunAdapter, c.iFaceDiscover, c.routeListener)
c.onHostDnsFn = func([]string) {}
return internal.RunClientMobile(ctx, cfg, c.recorder, c.tunAdapter, c.iFaceDiscover, c.routeListener, dns.items, dnsReadyListener)
}

// Stop the internal client and free the resources
Expand Down Expand Up @@ -126,6 +134,17 @@ func (c *Client) PeersList() *PeerInfoArray {
return &PeerInfoArray{items: peerInfos}
}

// OnUpdatedHostDNS update the DNS servers addresses for root zones
func (c *Client) OnUpdatedHostDNS(list *DNSList) error {
dnsServer, err := dns.GetServerDns()
if err != nil {
return err
}

dnsServer.OnUpdatedHostDNSServer(list.items)
return nil
}

// SetConnectionListener set the network connection listener
func (c *Client) SetConnectionListener(listener ConnectionListener) {
c.recorder.SetConnectionListener(listener)
Expand Down
26 changes: 26 additions & 0 deletions client/android/dns_list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package android

import "fmt"

// DNSList is a wrapper of []string
type DNSList struct {
items []string
}

// Add new DNS address to the collection
func (array *DNSList) Add(s string) {
array.items = append(array.items, s)
}

// Get return an element of the collection
func (array *DNSList) Get(i int) (string, error) {
if i >= len(array.items) || i < 0 {
return "", fmt.Errorf("out of range")
}
return array.items[i], nil
}

// Size return with the size of the collection
func (array *DNSList) Size() int {
return len(array.items)
}
24 changes: 24 additions & 0 deletions client/android/dns_list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package android

import "testing"

func TestDNSList_Get(t *testing.T) {
l := DNSList{
items: make([]string, 1),
}

_, err := l.Get(0)
if err != nil {
t.Errorf("invalid error: %s", err)
}

_, err = l.Get(-1)
if err == nil {
t.Errorf("expected error but got nil")
}

_, err = l.Get(1)
if err == nil {
t.Errorf("expected error but got nil")
}
}
2 changes: 1 addition & 1 deletion client/cmd/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func runInForegroundMode(ctx context.Context, cmd *cobra.Command) error {
var cancel context.CancelFunc
ctx, cancel = context.WithCancel(ctx)
SetupCloseHandler(ctx, cancel)
return internal.RunClient(ctx, config, peer.NewRecorder(config.ManagementURL.String()), nil, nil, nil)
return internal.RunClient(ctx, config, peer.NewRecorder(config.ManagementURL.String()))
}

func runInDaemonMode(ctx context.Context, cmd *cobra.Command) error {
Expand Down
29 changes: 20 additions & 9 deletions client/internal/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"google.golang.org/grpc/codes"
gstatus "google.golang.org/grpc/status"

"github.com/netbirdio/netbird/client/internal/dns"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/internal/routemanager"
"github.com/netbirdio/netbird/client/internal/stdnet"
Expand All @@ -24,7 +25,24 @@ import (
)

// RunClient with main logic.
func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover, routeListener routemanager.RouteListener) error {
func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status) error {
return runClient(ctx, config, statusRecorder, MobileDependency{})
}

// RunClientMobile with main logic on mobile system
func RunClientMobile(ctx context.Context, config *Config, statusRecorder *peer.Status, tunAdapter iface.TunAdapter, iFaceDiscover stdnet.ExternalIFaceDiscover, routeListener routemanager.RouteListener, dnsAddresses []string, dnsReadyListener dns.ReadyListener) error {
// in case of non Android os these variables will be nil
mobileDependency := MobileDependency{
TunAdapter: tunAdapter,
IFaceDiscover: iFaceDiscover,
RouteListener: routeListener,
HostDNSAddresses: dnsAddresses,
DnsReadyListener: dnsReadyListener,
}
return runClient(ctx, config, statusRecorder, mobileDependency)
}

func runClient(ctx context.Context, config *Config, statusRecorder *peer.Status, mobileDependency MobileDependency) error {
backOff := &backoff.ExponentialBackOff{
InitialInterval: time.Second,
RandomizationFactor: 1,
Expand Down Expand Up @@ -151,14 +169,7 @@ func RunClient(ctx context.Context, config *Config, statusRecorder *peer.Status,
return wrapErr(err)
}

// in case of non Android os these variables will be nil
md := MobileDependency{
TunAdapter: tunAdapter,
IFaceDiscover: iFaceDiscover,
RouteListener: routeListener,
}

engine := NewEngine(engineCtx, cancel, signalClient, mgmClient, engineConfig, md, statusRecorder)
engine := NewEngine(engineCtx, cancel, signalClient, mgmClient, engineConfig, mobileDependency, statusRecorder)
err = engine.Start()
if err != nil {
log.Errorf("error while starting Netbird Connection Engine: %s", err)
Expand Down
6 changes: 1 addition & 5 deletions client/internal/dns/host_android.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
package dns

import (
"github.com/netbirdio/netbird/iface"
)

type androidHostManager struct {
}

func newHostManager(wgInterface *iface.WGIface) (hostManager, error) {
func newHostManager(wgInterface WGIface) (hostManager, error) {
return &androidHostManager{}, nil
}

Expand Down
4 changes: 1 addition & 3 deletions client/internal/dns/host_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"strings"

log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/iface"
)

const (
Expand All @@ -34,7 +32,7 @@ type systemConfigurator struct {
createdKeys map[string]struct{}
}

func newHostManager(_ *iface.WGIface) (hostManager, error) {
func newHostManager(_ WGIface) (hostManager, error) {
return &systemConfigurator{
createdKeys: make(map[string]struct{}),
}, nil
Expand Down
6 changes: 3 additions & 3 deletions client/internal/dns/host_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ package dns
import (
"bufio"
"fmt"
"github.com/netbirdio/netbird/iface"
log "github.com/sirupsen/logrus"
"os"
"strings"

log "github.com/sirupsen/logrus"
)

const (
Expand All @@ -25,7 +25,7 @@ const (

type osManagerType int

func newHostManager(wgInterface *iface.WGIface) (hostManager, error) {
func newHostManager(wgInterface WGIface) (hostManager, error) {
osManager, err := getOSDNSManagerType()
if err != nil {
return nil, err
Expand Down
4 changes: 1 addition & 3 deletions client/internal/dns/host_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (

log "github.com/sirupsen/logrus"
"golang.org/x/sys/windows/registry"

"github.com/netbirdio/netbird/iface"
)

const (
Expand All @@ -33,7 +31,7 @@ type registryConfigurator struct {
existingSearchDomains []string
}

func newHostManager(wgInterface *iface.WGIface) (hostManager, error) {
func newHostManager(wgInterface WGIface) (hostManager, error) {
guid, err := wgInterface.GetInterfaceGUIDString()
if err != nil {
return nil, err
Expand Down
5 changes: 5 additions & 0 deletions client/internal/dns/mockServer.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ func (m *MockServer) DnsIP() string {
return ""
}

func (m *MockServer) OnUpdatedHostDNSServer(strings []string) {
//TODO implement me
panic("implement me")
}

// UpdateDNSServer mock implementation of UpdateDNSServer from Server interface
func (m *MockServer) UpdateDNSServer(serial uint64, update nbdns.Config) error {
if m.UpdateDNSServerFunc != nil {
Expand Down
4 changes: 1 addition & 3 deletions client/internal/dns/network_manager_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"github.com/hashicorp/go-version"
"github.com/miekg/dns"
log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/iface"
)

const (
Expand Down Expand Up @@ -72,7 +70,7 @@ func (s networkManagerConnSettings) cleanDeprecatedSettings() {
}
}

func newNetworkManagerDbusConfigurator(wgInterface *iface.WGIface) (hostManager, error) {
func newNetworkManagerDbusConfigurator(wgInterface WGIface) (hostManager, error) {
obj, closeConn, err := getDbusObject(networkManagerDest, networkManagerDbusObjectNode)
if err != nil {
return nil, err
Expand Down
4 changes: 1 addition & 3 deletions client/internal/dns/resolvconf_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"strings"

log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/iface"
)

const resolvconfCommand = "resolvconf"
Expand All @@ -18,7 +16,7 @@ type resolvconf struct {
ifaceName string
}

func newResolvConfConfigurator(wgInterface *iface.WGIface) (hostManager, error) {
func newResolvConfConfigurator(wgInterface WGIface) (hostManager, error) {
return &resolvconf{
ifaceName: wgInterface.Name(),
}, nil
Expand Down
Loading

0 comments on commit 7ebe58f

Please sign in to comment.