Fix static routes UI edit and delete: full signature tracking, force DOM redraw, clean kernel cleanup, deduplication, and ID propagation (v1.8.21)

VERSION

@@ -1 +1 @@
-1.8.20
+1.8.21

internal/api/firewall_handlers.go

@@ -373,7 +373,7 @@ func (s *Server) handleSaveRoute(w http.ResponseWriter, r *http.Request) {
 		writeError(w, http.StatusBadRequest, "invalid request body")
 		return
 	}
-	if err := s.routingMgr.SaveRoute(route); err != nil {
+	if err := s.routingMgr.SaveRoute(&route); err != nil {
 		writeError(w, http.StatusInternalServerError, err.Error())
 		return
 	}

internal/cli/executor.go

@@ -1493,7 +1493,7 @@ func (e *Executor) handleIPRoute(parts []string, user *auth.PublicUser) string {
 			Static:       true,
 			Disabled:     false,
 		}
-		if err := e.routingMgr.SaveRoute(r); err != nil {
+		if err := e.routingMgr.SaveRoute(&r); err != nil {
 			return ColorRed + "Error adding route: " + err.Error() + ColorReset + "\n"
 		}
 		return ColorGreen + fmt.Sprintf("Route to %s via %s added.\n", dst, gw) + ColorReset

internal/routing/manager.go

@@ -18,7 +18,7 @@ import (
 	"github.com/vishvananda/netlink"
 )
 
-const (
+var (
 	StaticRoutesStorePath = "/etc/debtik/routing/static_routes.json"
 	PolicyRulesStorePath  = "/etc/debtik/routing/policy_rules.json"
 	CustomTablesStorePath = "/etc/debtik/routing/custom_tables.json"
@@ -585,7 +585,10 @@ func (m *RoutingManager) ListRoutes() []RouteItem {
 	return liveRoutes
 }
 
-func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
+func (m *RoutingManager) SaveRoute(r *RouteItem) (err error) {
+	if r == nil {
+		return fmt.Errorf("route cannot be nil")
+	}
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	before := append([]RouteItem(nil), m.staticRoutes...)
@@ -674,6 +677,25 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 		}
 	}
 
+	// 4. Fallback match: if editing with OldDstAddress but gateway was changed and didn't match oldGw
+	if foundIndex == -1 && r.OldDstAddress != "" {
+		normOldDst := normalizeDst(r.OldDstAddress)
+		oldTable := r.OldRoutingTable
+		if oldTable == "" {
+			oldTable = "main"
+		}
+		for i, existing := range m.staticRoutes {
+			exTbl := existing.RoutingTable
+			if exTbl == "" {
+				exTbl = "main"
+			}
+			if normalizeDst(existing.DstAddress) == normOldDst && exTbl == oldTable {
+				foundIndex = i
+				break
+			}
+		}
+	}
+
 	if r.CheckInterval <= 0 {
 		r.CheckInterval = 5
 	}
@@ -699,7 +721,9 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 
 		if normalizeDst(oldExisting.DstAddress) != normalizeDst(r.DstAddress) ||
 			oldExisting.RoutingTable != r.RoutingTable ||
-			oldExisting.Distance != r.Distance {
+			oldExisting.Distance != r.Distance ||
+			oldExisting.Gateway != r.Gateway ||
+			oldExisting.Interface != r.Interface {
 			oldCopy := oldExisting
 			oldRouteToClean = &oldCopy
 		}
@@ -710,7 +734,7 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 			r.HealthLossPct = oldExisting.HealthLossPct
 			r.HealthLastCheck = oldExisting.HealthLastCheck
 		}
-		m.staticRoutes[foundIndex] = r
+		m.staticRoutes[foundIndex] = *r
 	} else {
 		if r.ID == "" || strings.HasPrefix(r.ID, "kr-") {
 			r.ID = fmt.Sprintf("route-%d", time.Now().UnixNano())
@@ -724,7 +748,7 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 				}
 			}
 		}
-		m.staticRoutes = append(m.staticRoutes, r)
+		m.staticRoutes = append(m.staticRoutes, *r)
 	}
 
 	if r.OldDstAddress != "" {
@@ -732,7 +756,11 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 		if r.OldDistance != nil {
 			oldDist = *r.OldDistance
 		}
-		if normalizeDst(r.OldDstAddress) != normalizeDst(r.DstAddress) || r.OldRoutingTable != r.RoutingTable || oldDist != r.Distance {
+		if normalizeDst(r.OldDstAddress) != normalizeDst(r.DstAddress) ||
+			r.OldRoutingTable != r.RoutingTable ||
+			oldDist != r.Distance ||
+			r.OldGateway != r.Gateway ||
+			r.OldInterface != r.Interface {
 			oldRouteToClean = &RouteItem{
 				DstAddress:   r.OldDstAddress,
 				Gateway:      r.OldGateway,
@@ -754,13 +782,26 @@ func (m *RoutingManager) SaveRoute(r RouteItem) (err error) {
 		}
 	}
 
+	// In-memory deduplication pass on static routes
+	seenKeys := make(map[string]bool)
+	var deduped []RouteItem
+	for _, sr := range m.staticRoutes {
+		k := fmt.Sprintf("%s|%s|%s|%s|%d", normalizeDst(sr.DstAddress), sr.RoutingTable, sr.Gateway, sr.Interface, sr.Distance)
+		if !seenKeys[k] {
+			seenKeys[k] = true
+			deduped = append(deduped, sr)
+		}
+	}
+	m.staticRoutes = deduped
+
 	if saveErr := m.save(); saveErr != nil {
 		return saveErr
 	}
 
-	// Clean up old route group if destination, routing table or distance changed
+	// Clean up old route group if destination, routing table, distance, gateway, or interface changed
 	if oldRouteToClean != nil {
 		_ = m.syncRouteGroupToKernel(oldRouteToClean.DstAddress, oldRouteToClean.RoutingTable, oldRouteToClean.Distance)
+		_ = m.deleteDirectKernelRoute(*oldRouteToClean)
 	}
 
 	// Apply updated/new route group to kernel
@@ -811,6 +852,7 @@ func (m *RoutingManager) DeleteRoute(id string) error {
 							updated = append(updated, r)
 						}
 					}
+					_ = m.deleteDirectKernelRoute(lr)
 					break
 				} else {
 					// Pure kernel route, not managed in static_routes.json
@@ -824,6 +866,7 @@ func (m *RoutingManager) DeleteRoute(id string) error {
 		m.staticRoutes = updated
 		_ = m.save()
 		_ = m.syncRouteGroupToKernel(target.DstAddress, target.RoutingTable, target.Distance)
+		_ = m.deleteDirectKernelRoute(*target)
 		return nil
 	}
 
@@ -1108,14 +1151,21 @@ func (m *RoutingManager) removeRouteFromKernel(r RouteItem) error {
 
 func (m *RoutingManager) deleteDirectKernelRoute(r RouteItem) error {
 	normDst := normalizeDst(r.DstAddress)
-	args := []string{"route", "del", normDst}
 	gw := strings.TrimSpace(r.Gateway)
-	if gw != "" && net.ParseIP(gw) != nil {
-		args = append(args, "via", gw)
-	}
-	iface := strings.TrimSpace(r.Interface)
-	if iface != "" {
-		args = append(args, "dev", iface)
+	isBlackhole := strings.EqualFold(gw, "blackhole") || strings.EqualFold(r.Action, "blackhole")
+
+	var args []string
+	if isBlackhole {
+		args = []string{"route", "del", "blackhole", normDst}
+	} else {
+		args = []string{"route", "del", normDst}
+		if gw != "" && net.ParseIP(gw) != nil {
+			args = append(args, "via", gw)
+		}
+		iface := strings.TrimSpace(r.Interface)
+		if iface != "" {
+			args = append(args, "dev", iface)
+		}
 	}
 	if r.RoutingTable != "" && r.RoutingTable != "main" {
 		args = append(args, "table", r.RoutingTable)
@@ -1130,6 +1180,20 @@ func (m *RoutingManager) deleteDirectKernelRoute(r RouteItem) error {
 		log.Printf("[RouteManager] direct ip route del (%s): %s (%v), attempting fallback", strings.Join(args, " "), string(out), err)
 	}
 
+	if isBlackhole {
+		fbBhArgs := []string{"route", "del", "blackhole", normDst}
+		if r.RoutingTable != "" && r.RoutingTable != "main" {
+			fbBhArgs = append(fbBhArgs, "table", r.RoutingTable)
+		}
+		if r.Distance > 0 {
+			fbBhArgs = append(fbBhArgs, "metric", strconv.Itoa(r.Distance))
+		}
+		if err := exec.Command("ip", fbBhArgs...).Run(); err == nil {
+			return nil
+		}
+		return exec.Command("ip", "route", "del", "blackhole", normDst).Run()
+	}
+
 	fbArgs := []string{"route", "del", normDst}
 	if r.RoutingTable != "" && r.RoutingTable != "main" {
 		fbArgs = append(fbArgs, "table", r.RoutingTable)

internal/routing/manager_test.go

@@ -275,3 +275,72 @@ func TestNormalizeDst(t *testing.T) {
 		t.Errorf("expected 10.0.0.0/8, got %s", normalizeDst("10.0.0.0/8"))
 	}
 }
+
+func TestSaveAndDeleteRouteLifecycle(t *testing.T) {
+	tmpDir := t.TempDir()
+	origStore := StaticRoutesStorePath
+	defer func() { StaticRoutesStorePath = origStore }()
+	StaticRoutesStorePath = filepath.Join(tmpDir, "static_routes.json")
+
+	mgr := &RoutingManager{
+		staticRoutes: make([]RouteItem, 0),
+	}
+
+	// 1. Save new route with empty ID
+	r1 := RouteItem{
+		DstAddress:   "198.51.100.0/24",
+		Gateway:      "blackhole",
+		Action:       "blackhole",
+		RoutingTable: "main",
+		Distance:     10,
+		Comment:      "Initial route",
+	}
+
+	if err := mgr.SaveRoute(&r1); err != nil {
+		t.Fatalf("expected nil error on SaveRoute, got %v", err)
+	}
+
+	if r1.ID == "" {
+		t.Fatalf("expected SaveRoute to populate r1.ID, got empty")
+	}
+	if len(mgr.staticRoutes) != 1 {
+		t.Fatalf("expected 1 static route in manager, got %d", len(mgr.staticRoutes))
+	}
+	savedID := r1.ID
+
+	// 2. Edit existing route with populated ID
+	r1Edit := RouteItem{
+		ID:           savedID,
+		DstAddress:   "198.51.100.0/24",
+		Gateway:      "blackhole",
+		Action:       "blackhole",
+		RoutingTable: "main",
+		Distance:     20,
+		Comment:      "Updated route",
+	}
+	if err := mgr.SaveRoute(&r1Edit); err != nil {
+		t.Fatalf("expected nil error on editing SaveRoute, got %v", err)
+	}
+
+	if len(mgr.staticRoutes) != 1 {
+		t.Fatalf("expected still 1 static route after edit, got %d", len(mgr.staticRoutes))
+	}
+	if mgr.staticRoutes[0].ID != savedID {
+		t.Errorf("expected ID %s preserved, got %s", savedID, mgr.staticRoutes[0].ID)
+	}
+	if mgr.staticRoutes[0].Distance != 20 {
+		t.Errorf("expected distance 20, got %d", mgr.staticRoutes[0].Distance)
+	}
+	if mgr.staticRoutes[0].Comment != "Updated route" {
+		t.Errorf("expected comment 'Updated route', got %s", mgr.staticRoutes[0].Comment)
+	}
+
+	// 3. Delete route
+	delErr := mgr.DeleteRoute(savedID)
+	if delErr != nil {
+		t.Fatalf("expected nil error on DeleteRoute, got %v", delErr)
+	}
+	if len(mgr.staticRoutes) != 0 {
+		t.Fatalf("expected 0 static routes after delete, got %d", len(mgr.staticRoutes))
+	}
+}

internal/web/static/app.js

@@ -7422,7 +7422,9 @@ function renderRoutesList() {
   }
   empty?.classList.add('hidden');
 
-  const sig = `${tableFilter}|${typeFilter}|` + routes.map(r => `${r.id}:${r.active}:${r.disabled}:${r.distance}:${r.protocol}:${r.health_status}:${(r.health_rtt_ms || 0).toFixed(0)}`).join(';');
+  const sig = `${tableFilter}|${typeFilter}|` + routes.map(r =>
+    `${r.id}:${r.dst_address}:${r.gateway}:${r.interface}:${r.distance}:${r.routing_table}:${r.active}:${r.disabled}:${r.protocol}:${r.comment}:${r.check_gateway}:${r.check_target}:${r.health_status}:${(r.health_rtt_ms || 0).toFixed(0)}`
+  ).join(';');
   if (tbody.dataset.sig === sig) {
     return; // Routes table unchanged! Do not wipe DOM or flicker!
   }
@@ -7763,10 +7765,12 @@ async function submitSaveRoute(e) {
     if (res.ok) {
       showToast('Route Saved', `Route to ${dst} installed`, 'success');
       closeRouteModal();
+      const tbody = document.getElementById('routes-table-body');
+      if (tbody) tbody.dataset.sig = '';
       loadRoutes();
     } else {
-      const err = await res.json();
-      showToast('Error', err.message || 'Failed to save route', 'error');
+      const err = await res.json().catch(() => ({}));
+      showToast('Error', err.message || err.error || 'Failed to save route', 'error');
     }
   } catch (err) {
     showToast('Error', err.message, 'error');
@@ -7780,7 +7784,14 @@ async function toggleRouteState(id, disabled) {
       headers: { 'Content-Type': 'application/json' },
       body: JSON.stringify({ disabled })
     });
-    if (res.ok) loadRoutes();
+    if (res.ok) {
+      const tbody = document.getElementById('routes-table-body');
+      if (tbody) tbody.dataset.sig = '';
+      loadRoutes();
+    } else {
+      const err = await res.json().catch(() => ({}));
+      showToast('Error', err.message || err.error || 'Failed to update route state', 'error');
+    }
   } catch (err) {
     showToast('Error', err.message, 'error');
   }
@@ -7792,7 +7803,12 @@ async function deleteRoute(id) {
     const res = await fetch(`${API_BASE}/routes/${encodeURIComponent(id)}`, { method: 'DELETE' });
     if (res.ok) {
       showToast('Deleted', 'Route removed', 'success');
+      const tbody = document.getElementById('routes-table-body');
+      if (tbody) tbody.dataset.sig = '';
       loadRoutes();
+    } else {
+      const err = await res.json().catch(() => ({}));
+      showToast('Error', err.message || err.error || 'Failed to delete route', 'error');
     }
   } catch (err) {
     showToast('Error', err.message, 'error');