From e38390df7d551f4aa36cf822fbcb2b4639a13537 Mon Sep 17 00:00:00 2001 From: adam Date: Sun, 18 Oct 2026 15:13:01 +0100 Subject: [PATCH] Zone settings: no empty changes, keep an outside name server absolute - A zone update that changes nothing stages nothing: saving a zone's settings unchanged says "Nothing changed" instead of staging an empty change. Staging operations that change nothing is a 400 "nothing to change". - The settings form showed the SOA MNAME without its final dot, which the server reads as relative to the zone, so saving wg.adbraun.lol turned ns1.adbraun.lol. into ns1.adbraun.lol.wg.adbraun.lol. Names outside the zone keep the dot, and MNAME is only sent when edited. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 +++++++++ docs/api.md | 6 ++-- internal/engine/engine.go | 3 ++ internal/engine/engine_test.go | 36 ++++++++++++++++++++++++ internal/engine/expand.go | 20 +++++++++++++ internal/web/static/assets/pages/zone.js | 21 +++++++++++--- 6 files changed, 94 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d6f977..121ed5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,20 @@ All notable changes to Jarl are documented here. The format follows - The New token form only offers templates within what the token's user may do. A token never does more than its user, so offering more only misled. +- A zone update that changes nothing is no change: saving a zone's + settings without changing them says "Nothing changed" instead of + staging an empty change. Staging operations that change nothing + (`POST /changesets/{id}/ops`, or `?stage=`) is a 400 "nothing to + change". + +### Fixed + +- Saving the settings of a zone whose primary name server is outside it + (`ns1.adbraun.lol.` for `wg.adbraun.lol`) changed the name server to + `ns1.adbraun.lol.wg.adbraun.lol.`. The form showed the name without its + final dot, which the server reads as relative to the zone. Names outside + the zone now keep the dot, and the name server is only sent when it was + changed. ## [0.6.0] - 2026-10-13 diff --git a/docs/api.md b/docs/api.md index 4e78a7a..71e337e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -411,7 +411,9 @@ meantime stops it (and gives a `disabled_conflict` warning until then). `PUT /rrsets` makes the RRset exactly the given data: a disabled record with one of the values is enabled, other disabled records are left alone. Disabling a record that is already disabled is a 409; with `missing_ok=1` -(by match) it's "nothing to do". +(by match) it's "nothing to do". A zone update that leaves the zone as it +is (the same SOA, TTL, comment...) changes nothing, so applying it is +"nothing to do" and staging it is "nothing to change". ### Views @@ -510,7 +512,7 @@ written in Jarl's fixed order (docs/concepts.md). | GET | `/changesets` | `?status=open\|applied\|discarded&mine=1&limit=100&before=ID` | | POST | `/changesets` | `{"description": "...", "ops": [...], "apply": false}`: create (and optionally stage or apply) | | GET / PATCH | `/changesets/{id}` | PATCH `{"description": "..."}` | -| POST | `/changesets/{id}/ops` | `{"ops": [Operation...]}`: stage more operations | +| POST | `/changesets/{id}/ops` | `{"ops": [Operation...]}`: stage more operations; 400 "nothing to change" if they change nothing | | DELETE | `/changesets/{id}/ops/{seq}` | remove one staged operation | | GET | `/changesets/{id}/preview` | dry run of an open changeset: problems, check messages, diff | | POST | `/changesets/{id}/apply` | `{"dry_run": false, "force": false}` | diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 98a610b..c85b7ad 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -330,6 +330,9 @@ func (e *Engine) Stage(ctx context.Context, csID int64, reqs []Request, p *auth. if err != nil { return nil, err } + if len(ops) == 0 { + return nil, errf(KindBadRequest, "nothing to change") + } intro := validate.Introduced(before, e.validate(work)) if validate.HasErrors(intro) { return nil, &Error{Kind: KindValidation, Message: describeErrors(intro), Problems: validate.Errors(intro)} diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index cc7f1c3..5c6fe95 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -462,3 +462,39 @@ func TestParseWarningsReachTheResult(t *testing.T) { t.Errorf("ops = %+v", res.Ops) } } + +func TestUnchangedZoneUpdateStagesNothing(t *testing.T) { + env := newEnv(t) + setupZone(env) + env.submit(env.admin, NewRequest(OpZoneCreate, 0, ZoneInput{Name: model.Ptr("wg.example.com")})) + z := env.state().FindZone("wg.example.com.", 0) + if z.SOA.MName != "ns1.example.com." { + env.submit(env.admin, NewRequest(OpZoneUpdate, z.ID, ZoneInput{SOA: &SOAInput{MName: model.Ptr("ns1.example.com.")}})) + z = env.state().FindZone("wg.example.com.", 0) + } + cs, err := env.e.CreateChangeset(env.ctx, env.admin, "web", "") + if err != nil { + t.Fatal(err) + } + // The settings form saved as it was. + same := ZoneInput{Comment: model.Ptr(z.Comment), DefaultTTL: model.Ptr(z.DefaultTTL), SerialPolicy: model.Ptr(z.SerialPolicy), + SOA: &SOAInput{RName: model.Ptr("hostmaster@example.com"), Refresh: model.Ptr(z.SOA.Refresh), Retry: model.Ptr(z.SOA.Retry), + Expire: model.Ptr(z.SOA.Expire), Minimum: model.Ptr(z.SOA.Minimum)}} + if z.SOA.RName != "hostmaster.example.com." { + same.SOA.RName = nil + } + _, err = env.e.Stage(env.ctx, cs.ID, []Request{NewRequest(OpZoneUpdate, z.ID, same)}, env.admin) + if e, ok := AsError(err); !ok || e.Kind != KindBadRequest || e.Message != "nothing to change" { + t.Fatalf("unchanged update: %v", err) + } + if e := env.submitErr(env.admin, NewRequest(OpZoneUpdate, z.ID, same)); e.Message != "nothing to do" { + t.Errorf("unchanged update applied: %+v", e) + } + // A real change still stages, and an absolute MNAME stays absolute. + if _, err := env.e.Stage(env.ctx, cs.ID, []Request{NewRequest(OpZoneUpdate, z.ID, ZoneInput{Comment: model.Ptr("the VPN")})}, env.admin); err != nil { + t.Fatal(err) + } + if got := env.state().FindZone("wg.example.com.", 0).SOA.MName; got != "ns1.example.com." { + t.Errorf("mname = %q", got) + } +} diff --git a/internal/engine/expand.go b/internal/engine/expand.go index 7d83e93..9d8bcfd 100644 --- a/internal/engine/expand.go +++ b/internal/engine/expand.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/netip" + "reflect" "slices" "strings" "time" @@ -193,6 +194,11 @@ func (x *expander) expandOne(req Request) ([]model.Op, error) { if in.LinkWith != "" { return nil, errf(KindBadRequest, "link_with is for new zones; link existing ones with zone.link (jarl zone link)") } + // An update that leaves the zone as it is stages nothing (a + // settings form saved without changes). + if zoneUnchanged(x.st, z, &in) { + return nil, nil + } } return ops, nil case OpRecordCreate: @@ -1555,3 +1561,17 @@ func firstNonEmptyStr(a, b string) string { func notLinked(st *model.State, z *model.Zone) error { return errf(KindBadRequest, "zone %s isn't linked with other views, so there is nothing to share its records with (link it first: jarl zone link)", zoneDesc(st, z)) } + +// zoneUnchanged reports whether an update would leave the zone as it is. +// Errors are left for applying the update to report. +func zoneUnchanged(st *model.State, z *model.Zone, in *ZoneInput) bool { + if len(in.Nameservers) > 0 || len(in.Records) > 0 || in.CloneFrom != 0 || len(in.Options) > 0 { + return false + } + c := st.Clone() + n := c.Zones[z.ID] + if err := setZoneFields(c, n, in, false); err != nil { + return false + } + return reflect.DeepEqual(n, z) +} diff --git a/internal/web/static/assets/pages/zone.js b/internal/web/static/assets/pages/zone.js index ab2a491..1ea8f33 100644 --- a/internal/web/static/assets/pages/zone.js +++ b/internal/web/static/assets/pages/zone.js @@ -804,12 +804,21 @@ async function recordsTab(el, zone, query) { // ---- settings +// inZone reports whether an absolute name is the zone or inside it. +function inZone(name, zoneName) { + if (!name) return true; + return name === zoneName || name.endsWith("." + zoneName); +} + async function settingsTab(el, zone) { const views = await get("/views"); const err = h("div"); const rows = []; const s = zone.soa || {}; - const mname = input({ value: display(s.mname), class: "mono" }); + // The server reads a name without a final dot as relative to the zone, + // so a name outside it keeps its dot. + const mnameText = inZone(s.mname, zone.name) ? display(s.mname) : (s.mname || ""); + const mname = input({ value: mnameText, class: "mono" }); const rname = input({ value: zone.rname_email || emailOf(s.rname) }); const refresh = input({ type: "number", value: s.refresh }); const retry = input({ type: "number", value: s.retry }); @@ -823,7 +832,7 @@ async function settingsTab(el, zone) { const autoPTR = checkbox("Keep PTR records in step with the A and AAAA records", !!zone.auto_ptr); if (zone.type === "primary") { rows.push( - field("Primary name server", mname, "SOA MNAME."), + field("Primary name server", mname, "SOA MNAME. A name outside " + zone.display_name + " ends with '.'; without it, the name is taken as relative to the zone."), field("Contact", rname, "SOA RNAME, as an email address."), field("Refresh", refresh, "Seconds between secondaries checking for updates."), field("Retry", retry, "Seconds before a failed refresh is retried."), @@ -841,7 +850,8 @@ async function settingsTab(el, zone) { mount(err); const body = { comment: comment.value }; if (zone.type === "primary") { - body.soa = { mname: mname.value.trim(), rname: rname.value.trim(), refresh: +refresh.value, retry: +retry.value, expire: +expire.value, minimum: +minimum.value }; + body.soa = { rname: rname.value.trim(), refresh: +refresh.value, retry: +retry.value, expire: +expire.value, minimum: +minimum.value }; + if (mname.value.trim() !== mnameText) body.soa.mname = mname.value.trim(); body.default_ttl = +ttl.value; body.serial_policy = policy.value; if (!!zone.auto_ptr !== autoPTR.input.checked) body.auto_ptr = autoPTR.input.checked; @@ -852,7 +862,10 @@ async function settingsTab(el, zone) { const r = await change("PATCH", "/zones/" + zone.id, body); toast(describeResult(r)); rerender(); - } catch (e) { mount(err, errorBox(e)); } + } catch (e) { + if (e.status === 400 && /^nothing to (change|do)/.test(e.message)) toast("Nothing changed"); + else mount(err, errorBox(e)); + } }; mount(el, h("div", { class: "panel" }, h("div", { class: "form-grid" }, rows), err, h("div", { class: "actions", style: { marginTop: "12px" } }, h("button", { class: "primary", onclick: save }, "Save settings"))));