From 429c910df148ebad8961ecbebc11a15fbcd929fe Mon Sep 17 00:00:00 2001
From: abrown38 <a.brown@epcc.ed.ac.uk>
Date: Tue, 20 Oct 2026 18:45:51 +0100
Subject: [PATCH] Counters: forget the totals of objects that no longer exist

hc counters kept listing the objects of a configuration replaced since
housecarld started (an import with --replace gives every object a new
ID) beside the current ones. Disabled objects, built-ins and policies
keep their totals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 CHANGELOG.md             |  4 ++++
 docs/operations.md       |  4 +++-
 internal/nft/nft.go      | 30 ++++++++++++++++++++++++++++++
 internal/nft/nft_test.go | 24 ++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5d346bb..b87b0a9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,6 +42,10 @@ All notable changes to Housecarl are documented here. The format follows
 
 ### Fixed
 
+- `hc counters` (`GET /counters`) kept listing the objects of a
+  configuration replaced since housecarld started (`hc import
+  --replace` gives every object a new ID), beside the current ones; the
+  totals of objects that no longer exist are now forgotten.
 - Import: two base chains on one hook had their rules imported twice,
   and the second chain's leading built-ins were moved above the first
   chain's rules; they now follow each other by priority (read in a
diff --git a/docs/operations.md b/docs/operations.md
index 4b01685..67b9227 100644
--- a/docs/operations.md
+++ b/docs/operations.md
@@ -154,7 +154,9 @@ table on its hook, so a drop in any of them wins.
 Every rule, NAT rule, built-in and policy has a counter. `hc rule list`,
 `hc forward list` and the web UI show packets; `hc counters` shows them
 all, with bytes. They count from housecarld's start, and carry across
-changes to the ruleset.
+changes to the ruleset. A deleted object's counter goes with it; an
+import with `--replace` gives every object a new ID, so its counters
+start again. A disabled rule keeps its count for when it is enabled.
 
 ## Quotas
 
diff --git a/internal/nft/nft.go b/internal/nft/nft.go
index b91c258..12858cc 100644
--- a/internal/nft/nft.go
+++ b/internal/nft/nft.go
@@ -556,9 +556,39 @@ func (f *Firewall) Apply(ctx context.Context, st *model.State) error {
 			f.base[k] = b
 		}
 	}
+	pruneCounters(f.base, st)
 	return nil
 }
 
+// pruneCounters forgets the totals of objects st no longer has (deleted,
+// or replaced by an import with new IDs), so they aren't listed beside
+// the current ones. A disabled object keeps its totals for when it comes
+// back; built-ins and policies are always kept.
+func pruneCounters(base map[string]Counter, st *model.State) {
+	ids := map[string]bool{}
+	for _, r := range st.Rules {
+		ids[fmt.Sprintf("r%d", r.ID)] = true
+	}
+	for _, x := range st.Forwards {
+		ids[fmt.Sprintf("f%d", x.ID)] = true
+	}
+	for _, x := range st.SNATs {
+		ids[fmt.Sprintf("s%d", x.ID)] = true
+	}
+	for _, x := range st.Redirects {
+		ids[fmt.Sprintf("d%d", x.ID)] = true
+	}
+	for mark := range base {
+		obj, _, _ := strings.Cut(mark, "-") // f3-hairpin, f3-6 are f3's
+		if len(obj) < 2 || !strings.ContainsRune("rfsd", rune(obj[0])) || strings.Trim(obj[1:], "0123456789") != "" {
+			continue // b-established, p-input
+		}
+		if !ids[obj] {
+			delete(base, mark)
+		}
+	}
+}
+
 // SaveBoot writes the ruleset of st for the boot unit, and the fallback
 // it loads if that fails (FallbackFile), atomically. The boot ruleset has
 // no flowtable: it loads before the network comes up, when its devices
diff --git a/internal/nft/nft_test.go b/internal/nft/nft_test.go
index 07ec081..b4ec22c 100644
--- a/internal/nft/nft_test.go
+++ b/internal/nft/nft_test.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"sort"
 	"strings"
 	"testing"
 
@@ -437,3 +438,26 @@ func TestShadowInContainer(t *testing.T) {
 		t.Errorf("the shadow fallback isn't recognised: %+v %v", tb, err)
 	}
 }
+
+// TestPruneCounters: the totals of objects gone (an import with --replace
+// gives every object a new ID) are forgotten; built-ins, policies and
+// disabled objects keep theirs.
+func TestPruneCounters(t *testing.T) {
+	base := map[string]Counter{}
+	for _, m := range []string{"r1", "r2", "r3", "f4", "f4-hairpin", "f4-6", "f5-hairpin", "s6", "d7", "d8", "b-established", "p-input", "fallback-input"} {
+		base[m] = Counter{Packets: 1}
+	}
+	st := model.NewState()
+	st.Rules = []*model.Rule{{ID: 1}, {ID: 3, Disabled: true}}
+	st.Forwards = []*model.PortForward{{ID: 4}}
+	st.Redirects = []*model.Redirect{{ID: 8}}
+	pruneCounters(base, st)
+	var got []string
+	for m := range base {
+		got = append(got, m)
+	}
+	sort.Strings(got)
+	if strings.Join(got, " ") != "b-established d8 f4 f4-6 f4-hairpin fallback-input p-input r1 r3" {
+		t.Errorf("kept %v", got)
+	}
+}