diff --git a/README.md b/README.md index d089ea3..12e1338 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,7 @@ TTY detection, and security headers are always applied. | `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` | | `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` | | `SENTRY_DSN` | Sentry error reporting DSN | `""` | +| `RETENTION_SWEEP_INTERVAL` | Retention reaper period (Go duration, must be positive) | `1h` | | `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` | | `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint | `120` | | `TRUSTED_PROXIES` | CIDRs whose forwarded headers are trusted | `""` (none) | @@ -173,8 +174,12 @@ its value and refuses to start, rather than silently running with a substituted default. `PORT=eighty`, `DEBUG=ture`, and `RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must additionally be a number in the range 1–65535, -`RECEIVER_RATE_LIMIT` must be at least 1, and every entry in -`TRUSTED_PROXIES` must be a CIDR block or a bare IP address. +`RECEIVER_RATE_LIMIT` must be at least 1, +`RETENTION_SWEEP_INTERVAL` must be greater than zero (it is a ticker +period, so `0s` or a negative value would crash the reaper after +startup), and every entry in `TRUSTED_PROXIES` must be a CIDR block or +a bare IP address. `SESSION_IDLE_TIMEOUT` is the exception: a +non-positive value there means idle expiry is disabled, not invalid. Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`, diff --git a/internal/config/config.go b/internal/config/config.go index 27e875f..cae3820 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -92,6 +92,7 @@ type Config struct { SentryDSN string // RetentionSweepInterval is how often the retention reaper runs. + // Always positive: it becomes a time.NewTicker period. RetentionSweepInterval time.Duration // SessionIdleTimeout is the sliding inactivity window after @@ -235,6 +236,34 @@ func envDuration( return d, nil } +// envPositiveDuration returns the value of the named environment +// variable parsed as a Go duration that must be greater than zero. +// Returns defaultValue if not set. A set value that is unparseable or +// non-positive is a hard error naming the key and the bad value. +// +// This is for durations that reach time.NewTicker, which panics on a +// non-positive period, in a goroutine started after startup has +// already reported success. It is deliberately not used for durations +// where non-positive means "disabled" (SESSION_IDLE_TIMEOUT). +func envPositiveDuration( + key string, + defaultValue time.Duration, +) (time.Duration, error) { + d, err := envDuration(key, defaultValue) + if err != nil { + return 0, err + } + + if d <= 0 { + return 0, fmt.Errorf( + "%w: %s must be greater than zero, got %s", + ErrNonPositiveValue, key, d, + ) + } + + return d, nil +} + // parseCIDR parses one trusted-proxy list entry, which may be a // CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated // as a single-host block). @@ -346,7 +375,7 @@ func loadFromEnv() (*Config, error) { return nil, err } - retentionSweepInterval, err := envDuration( + retentionSweepInterval, err := envPositiveDuration( "RETENTION_SWEEP_INTERVAL", defaultRetentionSweepInterval, ) @@ -354,6 +383,8 @@ func loadFromEnv() (*Config, error) { return nil, err } + // Non-positive is "disabled" here, not invalid, so this stays on + // envDuration. sessionIdleTimeout, err := envDuration( "SESSION_IDLE_TIMEOUT", defaultSessionIdleTimeout, diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 816d5a2..f8ff7be 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -139,7 +139,11 @@ func TestRetentionSweepInterval(t *testing.T) { set bool value string expectError bool - expected time.Duration + // sentinel, when set, must be wrapped by the startup + // error; every error case must additionally name the + // variable in its message. + sentinel error + expected time.Duration }{ { name: caseUnsetUsesDefault, @@ -158,6 +162,24 @@ func TestRetentionSweepInterval(t *testing.T) { value: "not-a-duration", expectError: true, }, + { + // A non-positive period panics the ticker in the + // reaper and archive-sweeper goroutines, long after + // startup has reported success, so it has to fail + // here instead. + name: "zero fails startup", + set: true, + value: "0s", + expectError: true, + sentinel: config.ErrNonPositiveValue, + }, + { + name: "negative fails startup", + set: true, + value: "-1h", + expectError: true, + sentinel: config.ErrNonPositiveValue, + }, } for _, tt := range tests { @@ -175,7 +197,9 @@ func TestRetentionSweepInterval(t *testing.T) { } if tt.expectError { - expectStartupError(t) + expectStartupErrorFor( + t, "RETENTION_SWEEP_INTERVAL", tt.sentinel, + ) } else { testRetentionSweepIntervalSuccess(t, tt.expected) } @@ -281,6 +305,22 @@ func TestSessionIdleTimeout(t *testing.T) { value: "not-a-duration", expectError: true, }, + { + // Non-positive is "idle expiry disabled" for this + // variable, not a configuration error: unlike + // RETENTION_SWEEP_INTERVAL it never becomes a ticker + // period. + name: "zero disables idle expiry", + set: true, + value: "0s", + expected: 0, + }, + { + name: "negative disables idle expiry", + set: true, + value: "-1h", + expected: -time.Hour, + }, } for _, tt := range tests {