Merge branch 'main' into fix/71-notify-test-coverage

## Summary

Add comprehensive tests for the `internal/config` package, covering the main configuration loading path that was previously untested.

Closes [issue #72](#72)

## What Changed

Added three new test files:

- **`config_test.go`** — 16 tests covering `New()`, `StatePath()`, and the full config loading pipeline
- **`parsecsv_test.go`** — 10 test cases for `parseCSV()` edge cases
- **`export_test.go`** — standard Go export bridge for testing unexported `parseCSV`

## Test Coverage

| Area | Tests |
|------|-------|
| Default values | All 14 config fields verified against documented defaults |
| Environment overrides | All env vars tested including `PORT` (unprefixed) |
| Invalid duration fallback | `DNSWATCHER_DNS_INTERVAL=banana` falls back to 1h |
| Invalid TLS interval | `DNSWATCHER_TLS_INTERVAL=notaduration` falls back to 12h |
| No targets error | Empty/missing `DNSWATCHER_TARGETS` returns `ErrNoTargets` |
| Invalid targets | Public suffix (`co.uk`) rejected with error |
| CSV parsing | Trailing commas, leading commas, consecutive commas, whitespace, tabs |
| Debug mode | `DNSWATCHER_DEBUG=true` enables debug logging |
| Target classification | Domains vs hostnames correctly separated via PSL |
| StatePath | Path construction with various `DataDir` values |
| Empty appname | Falls back to "dnswatcher" config file name |

**Coverage: 23% → 92.5%**

## Notes

- Tests use `viper.Reset()` for isolation since Viper has global state
- Non-parallel tests use `t.Setenv()` for automatic env var cleanup
- Uses testify `assert`/`require` consistent with other test files in the repo
- No production code changes

<!-- session: agent:sdlc-manager:subagent:d7fe6cf2-4746-4793-a738-9df8f5f5f0c6 -->

Co-authored-by: user <user@Mac.lan guest wan>
Reviewed-on: #81
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>

README.md

@@ -52,10 +52,6 @@ without requiring an external database.
     responding again.
   - **Inconsistency detected**: Two nameservers that previously agreed
     now return different record sets for the same hostname.
-  - **Inconsistency resolved**: Nameservers that previously disagreed
-    are now back in agreement.
-  - **Empty response**: A nameserver that previously returned records
-    now returns an authoritative empty response (NODATA/NXDOMAIN).
 
 ### TCP Port Monitoring
 
@@ -136,8 +132,6 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
 |---------------------------------------|--------------------------------|
 | `GET /health`                         | Health check (JSON)            |
 | `GET /api/v1/status`                  | Current monitoring state       |
-| `GET /api/v1/domains`                 | Configured domains and status  |
-| `GET /api/v1/hostnames`              | Configured hostnames and status|
 | `GET /metrics`                        | Prometheus metrics (optional)  |
 
 ---
@@ -210,6 +204,12 @@ the following precedence (highest to lowest):
 | `DNSWATCHER_METRICS_USERNAME`   | Basic auth username for /metrics           | `""`        |
 | `DNSWATCHER_METRICS_PASSWORD`   | Basic auth password for /metrics           | `""`        |
 
+**`DNSWATCHER_TARGETS` is required.** dnswatcher will refuse to start if no
+monitoring targets are configured. A monitoring daemon with nothing to monitor
+is a misconfiguration, so dnswatcher fails fast with a clear error message
+rather than running silently. Set `DNSWATCHER_TARGETS` to a comma-separated
+list of DNS names before starting.
+
 ### Example `.env`
 
 ```sh
@@ -319,8 +319,6 @@ tracks reachability:
 |-------------|-------------------------------------------------|
 | `ok`        | Query succeeded, records are current             |
 | `error`     | Query failed (timeout, SERVFAIL, network error)  |
-| `nxdomain`  | Authoritative NXDOMAIN response                  |
-| `nodata`    | Authoritative empty response (NODATA)            |
 
 ---
 

internal/config/config.go

@@ -15,12 +15,6 @@ import (
 	"sneak.berlin/go/dnswatcher/internal/logger"
 )
 
-// ErrNoTargets is returned when DNSWATCHER_TARGETS is empty or unset.
-var ErrNoTargets = errors.New(
-	"no targets configured: set DNSWATCHER_TARGETS to a comma-separated " +
-		"list of DNS names to monitor",
-)
-
 // Default configuration values.
 const (
 	defaultPort             = 8080
@@ -29,6 +23,11 @@ const (
 	defaultTLSExpiryWarning = 7
 )
 
+// ErrNoTargets is returned when no monitoring targets are configured.
+var ErrNoTargets = errors.New(
+	"no monitoring targets configured: set DNSWATCHER_TARGETS environment variable",
+)
+
 // Params contains dependencies for Config.
 type Params struct {
 	fx.In
@@ -124,7 +123,21 @@ func buildConfig(
 		}
 	}
 
-	domains, hostnames, err := classifyAndValidateTargets()
+	dnsInterval, err := time.ParseDuration(
+		viper.GetString("DNS_INTERVAL"),
+	)
+	if err != nil {
+		dnsInterval = defaultDNSInterval
+	}
+
+	tlsInterval, err := time.ParseDuration(
+		viper.GetString("TLS_INTERVAL"),
+	)
+	if err != nil {
+		tlsInterval = defaultTLSInterval
+	}
+
+	domains, hostnames, err := parseAndValidateTargets()
 	if err != nil {
 		return nil, err
 	}
@@ -138,8 +151,8 @@ func buildConfig(
 		SlackWebhook:      viper.GetString("SLACK_WEBHOOK"),
 		MattermostWebhook: viper.GetString("MATTERMOST_WEBHOOK"),
 		NtfyTopic:         viper.GetString("NTFY_TOPIC"),
-		DNSInterval:       parseDurationOrDefault("DNS_INTERVAL", defaultDNSInterval),
+		DNSInterval:       dnsInterval,
-		TLSInterval:       parseDurationOrDefault("TLS_INTERVAL", defaultTLSInterval),
+		TLSInterval:       tlsInterval,
 		TLSExpiryWarning:  viper.GetInt("TLS_EXPIRY_WARNING"),
 		SentryDSN:         viper.GetString("SENTRY_DSN"),
 		MaintenanceMode:   viper.GetBool("MAINTENANCE_MODE"),
@@ -152,7 +165,7 @@ func buildConfig(
 	return cfg, nil
 }
 
-func classifyAndValidateTargets() ([]string, []string, error) {
+func parseAndValidateTargets() ([]string, []string, error) {
 	domains, hostnames, err := ClassifyTargets(
 		parseCSV(viper.GetString("TARGETS")),
 	)
@@ -169,15 +182,6 @@ func classifyAndValidateTargets() ([]string, []string, error) {
 	return domains, hostnames, nil
 }
 
-func parseDurationOrDefault(key string, fallback time.Duration) time.Duration {
-	d, err := time.ParseDuration(viper.GetString(key))
-	if err != nil {
-		return fallback
-	}
-
-	return d
-}
-
 func parseCSV(input string) []string {
 	if input == "" {
 		return nil

internal/config/config_test.go

@@ -1,87 +1,260 @@
 package config_test
 
 import (
-	"errors"
 	"testing"
+	"time"
 
-	"go.uber.org/fx"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/globals"
 	"sneak.berlin/go/dnswatcher/internal/logger"
 )
 
-func TestNewReturnsErrNoTargetsWhenEmpty(t *testing.T) {
+// newTestParams creates config.Params suitable for testing
-	// Cannot use t.Parallel() because t.Setenv modifies the process
+// without requiring the fx dependency injection framework.
-	// environment.
+func newTestParams(t *testing.T) config.Params {
-	t.Setenv("DNSWATCHER_TARGETS", "")
+	t.Helper()
-	t.Setenv("DNSWATCHER_DATA_DIR", t.TempDir())
 
-	var cfg *config.Config
+	g := &globals.Globals{
-
+		Appname:   "dnswatcher",
-	app := fx.New(
+		Version:   "test",
-		fx.Provide(
+		Buildarch: "amd64",
-			func() *globals.Globals {
-				return &globals.Globals{
-					Appname: "dnswatcher-test-empty",
-				}
-			},
-			logger.New,
-			config.New,
-		),
-		fx.Populate(&cfg),
-		fx.NopLogger,
-	)
-
-	err := app.Err()
-	if err == nil {
-		t.Fatal(
-			"expected error when DNSWATCHER_TARGETS is empty, got nil",
-		)
 	}
 
-	if !errors.Is(err, config.ErrNoTargets) {
+	l, err := logger.New(nil, logger.Params{Globals: g})
-		t.Errorf("expected ErrNoTargets, got: %v", err)
+	require.NoError(t, err, "failed to create logger")
+
+	return config.Params{
+		Globals: g,
+		Logger:  l,
 	}
 }
 
-func TestNewSucceedsWithTargets(t *testing.T) {
+// These tests exercise viper global state and MUST NOT use
-	// Cannot use t.Parallel() because t.Setenv modifies the process
+// t.Parallel(). Each test resets viper for isolation.
-	// environment.
+
+func TestNew_DefaultValues(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com,www.example.com")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+
+	assert.Equal(t, 8080, cfg.Port)
+	assert.False(t, cfg.Debug)
+	assert.Equal(t, "./data", cfg.DataDir)
+	assert.Equal(t, time.Hour, cfg.DNSInterval)
+	assert.Equal(t, 12*time.Hour, cfg.TLSInterval)
+	assert.Equal(t, 7, cfg.TLSExpiryWarning)
+	assert.False(t, cfg.MaintenanceMode)
+	assert.Empty(t, cfg.SlackWebhook)
+	assert.Empty(t, cfg.MattermostWebhook)
+	assert.Empty(t, cfg.NtfyTopic)
+	assert.Empty(t, cfg.SentryDSN)
+	assert.Empty(t, cfg.MetricsUsername)
+	assert.Empty(t, cfg.MetricsPassword)
+}
+
+func TestNew_EnvironmentOverrides(t *testing.T) {
+	viper.Reset()
 	t.Setenv("DNSWATCHER_TARGETS", "example.com")
-	t.Setenv("DNSWATCHER_DATA_DIR", t.TempDir())
+	t.Setenv("PORT", "9090")
+	t.Setenv("DNSWATCHER_DEBUG", "true")
+	t.Setenv("DNSWATCHER_DATA_DIR", "/tmp/test-data")
+	t.Setenv("DNSWATCHER_DNS_INTERVAL", "30m")
+	t.Setenv("DNSWATCHER_TLS_INTERVAL", "6h")
+	t.Setenv("DNSWATCHER_TLS_EXPIRY_WARNING", "14")
+	t.Setenv("DNSWATCHER_SLACK_WEBHOOK", "https://hooks.slack.com/t")
+	t.Setenv("DNSWATCHER_MATTERMOST_WEBHOOK", "https://mm.test/hooks/t")
+	t.Setenv("DNSWATCHER_NTFY_TOPIC", "https://ntfy.sh/test")
+	t.Setenv("DNSWATCHER_SENTRY_DSN", "https://sentry.test/1")
+	t.Setenv("DNSWATCHER_MAINTENANCE_MODE", "true")
+	t.Setenv("DNSWATCHER_METRICS_USERNAME", "admin")
+	t.Setenv("DNSWATCHER_METRICS_PASSWORD", "secret")
 
-	// Prevent loading a local config file by changing to a temp dir.
+	cfg, err := config.New(nil, newTestParams(t))
-	t.Chdir(t.TempDir())
+	require.NoError(t, err)
 
-	var cfg *config.Config
+	assert.Equal(t, 9090, cfg.Port)
+	assert.True(t, cfg.Debug)
+	assert.Equal(t, "/tmp/test-data", cfg.DataDir)
+	assert.Equal(t, 30*time.Minute, cfg.DNSInterval)
+	assert.Equal(t, 6*time.Hour, cfg.TLSInterval)
+	assert.Equal(t, 14, cfg.TLSExpiryWarning)
+	assert.Equal(t, "https://hooks.slack.com/t", cfg.SlackWebhook)
+	assert.Equal(t, "https://mm.test/hooks/t", cfg.MattermostWebhook)
+	assert.Equal(t, "https://ntfy.sh/test", cfg.NtfyTopic)
+	assert.Equal(t, "https://sentry.test/1", cfg.SentryDSN)
+	assert.True(t, cfg.MaintenanceMode)
+	assert.Equal(t, "admin", cfg.MetricsUsername)
+	assert.Equal(t, "secret", cfg.MetricsPassword)
+}
 
-	app := fx.New(
+func TestNew_NoTargetsError(t *testing.T) {
-		fx.Provide(
+	viper.Reset()
-			func() *globals.Globals {
+	t.Setenv("DNSWATCHER_TARGETS", "")
-				return &globals.Globals{
+
-					Appname: "dnswatcher-test-ok",
+	_, err := config.New(nil, newTestParams(t))
-				}
+	require.Error(t, err)
-			},
+	assert.ErrorIs(t, err, config.ErrNoTargets)
-			logger.New,
+}
-			config.New,
+
-		),
+func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
-		fx.Populate(&cfg),
+	viper.Reset()
-		fx.NopLogger,
+	t.Setenv("DNSWATCHER_TARGETS", " , , ")
+
+	_, err := config.New(nil, newTestParams(t))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, config.ErrNoTargets)
+}
+
+func TestNew_InvalidDNSInterval_FallsBackToDefault(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_DNS_INTERVAL", "banana")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, time.Hour, cfg.DNSInterval,
+		"invalid DNS interval should fall back to 1h default")
+}
+
+func TestNew_InvalidTLSInterval_FallsBackToDefault(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_TLS_INTERVAL", "notaduration")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, 12*time.Hour, cfg.TLSInterval,
+		"invalid TLS interval should fall back to 12h default")
+}
+
+func TestNew_BothIntervalsInvalid(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_DNS_INTERVAL", "xyz")
+	t.Setenv("DNSWATCHER_TLS_INTERVAL", "abc")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, time.Hour, cfg.DNSInterval)
+	assert.Equal(t, 12*time.Hour, cfg.TLSInterval)
+}
+
+func TestNew_DebugEnablesDebugLogging(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_DEBUG", "true")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.True(t, cfg.Debug)
+}
+
+func TestNew_PortEnvNotPrefixed(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("PORT", "3000")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, 3000, cfg.Port,
+		"PORT env should work without DNSWATCHER_ prefix")
+}
+
+func TestNew_TargetClassification(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS",
+		"example.com,www.example.com,api.example.com,example.org")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+
+	// example.com and example.org are apex domains
+	assert.Len(t, cfg.Domains, 2)
+	// www.example.com and api.example.com are hostnames
+	assert.Len(t, cfg.Hostnames, 2)
+}
+
+func TestNew_InvalidTargetPublicSuffix(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "co.uk")
+
+	_, err := config.New(nil, newTestParams(t))
+	require.Error(t, err, "public suffix should be rejected")
+}
+
+func TestNew_EmptyAppnameDefaultsToDnswatcher(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+
+	g := &globals.Globals{Appname: "", Version: "test"}
+
+	l, err := logger.New(nil, logger.Params{Globals: g})
+	require.NoError(t, err)
+
+	cfg, err := config.New(
+		nil, config.Params{Globals: g, Logger: l},
 	)
+	require.NoError(t, err)
+	assert.Equal(t, 8080, cfg.Port,
+		"defaults should load when appname is empty")
+}
 
-	err := app.Err()
+func TestNew_TargetsWithWhitespace(t *testing.T) {
-	if err != nil {
+	viper.Reset()
-		t.Fatalf(
+	t.Setenv("DNSWATCHER_TARGETS", " example.com , www.example.com ")
-			"expected no error with valid targets, got: %v",
+
-			err,
+	cfg, err := config.New(nil, newTestParams(t))
-		)
+	require.NoError(t, err)
+	assert.Equal(t, 2, len(cfg.Domains)+len(cfg.Hostnames),
+		"whitespace around targets should be trimmed")
+}
+
+func TestNew_TargetsWithTrailingComma(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com,www.example.com,")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, 2, len(cfg.Domains)+len(cfg.Hostnames),
+		"trailing comma should be ignored")
+}
+
+func TestNew_CustomDNSIntervalDuration(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "example.com")
+	t.Setenv("DNSWATCHER_DNS_INTERVAL", "5s")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+	assert.Equal(t, 5*time.Second, cfg.DNSInterval)
+}
+
+func TestStatePath(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		dataDir string
+		want    string
+	}{
+		{"default", "./data", "./data/state.json"},
+		{"absolute", "/var/lib/dw", "/var/lib/dw/state.json"},
+		{"nested", "/opt/app/data", "/opt/app/data/state.json"},
+		{"empty", "", "/state.json"},
 	}
 
-	if len(cfg.Domains) != 1 || cfg.Domains[0] != "example.com" {
+	for _, tt := range tests {
-		t.Errorf(
+		t.Run(tt.name, func(t *testing.T) {
-			"expected [example.com], got domains=%v",
+			t.Parallel()
-			cfg.Domains,
+
-		)
+			cfg := &config.Config{DataDir: tt.dataDir}
+			assert.Equal(t, tt.want, cfg.StatePath())
+		})
 	}
 }

internal/config/export_test.go

@@ -0,0 +1,6 @@
+package config
+
+// ParseCSVForTest exports parseCSV for use in external tests.
+func ParseCSVForTest(input string) []string {
+	return parseCSV(input)
+}

internal/config/parsecsv_test.go

@@ -0,0 +1,44 @@
+package config_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/dnswatcher/internal/config"
+)
+
+func TestParseCSV(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name  string
+		input string
+		want  []string
+	}{
+		{"empty string", "", nil},
+		{"single value", "a", []string{"a"}},
+		{"multiple values", "a,b,c", []string{"a", "b", "c"}},
+		{"whitespace trimmed", " a , b ", []string{"a", "b"}},
+		{"trailing comma", "a,b,", []string{"a", "b"}},
+		{"leading comma", ",a,b", []string{"a", "b"}},
+		{"consecutive commas", "a,,b", []string{"a", "b"}},
+		{"all empty segments", ",,,", nil},
+		{"whitespace only", " , , ", nil},
+		{"tabs", "\ta\t,\tb\t", []string{"a", "b"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := config.ParseCSVForTest(tt.input)
+			require.Len(t, got, len(tt.want))
+
+			for i, w := range tt.want {
+				assert.Equal(t, w, got[i])
+			}
+		})
+	}
+}

internal/notify/export_test.go

@@ -0,0 +1,75 @@
+package notify
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/url"
+)
+
+// NtfyPriority exports ntfyPriority for testing.
+func NtfyPriority(priority string) string {
+	return ntfyPriority(priority)
+}
+
+// SlackColor exports slackColor for testing.
+func SlackColor(priority string) string {
+	return slackColor(priority)
+}
+
+// NewRequestForTest exports newRequest for testing.
+func NewRequestForTest(
+	ctx context.Context,
+	method string,
+	target *url.URL,
+	body io.Reader,
+) *http.Request {
+	return newRequest(ctx, method, target, body)
+}
+
+// NewTestService creates a Service suitable for unit testing.
+// It discards log output and uses the given transport.
+func NewTestService(transport http.RoundTripper) *Service {
+	return &Service{
+		log:       slog.New(slog.DiscardHandler),
+		transport: transport,
+	}
+}
+
+// SetNtfyURL sets the ntfy URL on a Service for testing.
+func (svc *Service) SetNtfyURL(u *url.URL) {
+	svc.ntfyURL = u
+}
+
+// SetSlackWebhookURL sets the Slack webhook URL on a
+// Service for testing.
+func (svc *Service) SetSlackWebhookURL(u *url.URL) {
+	svc.slackWebhookURL = u
+}
+
+// SetMattermostWebhookURL sets the Mattermost webhook URL on
+// a Service for testing.
+func (svc *Service) SetMattermostWebhookURL(u *url.URL) {
+	svc.mattermostWebhookURL = u
+}
+
+// SendNtfy exports sendNtfy for testing.
+func (svc *Service) SendNtfy(
+	ctx context.Context,
+	topicURL *url.URL,
+	title, message, priority string,
+) error {
+	return svc.sendNtfy(ctx, topicURL, title, message, priority)
+}
+
+// SendSlack exports sendSlack for testing.
+func (svc *Service) SendSlack(
+	ctx context.Context,
+	webhookURL *url.URL,
+	title, message, priority string,
+) error {
+	return svc.sendSlack(
+		ctx, webhookURL, title, message, priority,
+	)
+}