Release 0.2.0-rc.3+git62-0d16358

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -142,6 +142,16 @@ All notable changes to Housecarl are documented here. The format follows
 
 ### Fixed
 
+- A history entry's ruleset diff showed the object it changed as moved:
+  stepping back put the old form at the end of its list, so updating
+  one port forward (turning hairpin off) also showed its forward and
+  DNAT lines moving down. The diff is now only what the change did, and
+  its sides are named `before N` and `after N` instead of `now`.
+- Ruleset diffs numbered the hunks after one with a moved line too early
+  on the new side (`@@ -76,9 +74,9 @@` for `+76,9`), so the chain named
+  after `@@` could be the one before (`chain output` for a hunk in
+  `chain dstnat`). go-udiff miscounts them; Housecarl now numbers them
+  itself.
 - The dashboard's bar for a per-address quota showed only the busiest
   address, so one address that used up its allowance filled it (100%).
   The bar is now all the addresses together, and the dashboard and the

README.md

@@ -128,7 +128,7 @@ The web UI is on `http://127.0.0.1:8070/` (see `listen` in
 
 ## Status
 
-Housecarl 0.2.0 is in testing (0.2.0-rc.3+git60-ee27884): zones, address sets and
+Housecarl 0.2.0 is in testing (0.2.0-rc.3+git62-0d16358): zones, address sets and
 services with ordered rules, IPv4 and IPv6, port forwards with hairpin NAT
 and IPv6 pinholes, source NAT and redirects, built-ins, probation for
 network changes (confirmed by the client that made them), the ruleset

VERSION

@@ -1 +1 @@
-0.2.0-rc.3+git60-ee27884
+0.2.0-rc.3+git62-0d16358

internal/engine/engine.go

@@ -335,6 +335,8 @@ func Rewind(st *model.State, h *model.HistoryEntry) error {
 			return err
 		}
 	}
+	// put appends what it brings back; its position puts it in place.
+	st.Sort()
 	return nil
 }
 

internal/server/diff_test.go

@@ -1,6 +1,8 @@
 package server
 
 import (
+	"encoding/json"
+	"fmt"
 	"strings"
 	"testing"
 
@@ -70,3 +72,68 @@ func TestHistoryDiff(t *testing.T) {
 		t.Errorf("unknown entry: %d", code)
 	}
 }
+
+// TestHunkStarts: after a hunk with unchanged lines between its edits (a
+// moved line), the later hunks' new sides start where they do.
+func TestHunkStarts(t *testing.T) {
+	var a, b strings.Builder
+	for i := 1; i <= 20; i++ {
+		fmt.Fprintf(&a, "%d\n", i)
+		switch i {
+		case 4:
+		case 6:
+			b.WriteString("6\n4\n")
+		case 14:
+		default:
+			fmt.Fprintf(&b, "%d\n", i)
+		}
+	}
+	diff := hunkStarts(udiff.Unified("a", "b", a.String(), b.String()))
+	var heads []string
+	for _, l := range strings.Split(diff, "\n") {
+		if strings.HasPrefix(l, "@@") {
+			heads = append(heads, l)
+		}
+	}
+	// diff -u: the move nets nothing, the deleted 14 one line.
+	if want := []string{"@@ -1,9 +1,9 @@", "@@ -11,7 +11,6 @@"}; strings.Join(heads, "|") != strings.Join(want, "|") {
+		t.Fatalf("hunk headers %q, want %q\n%s", heads, want, diff)
+	}
+}
+
+// TestHistoryDiffUpdate: an update's diff is only what it changed; the
+// object stepped back to its old form stays where it was.
+func TestHistoryDiffUpdate(t *testing.T) {
+	e := newEnv(t)
+	e.call(e.token, "POST", "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}, nil)
+	e.call(e.token, "POST", "/zones", model.Zone{Name: "wan", Interfaces: []string{"wan"}, External: true}, nil)
+	var ids []int64
+	for i, port := range []string{"8096", "443", "50300", "7777", "25"} {
+		var mr api.MutationResponse
+		e.call(e.token, "POST", "/forwards", map[string]any{"name": "f" + port, "ports": port, "to": fmt.Sprintf("10.0.10.%d", 30+i),
+			"public_addresses": []string{"203.0.113.1"}, "hairpin": true}, &mr)
+		var f model.PortForward
+		if err := json.Unmarshal(mr.Object, &f); err != nil {
+			t.Fatal(err)
+		}
+		ids = append(ids, f.ID)
+	}
+	var mr api.MutationResponse
+	if code := e.call(e.token, "PATCH", "/forwards/"+itoa(ids[2]), map[string]any{"hairpin": false}, &mr); code != 200 {
+		t.Fatalf("update: %d", code)
+	}
+	var out struct{ Diff string }
+	if code := e.call(e.token, "GET", "/history/"+itoa(mr.HistoryID)+"/diff", nil, &out); code != 200 {
+		t.Fatalf("diff: %d", code)
+	}
+	var changed []string
+	for _, l := range strings.Split(out.Diff, "\n") {
+		if (strings.HasPrefix(l, "+") || strings.HasPrefix(l, "-")) && !strings.HasPrefix(l, "+++") && !strings.HasPrefix(l, "---") {
+			changed = append(changed, l)
+		}
+	}
+	if len(changed) != 1 || !strings.HasPrefix(changed[0], "-") || !strings.Contains(changed[0], "-hairpin") ||
+		!strings.Contains(out.Diff, "(before "+itoa(mr.HistoryID)+")") || !strings.Contains(out.Diff, "@@ chain srcnat") {
+		t.Errorf("entry %d's diff, want only the hairpin line gone, in chain srcnat:\n%s", mr.HistoryID, out.Diff)
+	}
+}

internal/server/h_config.go

@@ -3,6 +3,7 @@ package server
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/netip"
 	"sort"
@@ -98,11 +99,71 @@ func (s *Server) respondChange(w http.ResponseWriter, r *http.Request, p *auth.P
 // rulesetDiff is the unified diff of the rulesets two configurations
 // make: the file housecarl-ruleset.service loads, now and after.
 func (s *Server) rulesetDiff(before, after *model.State) string {
+	return s.labeledDiff(before, after, "now", "after")
+}
+
+// labeledDiff is rulesetDiff with its own names for the two sides.
+func (s *Server) labeledDiff(before, after *model.State, from, to string) string {
 	if before == nil || after == nil {
 		return ""
 	}
 	now, then := s.fw.Render(before).Text, s.fw.Render(after).Text
-	return blockHeaders(udiff.Unified("ruleset.nft (now)", "ruleset.nft (after)", now, then), now, then)
+	diff := hunkStarts(udiff.Unified("ruleset.nft ("+from+")", "ruleset.nft ("+to+")", now, then))
+	return blockHeaders(diff, now, then)
+}
+
+// hunkStarts numbers each hunk's new side again, from its old side and
+// the lines the hunks before it added and removed. go-udiff (v0.4.1)
+// leaves out of its count the unchanged lines between two edits of one
+// hunk, so after a hunk like a moved line every later one started too
+// early ("@@ -76,9 +74,9 @@" for "+76,9"), and blockHeaders named the
+// chain before it.
+func hunkStarts(diff string) string {
+	lines := strings.SplitAfter(diff, "\n")
+	delta := 0
+	for i := 0; i < len(lines); i++ {
+		f := strings.Fields(lines[i])
+		if !strings.HasPrefix(lines[i], "@@ ") || len(f) < 4 || f[3] != "@@" {
+			continue
+		}
+		start, _, _ := strings.Cut(f[1][1:], ",")
+		from, err := strconv.Atoi(start)
+		if err != nil {
+			continue
+		}
+		oldN, newN := 0, 0
+		for _, l := range lines[i+1:] {
+			if strings.HasPrefix(l, "@@ ") {
+				break
+			}
+			switch {
+			case strings.HasPrefix(l, "-"):
+				oldN++
+			case strings.HasPrefix(l, "+"):
+				newN++
+			case strings.HasPrefix(l, " "):
+				oldN++
+				newN++
+			}
+		}
+		// An empty side is numbered by the line before it.
+		first := from
+		if oldN == 0 {
+			first++
+		}
+		to := first + delta
+		if newN == 0 {
+			to--
+		}
+		side := "+" + strconv.Itoa(to)
+		if newN > 1 {
+			side += "," + strconv.Itoa(newN)
+		}
+		f[2] = side
+		lines[i] = strings.Join(f, " ") + "\n"
+		delta += newN - oldN
+	}
+	return strings.Join(lines, "")
 }
 
 // blockHeaders names, after each hunk's @@ line, the chain (or other
@@ -705,7 +766,8 @@ func (s *Server) historyDiff(w http.ResponseWriter, r *http.Request, p *auth.Pri
 			return err
 		}
 	}
-	writeJSON(w, http.StatusOK, map[string]string{"diff": s.rulesetDiff(st, after)})
+	before, done := fmt.Sprintf("before %d", id), fmt.Sprintf("after %d", id)
+	writeJSON(w, http.StatusOK, map[string]string{"diff": s.labeledDiff(st, after, before, done)})
 	return nil
 }