docker: a plain docker build . stamps the git version (closes #210)

A plain `docker build .`, which is how upaas builds, stamped `dev`:
`.dockerignore` left out `.git` and the builder declared
`ARG VERSION=dev`. `.dockerignore` now sends `.git` without
`.git/config`, which can hold a credential, and lists no tracked file,
which git in the build would count as deleted and mark `-dirty`.
`ARG VERSION` has no default. The Makefile takes a non-empty `VERSION`
from the command line or the environment, so a build arg still wins
(`script/docker` keeps passing one); otherwise `git describe` runs in
the builder. A new `make version` prints the version, and the builder
fails when the context carries `.git`, directory or file, and it comes
out empty, `dev` or `unknown`.

Model: opus-5-5

.dockerignore

@@ -1,9 +1,9 @@
-.git/
+# .git is sent, without its config: the builder stage derives the version it
+# stamps into the binary from it, and `git describe` does not need the config,
+# which can hold a credential (a password in the remote URL, a CI token). No
+# tracked file may be listed here: git in the build would see it as deleted
+# and mark the version -dirty, and an excluded .md would silently drop out of
+# the prettier check in Dockerfile.fmt.
+.git/config
 bin/
 node_modules/
-# No .md may be excluded: Dockerfile.fmt checks every document with
-# prettier, and an exclusion here would drop a file from that check while
-# prettier still reports every file it was handed clean.
-LICENSE
-.editorconfig
-.gitignore

Dockerfile

@@ -36,11 +36,26 @@ COPY . .
 # Run the tests - build fails if any test fails
 RUN make test
 
-# Build the binary. .dockerignore leaves out .git, so `git describe` in
-# the Makefile cannot find the version here: script/docker passes it as
-# --build-arg VERSION, and a build that passes none reports `dev`.
-ARG VERSION=dev
-RUN make build VERSION="${VERSION}"
+# Version stamped into the binary: the VERSION build arg when one is
+# given and not empty (script/docker passes one), otherwise what
+# `git describe` says of the .git in the build context, so a plain
+# `docker build .` of a clone stamps its tag or short commit. The build
+# arg reaches make through the environment.
+ARG VERSION
+
+# A context that carries .git, as a directory or as a file, must yield a
+# real version: one that is empty, `dev` or `unknown` cannot be traced
+# back to a commit.
+RUN version="$(make version)"; \
+    if [ -e .git ]; then \
+        case "$version" in \
+            "" | dev | unknown) \
+                echo "version is \"$version\" although the build context carries .git" >&2; \
+                exit 1 ;; \
+        esac; \
+    fi
+
+RUN make build
 
 # Runtime stage
 # alpine 3.21, 2026-02-28

Dockerfile.fmt

@@ -30,9 +30,8 @@ COPY . .
 # --config, not discovery: a .prettierrc that failed to arrive would
 # otherwise leave prettier on its defaults, where proseWrap is "preserve"
 # and every wrap this check exists to enforce passes. Missing the file is
-# a hard error instead. --no-editorconfig for the same reason in reverse:
-# .editorconfig is not in the build context, so honouring it here and on
-# a developer's machine would be two different answers.
+# a hard error instead. --no-editorconfig so that .prettierrc alone sets
+# the style.
 RUN prettier --config .prettierrc --no-editorconfig --check "**/*.md"
 
 # Write path. Not a check: script/fmt builds this and takes the files.

Makefile

@@ -1,9 +1,13 @@
-.PHONY: all bootstrap setup build lint fmt fmt-check test check clean hooks docker
+.PHONY: all bootstrap setup build version lint fmt fmt-check test check clean hooks docker
 
 BINARY := dnswatcher
-# `make build VERSION=...` overrides this; the Dockerfile does so, as the
-# image has no .git to describe.
-VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
+# VERSION given on the command line (`make build VERSION=...`) or in the
+# environment, which is how the Dockerfile's VERSION build arg arrives,
+# wins over what `git describe` says of this checkout. An empty one counts
+# as not given; `override` is what replaces an empty command-line value.
+ifeq ($(VERSION),)
+override VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
+endif
 LDFLAGS := -X main.Version=$(VERSION)
 
 # Standard targets are thin shims; the implementations live in script/
@@ -21,6 +25,10 @@ setup:
 build:
 	go build -ldflags "$(LDFLAGS)" -o bin/$(BINARY) ./cmd/dnswatcher
 
+# Prints the version `make build` stamps; the Dockerfile checks it.
+version:
+	@echo "$(VERSION)"
+
 test:
 	@script/test
 

README.md

@@ -407,6 +407,13 @@ it watches. Instead, it performs full iterative resolution:
 4. **Authoritative query**: Queries all discovered authoritative nameservers
    directly for the requested records.
 
+In steps 2 and 3 the servers are asked one at a time in a random order, chosen
+anew each time, so no one root server gets every first query. A server that does
+not reply, refuses the query, or gives an error reply such as SERVFAIL or a
+referral that leads no closer to the name is passed over for the next one. When
+a referral names a zone's nameservers without their addresses, the addresses of
+all of them are looked up, so that each can be asked.
+
 This approach ensures:
 
 - Independence from any upstream resolver's cache or filtering.
@@ -567,6 +574,7 @@ provide:
 
 ```sh
 make build      # Build binary to bin/dnswatcher
+make version    # Print the version make build stamps
 make test       # Run tests with race detector
 make lint       # Run golangci-lint in Docker (requires docker)
 make fmt        # Format code and Markdown (requires docker)
@@ -577,13 +585,21 @@ make clean      # Remove build artifacts
 ### Build-Time Variables
 
 `make build` sets the version with `-ldflags "-X main.Version=..."`, taking it
-from `git describe --tags --always --dirty`, or from `VERSION` when given on the
-command line (`make build VERSION=1.2.3`). The version appears in the startup
-log and in the health check response.
+from `VERSION` when given on the command line (`make build VERSION=1.2.3`) or in
+the environment, otherwise from `git describe --tags --always --dirty`, and
+`dev` without git metadata. An empty `VERSION` counts as not given. The version
+appears in the startup log and in the health check response.
 
-The Docker image has no `.git`, so the `Dockerfile` takes the version as
-`--build-arg VERSION`. `make docker` passes it; a plain `docker build` passes
-none, and that image reports `dev`.
+The image takes it the same way, from the `.git` the build context carries, so a
+plain `docker build .` of a clone stamps the commit it was built from; a clone
+without tags, such as a shallow one, stamps the short commit. `.dockerignore`
+sends `.git` without `.git/config`, which `git describe` does not need and which
+can hold a credential. A non-empty `--build-arg VERSION=...` takes precedence;
+`make docker` passes the version `git describe` gives on the host. The build
+fails when the context carries `.git`, as a directory or as a file, and the
+version comes out empty, `dev` or `unknown`. `.dockerignore` must list no
+tracked file: git in the build would see it as deleted and mark the version
+`-dirty`.
 
 ---
 

TODO.md

@@ -19,6 +19,10 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 # Completed Steps
 
+- 2026-10-02: a plain `docker build .` of a clone stamps its tag or short
+  commit, not `dev`: the build context now carries `.git` (closes #210).
+- 2026-10-02: the resolver tries root servers, and every other server list it
+  walks, in a random order each time, not always from the top (closes #138).
 - 2026-10-02: a name listed more than once in `DNSWATCHER_TARGETS`, in any
   letter case or with a trailing dot, is watched once (closes #207).
 - 2026-10-01: README checked against the code and corrected: metrics, CORS,
@@ -131,5 +135,4 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149
 
 - 1.0 readiness: run it with a real config and read the logs:
   https://git.eeqj.de/sneak/dnswatcher/issues/66
-- fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138
 - review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144

internal/livednstest/livednstest.go

@@ -9,11 +9,11 @@
 //
 //  1. Bounded concurrency. Tests run in parallel and the build hosts
 //     have many cores, so without a limit every test starts its own
-//     iterative resolution at the same instant and they all hit the
-//     first root server within a few milliseconds of each other. Root
-//     servers rate-limit that, which shows up as a different arbitrary
-//     subset of tests failing on each run. Run caps how many live
-//     operations are in flight at once in one test binary.
+//     iterative resolution at the same instant and they all send their
+//     first queries to the root servers within a few milliseconds of
+//     each other. Root servers rate-limit that, which shows up as a
+//     different arbitrary subset of tests failing on each run. Run caps
+//     how many live operations are in flight at once in one test binary.
 //
 //  2. Retry with exponential backoff. Each live operation gets several
 //     attempts with its own timeout. An attempt is retried when it

internal/resolver/export_test.go

@@ -36,3 +36,24 @@ func (r *Resolver) QueryEachNS(
 ) (map[string]*NameserverResponse, error) {
 	return r.queryEachNS(ctx, nameservers, hostname)
 }
+
+// ResolveNSIPs exports resolveNSIPs for testing.
+func (r *Resolver) ResolveNSIPs(
+	ctx context.Context,
+	nsNames []string,
+) []string {
+	return r.resolveNSIPs(ctx, nsNames)
+}
+
+// RootServerList exports rootServerList for testing.
+func RootServerList() []string {
+	return rootServerList()
+}
+
+// Shuffled exports shuffled for testing.
+func Shuffled(
+	servers []string,
+	shuffle func(n int, swap func(i, j int)),
+) []string {
+	return shuffled(servers, shuffle)
+}

internal/resolver/iterative.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/rand/v2"
 	"net"
+	"slices"
 	"sort"
 	"strings"
 	"time"
@@ -259,9 +261,25 @@ func (r *Resolver) followDelegation(
 	return nil, ErrNoNameservers
 }
 
-// queryServers asks servers, the servers of zone, about name until one
-// gives a usable reply. A server that times out, refuses or gives a
-// reply that is not usable is passed over for the next.
+// shuffled returns a copy of servers in the order shuffle puts them
+// in. The resolver passes rand.Shuffle, so each time it walks a list of
+// servers it starts at a random one, and no one server gets every
+// first query.
+func shuffled(
+	servers []string,
+	shuffle func(n int, swap func(i, j int)),
+) []string {
+	order := slices.Clone(servers)
+	shuffle(len(order), func(i, j int) {
+		order[i], order[j] = order[j], order[i]
+	})
+
+	return order
+}
+
+// queryServers asks servers, the servers of zone, about name in a random
+// order until one gives a usable reply. A server that times out, refuses
+// or gives a reply that is not usable is passed over for the next.
 func (r *Resolver) queryServers(
 	ctx context.Context,
 	servers []string,
@@ -271,7 +289,7 @@ func (r *Resolver) queryServers(
 ) (*dns.Msg, error) {
 	var lastErr error
 
-	for _, ip := range servers {
+	for _, ip := range shuffled(servers, rand.Shuffle) {
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
@@ -340,6 +358,10 @@ func nsSetFrom(resp *dns.Msg, domain string) []string {
 	return extractNSSet(resp.Answer)
 }
 
+// resolveNSIPs returns the addresses of every nameserver in nsNames
+// whose name resolves, for a referral that carries none. The walk can
+// then go on to the zone's other nameservers when one gives no usable
+// reply.
 func (r *Resolver) resolveNSIPs(
 	ctx context.Context,
 	nsNames []string,
@@ -351,10 +373,6 @@ func (r *Resolver) resolveNSIPs(
 		if err == nil {
 			ips = append(ips, resolved...)
 		}
-
-		if len(ips) > 0 {
-			break
-		}
 	}
 
 	return ips

internal/resolver/iterative_test.go

@@ -1,6 +1,8 @@
 package resolver_test
 
 import (
+	"math/rand/v2"
+	"slices"
 	"testing"
 
 	"github.com/miekg/dns"
@@ -235,3 +237,30 @@ func TestExtractRecordValue_LetterCase(t *testing.T) {
 		})
 	}
 }
+
+// TestShuffled shuffles the root servers with many seeds. Every order
+// must hold each root server once, so each is tried before a
+// resolution fails; each root server must come first for some seed, so
+// no one root server gets every first query; and the list passed in
+// must be left as it was.
+func TestShuffled(t *testing.T) {
+	t.Parallel()
+
+	const seeds = 1000
+
+	roots := resolver.RootServerList()
+	before := slices.Clone(roots)
+	first := make(map[string]bool)
+
+	for seed := range uint64(seeds) {
+		rng := rand.New(rand.NewPCG(seed, 0)) //nolint:gosec // seeded on purpose
+		order := resolver.Shuffled(roots, rng.Shuffle)
+
+		assert.ElementsMatch(t, roots, order)
+
+		first[order[0]] = true
+	}
+
+	assert.Len(t, first, len(roots))
+	assert.Equal(t, before, roots)
+}

internal/resolver/livedns_test.go

@@ -383,3 +383,37 @@ func liveResolveIPsAllowingEmpty(
 
 	return out
 }
+
+// liveResolveNSIPs looks up the addresses of the nameservers named
+// names, retrying until there are at least atLeast of them: a name
+// whose lookup got no reply is left out of the result, not an error.
+func liveResolveNSIPs(
+	t *testing.T,
+	r *resolver.Resolver,
+	names []string,
+	atLeast int,
+) []string {
+	t.Helper()
+
+	var out []string
+
+	livednstest.Retry(
+		t,
+		"ResolveNSIPs("+strings.Join(names, ", ")+")",
+		func(ctx context.Context) error {
+			ips := r.ResolveNSIPs(ctx, names)
+			if len(ips) < atLeast {
+				return fmt.Errorf(
+					"%w: %d addresses, expected at least %d",
+					livednstest.ErrNoAnswer, len(ips), atLeast,
+				)
+			}
+
+			out = ips
+
+			return nil
+		},
+	)
+
+	return out
+}

internal/resolver/resolver_test.go

@@ -139,6 +139,28 @@ func TestFindAuthoritativeNameservers_CloudflareDomain(
 	}
 }
 
+// TestResolveNSIPs_EveryNameserver looks up the addresses of two of
+// google.com's nameservers together, as the walk does when a referral
+// names a zone's nameservers without their addresses, and compares them
+// with each looked up alone. Together they must give the addresses of
+// both, not only of the first that resolves, so that when one gives no
+// usable reply the walk goes on to the other.
+func TestResolveNSIPs_EveryNameserver(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	names := []string{"ns3.google.com.", "ns4.google.com."}
+	want := make([]string, 0, len(names))
+
+	for _, name := range names {
+		want = append(want, liveResolveNSIPs(t, r, []string{name}, 1)...)
+	}
+
+	got := liveResolveNSIPs(t, r, names, len(want))
+
+	assert.ElementsMatch(t, want, got)
+}
+
 // ----------------------------------------------------------------
 // QueryNameserver tests
 // ----------------------------------------------------------------

script/docker

@@ -14,8 +14,8 @@ main() {
     cd "$ROOT"
     # Own line: a failing command substitution inside an argument does
     # not trip `set -e`, so the inline form degrades silently to an
-    # empty constant. VERSION is computed here because .dockerignore
-    # excludes .git, so `git describe` in a build stage cannot find it.
+    # empty constant. The VERSION build arg takes precedence over what
+    # the build would derive from the .git in its context.
     version="$(git describe --tags --always --dirty 2>/dev/null || true)"
     [ -n "$version" ] || version="unknown"
     docker build --no-cache-filter=lint,builder \