From 0795a1001e649efbb6727411b5f898fcf84ad20a Mon Sep 17 00:00:00 2001 From: abrown38 Date: Wed, 7 Oct 2026 15:36:24 +0000 Subject: [PATCH] Fix zone list filtering and BIND warning display; add a views guide - jarl zone list showed only primary zones. zone create binds its --type flag (default "primary") to the same variable as list's --type, so the default leaked into the list filter and hid hint, secondary and forward zones. list now has its own variable. TestListQueries runs list commands against a fake server and checks the filters they send. - named-checkconf/named-checkzone messages are classified by the tool's exit code: on success, everything printed is a warning. Warnings such as 'both "recursion no;" and "allow-recursion" active' were shown as errors. The CLI, jarld check and the web UI now label them as warnings and skip the routine "loaded serial" lines. - The zone_outside_view error points at --adopt-zones. - docs/views.md: step-by-step split DNS for a server that has no views yet, tested on a copy of a stock Ubuntu 26.04 setup. Linked from the README, concepts and validation docs. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 15 +++- README.md | 1 + cmd/jarl/main.go | 19 +++-- cmd/jarl/main_test.go | 41 ++++++++++ cmd/jarl/zones.go | 7 +- cmd/jarld/admin.go | 17 ++-- docs/concepts.md | 3 +- docs/validation.md | 2 +- docs/views.md | 134 +++++++++++++++++++++++++++++++ internal/bind/bind.go | 17 +++- internal/bind/bind_test.go | 19 +++++ internal/validate/zones.go | 2 +- internal/web/static/assets/ui.js | 10 ++- 13 files changed, 261 insertions(+), 26 deletions(-) create mode 100644 docs/views.md create mode 100644 internal/bind/bind_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 56cc0b8..a7a6e7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,16 @@ The first version, to be released as 0.1.0. - Problems printed by `jarld check` show zone names without the trailing dot. - `jarl` now shows the warnings a change introduces when it is applied or staged, not only on dry runs. -- `jarl check` and `jarl resync` print only named-checkconf errors, not its - "loaded serial" lines. +- `jarl check` and `jarl resync` no longer print named-checkconf's routine + "loaded serial" lines, only its errors and warnings. +- `jarl zone list` showed only primary zones: `zone create`'s default + `--type primary` leaked into the list filter through a shared variable. + Hint, secondary, forward and other zones are listed again. +- named-checkconf and named-checkzone output is classified by the tools' + exit code. Their warnings (e.g. `both "recursion no;" and "allow-recursion" + active`) were shown as errors. They are now shown as warnings in the + CLI, `jarld check` and the web UI. +- The `zone_outside_view` error says how to fix it (`--adopt-zones`). ### Added @@ -91,4 +99,5 @@ The first version, to be released as 0.1.0. end-to-end script (`scripts/e2e.sh`) that drives every CLI command against a running named. - **Documentation**: concepts, install and adoption, configuration, CLI, - API, permissions, ACME, validation, operations and development guides. + API, permissions, ACME, validation, operations and development guides, and + a split-DNS guide for adding views to a server that has none. diff --git a/README.md b/README.md index 76369b8..4a2e8e9 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ in `/etc/jarl/jarl.yaml` to reach it from your management network. | [docs/cli.md](docs/cli.md) | The `jarl` command line client | | [docs/api.md](docs/api.md) | REST API reference | | [docs/permissions.md](docs/permissions.md) | Users, tokens, grants and roles | +| [docs/views.md](docs/views.md) | Split DNS: adding views to a server that has none | | [docs/acme.md](docs/acme.md) | Let's Encrypt DNS-01 with certbot and acme.sh | | [docs/validation.md](docs/validation.md) | Every check Jarl makes | | [docs/operations.md](docs/operations.md) | Backups, drift, failures, recovery, monitoring | diff --git a/cmd/jarl/main.go b/cmd/jarl/main.go index dea6f78..a6e6fb6 100644 --- a/cmd/jarl/main.go +++ b/cmd/jarl/main.go @@ -237,15 +237,22 @@ func printProblems(w io.Writer, ps []client.Problem) { } } +// printCheck prints named-checkconf's errors and warnings, but not its +// routine "loaded serial" lines. func printCheck(w io.Writer, ms []client.CheckMessage) { for _, m := range ms { - if m.Error { - obj := "" - if m.Object != "" { - obj = " [" + m.Object + "]" - } - fmt.Fprintf(w, " named-checkconf: %s%s\n", m.Text, obj) + if !m.Error && (strings.HasPrefix(m.Text, "loaded serial") || m.Text == "OK") { + continue } + obj := "" + if m.Object != "" { + obj = " [" + m.Object + "]" + } + label := "named-checkconf" + if !m.Error { + label += " warning" + } + fmt.Fprintf(w, " %s: %s%s\n", label, m.Text, obj) } } diff --git a/cmd/jarl/main_test.go b/cmd/jarl/main_test.go index 7677f3e..501021f 100644 --- a/cmd/jarl/main_test.go +++ b/cmd/jarl/main_test.go @@ -2,6 +2,10 @@ package main import ( "encoding/json" + "io" + "net/http" + "net/http/httptest" + "path/filepath" "testing" "go.yaml.in/yaml/v3" @@ -102,3 +106,40 @@ func TestListValue(t *testing.T) { t.Errorf("listValue = %q", got) } } + +// TestListQueries runs list commands against a fake server and checks the +// filters they send: flags bound to variables shared between subcommands +// must not leak defaults (zone create's --type primary once hid every +// non-primary zone from 'zone list'). +func TestListQueries(t *testing.T) { + var got string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.URL.Path + "?" + r.URL.RawQuery + w.Header().Set("Content-Type", "application/json") + w.Write([]byte("[]")) + })) + defer srv.Close() + t.Setenv("JARL_CLI_CONFIG", filepath.Join(t.TempDir(), "cli.yaml")) + for _, tc := range []struct { + args []string + want string + }{ + {[]string{"zone", "list"}, "/api/v1/zones?"}, + {[]string{"zone", "list", "--type", "hint"}, "/api/v1/zones?type=hint"}, + {[]string{"zone", "list", "--view", "internal"}, "/api/v1/zones?view=internal"}, + {[]string{"changeset", "list"}, "/api/v1/changesets?status=open"}, + {[]string{"option", "list", "--all"}, "/api/v1/options?"}, + } { + got = "" + root := rootCmd() + root.SetArgs(append(tc.args, "--server", srv.URL, "--token", "jarl_test_token", "-o", "json")) + root.SetOut(io.Discard) + if err := root.Execute(); err != nil { + t.Errorf("%v: %v", tc.args, err) + continue + } + if got != tc.want { + t.Errorf("%v sent %q, want %q", tc.args, got, tc.want) + } + } +} diff --git a/cmd/jarl/zones.go b/cmd/jarl/zones.go index a3327f4..7c3cadc 100644 --- a/cmd/jarl/zones.go +++ b/cmd/jarl/zones.go @@ -18,6 +18,9 @@ import ( func zoneCmd() *cobra.Command { cmd := &cobra.Command{Use: "zone", Aliases: []string{"zones"}, Short: "Manage zones"} var view, typ string + // list has its own type filter: create's --type defaults to "primary" + // and would otherwise leak into list through the shared variable. + var listType string list := &cobra.Command{ Use: "list", @@ -28,7 +31,7 @@ func zoneCmd() *cobra.Command { return err } var zones []api.Zone - if err := c.Get(ctx(), "/zones", q("view", view, "type", typ), &zones); err != nil { + if err := c.Get(ctx(), "/zones", q("view", view, "type", listType), &zones); err != nil { return err } return output(zones, func(w io.Writer) { @@ -44,7 +47,7 @@ func zoneCmd() *cobra.Command { }, } list.Flags().StringVar(&view, "view", "", "only zones in this view") - list.Flags().StringVar(&typ, "type", "", "only zones of this type") + list.Flags().StringVar(&listType, "type", "", "only zones of this type") show := &cobra.Command{ Use: "show ZONE", diff --git a/cmd/jarld/admin.go b/cmd/jarld/admin.go index ee3103e..fb32014 100644 --- a/cmd/jarld/admin.go +++ b/cmd/jarld/admin.go @@ -88,13 +88,18 @@ func printResult(res *engine.Result) { printProblems(res.Problems) } for _, m := range res.Check { - if m.Error { - obj := "" - if m.Object != "" { - obj = " [" + m.Object + "]" - } - fmt.Printf(" named-checkconf: %s%s\n", m.Text, obj) + if !m.Error && (strings.HasPrefix(m.Text, "loaded serial") || m.Text == "OK") { + continue } + obj := "" + if m.Object != "" { + obj = " [" + m.Object + "]" + } + label := "named-checkconf" + if !m.Error { + label += " warning" + } + fmt.Printf(" %s: %s%s\n", label, m.Text, obj) } for _, w := range res.Warnings { fmt.Println("warning:", w) diff --git a/docs/concepts.md b/docs/concepts.md index 46b7f3c..f801464 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -133,7 +133,8 @@ RFC 1982 arithmetic. * **Views** are ordered; named uses the first view whose `match-clients` and `match-destinations` match. Once one view exists, named requires every zone to be in a view, so creating the first view offers to move existing - zones into it (`adopt_zones`). + zones into it (`adopt_zones`). See [views.md](views.md) for a split-DNS + walkthrough. * **Zones** have a type: `primary`, `secondary`, `stub`, `static-stub`, `forward`, `mirror`, `hint`, `redirect`, or `in-view` (share a zone from another view). Only primary zones have records in Jarl. The same zone name diff --git a/docs/validation.md b/docs/validation.md index e03db5c..1f70adc 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -52,7 +52,7 @@ showing them. | `wildcard_unusual` | warning | Wildcard NS records | | `soa_invalid`, `soa_warning`, `ttl_invalid` | error / warning | SOA fields and TTLs in range and sensible. One warning covers negative caching longer than 3 hours: resolvers cache "no such name" answers for the smaller of the SOA minimum and the zone's default TTL (RFC 2308), so a new name can stay invisible that long. 1 hour (`--minimum 3600`) is a good value | | `zone_name_invalid`, `zone_type_invalid`, `zone_duplicate` | error | Valid zone names and types. A zone exists once per view | -| `zone_outside_view` | error | Once views exist, every zone must be in one (named requires this) | +| `zone_outside_view` | error | Once views exist, every zone must be in one (named requires this). Create the first view with `--adopt-zones` to move all zones into it ([views.md](views.md)) | | `zone_missing_option` | error | Secondary, stub and mirror zones need `primaries` | | `zone_file_required` / `zone_file_invalid` / `zone_file_managed` | error | Hint zones need a file. Forward zones have none. Primary zone files are Jarl's | | `zone_inview_invalid`, `zone_inview_option` | error | `in-view` zones only inside views, and only with `forward`/`forwarders` | diff --git a/docs/views.md b/docs/views.md new file mode 100644 index 0000000..b4fc626 --- /dev/null +++ b/docs/views.md @@ -0,0 +1,134 @@ +# Split DNS with views + +Views let named answer differently depending on who asks. The usual setup +for a homelab that is also authoritative on the internet has two views: + +* **internal**: your networks get recursion (they use the server as their + resolver) and see private addresses (`nas.adbraun.lol → 10.10.0.20`) and + internal-only zones such as reverse zones for private networks. +* **external**: everyone else gets authoritative answers only. That means + public addresses, no recursion, and no internal zones. + +## How named treats views (and why Jarl asks to move your zones) + +Without any `view` statements, named puts every zone into an implicit +default view. That's why an imported server shows no views. **As soon as +one view exists, named requires every zone to be inside a view.** So the +first view you create has to take over the existing zones, including the +root hints zone `.` that Debian/Ubuntu configure. + +Jarl does this in the same changeset that creates the view: + +* web UI: Views → New view. The box "Move the N zone(s) that are not in a + view into this view" is ticked by default. +* CLI: `jarl view create NAME --adopt-zones` +* API: `"adopt_zones": true` + +Creating the first view without it is refused before anything reaches +named (`zone_outside_view`). + +Views are matched in order: the first view whose `match-clients` (and +`match-destinations`) fit the client wins. Put the specific view first and +the catch-all last. + +## Step by step + +### 1. Check the ACL for your networks + +The internal view matches your networks. Usually that's the ACL you already +use for `allow-recursion`: + +```sh +jarl acl list +jarl acl update homelab --add localhost # if missing; see the note below +``` + +Include **localhost** if the DNS server itself uses `127.0.0.1` as its +resolver. Otherwise the server's own lookups land in the external view and +get no recursion. + +### 2. Create the internal view and move the existing zones into it + +```sh +jarl view create internal --match-clients homelab --adopt-zones --dry-run # look first +jarl view create internal --match-clients homelab --adopt-zones +``` + +``` +create view internal +move zone . into view internal +move zone 0.10.10.in-addr.arpa into view internal +move zone adbraun.lol into view internal +applied (changeset 4) +``` + +Nothing changes for your clients: they match `internal` and get the same +answers as before. The server-wide options (`recursion`, `allow-recursion`, +`forwarders`, ...) still apply, because views inherit them. + +### 3. Create the external view + +```sh +jarl view create external --match-clients any +jarl option set recursion no --view external +jarl option set allow-recursion '{ none; }' --view external +``` + +The inherited `allow-recursion { homelab; }` already keeps outsiders from +recursing. The explicit `recursion no` also stops them reading the cache, +and makes the intent obvious. Setting `allow-recursion { none; }` as well +avoids named-checkconf's warning that `allow-recursion` is still active in a +view with recursion off. Jarl warns (`security_open_resolver`) if a view +ever offers recursion to everyone. + +### 4. Give the external view its zones + +**Different data per view (split DNS).** Copy the zone, then change what +should differ: + +```sh +jarl zone create adbraun.lol --view external --clone-from adbraun.lol --clone-view internal +jarl record list adbraun.lol --view external +jarl record update --data 203.0.113.20 # public address instead of 10.10.0.20 +jarl record delete adbraun.lol nas-internal A --view external +``` + +From then on the two copies are separate zones. A record added to one isn't +in the other, and the serials are separate. + +**The same data in both views.** Use an `in-view` zone. The external view +then serves the internal view's zone, so there is one copy to maintain: + +```sh +jarl zone create example.org --view external --type in-view --in-view internal +``` + +Don't do that for zones with private data. Internal-only zones (reverse +zones for 10.x / 192.168.x) simply don't get an external copy: named +answers outsiders with NXDOMAIN from its built-in empty zones. + +### 5. Check from both sides + +```sh +dig @ns1.adbraun.lol nas.adbraun.lol +short # from the LAN: 10.10.0.20 +dig @ nas.adbraun.lol +short # from outside: 203.0.113.20 +dig @ example.com # from outside: status REFUSED +jarl check +``` + +On the server, `dig -b ` picks which view you test. + +## Living with views + +* Commands that name a zone that exists in several views need `--view` + (`jarl record add adbraun.lol www A ... --view external`). Jarl refuses to + guess. +* `jarl acme` writes challenge records into **every** view that has the + zone, so Let's Encrypt (external) and internal clients see the same + thing. Use `--view` to limit it. +* `jarl view order internal external` changes the match order, and + `jarl view list` shows it. +* Deleting a view is refused while it still has zones, unless you pass + `--cascade`. +* Zone files are stored per view: `/var/lib/bind/jarl/internal/adbraun.lol.zone`, + `/var/lib/bind/jarl/external/adbraun.lol.zone`. diff --git a/internal/bind/bind.go b/internal/bind/bind.go index 17a11ef..3d98575 100644 --- a/internal/bind/bind.go +++ b/internal/bind/bind.go @@ -141,7 +141,20 @@ func (t *Tools) CheckConfig(ctx context.Context, path string, loadZones bool) (* if err != nil { return nil, err } - return &Result{OK: code == 0, Messages: ParseOutput(out), Output: out}, nil + return newResult(out, code), nil +} + +// newResult builds a checker result. The exit code is authoritative: when +// the checker succeeded, whatever it printed is a warning, even if the text +// doesn't say so (e.g. 'both "recursion no;" and "allow-recursion" active'). +func newResult(out string, code int) *Result { + r := &Result{OK: code == 0, Messages: ParseOutput(out), Output: out} + if r.OK { + for i := range r.Messages { + r.Messages[i].Error = false + } + } + return r } // CheckZone runs named-checkzone on a zone file with the same strictness @@ -152,7 +165,7 @@ func (t *Tools) CheckZone(ctx context.Context, zone, file string) (*Result, erro if err != nil { return nil, err } - return &Result{OK: code == 0, Messages: ParseOutput(out), Output: out}, nil + return newResult(out, code), nil } // DumpZone loads a zone file and prints it in canonical form (used by the diff --git a/internal/bind/bind_test.go b/internal/bind/bind_test.go new file mode 100644 index 0000000..3152714 --- /dev/null +++ b/internal/bind/bind_test.go @@ -0,0 +1,19 @@ +package bind + +import "testing" + +func TestResultClassification(t *testing.T) { + // named-checkconf exited 0: everything it printed is a warning. + ok := newResult(`both "recursion no;" and "allow-recursion" active for view external +zone example.com/IN/internal: loaded serial 1 +`, 0) + if !ok.OK || len(ok.Messages) != 2 || ok.Messages[0].Error || ok.Messages[1].Error { + t.Errorf("exit 0: %+v", ok) + } + // Non-zero exit: real problems stay errors. + bad := newResult(`/etc/bind/jarl/named.conf:12: unknown option 'recursoin' +`, 1) + if bad.OK || len(bad.Messages) != 1 || !bad.Messages[0].Error || bad.Messages[0].Line != 12 { + t.Errorf("exit 1: %+v", bad) + } +} diff --git a/internal/validate/zones.go b/internal/validate/zones.go index 995ec27..290121d 100644 --- a/internal/validate/zones.go +++ b/internal/validate/zones.go @@ -43,7 +43,7 @@ func (c *checker) checkZones() { continue } if hasViews && z.ViewID == 0 { - c.zoneProblem(z, Error, "zone_outside_view", "when views are used, every zone must be inside a view (named requires this); move the zone into a view") + c.zoneProblem(z, Error, "zone_outside_view", "when views are used, every zone must be inside a view (named requires this); move the zone into a view, or create the first view with adopt_zones (jarl view create NAME --adopt-zones) to move all zones at once") } if err := dnsname.CheckZoneName(z.Name); err != nil { c.zoneProblem(z, Error, "zone_name_invalid", "invalid zone name: %v", err) diff --git a/internal/web/static/assets/ui.js b/internal/web/static/assets/ui.js index 858477d..b186964 100644 --- a/internal/web/static/assets/ui.js +++ b/internal/web/static/assets/ui.js @@ -193,11 +193,13 @@ export function problemsList(problems) { })); } +// checkList shows named-checkconf's errors and warnings (not its routine +// "loaded serial" lines). export function checkList(msgs) { - const errs = (msgs || []).filter((m) => m.error); - if (!errs.length) return null; - return h("ul", { class: "problems" }, errs.map((m) => - h("li", { class: "error" }, h("strong", null, "named-checkconf: "), m.text, m.object ? h("span", { class: "where" }, m.object) : null))); + const shown = (msgs || []).filter((m) => m.error || !(m.text.startsWith("loaded serial") || m.text === "OK")); + if (!shown.length) return null; + return h("ul", { class: "problems" }, shown.map((m) => + h("li", { class: m.error ? "error" : "warning" }, h("strong", null, m.error ? "named-checkconf: " : "named-checkconf warning: "), m.text, m.object ? h("span", { class: "where" }, m.object) : null))); } export function diffView(text) {