Parcourir la source

fix(routing): implement atomic Move Up / Move Down for routing filter rules (v1.8.27)

Proxmox Router il y a 9 heures
Parent
commit
fe98ff4462

+ 1 - 1
VERSION

@@ -1 +1 @@
-1.8.26
+1.8.27

BIN
bin/debtik


BIN
bin/debtik-arm64


BIN
bin/debtik-cli


BIN
bin/debtik-cli-arm64


+ 2 - 0
internal/api/api.go

@@ -478,11 +478,13 @@ func (s *Server) registerRoutes() {
 	s.mux.HandleFunc("POST /api/routing/filters", s.handleSaveRoutingFilter)
 	s.mux.HandleFunc("DELETE /api/routing/filters/{id}", s.handleDeleteRoutingFilter)
 	s.mux.HandleFunc("POST /api/routing/filters/{id}/state", s.handleSetRoutingFilterState)
+	s.mux.HandleFunc("POST /api/routing/filters/{id}/move", s.handleMoveRoutingFilter)
 
 	s.mux.HandleFunc("GET /api/routing/filters/rules", s.handleListRoutingFilters)
 	s.mux.HandleFunc("POST /api/routing/filters/rules", s.handleSaveRoutingFilter)
 	s.mux.HandleFunc("DELETE /api/routing/filters/rules/{id}", s.handleDeleteRoutingFilter)
 	s.mux.HandleFunc("POST /api/routing/filters/rules/{id}/state", s.handleSetRoutingFilterState)
+	s.mux.HandleFunc("POST /api/routing/filters/rules/{id}/move", s.handleMoveRoutingFilter)
 
 	s.mux.HandleFunc("GET /api/routing/filters/community-lists", s.handleListCommunityLists)
 	s.mux.HandleFunc("POST /api/routing/filters/community-lists", s.handleSaveCommunityList)

+ 25 - 0
internal/api/dynamic_routing_handlers.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"encoding/json"
+	"io"
 	"net/http"
 	"strings"
 
@@ -510,6 +511,30 @@ func (s *Server) handleSetRoutingFilterState(w http.ResponseWriter, r *http.Requ
 	writeJSON(w, http.StatusOK, map[string]interface{}{"status": "success", "disabled": req.Disabled})
 }
 
+func (s *Server) handleMoveRoutingFilter(w http.ResponseWriter, r *http.Request) {
+	id := r.PathValue("id")
+	if id == "" {
+		writeJSON(w, http.StatusBadRequest, map[string]string{"error": "Missing filter rule ID"})
+		return
+	}
+	var req struct {
+		Direction string `json:"direction"`
+	}
+	if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err != io.EOF {
+		writeJSON(w, http.StatusBadRequest, map[string]string{"error": "Invalid JSON: " + err.Error()})
+		return
+	}
+	dir := strings.ToLower(strings.TrimSpace(req.Direction))
+	if dir != "up" && dir != "down" {
+		dir = "up"
+	}
+	if err := s.dynRoutingMgr.MoveRoutingFilter(id, dir); err != nil {
+		writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
+		return
+	}
+	writeJSON(w, http.StatusOK, map[string]any{"status": "success", "id": id, "direction": dir})
+}
+
 // -------------------------------------------------------------
 // Community Lists Handlers (/routing filter community-list)
 // -------------------------------------------------------------

+ 34 - 1
internal/cli/executor.go

@@ -5884,8 +5884,41 @@ func (e *Executor) handleRoutingFilterRule(parts []string, user *auth.PublicUser
 		}
 		return ColorGreen + fmt.Sprintf("Routing filter rule %s disabled.\n", target) + ColorReset
 
+	case "move":
+		pos, params := parseArgs(parts[1:])
+		if len(pos) == 0 && params["numbers"] == "" {
+			return ColorRed + "Usage: /routing filter rule move <#|id> <up|down>\n" + ColorReset
+		}
+		target := ""
+		direction := "up"
+		if len(pos) >= 1 {
+			target = pos[0]
+			if len(pos) >= 2 {
+				direction = pos[1]
+			}
+		} else {
+			target = params["numbers"]
+		}
+		if v, ok := params["direction"]; ok {
+			direction = v
+		} else if v, ok := params["destination"]; ok {
+			direction = v
+		}
+
+		rules := e.dynRoutingMgr.ListRoutingFilters()
+		var ruleID string
+		if idx, err := strconv.Atoi(target); err == nil && idx >= 0 && idx < len(rules) {
+			ruleID = rules[idx].ID
+		} else {
+			ruleID = target
+		}
+		if err := e.dynRoutingMgr.MoveRoutingFilter(ruleID, direction); err != nil {
+			return ColorRed + "Error moving filter rule: " + err.Error() + "\n" + ColorReset
+		}
+		return ColorGreen + fmt.Sprintf("Routing filter rule %s moved %s.\n", target, direction) + ColorReset
+
 	default:
-		return ColorRed + "Usage: /routing filter rule [print|add|remove|enable|disable]\n" + ColorReset
+		return ColorRed + "Usage: /routing filter rule [print|add|remove|enable|disable|move]\n" + ColorReset
 	}
 }
 

+ 82 - 1
internal/dynamicrouting/manager.go

@@ -2602,6 +2602,81 @@ func (m *Manager) SetRoutingFilterState(id string, disabled bool) error {
 	return m.applyBirdConfig()
 }
 
+func (m *Manager) MoveRoutingFilter(id string, direction string) error {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	var targetRule *RoutingFilterRule
+	for i := range m.config.RoutingFilters {
+		if m.config.RoutingFilters[i].ID == id {
+			targetRule = &m.config.RoutingFilters[i]
+			break
+		}
+	}
+	if targetRule == nil {
+		return fmt.Errorf("routing filter rule not found: %s", id)
+	}
+
+	chain := targetRule.Chain
+	dir := strings.ToLower(strings.TrimSpace(direction))
+	if dir != "up" && dir != "down" {
+		return fmt.Errorf("invalid direction %q: must be 'up' or 'down'", direction)
+	}
+
+	type chainItem struct {
+		globalIndex int
+		rule        RoutingFilterRule
+	}
+	var items []chainItem
+	for i, r := range m.config.RoutingFilters {
+		if r.Chain == chain {
+			items = append(items, chainItem{globalIndex: i, rule: r})
+		}
+	}
+
+	sort.SliceStable(items, func(i, j int) bool {
+		return items[i].rule.RuleOrder < items[j].rule.RuleOrder
+	})
+
+	chainIdx := -1
+	for i, it := range items {
+		if it.rule.ID == id {
+			chainIdx = i
+			break
+		}
+	}
+	if chainIdx == -1 {
+		return fmt.Errorf("rule not found in chain")
+	}
+
+	targetIdx := chainIdx
+	if dir == "up" {
+		targetIdx = chainIdx - 1
+	} else if dir == "down" {
+		targetIdx = chainIdx + 1
+	}
+
+	if targetIdx < 0 || targetIdx >= len(items) {
+		// Already at boundary, normalize order and return
+		for i, it := range items {
+			m.config.RoutingFilters[it.globalIndex].RuleOrder = (i + 1) * 10
+		}
+		_ = m.save()
+		return nil
+	}
+
+	// Swap positions in the sorted chain list
+	items[chainIdx], items[targetIdx] = items[targetIdx], items[chainIdx]
+
+	// Reassign spaced rule_order values (10, 20, 30...)
+	for i, it := range items {
+		m.config.RoutingFilters[it.globalIndex].RuleOrder = (i + 1) * 10
+	}
+
+	_ = m.save()
+	return m.applyBirdConfig()
+}
+
 // translateRos7RuleToBird converts a RouterOS v7 rule script (or fallback fields) to valid BIRD 2 syntax
 func translateRos7RuleToBird(r *RoutingFilterRule) string {
 	script := strings.TrimSpace(r.Rule)
@@ -4185,13 +4260,19 @@ func (m *Manager) GetSummary() DynamicRoutingSummary {
 // ==========================================
 
 func generateFRRPrefixList(name string, rules []RoutingFilterRule) string {
+	sortedRules := make([]RoutingFilterRule, len(rules))
+	copy(sortedRules, rules)
+	sort.SliceStable(sortedRules, func(i, j int) bool {
+		return sortedRules[i].RuleOrder < sortedRules[j].RuleOrder
+	})
+
 	var sb strings.Builder
 	seq := 10
 	reDstRange := regexp.MustCompile(`dst\s+in\s+([0-9\.\/]+)\{([0-9]+),([0-9]+)\}`)
 	reDstIn := regexp.MustCompile(`dst\s+in\s+([0-9\.\/]+)`)
 	reDstEq := regexp.MustCompile(`\bdst\s*={1,2}\s*([0-9\.\/]+)`)
 
-	for _, r := range rules {
+	for _, r := range sortedRules {
 		if r.Disabled {
 			continue
 		}

+ 65 - 0
internal/dynamicrouting/protocols_test.go

@@ -628,3 +628,68 @@ func TestDynamicRoutingFiltersAllProtocols(t *testing.T) {
 		t.Errorf("FRR config missing distribute-list prefix out;\nConfig:\n%s", frrCfg)
 	}
 }
+
+func TestMoveRoutingFilter(t *testing.T) {
+	mgr := &Manager{
+		config: DynamicConfig{
+			RoutingFilters: []RoutingFilterRule{
+				{ID: "r1", Chain: "chain-A", RuleOrder: 10, Rule: "dst in 10.1.0.0/16"},
+				{ID: "r2", Chain: "chain-A", RuleOrder: 20, Rule: "dst in 10.2.0.0/16"},
+				{ID: "r3", Chain: "chain-A", RuleOrder: 30, Rule: "dst in 10.3.0.0/16"},
+				{ID: "r-other", Chain: "chain-B", RuleOrder: 10, Rule: "dst in 172.16.0.0/12"},
+			},
+		},
+	}
+
+	// 1. Move r1 "up" (already at top) -> should succeed with no order change
+	if err := mgr.MoveRoutingFilter("r1", "up"); err != nil {
+		t.Fatalf("Move r1 up failed: %v", err)
+	}
+	filters := mgr.ListRoutingFilters()
+	if filters[0].ID != "r1" || filters[0].RuleOrder != 10 {
+		t.Errorf("Expected r1 still at order 10, got %d", filters[0].RuleOrder)
+	}
+
+	// 2. Move r1 "down" -> r2 should become order 10, r1 should become order 20
+	if err := mgr.MoveRoutingFilter("r1", "down"); err != nil {
+		t.Fatalf("Move r1 down failed: %v", err)
+	}
+	filters = mgr.ListRoutingFilters()
+	var r1Order, r2Order int
+	for _, f := range filters {
+		if f.ID == "r1" {
+			r1Order = f.RuleOrder
+		}
+		if f.ID == "r2" {
+			r2Order = f.RuleOrder
+		}
+	}
+	if r2Order != 10 || r1Order != 20 {
+		t.Errorf("Expected r2 order 10 and r1 order 20, got r2=%d, r1=%d", r2Order, r1Order)
+	}
+
+	// 3. Chain B should be unaffected
+	for _, f := range filters {
+		if f.ID == "r-other" && f.RuleOrder != 10 {
+			t.Errorf("Chain B rule modified unexpectedly: %d", f.RuleOrder)
+		}
+	}
+
+	// 4. Move r3 (now at index 2 in chain-A) "up" -> should swap with r1 (at order 20)
+	if err := mgr.MoveRoutingFilter("r3", "up"); err != nil {
+		t.Fatalf("Move r3 up failed: %v", err)
+	}
+	filters = mgr.ListRoutingFilters()
+	var r3Order int
+	for _, f := range filters {
+		if f.ID == "r1" {
+			r1Order = f.RuleOrder
+		}
+		if f.ID == "r3" {
+			r3Order = f.RuleOrder
+		}
+	}
+	if r3Order != 20 || r1Order != 30 {
+		t.Errorf("Expected r3 order 20 and r1 order 30, got r3=%d, r1=%d", r3Order, r1Order)
+	}
+}

+ 21 - 23
internal/web/static/app.js

@@ -12965,8 +12965,8 @@ function renderRoutingFilterRules() {
         <td class="py-2 px-3 break-all max-w-xl">${ruleDisplay}</td>
         <td class="py-2 px-3 text-slate-400 font-sans truncate max-w-xs">${escapeHtml(r.comment || '')}</td>
         <td class="py-2 px-3 text-right space-x-1 whitespace-nowrap font-sans" onclick="event.stopPropagation()">
-          <button onclick="moveRoutingFilterOrder('${r.id}', -1)" title="Move Up" class="px-1.5 py-0.5 rounded text-[10px] bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700">▲</button>
-          <button onclick="moveRoutingFilterOrder('${r.id}', 1)" title="Move Down" class="px-1.5 py-0.5 rounded text-[10px] bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700">▼</button>
+          <button onclick="event.stopPropagation(); moveRoutingFilterOrder('${r.id}', -1)" title="Move Up" class="px-1.5 py-0.5 rounded text-[10px] bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700">▲</button>
+          <button onclick="event.stopPropagation(); moveRoutingFilterOrder('${r.id}', 1)" title="Move Down" class="px-1.5 py-0.5 rounded text-[10px] bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700">▼</button>
           <button onclick="toggleRoutingFilterState('${r.id}', ${!r.disabled})" class="px-2 py-0.5 rounded text-[10px] ${r.disabled ? 'bg-emerald-950 hover:bg-emerald-900 text-emerald-400 border border-emerald-800/50' : 'bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700'}">
             ${r.disabled ? 'Enable' : 'Disable'}
           </button>
@@ -13318,33 +13318,31 @@ async function applyRoutingFilterForm() {
 }
 
 async function moveRoutingFilterOrder(id, delta) {
-  const list = state.routingFilters || [];
-  const idx = list.findIndex(r => r.id === id);
-  if (idx < 0) return;
-  const targetIdx = idx + delta;
-  if (targetIdx < 0 || targetIdx >= list.length) return;
-
-  const currentOrder = list[idx].rule_order || 10;
-  const targetOrder = list[targetIdx].rule_order || 20;
-
-  list[idx].rule_order = targetOrder;
-  list[targetIdx].rule_order = currentOrder;
-
+  const dir = delta < 0 ? 'up' : 'down';
   try {
-    await fetch(`${API_BASE}/routing/filters`, {
+    const res = await fetch(`${API_BASE}/routing/filters/${encodeURIComponent(id)}/move`, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify(list[idx])
-    });
-    await fetch(`${API_BASE}/routing/filters`, {
-      method: 'POST',
-      headers: { 'Content-Type': 'application/json' },
-      body: JSON.stringify(list[targetIdx])
+      body: JSON.stringify({ direction: dir })
     });
-    loadDynamicRouting();
+    if (!res.ok) {
+      const err = await res.json().catch(() => ({}));
+      showToast('Error', err.error || 'Failed to move filter rule', 'error');
+      return;
+    }
+    await loadDynamicRouting(false);
   } catch (e) {
-    console.error('Reorder error:', e);
+    showToast('Error', e.message, 'error');
+  }
+}
+
+async function moveSelectedRoutingFilter(delta) {
+  const id = state.selectedFilterRow?.['rules'];
+  if (!id) {
+    showToast('Notice', 'Please select a routing filter rule first', 'info');
+    return;
   }
+  await moveRoutingFilterOrder(id, delta);
 }
 
 async function deleteRoutingFilter(id) {

+ 6 - 0
internal/web/static/index.html

@@ -3733,6 +3733,12 @@
                 <button id="btn-rf-disable-selected" onclick="toggleSelectedRoutingFilter(true)" class="px-2.5 py-1 rounded font-medium bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700 transition">
                   Disable
                 </button>
+                <button id="btn-rf-move-up-selected" onclick="moveSelectedRoutingFilter(-1)" title="Move Up" class="px-2 py-1 rounded font-medium bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700 transition">
+                  ▲ Up
+                </button>
+                <button id="btn-rf-move-down-selected" onclick="moveSelectedRoutingFilter(1)" title="Move Down" class="px-2 py-1 rounded font-medium bg-slate-800 hover:bg-slate-700 text-slate-300 border border-slate-700 transition">
+                  ▼ Down
+                </button>
                 <button id="btn-rf-remove-selected" onclick="removeSelectedRoutingFilter()" class="px-2.5 py-1 rounded font-medium bg-slate-800 hover:bg-rose-950 text-rose-400 border border-slate-700 transition">
                   Remove
                 </button>