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

@@ -23,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
@@ -132,11 +137,9 @@ func buildConfig(
 		tlsInterval = defaultTLSInterval
 	}
 
-	domains, hostnames, err := ClassifyTargets(
-		parseCSV(viper.GetString("TARGETS")),
-	)
+	domains, hostnames, err := parseAndValidateTargets()
 	if err != nil {
-		return nil, fmt.Errorf("invalid targets configuration: %w", err)
+		return nil, err
 	}
 
 	cfg := &Config{
@@ -162,6 +165,23 @@ func buildConfig(
 	return cfg, nil
 }
 
+func parseAndValidateTargets() ([]string, []string, error) {
+	domains, hostnames, err := ClassifyTargets(
+		parseCSV(viper.GetString("TARGETS")),
+	)
+	if err != nil {
+		return nil, nil, fmt.Errorf(
+			"invalid targets configuration: %w", err,
+		)
+	}
+
+	if len(domains) == 0 && len(hostnames) == 0 {
+		return nil, nil, ErrNoTargets
+	}
+
+	return domains, hostnames, nil
+}
+
 func parseCSV(input string) []string {
 	if input == "" {
 		return nil

internal/config/config_test.go

@@ -0,0 +1,260 @@
+package config_test
+
+import (
+	"testing"
+	"time"
+
+	"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"
+)
+
+// newTestParams creates config.Params suitable for testing
+// without requiring the fx dependency injection framework.
+func newTestParams(t *testing.T) config.Params {
+	t.Helper()
+
+	g := &globals.Globals{
+		Appname:   "dnswatcher",
+		Version:   "test",
+		Buildarch: "amd64",
+	}
+
+	l, err := logger.New(nil, logger.Params{Globals: g})
+	require.NoError(t, err, "failed to create logger")
+
+	return config.Params{
+		Globals: g,
+		Logger:  l,
+	}
+}
+
+// These tests exercise viper global state and MUST NOT use
+// t.Parallel(). Each test resets viper for isolation.
+
+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("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")
+
+	cfg, err := config.New(nil, newTestParams(t))
+	require.NoError(t, err)
+
+	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)
+}
+
+func TestNew_NoTargetsError(t *testing.T) {
+	viper.Reset()
+	t.Setenv("DNSWATCHER_TARGETS", "")
+
+	_, err := config.New(nil, newTestParams(t))
+	require.Error(t, err)
+	assert.ErrorIs(t, err, config.ErrNoTargets)
+}
+
+func TestNew_OnlyEmptyCSVSegments(t *testing.T) {
+	viper.Reset()
+	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")
+}
+
+func TestNew_TargetsWithWhitespace(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),
+		"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"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			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,
+	)
+}