Diffs name their chain; history shows the ruleset's diff
The ruleset's diff names the chain each hunk is in on its @@ line, as
git names the function. A history entry shows what it did to the
ruleset (web page, hc history diff, GET /history/{id}/diff), rebuilt
from the current configuration by stepping back through the later
entries. A changed setting is listed by its own fields
(builtins.drop_invalid) instead of the whole object.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
4e35236dba
commit
92147c8681
12 files changed
+289
-11
No files matched your search
@@ -322,6 +322,22 @@ func Undo(st *model.State, h *model.HistoryEntry) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Rewind applies a history entry backwards to st without Undo's check:
|
||||
// stepping back through the entries newest first brings back the
|
||||
// configuration as it was before each.
|
||||
func Rewind(st *model.State, h *model.HistoryEntry) error {
|
||||
for i := len(h.Changes) - 1; i >= 0; i-- {
|
||||
c := h.Changes[i]
|
||||
if c.Kind == model.KindSettings && c.Before == nil {
|
||||
continue
|
||||
}
|
||||
if err := put(st, c.Kind, c.ID, c.Before); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Replace makes the configuration next as a whole (imports).
|
||||
func (e *Engine) Replace(ctx context.Context, p *auth.Principal, description string, opts Options, next *model.State) (*Result, error) {
|
||||
e.mu.Lock()
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
udiff "github.com/aymanbagabas/go-udiff"
|
||||
|
||||
"housecarl/internal/api"
|
||||
"housecarl/internal/model"
|
||||
)
|
||||
|
||||
// TestBlockHeaders: each hunk says which chain it is in, even when the
|
||||
// chain's own line is far above it.
|
||||
func TestBlockHeaders(t *testing.T) {
|
||||
var b strings.Builder
|
||||
b.WriteString("table inet housecarl {\n\tset lan_v4 { type ipv4_addr; }\n\tchain input {\n")
|
||||
for i := 0; i < 20; i++ {
|
||||
b.WriteString("\t\tcounter comment \"in\"\n")
|
||||
}
|
||||
b.WriteString("\t}\n\tchain forward {\n")
|
||||
for i := 0; i < 20; i++ {
|
||||
b.WriteString("\t\tcounter comment \"fw\"\n")
|
||||
}
|
||||
b.WriteString("\t}\n}\n")
|
||||
now := b.String()
|
||||
then := strings.Replace(now, "\t\tcounter comment \"in\"\n\t}", "\t\tcounter comment \"in\"\n\t\taccept comment \"new in\"\n\t}", 1)
|
||||
then = strings.Replace(then, "\t\tcounter comment \"fw\"\n\t}", "\t\taccept comment \"new fw\"\n\t}", 1)
|
||||
|
||||
diff := blockHeaders(udiff.Unified("a", "b", now, then), now, then)
|
||||
var heads []string
|
||||
for _, l := range strings.Split(diff, "\n") {
|
||||
if strings.HasPrefix(l, "@@") {
|
||||
heads = append(heads, l)
|
||||
}
|
||||
}
|
||||
if len(heads) != 2 || !strings.HasSuffix(heads[0], "@@ chain input") || !strings.HasSuffix(heads[1], "@@ chain forward") {
|
||||
t.Fatalf("hunk headers %q, want chain input then chain forward\n%s", heads, diff)
|
||||
}
|
||||
if blockHeaders("", now, then) != "" {
|
||||
t.Fatal("no diff should stay empty")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHistoryDiff: an entry's diff is its own change, rebuilt from the
|
||||
// current configuration however many entries came after it.
|
||||
func TestHistoryDiff(t *testing.T) {
|
||||
e := newEnv(t)
|
||||
var mr api.MutationResponse
|
||||
e.call(e.token, "POST", "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}, nil)
|
||||
e.call(e.token, "POST", "/rules", model.Rule{Chain: "input", Match: model.Match{Services: []string{"ssh"}}, Comment: "first"}, &mr)
|
||||
first := mr.HistoryID
|
||||
e.call(e.token, "POST", "/rules", model.Rule{Chain: "forward", Match: model.Match{Services: []string{"http"}}, Comment: "second"}, nil)
|
||||
e.call(e.token, "PATCH", "/settings", map[string]any{"builtins": map[string]any{"drop_invalid": true}}, nil)
|
||||
|
||||
var out struct{ Diff string }
|
||||
if code := e.call(e.token, "GET", "/history/"+itoa(first)+"/diff", nil, &out); code != 200 {
|
||||
t.Fatalf("diff: %d", code)
|
||||
}
|
||||
var added []string
|
||||
for _, l := range strings.Split(out.Diff, "\n") {
|
||||
if strings.HasPrefix(l, "+") && !strings.HasPrefix(l, "+++") {
|
||||
added = append(added, l)
|
||||
}
|
||||
}
|
||||
if len(added) == 0 || !strings.Contains(strings.Join(added, "\n"), ` first"`) || strings.Contains(out.Diff, ` second"`) || !strings.Contains(out.Diff, "@@ chain input") {
|
||||
t.Errorf("entry %d's diff:\n%s", first, out.Diff)
|
||||
}
|
||||
if code := e.call(e.token, "GET", "/history/9999/diff", nil, nil); code != 404 {
|
||||
t.Errorf("unknown entry: %d", code)
|
||||
}
|
||||
}
|
||||
@@ -101,7 +101,68 @@ func (s *Server) rulesetDiff(before, after *model.State) string {
|
||||
if before == nil || after == nil {
|
||||
return ""
|
||||
}
|
||||
return udiff.Unified("ruleset.nft (now)", "ruleset.nft (after)", s.fw.Render(before).Text, s.fw.Render(after).Text)
|
||||
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)
|
||||
}
|
||||
|
||||
// blockHeaders names, after each hunk's @@ line, the chain (or other
|
||||
// block of the table) the hunk starts in, as git names the function:
|
||||
// "@@ -40,6 +40,8 @@ chain forward". Without it a change deep in a long
|
||||
// chain doesn't say which chain it is in.
|
||||
func blockHeaders(diff, now, then string) string {
|
||||
if diff == "" {
|
||||
return diff
|
||||
}
|
||||
oldBlocks, newBlocks := enclosingBlocks(now), enclosingBlocks(then)
|
||||
lines := strings.SplitAfter(diff, "\n")
|
||||
for i, l := range lines {
|
||||
if !strings.HasPrefix(l, "@@ ") {
|
||||
continue
|
||||
}
|
||||
// "@@ -a,b +c,d @@": the new side's start, or the old side's
|
||||
// when nothing of the hunk is left.
|
||||
f := strings.Fields(l)
|
||||
if len(f) < 4 || f[3] != "@@" {
|
||||
continue
|
||||
}
|
||||
block := blockAt(newBlocks, f[2])
|
||||
if block == "" {
|
||||
block = blockAt(oldBlocks, f[1])
|
||||
}
|
||||
if block != "" {
|
||||
lines[i] = strings.TrimRight(l, "\n") + " " + block + "\n"
|
||||
}
|
||||
}
|
||||
return strings.Join(lines, "")
|
||||
}
|
||||
|
||||
// enclosingBlocks gives each line of a rendered ruleset the block of the
|
||||
// table it is in ("chain input"), "" outside one. A block opens with a
|
||||
// line of one tab ending in "{" and closes with "\t}".
|
||||
func enclosingBlocks(text string) []string {
|
||||
var out []string
|
||||
cur := ""
|
||||
for _, l := range strings.Split(text, "\n") {
|
||||
if strings.HasPrefix(l, "\t") && !strings.HasPrefix(l, "\t\t") && strings.HasSuffix(l, "{") {
|
||||
cur = strings.TrimSpace(strings.TrimSuffix(l, "{"))
|
||||
}
|
||||
out = append(out, cur)
|
||||
if l == "\t}" {
|
||||
cur = ""
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// blockAt is the block at a hunk side's start ("+40,8" or "-40"); lines
|
||||
// count from 1, and an empty side (",0") sits after its line.
|
||||
func blockAt(blocks []string, side string) string {
|
||||
start, _, _ := strings.Cut(side[1:], ",")
|
||||
n, err := strconv.Atoi(start)
|
||||
if err != nil || n < 1 || n > len(blocks) {
|
||||
return ""
|
||||
}
|
||||
return blocks[n-1]
|
||||
}
|
||||
|
||||
// kindOf maps the API's plural path names to kinds.
|
||||
@@ -616,6 +677,38 @@ func (s *Server) getHistory(w http.ResponseWriter, r *http.Request, p *auth.Prin
|
||||
return nil
|
||||
}
|
||||
|
||||
// historyDiff is the ruleset's diff for one history entry: the
|
||||
// configurations before and after it, got back by stepping back from
|
||||
// the current one through every later entry, rendered by this version.
|
||||
func (s *Server) historyDiff(w http.ResponseWriter, r *http.Request, p *auth.Principal) error {
|
||||
if err := require(p, auth.ResConfig, false); err != nil {
|
||||
return err
|
||||
}
|
||||
id, err := pathID(r, "id")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
hs, err := s.store.HistorySince(r.Context(), id)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if len(hs) == 0 || hs[len(hs)-1].ID != id {
|
||||
return notFound("history entry %d not found", id)
|
||||
}
|
||||
st := s.eng.State()
|
||||
var after *model.State
|
||||
for _, h := range hs {
|
||||
if h.ID == id {
|
||||
after = st.Clone()
|
||||
}
|
||||
if err := engine.Rewind(st, h); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]string{"diff": s.rulesetDiff(st, after)})
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Server) revertHistory(w http.ResponseWriter, r *http.Request, p *auth.Principal) error {
|
||||
if err := require(p, auth.ResConfig, true); err != nil {
|
||||
return err
|
||||
|
||||
@@ -75,6 +75,7 @@ func (s *Server) routes() {
|
||||
m.HandleFunc("GET "+p+"/stats/heatmap", s.authed(s.statsHeatmap))
|
||||
m.HandleFunc("GET "+p+"/history", s.authed(s.listHistory))
|
||||
m.HandleFunc("GET "+p+"/history/{id}", s.authed(s.getHistory))
|
||||
m.HandleFunc("GET "+p+"/history/{id}/diff", s.authed(s.historyDiff))
|
||||
m.HandleFunc("POST "+p+"/history/{id}/revert", s.authed(s.revertHistory))
|
||||
m.HandleFunc("GET "+p+"/probation", s.authed(s.getProbation))
|
||||
m.HandleFunc("POST "+p+"/confirm", s.authed(s.confirmChanges))
|
||||
|
||||
@@ -63,3 +63,21 @@ func (s *Store) GetHistory(ctx context.Context, id int64) (*model.HistoryEntry,
|
||||
}
|
||||
return e, err
|
||||
}
|
||||
|
||||
// HistorySince lists the entries from id on, newest first.
|
||||
func (s *Store) HistorySince(ctx context.Context, id int64) ([]*model.HistoryEntry, error) {
|
||||
rows, err := s.db.QueryContext(ctx, `SELECT `+historyCols+` FROM history WHERE id >= ? ORDER BY id DESC`, id)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
var out []*model.HistoryEntry
|
||||
for rows.Next() {
|
||||
e, err := scanHistory(rows)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out = append(out, e)
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
// History: every configuration change, with undo.
|
||||
import { h, mount, table, time, confirmDialog, modal, errorBox, badge } from "../ui.js";
|
||||
import { h, mount, table, time, confirmDialog, modal, errorBox, badge, diffView } from "../ui.js";
|
||||
import { get, change, can, state } from "../api.js";
|
||||
import { rerender } from "../app.js";
|
||||
|
||||
@@ -29,7 +29,7 @@ async function undo(e) {
|
||||
}
|
||||
|
||||
async function entry(el, id) {
|
||||
const e = await get("/history/" + id);
|
||||
const [e, d] = await Promise.all([get("/history/" + id), get("/history/" + id + "/diff").catch(() => null)]);
|
||||
mount(el,
|
||||
h("nav", { class: "back-link" }, h("a", { href: "#/history" }, "← History")),
|
||||
h("div", { class: "page-head" },
|
||||
@@ -40,7 +40,11 @@ async function entry(el, id) {
|
||||
can("config", true) && !e.reverted_by ? h("div", { class: "actions" }, h("button", { onclick: () => undo(e) }, "Undo")) : null),
|
||||
e.changes.map((c) => h("div", { class: "panel" },
|
||||
h("h3", null, action(c) + " " + c.kind + " ", h("span", { class: "mono" }, c.label)),
|
||||
fieldsTable(c))));
|
||||
fieldsTable(c))),
|
||||
d ? h("div", { class: "panel" },
|
||||
h("h3", null, "Ruleset"),
|
||||
h("p", { class: "muted small" }, "What this change did to the ruleset, rendered by this version of Housecarl."),
|
||||
diffView(d.diff)) : null);
|
||||
}
|
||||
|
||||
function action(c) {
|
||||
@@ -50,9 +54,10 @@ function action(c) {
|
||||
}
|
||||
|
||||
// fieldsTable lists the fields that differ (all of them for a create or
|
||||
// delete).
|
||||
// delete). An object within (settings' builtins) is listed by its own
|
||||
// fields, "builtins.drop_invalid", so only what changed shows.
|
||||
function fieldsTable(c) {
|
||||
const a = c.before || {}, b = c.after || {};
|
||||
const a = flatten(c.before || {}), b = flatten(c.after || {});
|
||||
const keys = [...new Set([...Object.keys(a), ...Object.keys(b)])].sort();
|
||||
const show = (v, k) => {
|
||||
if (v === undefined || v === "") return "–";
|
||||
@@ -68,3 +73,14 @@ function fieldsTable(c) {
|
||||
c.after ? { label: c.before ? "After" : "Value", render: (k) => h("span", { class: "mono small" }, show(b[k], k)) } : null,
|
||||
].filter(Boolean), rows);
|
||||
}
|
||||
|
||||
// flatten lifts the fields of objects within an object to dotted keys
|
||||
// ({builtins: {loopback: true}} → {"builtins.loopback": true}); lists
|
||||
// stay whole.
|
||||
function flatten(o, prefix = "", out = {}) {
|
||||
for (const [k, v] of Object.entries(o)) {
|
||||
if (v && typeof v === "object" && !Array.isArray(v) && Object.keys(v).length) flatten(v, prefix + k + ".", out);
|
||||
else out[prefix + k] = v;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
@@ -409,6 +409,7 @@ input.wide { width: 100%; }
|
||||
.diff .add { background: var(--add); color: var(--add-text); }
|
||||
.diff .del { background: var(--del); color: var(--del-text); }
|
||||
.diff .hunk { background: var(--hunk); color: var(--accent); }
|
||||
.diff .hunk .block { font-weight: 700; color: var(--text); }
|
||||
.diff .file { font-weight: 700; padding-top: 6px; }
|
||||
select.inline { width: auto; display: inline-block; }
|
||||
/* The rule dialog's optional parts: "+" buttons, and ✕ to close one. */
|
||||
|
||||
@@ -200,7 +200,12 @@ export function diffView(text) {
|
||||
if (line.startsWith("+++") || line.startsWith("---")) cls = "file";
|
||||
else if (line.startsWith("+")) cls = "add";
|
||||
else if (line.startsWith("-")) cls = "del";
|
||||
else if (line.startsWith("@@")) cls = "hunk";
|
||||
else if (line.startsWith("@@")) {
|
||||
// "@@ -a,b +c,d @@ chain forward": the chain the hunk is in.
|
||||
const m = line.match(/^(@@ .*? @@)(?: (.+))?$/);
|
||||
box.append(h("div", { class: "hunk" }, m ? [m[1], m[2] ? [" ", h("span", { class: "block" }, m[2])] : null] : line));
|
||||
continue;
|
||||
}
|
||||
box.append(h("div", { class: cls }, line || " "));
|
||||
}
|
||||
return box;
|
||||
|
||||
Reference in new issue
Block a user