3 Commits

Author SHA1 Message Date
user
a2819c34d9 fix: skip real-DNS resolver tests in make check, add timeout
All checks were successful
Check / check (pull_request) Successful in 10m9s
Resolver tests perform iterative DNS resolution from root nameservers,
which can hang indefinitely. This broke make check on main.

Changes:
- Add -short and -timeout 30s flags to go test in make check
- Skip real-DNS integration tests when -short is set (via testContext helper)
- Context-canceled tests still run (no network needed)
- Also add -timeout 30s to make test target

Run integration tests explicitly with: go test -v -race ./internal/resolver/...
2026-02-21 02:37:38 -08:00
8cfff5dcc8 Merge pull request 'fix: use full Lock in State.Save() to prevent data race (closes #17)' (#20) from fix/state-save-data-race into main
Some checks failed
Check / check (push) Failing after 5m43s
Reviewed-on: #20
2026-02-21 11:22:46 +01:00
clawbot
b162ca743b fix: use full Lock in State.Save() to prevent data race (closes #17)
Some checks failed
Check / check (pull_request) Failing after 5m31s
State.Save() was using RLock but mutating s.snapshot.LastUpdated,
which is a write operation. This created a data race since other
goroutines could also hold a read lock and observe a partially
written timestamp. Changed to full Lock to ensure exclusive access
during the mutation.
2026-02-21 00:51:58 -08:00
5 changed files with 34 additions and 106 deletions

View File

@@ -18,7 +18,7 @@ fmt:
goimports -w . goimports -w .
test: test:
go test -v -race -cover ./... go test -v -race -cover -timeout 30s ./...
# Check runs all validation without making changes # Check runs all validation without making changes
# Used by CI and Docker build - fails if anything is wrong # Used by CI and Docker build - fails if anything is wrong
@@ -28,7 +28,7 @@ check:
@echo "==> Running linter..." @echo "==> Running linter..."
golangci-lint run --config .golangci.yml ./... golangci-lint run --config .golangci.yml ./...
@echo "==> Running tests..." @echo "==> Running tests..."
go test -v -race ./... go test -v -race -short -timeout 30s ./...
@echo "==> Building..." @echo "==> Building..."
go build -ldflags "$(LDFLAGS)" -o /dev/null ./cmd/dnswatcher go build -ldflags "$(LDFLAGS)" -o /dev/null ./cmd/dnswatcher
@echo "==> All checks passed!" @echo "==> All checks passed!"

View File

@@ -34,8 +34,12 @@ func newTestResolver(t *testing.T) *resolver.Resolver {
func testContext(t *testing.T) context.Context { func testContext(t *testing.T) context.Context {
t.Helper() t.Helper()
if testing.Short() {
t.Skip("skipping integration test requiring real DNS")
}
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 60*time.Second, context.Background(), 15*time.Second,
) )
t.Cleanup(cancel) t.Cleanup(cancel)

View File

@@ -156,8 +156,8 @@ func (s *State) Load() error {
// Save writes the current state to disk atomically. // Save writes the current state to disk atomically.
func (s *State) Save() error { func (s *State) Save() error {
s.mu.RLock() s.mu.Lock()
defer s.mu.RUnlock() defer s.mu.Unlock()
s.snapshot.LastUpdated = time.Now().UTC() s.snapshot.LastUpdated = time.Now().UTC()

View File

@@ -6,7 +6,6 @@ import (
"log/slog" "log/slog"
"sort" "sort"
"strings" "strings"
"sync"
"time" "time"
"go.uber.org/fx" "go.uber.org/fx"
@@ -41,17 +40,15 @@ type Params struct {
// Watcher orchestrates all monitoring checks on a schedule. // Watcher orchestrates all monitoring checks on a schedule.
type Watcher struct { type Watcher struct {
log *slog.Logger log *slog.Logger
config *config.Config config *config.Config
state *state.State state *state.State
resolver DNSResolver resolver DNSResolver
portCheck PortChecker portCheck PortChecker
tlsCheck TLSChecker tlsCheck TLSChecker
notify Notifier notify Notifier
cancel context.CancelFunc cancel context.CancelFunc
firstRun bool firstRun bool
expiryNotifiedMu sync.Mutex
expiryNotified map[string]time.Time
} }
// New creates a new Watcher instance wired into the fx lifecycle. // New creates a new Watcher instance wired into the fx lifecycle.
@@ -60,15 +57,14 @@ func New(
params Params, params Params,
) (*Watcher, error) { ) (*Watcher, error) {
w := &Watcher{ w := &Watcher{
log: params.Logger.Get(), log: params.Logger.Get(),
config: params.Config, config: params.Config,
state: params.State, state: params.State,
resolver: params.Resolver, resolver: params.Resolver,
portCheck: params.PortCheck, portCheck: params.PortCheck,
tlsCheck: params.TLSCheck, tlsCheck: params.TLSCheck,
notify: params.Notify, notify: params.Notify,
firstRun: true, firstRun: true,
expiryNotified: make(map[string]time.Time),
} }
lifecycle.Append(fx.Hook{ lifecycle.Append(fx.Hook{
@@ -104,15 +100,14 @@ func NewForTest(
n Notifier, n Notifier,
) *Watcher { ) *Watcher {
return &Watcher{ return &Watcher{
log: slog.Default(), log: slog.Default(),
config: cfg, config: cfg,
state: st, state: st,
resolver: res, resolver: res,
portCheck: pc, portCheck: pc,
tlsCheck: tc, tlsCheck: tc,
notify: n, notify: n,
firstRun: true, firstRun: true,
expiryNotified: make(map[string]time.Time),
} }
} }
@@ -696,22 +691,6 @@ func (w *Watcher) checkTLSExpiry(
return return
} }
// Deduplicate expiry warnings: don't re-notify for the same
// hostname within the TLS check interval.
dedupKey := fmt.Sprintf("expiry:%s:%s", hostname, ip)
w.expiryNotifiedMu.Lock()
lastNotified, seen := w.expiryNotified[dedupKey]
if seen && time.Since(lastNotified) < w.config.TLSInterval {
w.expiryNotifiedMu.Unlock()
return
}
w.expiryNotified[dedupKey] = time.Now()
w.expiryNotifiedMu.Unlock()
msg := fmt.Sprintf( msg := fmt.Sprintf(
"Host: %s\nIP: %s\nCN: %s\n"+ "Host: %s\nIP: %s\nCN: %s\n"+
"Expires: %s (%.0f days)", "Expires: %s (%.0f days)",

View File

@@ -506,61 +506,6 @@ func TestTLSExpiryWarning(t *testing.T) {
} }
} }
func TestTLSExpiryWarningDedup(t *testing.T) {
t.Parallel()
cfg := defaultTestConfig(t)
cfg.Hostnames = []string{"www.example.com"}
cfg.TLSInterval = 24 * time.Hour
w, deps := newTestWatcher(t, cfg)
deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"1.2.3.4"}},
}
deps.resolver.ipAddresses["www.example.com"] = []string{
"1.2.3.4",
}
deps.portChecker.results["1.2.3.4:80"] = true
deps.portChecker.results["1.2.3.4:443"] = true
deps.tlsChecker.certs["1.2.3.4:www.example.com"] = &tlscheck.CertificateInfo{
CommonName: "www.example.com",
Issuer: "DigiCert",
NotAfter: time.Now().Add(3 * 24 * time.Hour),
SubjectAlternativeNames: []string{
"www.example.com",
},
}
ctx := t.Context()
// First run = baseline, no notifications
w.RunOnce(ctx)
// Second run should fire one expiry warning
w.RunOnce(ctx)
// Third run should NOT fire another warning (dedup)
w.RunOnce(ctx)
notifications := deps.notifier.getNotifications()
expiryCount := 0
for _, n := range notifications {
if n.Title == "TLS Expiry Warning: www.example.com" {
expiryCount++
}
}
if expiryCount != 1 {
t.Errorf(
"expected exactly 1 expiry warning (dedup), got %d",
expiryCount,
)
}
}
func TestGracefulShutdown(t *testing.T) { func TestGracefulShutdown(t *testing.T) {
t.Parallel() t.Parallel()