Fix: Skip state notification should use a copy of the previous peer state (#960)

This was affecting the behavior of the route manager,
causing issues with HA and with cases of flaky connections

client/internal/dns/mockServer.go

@@ -7,16 +7,17 @@ import (
 
 // MockServer is the mock instance of a dns server
 type MockServer struct {
-	StartFunc           func()
+	InitializeFunc      func() error
 	StopFunc            func()
 	UpdateDNSServerFunc func(serial uint64, update nbdns.Config) error
 }
 
-// Start mock implementation of Start from Server interface
-func (m *MockServer) Start() {
-	if m.StartFunc != nil {
-		m.StartFunc()
+// Initialize mock implementation of Initialize from Server interface
+func (m *MockServer) Initialize() error {
+	if m.InitializeFunc != nil {
+		return m.InitializeFunc()
 	}
+	return nil
 }
 
 // Stop mock implementation of Stop from Server interface

client/internal/dns/server.go

@@ -29,7 +29,7 @@ const (
 
 // Server is a dns server interface
 type Server interface {
-	Start()
+	Initialize() error
 	Stop()
 	DnsIP() string
 	UpdateDNSServer(serial uint64, update nbdns.Config) error
@@ -82,11 +82,6 @@ func NewDefaultServer(ctx context.Context, wgInterface *iface.WGIface, customAdd
 		addrPort = &parsedAddrPort
 	}
 
-	hostManager, err := newHostManager(wgInterface)
-	if err != nil {
-		return nil, err
-	}
-
 	ctx, stop := context.WithCancel(ctx)
 
 	defaultServer := &DefaultServer{
@@ -104,7 +99,6 @@ func NewDefaultServer(ctx context.Context, wgInterface *iface.WGIface, customAdd
 		},
 		wgInterface:   wgInterface,
 		customAddress: addrPort,
-		hostManager:   hostManager,
 	}
 
 	if initialDnsCfg != nil {
@@ -115,8 +109,21 @@ func NewDefaultServer(ctx context.Context, wgInterface *iface.WGIface, customAdd
 	return defaultServer, nil
 }
 
-// Start runs the listener in a go routine
-func (s *DefaultServer) Start() {
+// Initialize instantiate host manager. It required to be initialized wginterface
+func (s *DefaultServer) Initialize() (err error) {
+	s.mux.Lock()
+	defer s.mux.Unlock()
+
+	if s.hostManager != nil {
+		return nil
+	}
+
+	s.hostManager, err = newHostManager(s.wgInterface)
+	return
+}
+
+// listen runs the listener in a go routine
+func (s *DefaultServer) listen() {
 	// nil check required in unit tests
 	if s.wgInterface != nil && s.wgInterface.IsUserspaceBind() {
 		s.fakeResolverWG.Add(1)
@@ -231,6 +238,10 @@ func (s *DefaultServer) UpdateDNSServer(serial uint64, update nbdns.Config) erro
 		s.mux.Lock()
 		defer s.mux.Unlock()
 
+		if s.hostManager == nil {
+			return fmt.Errorf("dns service is not initialized yet")
+		}
+
 		hash, err := hashstructure.Hash(update, hashstructure.FormatV2, &hashstructure.HashOptions{
 			ZeroNil:         true,
 			IgnoreZeroValue: true,
@@ -270,7 +281,7 @@ func (s *DefaultServer) applyConfiguration(update nbdns.Config) error {
 			}
 		}
 	} else if !s.listenerIsRunning {
-		s.Start()
+		s.listen()
 	}
 
 	localMuxUpdates, localRecords, err := s.buildLocalHandlerUpdate(update.CustomZones)

client/internal/dns/server_test.go

@@ -225,6 +225,10 @@ func TestUpdateDNSServer(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
+			err = dnsServer.Initialize()
+			if err != nil {
+				t.Fatal(err)
+			}
 			defer func() {
 				err = dnsServer.hostManager.restoreHostDNS()
 				if err != nil {
@@ -291,7 +295,7 @@ func TestDNSServerStartStop(t *testing.T) {
 			dnsServer := getDefaultServerWithNoHostManager(t, testCase.addrPort)
 
 			dnsServer.hostManager = newNoopHostMocker()
-			dnsServer.Start()
+			dnsServer.listen()
 			time.Sleep(100 * time.Millisecond)
 			if !dnsServer.listenerIsRunning {
 				t.Fatal("dns server listener is not running")

client/internal/engine.go

@@ -259,6 +259,12 @@ func (e *Engine) Start() error {
 		e.acl = acl
 	}
 
+	err = e.dnsServer.Initialize()
+	if err != nil {
+		e.close()
+		return err
+	}
+
 	e.receiveSignalEvents()
 	e.receiveManagementEvents()
 

client/internal/peer/status.go

@@ -135,6 +135,8 @@ func (d *Status) UpdatePeerState(receivedState State) error {
 		peerState.IP = receivedState.IP
 	}
 
+	skipNotification := shouldSkipNotify(receivedState, peerState)
+
 	if receivedState.ConnStatus != peerState.ConnStatus {
 		peerState.ConnStatus = receivedState.ConnStatus
 		peerState.ConnStatusUpdate = receivedState.ConnStatusUpdate
@@ -146,7 +148,7 @@ func (d *Status) UpdatePeerState(receivedState State) error {
 
 	d.peers[receivedState.PubKey] = peerState
 
-	if shouldSkipNotify(receivedState, peerState) {
+	if skipNotification {
 		return nil
 	}
 

management/server/http/api/openapi.yml

@@ -1102,8 +1102,8 @@ paths:
         '500':
           "$ref": "#/components/responses/internal_error"
     delete:
-      summary: Delete a User
-      description: Delete a user
+      summary: Block a User
+      description: This method blocks a user from accessing the system, but leaves the IDP user intact.
       tags: [ Users ]
       security:
         - BearerAuth: [ ]