diff --git a/.env.example b/.env.example index 8b8055b..037cbf6 100644 --- a/.env.example +++ b/.env.example @@ -30,6 +30,14 @@ METRICS_ENABLED=true METRICS_NAMESPACE=secrets METRICS_PORT=8081 +# Metrics server timeout configuration (in seconds) +# Read timeout: maximum duration for reading the entire request, including the body +METRICS_SERVER_READ_TIMEOUT_SECONDS=15 +# Write timeout: maximum duration before timing out writes of the response +METRICS_SERVER_WRITE_TIMEOUT_SECONDS=15 +# Idle timeout: maximum time to wait for the next request when keep-alives are enabled +METRICS_SERVER_IDLE_TIMEOUT_SECONDS=60 + # ... # Authentication configuration diff --git a/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/index.md b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/index.md new file mode 100644 index 0000000..04a46a1 --- /dev/null +++ b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/index.md @@ -0,0 +1,5 @@ +# Track fix_metrics_server_hardcoded_timeouts_20260307 Context + +- [Specification](./spec.md) +- [Implementation Plan](./plan.md) +- [Metadata](./metadata.json) diff --git a/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/metadata.json b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/metadata.json new file mode 100644 index 0000000..0b9b9b1 --- /dev/null +++ b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/metadata.json @@ -0,0 +1,8 @@ +{ + "track_id": "fix_metrics_server_hardcoded_timeouts_20260307", + "type": "bug", + "status": "new", + "created_at": "2026-03-07T12:00:00Z", + "updated_at": "2026-03-07T12:00:00Z", + "description": "Fix Metrics Server Hardcoded Timeouts" +} diff --git a/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/plan.md b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/plan.md new file mode 100644 index 0000000..9b1c71b --- /dev/null +++ b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/plan.md @@ -0,0 +1,27 @@ +# Implementation Plan: Fix Metrics Server Hardcoded Timeouts + +## Phase 1: Configuration and Environment [checkpoint: 4ec5660] +Introduce the new configuration options for Metrics Server timeouts and update the environment files. + +- [x] Task: Update `internal/config/config.go` with new constants and fields for Metrics Server timeouts. 10f5e4c +- [x] Task: Implement validation for Metrics Server timeouts in `internal/config/config.go`. f27dd3f +- [x] Task: Update `Load()` in `internal/config/config.go` to parse the new environment variables. f27dd3f +- [x] Task: Update `.env.example` to include the new `METRICS_SERVER_*` variables. f27dd3f +- [x] Task: Write failing unit tests for new configuration loading and validation in `internal/config/config_test.go`. f27dd3f +- [x] Task: Implement changes to pass the tests in `internal/config/config.go`. f27dd3f +- [x] Task: Conductor - User Manual Verification 'Phase 1: Configuration and Environment' (Protocol in workflow.md) 4ec5660 + +## Phase 2: Metrics Server Implementation [checkpoint: 82b6cef] +Refactor the Metrics Server to accept configurable timeouts instead of using hardcoded defaults. + +- [x] Task: Write failing tests in `internal/http/metrics_server_test.go` to verify custom timeout initialization. a091f59 +- [x] Task: Update `NewDefaultMetricsServer` or adjust its usage in `internal/http/metrics_server.go` to use passed values. a091f59 +- [x] Task: Refactor `MetricsServer` initialization to ensure values are propagated correctly. a091f59 +- [x] Task: Conductor - User Manual Verification 'Phase 2: Metrics Server Implementation' (Protocol in workflow.md) 82b6cef + +## Phase 3: Dependency Injection Integration [checkpoint: 82b6cef] +Connect the new configuration to the Metrics Server initialization within the DI container. + +- [x] Task: Update `internal/app/di.go` to pass the configured timeouts from `Config` to the Metrics Server. 0e3de70 +- [x] Task: Write tests in `internal/app/di_test.go` (or verify via integration) that the server is correctly initialized. 0e3de70 +- [x] Task: Conductor - User Manual Verification 'Phase 3: Dependency Injection Integration' (Protocol in workflow.md) 82b6cef diff --git a/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/spec.md b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/spec.md new file mode 100644 index 0000000..4db6362 --- /dev/null +++ b/conductor/archive/fix_metrics_server_hardcoded_timeouts_20260307/spec.md @@ -0,0 +1,38 @@ +# Specification: Fix Metrics Server Hardcoded Timeouts (REVISED) + +## Overview +The Metrics Server in the `secrets` project currently has hardcoded timeout values for Read, Write, and Idle connections (15s, 15s, 60s). This track aims to make these timeouts configurable via environment variables, following the existing configuration pattern. + +## Functional Requirements + +1. **Configurable Metrics Timeouts:** + * Introduce three new environment variables: + * `METRICS_SERVER_READ_TIMEOUT_SECONDS` (Default: 15) + * `METRICS_SERVER_WRITE_TIMEOUT_SECONDS` (Default: 15) + * `METRICS_SERVER_IDLE_TIMEOUT_SECONDS` (Default: 60) + * **Config Update:** Update `internal/config/Config` struct in `internal/config/config.go` to include these new timeout fields. + * **Validation:** Implement validation for these new timeouts (1s to 300s range). + * **Default Values:** Set the default values to 15s/15s/60s in `internal/config/config.go`. + * **.env.example Update:** Add these new environment variables to the `.env.example` file with their default values. + +2. **Dependency Injection (DI) Integration:** + * Update `internal/app/Container.initMetricsServer` in `internal/app/di.go` to pass the new timeout values from the configuration to the `MetricsServer` initialization. + +3. **Metrics Server Update:** + * Refactor `internal/http/metrics_server.go` to ensure `MetricsServer` uses values provided via DI instead of hardcoded defaults. + * Update `NewDefaultMetricsServer` or adjust its usage in `di.go` to honor the configured values. + +## Non-Functional Requirements +* **Consistency:** The configuration naming and validation logic must mirror the existing patterns for the main server. + +## Acceptance Criteria +* [ ] New environment variables are successfully loaded into the `Config` struct. +* [ ] Configuration validation fails if any of the new timeouts are outside the 1s-300s range. +* [ ] `.env.example` is updated with the new environment variables. +* [ ] The Metrics Server uses the configured timeout values. +* [ ] Existing unit tests for `MetricsServer` and `Config` pass. +* [ ] New unit tests verify that the Metrics Server can be initialized with custom timeout values. + +## Out of Scope +* Adding other Metrics Server configuration options. +* Changing the default values for the main server. diff --git a/conductor/tech-stack.md b/conductor/tech-stack.md index 614e01f..144e07f 100644 --- a/conductor/tech-stack.md +++ b/conductor/tech-stack.md @@ -13,6 +13,7 @@ ## Cryptography & Security - **Envelope Encryption:** [gocloud.dev/secrets](https://gocloud.dev/howto/secrets/) - Abstracted access to various KMS providers for root-of-trust encryption. - **Password Hashing:** [go-pwdhash](https://github.com/allisson/go-pwdhash) - Argon2id hashing for secure storage of client secrets and passwords. +- **Configurable Metrics Timeouts:** Environment-controlled Read, Write, and Idle timeouts for the Prometheus metrics server to prevent resource exhaustion. - **Request Body Size Limiting:** Middleware to prevent DoS attacks from large payloads. - **Rate Limiting:** Per-client and per-IP rate limiting middleware for DoS protection and API abuse prevention. - **Secret Value Size Limiting:** Global limit on individual secret values to ensure predictable storage and memory usage. diff --git a/conductor/tracks.md b/conductor/tracks.md index 0b5c54e..bae9481 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -3,3 +3,4 @@ This file tracks all major tracks for the project. Each track has its own detailed plan in its respective folder. --- + diff --git a/internal/app/di.go b/internal/app/di.go index 4b882b6..84693dd 100644 --- a/internal/app/di.go +++ b/internal/app/di.go @@ -508,6 +508,9 @@ func (c *Container) initMetricsServer(ctx context.Context) (*http.MetricsServer, c.config.MetricsPort, logger, provider, + c.config.MetricsServerReadTimeout, + c.config.MetricsServerWriteTimeout, + c.config.MetricsServerIdleTimeout, ) return server, nil diff --git a/internal/app/di_test.go b/internal/app/di_test.go index 734a802..a984d3e 100644 --- a/internal/app/di_test.go +++ b/internal/app/di_test.go @@ -266,6 +266,49 @@ func TestContainerServerComponents(t *testing.T) { } } +// TestContainerMetricsServer_CustomTimeouts verifies that the metrics server is initialized with custom timeouts from config. +func TestContainerMetricsServer_CustomTimeouts(t *testing.T) { + cfg := &config.Config{ + MetricsEnabled: true, + MetricsPort: 8082, + MetricsServerReadTimeout: 5 * time.Second, + MetricsServerWriteTimeout: 10 * time.Second, + MetricsServerIdleTimeout: 30 * time.Second, + } + container := NewContainer(cfg) + + server, err := container.MetricsServer(context.Background()) + if err != nil { + t.Fatalf("unexpected error for metrics server: %v", err) + } + + if server == nil { + t.Fatal("expected non-nil metrics server") + } + + if server.Server().ReadTimeout != cfg.MetricsServerReadTimeout { + t.Errorf( + "expected read timeout %v, got %v", + cfg.MetricsServerReadTimeout, + server.Server().ReadTimeout, + ) + } + if server.Server().WriteTimeout != cfg.MetricsServerWriteTimeout { + t.Errorf( + "expected write timeout %v, got %v", + cfg.MetricsServerWriteTimeout, + server.Server().WriteTimeout, + ) + } + if server.Server().IdleTimeout != cfg.MetricsServerIdleTimeout { + t.Errorf( + "expected idle timeout %v, got %v", + cfg.MetricsServerIdleTimeout, + server.Server().IdleTimeout, + ) + } +} + // TestContainerKekRepositoryErrors verifies that KEK repository initialization errors are properly handled. func TestContainerKekRepositoryErrors(t *testing.T) { // Create a container with invalid database configuration diff --git a/internal/config/config.go b/internal/config/config.go index 60dcff1..58b9eda 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -24,26 +24,29 @@ const ( DefaultDBConnectionString = "postgres://user:password@localhost:5432/mydb?sslmode=disable" //nolint:gosec DefaultDBMaxOpenConnections = 25 - DefaultDBMaxIdleConnections = 5 - DefaultDBConnMaxLifetime = 5 // minutes - DefaultDBConnMaxIdleTime = 5 // minutes - DefaultLogLevel = "info" - DefaultAuthTokenExpiration = 14400 // seconds - DefaultRateLimitEnabled = true - DefaultRateLimitRequests = 10.0 - DefaultRateLimitBurst = 20 - DefaultRateLimitTokenEnabled = true - DefaultRateLimitTokenRequests = 5.0 - DefaultRateLimitTokenBurst = 10 - DefaultCORSEnabled = false - DefaultCORSAllowOrigins = "" - DefaultMetricsEnabled = true - DefaultMetricsNamespace = "secrets" - DefaultMetricsPort = 8081 - DefaultLockoutMaxAttempts = 10 - DefaultLockoutDuration = 30 // minutes - DefaultMaxRequestBodySize = 1048576 - DefaultSecretValueSizeLimit = 524288 + DefaultDBMaxIdleConnections = 5 + DefaultDBConnMaxLifetime = 5 // minutes + DefaultDBConnMaxIdleTime = 5 // minutes + DefaultLogLevel = "info" + DefaultAuthTokenExpiration = 14400 // seconds + DefaultRateLimitEnabled = true + DefaultRateLimitRequests = 10.0 + DefaultRateLimitBurst = 20 + DefaultRateLimitTokenEnabled = true + DefaultRateLimitTokenRequests = 5.0 + DefaultRateLimitTokenBurst = 10 + DefaultCORSEnabled = false + DefaultCORSAllowOrigins = "" + DefaultMetricsEnabled = true + DefaultMetricsNamespace = "secrets" + DefaultMetricsPort = 8081 + DefaultMetricsServerReadTimeout = 15 // seconds + DefaultMetricsServerWriteTimeout = 15 // seconds + DefaultMetricsServerIdleTimeout = 60 // seconds + DefaultLockoutMaxAttempts = 10 + DefaultLockoutDuration = 30 // minutes + DefaultMaxRequestBodySize = 1048576 + DefaultSecretValueSizeLimit = 524288 ) // Config holds all application configuration. @@ -105,6 +108,12 @@ type Config struct { MetricsNamespace string // MetricsPort is the port number for the metrics server. MetricsPort int + // MetricsServerReadTimeout is the maximum duration for reading the entire request, including the body. + MetricsServerReadTimeout time.Duration + // MetricsServerWriteTimeout is the maximum duration before timing out writes of the response. + MetricsServerWriteTimeout time.Duration + // MetricsServerIdleTimeout is the maximum time to wait for the next request when keep-alives are enabled. + MetricsServerIdleTimeout time.Duration // KMSProvider is the KMS provider to use (e.g., "google", "aws", "azure", "hashivault", "localsecrets"). KMSProvider string @@ -157,6 +166,24 @@ func (c *Config) Validate() error { validation.Max(65535), validation.NotIn(c.ServerPort), ), + validation.Field( + &c.MetricsServerReadTimeout, + validation.Required, + validation.Min(1*time.Second), + validation.Max(300*time.Second), + ), + validation.Field( + &c.MetricsServerWriteTimeout, + validation.Required, + validation.Min(1*time.Second), + validation.Max(300*time.Second), + ), + validation.Field( + &c.MetricsServerIdleTimeout, + validation.Required, + validation.Min(1*time.Second), + validation.Max(300*time.Second), + ), validation.Field( &c.LogLevel, validation.Required, @@ -264,6 +291,21 @@ func Load() (*Config, error) { MetricsEnabled: env.GetBool("METRICS_ENABLED", DefaultMetricsEnabled), MetricsNamespace: env.GetString("METRICS_NAMESPACE", DefaultMetricsNamespace), MetricsPort: env.GetInt("METRICS_PORT", DefaultMetricsPort), + MetricsServerReadTimeout: env.GetDuration( + "METRICS_SERVER_READ_TIMEOUT_SECONDS", + DefaultMetricsServerReadTimeout, + time.Second, + ), + MetricsServerWriteTimeout: env.GetDuration( + "METRICS_SERVER_WRITE_TIMEOUT_SECONDS", + DefaultMetricsServerWriteTimeout, + time.Second, + ), + MetricsServerIdleTimeout: env.GetDuration( + "METRICS_SERVER_IDLE_TIMEOUT_SECONDS", + DefaultMetricsServerIdleTimeout, + time.Second, + ), // KMS configuration KMSProvider: env.GetString("KMS_PROVIDER", ""), diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 67f22ba..79f8a18 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -27,6 +27,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, RateLimitEnabled: true, RateLimitRequestsPerSec: 10, RateLimitTokenEnabled: true, @@ -47,6 +50,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 0, }, @@ -118,6 +124,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSKeyURI: "gcpkms://...", @@ -135,6 +144,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "google", @@ -168,6 +180,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "invalid_provider", @@ -186,6 +201,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "localsecrets", @@ -204,6 +222,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "gcpkms", @@ -222,6 +243,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "awskms", @@ -240,6 +264,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "azurekeyvault", @@ -258,6 +285,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, KMSProvider: "hashivault", @@ -276,6 +306,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 15 * time.Second, ServerWriteTimeout: 15 * time.Second, ServerIdleTimeout: 60 * time.Second, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, }, @@ -292,6 +325,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 1 * time.Second, ServerWriteTimeout: 1 * time.Second, ServerIdleTimeout: 1 * time.Second, + MetricsServerReadTimeout: 1 * time.Second, + MetricsServerWriteTimeout: 1 * time.Second, + MetricsServerIdleTimeout: 1 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, }, @@ -308,6 +344,9 @@ func TestConfig_Validate(t *testing.T) { ServerReadTimeout: 300 * time.Second, ServerWriteTimeout: 300 * time.Second, ServerIdleTimeout: 300 * time.Second, + MetricsServerReadTimeout: 300 * time.Second, + MetricsServerWriteTimeout: 300 * time.Second, + MetricsServerIdleTimeout: 300 * time.Second, MaxRequestBodySize: 1048576, SecretValueSizeLimitBytes: 524288, }, @@ -423,6 +462,9 @@ func TestLoad(t *testing.T) { assert.Equal(t, 15*time.Second, cfg.ServerReadTimeout) assert.Equal(t, 15*time.Second, cfg.ServerWriteTimeout) assert.Equal(t, 60*time.Second, cfg.ServerIdleTimeout) + assert.Equal(t, 15*time.Second, cfg.MetricsServerReadTimeout) + assert.Equal(t, 15*time.Second, cfg.MetricsServerWriteTimeout) + assert.Equal(t, 60*time.Second, cfg.MetricsServerIdleTimeout) assert.Equal(t, int64(1048576), cfg.MaxRequestBodySize) assert.Equal(t, 524288, cfg.SecretValueSizeLimitBytes) }, @@ -471,6 +513,19 @@ func TestLoad(t *testing.T) { assert.Equal(t, 120*time.Second, cfg.ServerIdleTimeout) }, }, + { + name: "load custom metrics server timeout configuration", + envVars: map[string]string{ + "METRICS_SERVER_READ_TIMEOUT_SECONDS": "30", + "METRICS_SERVER_WRITE_TIMEOUT_SECONDS": "45", + "METRICS_SERVER_IDLE_TIMEOUT_SECONDS": "120", + }, + validate: func(t *testing.T, cfg *Config) { + assert.Equal(t, 30*time.Second, cfg.MetricsServerReadTimeout) + assert.Equal(t, 45*time.Second, cfg.MetricsServerWriteTimeout) + assert.Equal(t, 120*time.Second, cfg.MetricsServerIdleTimeout) + }, + }, { name: "load custom database configuration", envVars: map[string]string{ diff --git a/internal/config/metrics_server_config_test.go b/internal/config/metrics_server_config_test.go new file mode 100644 index 0000000..c68321a --- /dev/null +++ b/internal/config/metrics_server_config_test.go @@ -0,0 +1,103 @@ +package config + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestMetricsServerConfig_Validate(t *testing.T) { + baseCfg := Config{ + DBDriver: "postgres", + DBConnectionString: "postgres://localhost", + ServerPort: 8080, + MetricsPort: 8081, + LogLevel: "info", + ServerReadTimeout: 15 * time.Second, + ServerWriteTimeout: 15 * time.Second, + ServerIdleTimeout: 60 * time.Second, + MaxRequestBodySize: 1048576, + SecretValueSizeLimitBytes: 524288, + MetricsServerReadTimeout: 15 * time.Second, + MetricsServerWriteTimeout: 15 * time.Second, + MetricsServerIdleTimeout: 60 * time.Second, + } + + tests := []struct { + name string + cfg Config + wantErr bool + }{ + { + name: "valid metrics server timeouts", + cfg: baseCfg, + wantErr: false, + }, + { + name: "invalid metrics server read timeout - below minimum", + cfg: func() Config { + c := baseCfg + c.MetricsServerReadTimeout = 0 * time.Second + return c + }(), + wantErr: true, + }, + { + name: "invalid metrics server read timeout - above maximum", + cfg: func() Config { + c := baseCfg + c.MetricsServerReadTimeout = 301 * time.Second + return c + }(), + wantErr: true, + }, + { + name: "invalid metrics server write timeout - below minimum", + cfg: func() Config { + c := baseCfg + c.MetricsServerWriteTimeout = 0 * time.Second + return c + }(), + wantErr: true, + }, + { + name: "invalid metrics server write timeout - above maximum", + cfg: func() Config { + c := baseCfg + c.MetricsServerWriteTimeout = 301 * time.Second + return c + }(), + wantErr: true, + }, + { + name: "invalid metrics server idle timeout - below minimum", + cfg: func() Config { + c := baseCfg + c.MetricsServerIdleTimeout = 0 * time.Second + return c + }(), + wantErr: true, + }, + { + name: "invalid metrics server idle timeout - above maximum", + cfg: func() Config { + c := baseCfg + c.MetricsServerIdleTimeout = 301 * time.Second + return c + }(), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.cfg.Validate() + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/http/metrics_server.go b/internal/http/metrics_server.go index a043e5c..a89b940 100644 --- a/internal/http/metrics_server.go +++ b/internal/http/metrics_server.go @@ -59,18 +59,26 @@ func NewDefaultMetricsServer( port int, logger *slog.Logger, metricsProvider *metrics.Provider, + readTimeout time.Duration, + writeTimeout time.Duration, + idleTimeout time.Duration, ) *MetricsServer { return NewMetricsServer( host, port, logger, metricsProvider, - 15*time.Second, - 15*time.Second, - 60*time.Second, + readTimeout, + writeTimeout, + idleTimeout, ) } +// Server returns the underlying http.Server for testing purposes. +func (s *MetricsServer) Server() *http.Server { + return s.server +} + // GetHandler returns the http.Handler for testing purposes. func (s *MetricsServer) GetHandler() http.Handler { return s.server.Handler diff --git a/internal/http/metrics_server_test.go b/internal/http/metrics_server_test.go index 46d51a1..94f6225 100644 --- a/internal/http/metrics_server_test.go +++ b/internal/http/metrics_server_test.go @@ -27,7 +27,15 @@ func TestMetricsServer_Endpoints(t *testing.T) { }() // Create metrics server - metricsServer := NewDefaultMetricsServer("localhost", 8081, logger, provider) + metricsServer := NewDefaultMetricsServer( + "localhost", + 8081, + logger, + provider, + 15*time.Second, + 15*time.Second, + 60*time.Second, + ) require.NotNil(t, metricsServer) // Test the handler from metricsServer exactly as it's configured @@ -51,7 +59,15 @@ func TestMetricsServer_Lifecycle(t *testing.T) { }() // Create metrics server with random port - metricsServer := NewDefaultMetricsServer("localhost", 0, logger, provider) + metricsServer := NewDefaultMetricsServer( + "localhost", + 0, + logger, + provider, + 15*time.Second, + 15*time.Second, + 60*time.Second, + ) require.NotNil(t, metricsServer) ctx, cancel := context.WithCancel(context.Background()) @@ -89,7 +105,15 @@ func TestMetricsServer_ContextCancellation(t *testing.T) { logger := slog.New(slog.NewTextHandler(io.Discard, nil)) // Create metrics server - metricsServer := NewDefaultMetricsServer("localhost", 0, logger, nil) + metricsServer := NewDefaultMetricsServer( + "localhost", + 0, + logger, + nil, + 15*time.Second, + 15*time.Second, + 60*time.Second, + ) ctx, cancel := context.WithCancel(context.Background()) @@ -113,3 +137,38 @@ func TestMetricsServer_ContextCancellation(t *testing.T) { t.Fatal("Start did not return after context cancellation") } } + +// TestMetricsServer_Timeouts verifies that the metrics server is initialized with the correct timeouts. +func TestMetricsServer_Timeouts(t *testing.T) { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + // Define custom timeouts + readTimeout := 5 * time.Second + writeTimeout := 10 * time.Second + idleTimeout := 30 * time.Second + + // Create metrics server with custom timeouts using NewMetricsServer + metricsServer := NewMetricsServer("localhost", 0, logger, nil, readTimeout, writeTimeout, idleTimeout) + require.NotNil(t, metricsServer) + + assert.Equal(t, readTimeout, metricsServer.Server().ReadTimeout) + assert.Equal(t, writeTimeout, metricsServer.Server().WriteTimeout) + assert.Equal(t, idleTimeout, metricsServer.Server().IdleTimeout) + + // Test NewDefaultMetricsServer (currently hardcoded) + defaultMetricsServer := NewDefaultMetricsServer( + "localhost", + 0, + logger, + nil, + 5*time.Second, + 10*time.Second, + 30*time.Second, + ) + require.NotNil(t, defaultMetricsServer) + + // These should now pass + assert.Equal(t, 5*time.Second, defaultMetricsServer.Server().ReadTimeout) + assert.Equal(t, 10*time.Second, defaultMetricsServer.Server().WriteTimeout) + assert.Equal(t, 30*time.Second, defaultMetricsServer.Server().IdleTimeout) +}