Cleanup route handling (#542)

* mikrotik: allow to set DNS, wip: handle routes in wg-controller

* replace old route handling for local controller

* cleanup route handling for local backend

* implement route handling for mikrotik controller

internal/app/wireguard/controller_manager.go

@@ -1,7 +1,6 @@
 package wireguard
 
 import (
-	"context"
 	"fmt"
 	"log/slog"
 	"maps"
@@ -12,33 +11,9 @@ import (
 	"github.com/h44z/wg-portal/internal/domain"
 )
 
-type InterfaceController interface {
-	GetId() domain.InterfaceBackend
-	GetInterfaces(_ context.Context) ([]domain.PhysicalInterface, error)
-	GetInterface(_ context.Context, id domain.InterfaceIdentifier) (*domain.PhysicalInterface, error)
-	GetPeers(_ context.Context, deviceId domain.InterfaceIdentifier) ([]domain.PhysicalPeer, error)
-	SaveInterface(
-		_ context.Context,
-		id domain.InterfaceIdentifier,
-		updateFunc func(pi *domain.PhysicalInterface) (*domain.PhysicalInterface, error),
-	) error
-	DeleteInterface(_ context.Context, id domain.InterfaceIdentifier) error
-	SavePeer(
-		_ context.Context,
-		deviceId domain.InterfaceIdentifier,
-		id domain.PeerIdentifier,
-		updateFunc func(pp *domain.PhysicalPeer) (*domain.PhysicalPeer, error),
-	) error
-	DeletePeer(_ context.Context, deviceId domain.InterfaceIdentifier, id domain.PeerIdentifier) error
-	PingAddresses(
-		ctx context.Context,
-		addr string,
-	) (*domain.PingerResult, error)
-}
-
 type backendInstance struct {
 	Config         config.BackendBase // Config is the configuration for the backend instance.
-	Implementation InterfaceController
+	Implementation domain.InterfaceController
 }
 
 type ControllerManager struct {
@@ -118,11 +93,11 @@ func (c *ControllerManager) logRegisteredControllers() {
 	}
 }
 
-func (c *ControllerManager) GetControllerByName(backend domain.InterfaceBackend) InterfaceController {
+func (c *ControllerManager) GetControllerByName(backend domain.InterfaceBackend) domain.InterfaceController {
 	return c.getController(backend, "").Implementation
 }
 
-func (c *ControllerManager) GetController(iface domain.Interface) InterfaceController {
+func (c *ControllerManager) GetController(iface domain.Interface) domain.InterfaceController {
 	return c.getController(iface.Backend, iface.Identifier).Implementation
 }
 

internal/app/wireguard/wireguard.go

@@ -38,9 +38,9 @@ type InterfaceAndPeerDatabaseRepo interface {
 }
 
 type WgQuickController interface {
-	ExecuteInterfaceHook(id domain.InterfaceIdentifier, hookCmd string) error
-	SetDNS(id domain.InterfaceIdentifier, dnsStr, dnsSearchStr string) error
-	UnsetDNS(id domain.InterfaceIdentifier) error
+	ExecuteInterfaceHook(ctx context.Context, id domain.InterfaceIdentifier, hookCmd string) error
+	SetDNS(ctx context.Context, id domain.InterfaceIdentifier, dnsStr, dnsSearchStr string) error
+	UnsetDNS(ctx context.Context, id domain.InterfaceIdentifier, dnsStr, dnsSearchStr string) error
 }
 
 type EventBus interface {
@@ -53,11 +53,10 @@ type EventBus interface {
 // endregion dependencies
 
 type Manager struct {
-	cfg   *config.Config
-	bus   EventBus
-	db    InterfaceAndPeerDatabaseRepo
-	wg    *ControllerManager
-	quick WgQuickController
+	cfg *config.Config
+	bus EventBus
+	db  InterfaceAndPeerDatabaseRepo
+	wg  *ControllerManager
 
 	userLockMap *sync.Map
 }
@@ -66,7 +65,6 @@ func NewWireGuardManager(
 	cfg *config.Config,
 	bus EventBus,
 	wg *ControllerManager,
-	quick WgQuickController,
 	db InterfaceAndPeerDatabaseRepo,
 ) (*Manager, error) {
 	m := &Manager{
@@ -74,7 +72,6 @@ func NewWireGuardManager(
 		bus:         bus,
 		wg:          wg,
 		db:          db,
-		quick:       quick,
 		userLockMap: &sync.Map{},
 	}
 

internal/app/wireguard/wireguard_interfaces.go

@@ -453,7 +453,7 @@ func (m Manager) DeleteInterface(ctx context.Context, id domain.InterfaceIdentif
 		return err
 	}
 
-	existingInterface, err := m.db.GetInterface(ctx, id)
+	existingInterface, existingPeers, err := m.db.GetInterfaceAndPeers(ctx, id)
 	if err != nil {
 		return fmt.Errorf("unable to find interface %s: %w", id, err)
 	}
@@ -462,21 +462,29 @@ func (m Manager) DeleteInterface(ctx context.Context, id domain.InterfaceIdentif
 		return fmt.Errorf("deletion not allowed: %w", err)
 	}
 
+	m.bus.Publish(app.TopicRouteRemove, domain.RoutingTableInfo{
+		Interface:  *existingInterface,
+		AllowedIps: existingInterface.GetAllowedIPs(existingPeers),
+		FwMark:     existingInterface.FirewallMark,
+		Table:      existingInterface.GetRoutingTable(),
+		TableStr:   existingInterface.RoutingTable,
+		IsDeleted:  true,
+	})
+
 	now := time.Now()
 	existingInterface.Disabled = &now // simulate a disabled interface
 	existingInterface.DisabledReason = domain.DisabledReasonDeleted
 
-	physicalInterface, _ := m.wg.GetController(*existingInterface).GetInterface(ctx, id)
-
-	if err := m.handleInterfacePreSaveHooks(existingInterface, !existingInterface.IsDisabled(), false); err != nil {
+	if err := m.handleInterfacePreSaveHooks(ctx, existingInterface, !existingInterface.IsDisabled(),
+		false); err != nil {
 		return fmt.Errorf("pre-delete hooks failed: %w", err)
 	}
 
-	if err := m.handleInterfacePreSaveActions(existingInterface); err != nil {
+	if err := m.handleInterfacePreSaveActions(ctx, existingInterface); err != nil {
 		return fmt.Errorf("pre-delete actions failed: %w", err)
 	}
 
-	if err := m.deleteInterfacePeers(ctx, id); err != nil {
+	if err := m.deleteInterfacePeers(ctx, existingInterface, existingPeers); err != nil {
 		return fmt.Errorf("peer deletion failure: %w", err)
 	}
 
@@ -488,16 +496,12 @@ func (m Manager) DeleteInterface(ctx context.Context, id domain.InterfaceIdentif
 		return fmt.Errorf("deletion failure: %w", err)
 	}
 
-	fwMark := existingInterface.FirewallMark
-	if physicalInterface != nil && fwMark == 0 {
-		fwMark = physicalInterface.FirewallMark
-	}
-	m.bus.Publish(app.TopicRouteRemove, domain.RoutingTableInfo{
-		FwMark: fwMark,
-		Table:  existingInterface.GetRoutingTable(),
-	})
-
-	if err := m.handleInterfacePostSaveHooks(existingInterface, !existingInterface.IsDisabled(), false); err != nil {
+	if err := m.handleInterfacePostSaveHooks(
+		ctx,
+		existingInterface,
+		!existingInterface.IsDisabled(),
+		false,
+	); err != nil {
 		return fmt.Errorf("post-delete hooks failed: %w", err)
 	}
 
@@ -516,17 +520,21 @@ func (m Manager) saveInterface(ctx context.Context, iface *domain.Interface) (
 		return nil, fmt.Errorf("interface validation failed: %w", err)
 	}
 
-	oldEnabled, newEnabled := m.getInterfaceStateHistory(ctx, iface)
+	oldEnabled, newEnabled, routeTableChanged := false, !iface.IsDisabled(), false // if the interface did not exist, we assume it was not enabled
+	oldInterface, err := m.db.GetInterface(ctx, iface.Identifier)
+	if err == nil {
+		oldEnabled, newEnabled, routeTableChanged = m.getInterfaceStateHistory(oldInterface, iface)
+	}
 
-	if err := m.handleInterfacePreSaveHooks(iface, oldEnabled, newEnabled); err != nil {
+	if err := m.handleInterfacePreSaveHooks(ctx, iface, oldEnabled, newEnabled); err != nil {
 		return nil, fmt.Errorf("pre-save hooks failed: %w", err)
 	}
 
-	if err := m.handleInterfacePreSaveActions(iface); err != nil {
+	if err := m.handleInterfacePreSaveActions(ctx, iface); err != nil {
 		return nil, fmt.Errorf("pre-save actions failed: %w", err)
 	}
 
-	err := m.db.SaveInterface(ctx, iface.Identifier, func(i *domain.Interface) (*domain.Interface, error) {
+	err = m.db.SaveInterface(ctx, iface.Identifier, func(i *domain.Interface) (*domain.Interface, error) {
 		iface.CopyCalculatedAttributes(i)
 
 		err := m.wg.GetController(*iface).SaveInterface(ctx, iface.Identifier,
@@ -569,20 +577,35 @@ func (m Manager) saveInterface(ctx context.Context, iface *domain.Interface) (
 	}
 
 	if iface.IsDisabled() {
-		physicalInterface, _ := m.wg.GetController(*iface).GetInterface(ctx, iface.Identifier)
-		fwMark := iface.FirewallMark
-		if physicalInterface != nil && fwMark == 0 {
-			fwMark = physicalInterface.FirewallMark
-		}
 		m.bus.Publish(app.TopicRouteRemove, domain.RoutingTableInfo{
-			FwMark: fwMark,
-			Table:  iface.GetRoutingTable(),
+			Interface:  *iface,
+			AllowedIps: iface.GetAllowedIPs(peers),
+			FwMark:     iface.FirewallMark,
+			Table:      iface.GetRoutingTable(),
+			TableStr:   iface.RoutingTable,
 		})
 	} else {
-		m.bus.Publish(app.TopicRouteUpdate, "interface updated: "+string(iface.Identifier))
+		m.bus.Publish(app.TopicRouteUpdate, domain.RoutingTableInfo{
+			Interface:  *iface,
+			AllowedIps: iface.GetAllowedIPs(peers),
+			FwMark:     iface.FirewallMark,
+			Table:      iface.GetRoutingTable(),
+			TableStr:   iface.RoutingTable,
+		})
+		// if the route table changed, ensure that the old entries are remove
+		if routeTableChanged {
+			m.bus.Publish(app.TopicRouteRemove, domain.RoutingTableInfo{
+				Interface:  *oldInterface,
+				AllowedIps: oldInterface.GetAllowedIPs(peers),
+				FwMark:     oldInterface.FirewallMark,
+				Table:      oldInterface.GetRoutingTable(),
+				TableStr:   oldInterface.RoutingTable,
+				IsDeleted:  true, // mark the old entries as deleted
+			})
+		}
 	}
 
-	if err := m.handleInterfacePostSaveHooks(iface, oldEnabled, newEnabled); err != nil {
+	if err := m.handleInterfacePostSaveHooks(ctx, iface, oldEnabled, newEnabled); err != nil {
 		return nil, fmt.Errorf("post-save hooks failed: %w", err)
 	}
 
@@ -618,60 +641,90 @@ func (m Manager) saveInterface(ctx context.Context, iface *domain.Interface) (
 	return iface, nil
 }
 
-func (m Manager) getInterfaceStateHistory(ctx context.Context, iface *domain.Interface) (oldEnabled, newEnabled bool) {
-	oldInterface, err := m.db.GetInterface(ctx, iface.Identifier)
-	if err != nil {
-		return false, !iface.IsDisabled() // if the interface did not exist, we assume it was not enabled
-	}
-
-	return !oldInterface.IsDisabled(), !iface.IsDisabled()
+func (m Manager) getInterfaceStateHistory(
+	oldInterface *domain.Interface,
+	iface *domain.Interface,
+) (oldEnabled, newEnabled, routeTableChanged bool) {
+	return !oldInterface.IsDisabled(), !iface.IsDisabled(), oldInterface.RoutingTable != iface.RoutingTable
 }
 
-func (m Manager) handleInterfacePreSaveActions(iface *domain.Interface) error {
-	if !iface.IsDisabled() {
-		if err := m.quick.SetDNS(iface.Identifier, iface.DnsStr, iface.DnsSearchStr); err != nil {
-			return fmt.Errorf("failed to update dns settings: %w", err)
-		}
-	} else {
-		if err := m.quick.UnsetDNS(iface.Identifier); err != nil {
-			return fmt.Errorf("failed to clear dns settings: %w", err)
+func (m Manager) handleInterfacePreSaveActions(ctx context.Context, iface *domain.Interface) error {
+	wgQuickController, ok := m.wg.GetController(*iface).(WgQuickController)
+	if !ok {
+		slog.Warn("failed to perform pre-save actions", "interface", iface.Identifier,
+			"error", "no capable controller found")
+		return nil
+	}
+
+	// update DNS settings only for client interfaces
+	if iface.Type == domain.InterfaceTypeClient || iface.Type == domain.InterfaceTypeAny {
+		if !iface.IsDisabled() {
+			if err := wgQuickController.SetDNS(ctx, iface.Identifier, iface.DnsStr, iface.DnsSearchStr); err != nil {
+				return fmt.Errorf("failed to update dns settings: %w", err)
+			}
+		} else {
+			if err := wgQuickController.UnsetDNS(ctx, iface.Identifier, iface.DnsStr, iface.DnsSearchStr); err != nil {
+				return fmt.Errorf("failed to clear dns settings: %w", err)
+			}
 		}
 	}
 	return nil
 }
 
-func (m Manager) handleInterfacePreSaveHooks(iface *domain.Interface, oldEnabled, newEnabled bool) error {
+func (m Manager) handleInterfacePreSaveHooks(
+	ctx context.Context,
+	iface *domain.Interface,
+	oldEnabled, newEnabled bool,
+) error {
 	if oldEnabled == newEnabled {
 		return nil // do nothing if state did not change
 	}
 
 	slog.Debug("executing pre-save hooks", "interface", iface.Identifier, "up", newEnabled)
 
+	wgQuickController, ok := m.wg.GetController(*iface).(WgQuickController)
+	if !ok {
+		slog.Warn("failed to execute pre-save hooks", "interface", iface.Identifier, "up", newEnabled,
+			"error", "no capable controller found")
+		return nil
+	}
+
 	if newEnabled {
-		if err := m.quick.ExecuteInterfaceHook(iface.Identifier, iface.PreUp); err != nil {
+		if err := wgQuickController.ExecuteInterfaceHook(ctx, iface.Identifier, iface.PreUp); err != nil {
 			return fmt.Errorf("failed to execute pre-up hook: %w", err)
 		}
 	} else {
-		if err := m.quick.ExecuteInterfaceHook(iface.Identifier, iface.PreDown); err != nil {
+		if err := wgQuickController.ExecuteInterfaceHook(ctx, iface.Identifier, iface.PreDown); err != nil {
 			return fmt.Errorf("failed to execute pre-down hook: %w", err)
 		}
 	}
 	return nil
 }
 
-func (m Manager) handleInterfacePostSaveHooks(iface *domain.Interface, oldEnabled, newEnabled bool) error {
+func (m Manager) handleInterfacePostSaveHooks(
+	ctx context.Context,
+	iface *domain.Interface,
+	oldEnabled, newEnabled bool,
+) error {
 	if oldEnabled == newEnabled {
 		return nil // do nothing if state did not change
 	}
 
 	slog.Debug("executing post-save hooks", "interface", iface.Identifier, "up", newEnabled)
 
+	wgQuickController, ok := m.wg.GetController(*iface).(WgQuickController)
+	if !ok {
+		slog.Warn("failed to execute post-save hooks", "interface", iface.Identifier, "up", newEnabled,
+			"error", "no capable controller found")
+		return nil
+	}
+
 	if newEnabled {
-		if err := m.quick.ExecuteInterfaceHook(iface.Identifier, iface.PostUp); err != nil {
+		if err := wgQuickController.ExecuteInterfaceHook(ctx, iface.Identifier, iface.PostUp); err != nil {
 			return fmt.Errorf("failed to execute post-up hook: %w", err)
 		}
 	} else {
-		if err := m.quick.ExecuteInterfaceHook(iface.Identifier, iface.PostDown); err != nil {
+		if err := wgQuickController.ExecuteInterfaceHook(ctx, iface.Identifier, iface.PostDown); err != nil {
 			return fmt.Errorf("failed to execute post-down hook: %w", err)
 		}
 	}
@@ -799,7 +852,7 @@ func (m Manager) getFreshListenPort(ctx context.Context) (port int, err error) {
 
 func (m Manager) importInterface(
 	ctx context.Context,
-	backend InterfaceController,
+	backend domain.InterfaceController,
 	in *domain.PhysicalInterface,
 	peers []domain.PhysicalPeer,
 ) error {
@@ -901,13 +954,9 @@ func (m Manager) importPeer(ctx context.Context, in *domain.Interface, p *domain
 	return nil
 }
 
-func (m Manager) deleteInterfacePeers(ctx context.Context, id domain.InterfaceIdentifier) error {
-	iface, allPeers, err := m.db.GetInterfaceAndPeers(ctx, id)
-	if err != nil {
-		return err
-	}
+func (m Manager) deleteInterfacePeers(ctx context.Context, iface *domain.Interface, allPeers []domain.Peer) error {
 	for _, peer := range allPeers {
-		err = m.wg.GetController(*iface).DeletePeer(ctx, id, peer.Identifier)
+		err := m.wg.GetController(*iface).DeletePeer(ctx, iface.Identifier, peer.Identifier)
 		if err != nil && !errors.Is(err, os.ErrNotExist) {
 			return fmt.Errorf("wireguard peer deletion failure for %s: %w", peer.Identifier, err)
 		}

internal/app/wireguard/wireguard_peers.go

@@ -388,9 +388,20 @@ func (m Manager) DeletePeer(ctx context.Context, id domain.PeerIdentifier) error
 		return fmt.Errorf("failed to delete peer %s: %w", id, err)
 	}
 
+	peers, err := m.db.GetInterfacePeers(ctx, iface.Identifier)
+	if err != nil {
+		return fmt.Errorf("failed to load peers for interface %s: %w", iface.Identifier, err)
+	}
+
 	m.bus.Publish(app.TopicPeerDeleted, *peer)
 	// Update routes after peers have changed
-	m.bus.Publish(app.TopicRouteUpdate, "peers updated")
+	m.bus.Publish(app.TopicRouteUpdate, domain.RoutingTableInfo{
+		Interface:  *iface,
+		AllowedIps: iface.GetAllowedIPs(peers),
+		FwMark:     iface.FirewallMark,
+		Table:      iface.GetRoutingTable(),
+		TableStr:   iface.RoutingTable,
+	})
 	// Update interface after peers have changed
 	m.bus.Publish(app.TopicPeerInterfaceUpdated, peer.InterfaceIdentifier)
 
@@ -438,20 +449,26 @@ func (m Manager) GetUserPeerStats(ctx context.Context, id domain.UserIdentifier)
 // region helper-functions
 
 func (m Manager) savePeers(ctx context.Context, peers ...*domain.Peer) error {
-	interfaces := make(map[domain.InterfaceIdentifier]struct{})
+	interfaces := make(map[domain.InterfaceIdentifier]domain.Interface)
 
 	for _, peer := range peers {
-		iface, err := m.db.GetInterface(ctx, peer.InterfaceIdentifier)
-		if err != nil {
-			return fmt.Errorf("unable to find interface %s: %w", peer.InterfaceIdentifier, err)
+		// get interface from db if it is not yet in the map
+		if _, ok := interfaces[peer.InterfaceIdentifier]; !ok {
+			iface, err := m.db.GetInterface(ctx, peer.InterfaceIdentifier)
+			if err != nil {
+				return fmt.Errorf("unable to find interface %s: %w", peer.InterfaceIdentifier, err)
+			}
+			interfaces[peer.InterfaceIdentifier] = *iface
 		}
 
+		iface := interfaces[peer.InterfaceIdentifier]
+
 		// Always save the peer to the backend, regardless of disabled/expired state
 		// The backend will handle the disabled state appropriately
-		err = m.db.SavePeer(ctx, peer.Identifier, func(p *domain.Peer) (*domain.Peer, error) {
+		err := m.db.SavePeer(ctx, peer.Identifier, func(p *domain.Peer) (*domain.Peer, error) {
 			peer.CopyCalculatedAttributes(p)
 
-			err := m.wg.GetController(*iface).SavePeer(ctx, peer.InterfaceIdentifier, peer.Identifier,
+			err := m.wg.GetController(iface).SavePeer(ctx, peer.InterfaceIdentifier, peer.Identifier,
 				func(pp *domain.PhysicalPeer) (*domain.PhysicalPeer, error) {
 					domain.MergeToPhysicalPeer(pp, peer)
 					return pp, nil
@@ -475,13 +492,22 @@ func (m Manager) savePeers(ctx context.Context, peers ...*domain.Peer) error {
 				Peer:   *peer,
 			},
 		})
-
-		interfaces[peer.InterfaceIdentifier] = struct{}{}
 	}
 
 	// Update routes after peers have changed
-	if len(interfaces) != 0 {
-		m.bus.Publish(app.TopicRouteUpdate, "peers updated")
+	for id, iface := range interfaces {
+		interfacePeers, err := m.db.GetInterfacePeers(ctx, id)
+		if err != nil {
+			return fmt.Errorf("failed to re-load peers for interface %s: %w", id, err)
+		}
+
+		m.bus.Publish(app.TopicRouteUpdate, domain.RoutingTableInfo{
+			Interface:  iface,
+			AllowedIps: iface.GetAllowedIPs(interfacePeers),
+			FwMark:     iface.FirewallMark,
+			Table:      iface.GetRoutingTable(),
+			TableStr:   iface.RoutingTable,
+		})
 	}
 
 	for iface := range interfaces {