1 Commits
Author SHA1 Message Date
sneak ef6e3d13a1 config: stop startup on an invalid DNS or TLS interval (closes #177)
check / check (push) Successful in 1m8s
DNSWATCHER_DNS_INTERVAL and DNSWATCHER_TLS_INTERVAL were parsed with
time.ParseDuration and silently replaced by the default when that failed,
so a value like 5 or 1d gave hourly checks with no hint why, and zero or
negative values were accepted. Both now go through parseInterval, which
returns an error naming the variable and the value, and startup stops the
same way it does for invalid targets. An unset or empty variable still gets
its default from setupViper. The three tests that pinned the old fallback
are replaced. The README says what a valid value looks like.

Model: opus-5-5
2026-10-01 19:41:53 +00:00
9 changed files with 23 additions and 63 deletions
+2 -5
View File
@@ -34,11 +34,8 @@ COPY . .
# Run the tests - build fails if any test fails # Run the tests - build fails if any test fails
RUN make test RUN make test
# Build the binary. .dockerignore leaves out .git, so `git describe` in # Build the binary
# the Makefile cannot find the version here: script/docker passes it as RUN make build
# --build-arg VERSION, and a build that passes none reports `dev`.
ARG VERSION=dev
RUN make build VERSION="${VERSION}"
# Runtime stage # Runtime stage
# alpine 3.21, 2026-02-28 # alpine 3.21, 2026-02-28
-2
View File
@@ -1,8 +1,6 @@
.PHONY: all bootstrap setup build lint fmt fmt-check test check clean hooks docker .PHONY: all bootstrap setup build lint fmt fmt-check test check clean hooks docker
BINARY := dnswatcher 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 := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
LDFLAGS := -X main.Version=$(VERSION) LDFLAGS := -X main.Version=$(VERSION)
+5 -9
View File
@@ -488,8 +488,7 @@ them. We provide:
- `script/check` — run test, lint, and fmt-check - `script/check` — run test, lint, and fmt-check
- `script/docker` — build the Docker image tagged via `script/projectname`, with - `script/docker` — build the Docker image tagged via `script/projectname`, with
`--no-cache-filter=lint,builder` so the lint stage and the builder stage, `--no-cache-filter=lint,builder` so the lint stage and the builder stage,
which runs the tests, run on every invocation, and with the version from which runs the tests, run on every invocation
`git describe` passed as `--build-arg VERSION`
- `script/cibuild` — CI entrypoint: `docker build` with - `script/cibuild` — CI entrypoint: `docker build` with
`--no-cache-filter=lint,builder`, so the lint stage and the builder stage, `--no-cache-filter=lint,builder`, so the lint stage and the builder stage,
which runs the tests, run on every invocation, because a cached build lints which runs the tests, run on every invocation, because a cached build lints
@@ -511,14 +510,11 @@ make clean # Remove build artifacts
### Build-Time Variables ### Build-Time Variables
`make build` sets the version with `-ldflags "-X main.Version=..."`, taking Version is injected via `-ldflags`:
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.
The Docker image has no `.git`, so the `Dockerfile` takes the version as ```sh
`--build-arg VERSION`. `make docker` passes it; a plain `docker build` go build -ldflags "-X main.Version=$(git describe --tags --always)" ./cmd/dnswatcher
passes none, and that image reports `dev`. ```
--- ---
+6 -7
View File
@@ -21,13 +21,7 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
# Completed Steps # Completed Steps
- 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is - 2026-10-01: a `DNSWATCHER_DNS_INTERVAL` or `DNSWATCHER_TLS_INTERVAL` that is
not a positive duration stops startup; empty means the default (closes #177). not a positive duration stops startup instead of being ignored (closes #177).
- 2026-10-01: the image built by `make docker` reports the `git describe`
version, not `dev`, and the startup log now shows it (closes #109).
- 2026-10-01: two notify shutdown tests always release the delivery they hold,
so a drain that returns early fails them instead of hanging (closes #176).
- 2026-10-01: `script/install-precommit` asks git for the repository's git
directory, so `make hooks` also works where `.git` is a file (closes #129).
- 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every - 2026-10-01: `TODO.md` brought up to date: open issues listed by URL, every
Completed Steps entry cut to at most two lines (closes #146). Completed Steps entry cut to at most two lines (closes #146).
- 2026-10-01: wildcard CORS now applies only to the public routes, not to - 2026-10-01: wildcard CORS now applies only to the public routes, not to
@@ -99,6 +93,7 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
https://git.eeqj.de/sneak/dnswatcher/issues/107 https://git.eeqj.de/sneak/dnswatcher/issues/107
- rate limit on `/metrics` Basic Auth: - rate limit on `/metrics` Basic Auth:
https://git.eeqj.de/sneak/dnswatcher/issues/101 https://git.eeqj.de/sneak/dnswatcher/issues/101
- images report version `dev`: https://git.eeqj.de/sneak/dnswatcher/issues/109
- trial run of the finished image: - trial run of the finished image:
https://git.eeqj.de/sneak/dnswatcher/issues/149 https://git.eeqj.de/sneak/dnswatcher/issues/149
- 1.0 readiness: run it with a real config and read the logs: - 1.0 readiness: run it with a real config and read the logs:
@@ -106,8 +101,12 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104
- `goimports` in `make fmt-check`, Markdown formatting: - `goimports` in `make fmt-check`, Markdown formatting:
https://git.eeqj.de/sneak/dnswatcher/issues/119 https://git.eeqj.de/sneak/dnswatcher/issues/119
- final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114 - final state save at shutdown: https://git.eeqj.de/sneak/dnswatcher/issues/114
- `internal/notify` shutdown tests hang when a drain returns early:
https://git.eeqj.de/sneak/dnswatcher/issues/176
- README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108 - README accuracy sweep: https://git.eeqj.de/sneak/dnswatcher/issues/108
- README sections required by policy: - README sections required by policy:
https://git.eeqj.de/sneak/dnswatcher/issues/173 https://git.eeqj.de/sneak/dnswatcher/issues/173
- `script/install-precommit` in a linked worktree:
https://git.eeqj.de/sneak/dnswatcher/issues/129
- fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138 - fixed root server order: https://git.eeqj.de/sneak/dnswatcher/issues/138
- review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144 - review toward 1.0: https://git.eeqj.de/sneak/dnswatcher/issues/144
-1
View File
@@ -63,7 +63,6 @@ func main() {
return n return n
}, },
), ),
fx.Invoke(func(l *logger.Logger) { l.Identify() }),
fx.Invoke(func(*server.Server, *watcher.Watcher) {}), fx.Invoke(func(*server.Server, *watcher.Watcher) {}),
).Run() ).Run()
} }
+1 -2
View File
@@ -29,8 +29,7 @@ var ErrNoTargets = errors.New(
) )
// ErrInvalidInterval is returned when DNSWATCHER_DNS_INTERVAL or // ErrInvalidInterval is returned when DNSWATCHER_DNS_INTERVAL or
// DNSWATCHER_TLS_INTERVAL is set but is not a positive duration. An empty // DNSWATCHER_TLS_INTERVAL is set to something other than a positive duration.
// value counts as unset and means the default.
var ErrInvalidInterval = errors.New( var ErrInvalidInterval = errors.New(
"interval must be a positive duration such as 30m or 1h", "interval must be a positive duration such as 30m or 1h",
) )
+7 -14
View File
@@ -143,12 +143,6 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
srv := blockingNtfyServer(entered, release, &served) srv := blockingNtfyServer(entered, release, &served)
defer srv.Close() defer srv.Close()
// srv.Close waits for the handler, so release it however the
// test ends; otherwise a drain that returns early hangs the
// package instead of failing this test.
releaseHandler := sync.OnceFunc(func() { close(release) })
defer releaseHandler()
topicURL, _ := url.Parse(srv.URL) topicURL, _ := url.Parse(srv.URL)
svc := notify.NewTestService(http.DefaultTransport) svc := notify.NewTestService(http.DefaultTransport)
@@ -173,7 +167,9 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
// delay alone. // delay alone.
start := time.Now() start := time.Now()
timer := time.AfterFunc(inFlightHold, releaseHandler) timer := time.AfterFunc(inFlightHold, func() {
close(release)
})
defer timer.Stop() defer timer.Stop()
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
@@ -272,7 +268,7 @@ func TestDrainBoundedByContextDeadline(t *testing.T) {
// all never returns here (the delivery is parked in a backoff // all never returns here (the delivery is parked in a backoff
// that never fires), so an unbounded drain must fail this // that never fires), so an unbounded drain must fail this
// test promptly instead of hanging the package until the test // test promptly instead of hanging the package until the test
// binary's -timeout. // binary's 30s timeout.
returned := make(chan struct{}) returned := make(chan struct{})
go func() { go func() {
@@ -451,11 +447,6 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
srv := blockingNtfyServer(entered, release, &served) srv := blockingNtfyServer(entered, release, &served)
defer srv.Close() defer srv.Close()
// As in TestDrainWaitsForInFlightDelivery: release the handler
// however the test ends, before srv.Close waits for it.
releaseHandler := sync.OnceFunc(func() { close(release) })
defer releaseHandler()
lifecycle := &recordingLifecycle{} lifecycle := &recordingLifecycle{}
svc := newNotifyService(t, lifecycle, srv.URL) svc := newNotifyService(t, lifecycle, srv.URL)
@@ -481,7 +472,9 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
t.Fatal("delivery never reached the endpoint") t.Fatal("delivery never reached the endpoint")
} }
timer := time.AfterFunc(inFlightHold, releaseHandler) timer := time.AfterFunc(inFlightHold, func() {
close(release)
})
defer timer.Stop() defer timer.Stop()
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
+1 -9
View File
@@ -12,15 +12,7 @@ ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Own line: a failing command substitution inside an argument does docker build --no-cache-filter=lint,builder -t "$("$SCRIPT_DIR/projectname")" .
# 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.
version="$(git describe --tags --always --dirty 2>/dev/null || true)"
[ -n "$version" ] || version="unknown"
docker build --no-cache-filter=lint,builder \
--build-arg VERSION="$version" \
-t "$("$SCRIPT_DIR/projectname")" .
} }
main "$@" main "$@"
+1 -14
View File
@@ -7,20 +7,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
# Stop if this directory is not the top of its own git checkout, for hook=".git/hooks/pre-commit"
# example a copy inside another repository, whose hook must not be
# replaced.
if [ "$(git rev-parse --show-toplevel)" != "$ROOT" ]; then
echo "install-precommit: $ROOT is not the top of a git checkout" >&2
exit 1
fi
# Ask git for the repository's own git directory: .git is a file, not
# a directory, in some checkouts (for example a clone made with
# --separate-git-dir). core.hooksPath is deliberately not followed, so
# the hook is never written outside this repository.
hooks="$(git rev-parse --git-common-dir)/hooks"
mkdir -p "$hooks"
hook="$hooks/pre-commit"
printf '#!/bin/sh\nset -e\nscript/precommit\n' > "$hook" printf '#!/bin/sh\nset -e\nscript/precommit\n' > "$hook"
chmod +x "$hook" chmod +x "$hook"
echo "pre-commit hook installed: runs script/precommit" echo "pre-commit hook installed: runs script/precommit"