Compare commits
1 Commits
fix/115-ci
...
remove-dns
| Author | SHA1 | Date | |
|---|---|---|---|
| a535ae864b |
21
Dockerfile
21
Dockerfile
@@ -15,25 +15,8 @@ RUN go mod download
|
|||||||
|
|
||||||
COPY . .
|
COPY . .
|
||||||
|
|
||||||
# Run all checks - build fails if any check fails.
|
# Run all checks - build fails if any check fails
|
||||||
#
|
RUN make check
|
||||||
# CHECK_EPOCH is a cache-busting build argument. Without it, an
|
|
||||||
# unchanged tree leaves this layer's cache key identical and Docker
|
|
||||||
# serves the previous verdict instead of re-running the suite, so the
|
|
||||||
# build reports a green it did not earn. A build argument's value
|
|
||||||
# participates in the cache key of later instructions in the stage even
|
|
||||||
# when they do not reference it, so a fresh value busts this layer
|
|
||||||
# either way. It is expanded into the command deliberately: that makes
|
|
||||||
# the invalidation a property of the command string itself rather than
|
|
||||||
# of how a given builder treats unreferenced args, and it surfaces the
|
|
||||||
# epoch in the build log as a diagnostic.
|
|
||||||
#
|
|
||||||
# Placing the ARG here and nowhere earlier keeps everything above it
|
|
||||||
# (toolchain install, go mod download) cached, so only the check and the
|
|
||||||
# steps after it re-run. script/cibuild passes a fresh value per run; a
|
|
||||||
# plain `docker build` without it caches as before.
|
|
||||||
ARG CHECK_EPOCH
|
|
||||||
RUN echo "check epoch: ${CHECK_EPOCH}" && make check
|
|
||||||
|
|
||||||
# Build the binary
|
# Build the binary
|
||||||
RUN make build
|
RUN make build
|
||||||
|
|||||||
@@ -393,10 +393,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/docker` — build the Docker image tagged via
|
||||||
`script/projectname`
|
`script/projectname`
|
||||||
- `script/cibuild` — CI entrypoint: `docker build .` with a fresh
|
- `script/cibuild` — CI entrypoint: plain `docker build .`
|
||||||
`CHECK_EPOCH` build argument, so the Dockerfile's `make check` layer
|
|
||||||
is never served from the cache and a green build always means the
|
|
||||||
checks ran on this invocation
|
|
||||||
- `script/precommit` — run by the git pre-commit hook; `go mod tidy`
|
- `script/precommit` — run by the git pre-commit hook; `go mod tidy`
|
||||||
guard, then `script/check`
|
guard, then `script/check`
|
||||||
- `script/install-precommit` — install the git pre-commit hook
|
- `script/install-precommit` — install the git pre-commit hook
|
||||||
|
|||||||
15
TESTING.md
15
TESTING.md
@@ -2,8 +2,10 @@
|
|||||||
|
|
||||||
## DNS Resolution Tests
|
## DNS Resolution Tests
|
||||||
|
|
||||||
All resolver tests **MUST** use live queries against real DNS servers.
|
All tests that involve DNS resolution — in every package, including
|
||||||
No mocking of the DNS client layer is permitted.
|
consumers of the resolver such as the watcher — **MUST** use live
|
||||||
|
queries against real DNS servers. No mocking, faking, or stubbing of
|
||||||
|
DNS at any layer is permitted.
|
||||||
|
|
||||||
### Rationale
|
### Rationale
|
||||||
|
|
||||||
@@ -12,6 +14,8 @@ the full delegation chain. Mocked responses cannot faithfully represent
|
|||||||
the variety of real-world DNS behavior (truncation, referrals, glue
|
the variety of real-world DNS behavior (truncation, referrals, glue
|
||||||
records, DNSSEC, varied response times, EDNS, etc.). Testing against
|
records, DNSSEC, varied response times, EDNS, etc.). Testing against
|
||||||
real servers ensures the resolver works correctly in production.
|
real servers ensures the resolver works correctly in production.
|
||||||
|
Robustness comes from handling real-world DNS behavior with tolerant
|
||||||
|
assertions and sensible timeouts, not from mocks.
|
||||||
|
|
||||||
### Constraints
|
### Constraints
|
||||||
|
|
||||||
@@ -24,11 +28,14 @@ real servers ensures the resolver works correctly in production.
|
|||||||
- Flaky failures from transient network issues are acceptable and
|
- Flaky failures from transient network issues are acceptable and
|
||||||
should be investigated as potential resolver bugs, not papered over
|
should be investigated as potential resolver bugs, not papered over
|
||||||
with mocks or skip flags
|
with mocks or skip flags
|
||||||
|
- Watcher change-detection tests seed a synthetic *previous state*
|
||||||
|
and compare it against fresh live lookups; the DNS side is never
|
||||||
|
faked
|
||||||
|
|
||||||
### What NOT to do
|
### What NOT to do
|
||||||
|
|
||||||
- **Do not mock `DNSClient`** for resolver tests (the mock constructor
|
- **Do not mock `DNSClient`**, the watcher's `DNSResolver` interface,
|
||||||
exists for unit-testing other packages that consume the resolver)
|
or any other DNS abstraction — in any package, for any reason
|
||||||
- **Do not add `-short` flags** to skip slow tests
|
- **Do not add `-short` flags** to skip slow tests
|
||||||
- **Do not increase `-timeout`** to hide hanging queries
|
- **Do not increase `-timeout`** to hide hanging queries
|
||||||
- **Do not modify linter configuration** to suppress findings
|
- **Do not modify linter configuration** to suppress findings
|
||||||
|
|||||||
34
TODO.md
34
TODO.md
@@ -14,7 +14,10 @@ pre-1.0. No git tags. Core resolver work in flight on feature/resolver
|
|||||||
(dirty: internal/resolver/resolver_test.go). Local checkout has diverged
|
(dirty: internal/resolver/resolver_test.go). Local checkout has diverged
|
||||||
from origin: origin/main is 8 commits ahead (watcher orchestrator,
|
from origin: origin/main is 8 commits ahead (watcher orchestrator,
|
||||||
unified TARGETS) and origin/feature/resolver already contains the full
|
unified TARGETS) and origin/feature/resolver already contains the full
|
||||||
iterative resolver implementation with hermetic mocked tests.
|
iterative resolver implementation. DNS mocking is banned in this repo
|
||||||
|
(see `TESTING.md`): all tests use live DNS only. The hermetic mocked
|
||||||
|
tests previously noted on `feature/resolver` are gone from its current
|
||||||
|
tip, which carries a live-DNS suite against `*.dns.sneak.cloud`.
|
||||||
|
|
||||||
# Next Step
|
# Next Step
|
||||||
|
|
||||||
@@ -25,16 +28,10 @@ confirm make check still passes.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-08-09: `script/cibuild` can no longer report a green it did not
|
- 2026-08-07: DNS mocking removed from the entire test suite; watcher
|
||||||
earn. The Dockerfile declares `ARG CHECK_EPOCH` immediately above the
|
tests now drive the real iterative resolver against live DNS and
|
||||||
check step and expands it into the `RUN` command, and `script/cibuild`
|
`TESTING.md` bans DNS mocks in every package (`remove-dns-mocking`
|
||||||
passes a fresh `$(date +%s%N)` per invocation, so the `make check`
|
branch)
|
||||||
layer is always re-executed while the pinned toolchain install and
|
|
||||||
`go mod download` stay cached. Verified by experiment: before the fix
|
|
||||||
a second run on an unchanged tree returned in 283 ms with the check
|
|
||||||
layer `CACHED`; after it the check runs every time, and a deliberately
|
|
||||||
planted always-failing test made the build fail with exactly that
|
|
||||||
test's message
|
|
||||||
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
||||||
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
||||||
org-standard v2-schema config used across the org's repos
|
org-standard v2-schema config used across the org's repos
|
||||||
@@ -46,7 +43,8 @@ confirm make check still passes.
|
|||||||
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
|
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
|
||||||
Makefile shims, README Entrypoints section
|
Makefile shims, README Entrypoints section
|
||||||
- 2026-02-20: iterative DNS resolver implemented; tests made hermetic
|
- 2026-02-20: iterative DNS resolver implemented; tests made hermetic
|
||||||
with mocked DNS (origin/feature/resolver, unmerged)
|
with mocked DNS (origin/feature/resolver, unmerged; superseded — DNS
|
||||||
|
mocking is banned, see `TESTING.md`)
|
||||||
- 2026-02-20: CI actions and go install refs pinned to commit SHAs;
|
- 2026-02-20: CI actions and go install refs pinned to commit SHAs;
|
||||||
Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
|
Gitea Actions workflow for make check (origin/ci/make-check, unmerged)
|
||||||
- 2026-02-20: watcher monitoring orchestrator merged to main (#8)
|
- 2026-02-20: watcher monitoring orchestrator merged to main (#8)
|
||||||
@@ -73,8 +71,9 @@ Branch reconciliation:
|
|||||||
- Sync local checkout with origin: local main is 8 commits behind
|
- Sync local checkout with origin: local main is 8 commits behind
|
||||||
origin/main; local feature/resolver has diverged from
|
origin/main; local feature/resolver has diverged from
|
||||||
origin/feature/resolver, which already implements the resolver
|
origin/feature/resolver, which already implements the resolver
|
||||||
- Merge in-flight branches to main once green: feature/resolver,
|
- Merge in-flight branches to main once green: feature/resolver
|
||||||
ci/make-check, feature/portcheck-implementation,
|
(confirm its tests remain live-DNS — DNS mocking is banned, see
|
||||||
|
`TESTING.md`), ci/make-check, feature/portcheck-implementation,
|
||||||
feature/tlscheck-implementation
|
feature/tlscheck-implementation
|
||||||
|
|
||||||
Resolver (plan from untracked TODO.md; largely implemented on
|
Resolver (plan from untracked TODO.md; largely implemented on
|
||||||
@@ -158,6 +157,7 @@ Infrastructure notes (from untracked TODO.md):
|
|||||||
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
|
- Module path sneak.berlin/go/dnswatcher differs from the git.eeqj.de
|
||||||
remote intentionally; do not "fix" it
|
remote intentionally; do not "fix" it
|
||||||
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
|
- Dependencies: github.com/miekg/dns, golang.org/x/net/publicsuffix
|
||||||
- Resolver tests originally used live DNS against *.dns.sneak.cloud
|
- Resolver tests originally used live DNS against `*.dns.sneak.cloud`
|
||||||
(required records documented in the test file header); origin now has
|
(required records documented in the test file header); `main` now
|
||||||
mocked hermetic tests, keep them hermetic
|
tests against live public DNS. DNS mocking is banned (see
|
||||||
|
`TESTING.md`); never reintroduce hermetic mocked DNS tests
|
||||||
|
|||||||
@@ -7,8 +7,8 @@ import (
|
|||||||
"github.com/miekg/dns"
|
"github.com/miekg/dns"
|
||||||
)
|
)
|
||||||
|
|
||||||
// DNSClient abstracts DNS wire-protocol exchanges so the resolver
|
// DNSClient abstracts DNS wire-protocol exchanges over a single
|
||||||
// can be tested without hitting real nameservers.
|
// transport, letting the resolver switch between UDP and TCP.
|
||||||
type DNSClient interface {
|
type DNSClient interface {
|
||||||
ExchangeContext(
|
ExchangeContext(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
|
|||||||
@@ -67,17 +67,4 @@ func NewFromLogger(log *slog.Logger) *Resolver {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewFromLoggerWithClient creates a Resolver with a custom DNS
|
|
||||||
// client, useful for testing with mock DNS responses.
|
|
||||||
func NewFromLoggerWithClient(
|
|
||||||
log *slog.Logger,
|
|
||||||
client DNSClient,
|
|
||||||
) *Resolver {
|
|
||||||
return &Resolver{
|
|
||||||
log: log,
|
|
||||||
client: client,
|
|
||||||
tcp: client,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Method implementations are in iterative.go.
|
// Method implementations are in iterative.go.
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/miekg/dns"
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
@@ -624,57 +623,40 @@ func TestQueryAllNameservers_ContextCanceled(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ----------------------------------------------------------------
|
// ----------------------------------------------------------------
|
||||||
// Timeout tests
|
// Unreachable nameserver tests
|
||||||
// ----------------------------------------------------------------
|
// ----------------------------------------------------------------
|
||||||
|
|
||||||
func TestQueryNameserverIP_Timeout(t *testing.T) {
|
func TestQueryNameserverIP_UnreachableServer(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
log := slog.New(slog.NewTextHandler(
|
r := newTestResolver(t)
|
||||||
os.Stderr,
|
|
||||||
&slog.HandlerOptions{Level: slog.LevelDebug},
|
|
||||||
))
|
|
||||||
|
|
||||||
r := resolver.NewFromLoggerWithClient(
|
|
||||||
log, &timeoutClient{},
|
|
||||||
)
|
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(
|
ctx, cancel := context.WithTimeout(
|
||||||
context.Background(), 10*time.Second,
|
context.Background(), 10*time.Second,
|
||||||
)
|
)
|
||||||
t.Cleanup(cancel)
|
t.Cleanup(cancel)
|
||||||
|
|
||||||
// Query any IP — the client always returns a timeout error.
|
// 192.0.2.1 is an RFC 5737 documentation address: no
|
||||||
|
// nameserver can exist there. Depending on the network
|
||||||
|
// path the queries either time out (silent drop) or fail
|
||||||
|
// fast (ICMP unreachable), so accept any non-OK status;
|
||||||
|
// the resolver must return a classified response with no
|
||||||
|
// records rather than an error or a hang.
|
||||||
resp, err := r.QueryNameserverIP(
|
resp, err := r.QueryNameserverIP(
|
||||||
ctx, "unreachable.test.", "192.0.2.1",
|
ctx, "unreachable.test.", "192.0.2.1",
|
||||||
"example.com",
|
"example.com",
|
||||||
)
|
)
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
assert.Equal(t, resolver.StatusTimeout, resp.Status)
|
assert.NotEqual(t, resolver.StatusOK, resp.Status)
|
||||||
assert.NotEmpty(t, resp.Error)
|
|
||||||
|
totalRecords := 0
|
||||||
|
for _, values := range resp.Records {
|
||||||
|
totalRecords += len(values)
|
||||||
}
|
}
|
||||||
|
|
||||||
// timeoutClient simulates DNS timeout errors for testing.
|
assert.Zero(t, totalRecords)
|
||||||
type timeoutClient struct{}
|
|
||||||
|
|
||||||
func (c *timeoutClient) ExchangeContext(
|
|
||||||
_ context.Context,
|
|
||||||
_ *dns.Msg,
|
|
||||||
_ string,
|
|
||||||
) (*dns.Msg, time.Duration, error) {
|
|
||||||
return nil, 0, &net.OpError{
|
|
||||||
Op: "read",
|
|
||||||
Net: "udp",
|
|
||||||
Err: &timeoutError{},
|
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
type timeoutError struct{}
|
|
||||||
|
|
||||||
func (e *timeoutError) Error() string { return "i/o timeout" }
|
|
||||||
func (e *timeoutError) Timeout() bool { return true }
|
|
||||||
func (e *timeoutError) Temporary() bool { return true }
|
|
||||||
|
|
||||||
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
func TestResolveIPAddresses_ContextCanceled(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
@@ -1,18 +1,13 @@
|
|||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
# script/cibuild: run the CI build. The Dockerfile runs make check, and
|
# script/cibuild: run the CI build. The Dockerfile runs make check, so
|
||||||
# the CHECK_EPOCH build argument below is fresh on every invocation, so
|
# a successful build implies all checks pass.
|
||||||
# the check layer is never served from the Docker layer cache: a
|
|
||||||
# successful build means the checks were executed and passed on this
|
|
||||||
# run, not on some earlier one. Only the check step and the steps after
|
|
||||||
# it are invalidated; the toolchain install and go mod download stay
|
|
||||||
# cached.
|
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||||
|
|
||||||
main() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
docker build --build-arg CHECK_EPOCH="$(date +%s%N)" .
|
docker build .
|
||||||
}
|
}
|
||||||
|
|
||||||
main "$@"
|
main "$@"
|
||||||
|
|||||||
Reference in New Issue
Block a user