Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Track fix_metrics_server_hardcoded_timeouts_20260307 Context

- [Specification](./spec.md)
- [Implementation Plan](./plan.md)
- [Metadata](./metadata.json)
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions conductor/tech-stack.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions conductor/tracks.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
This file tracks all major tracks for the project. Each track has its own detailed plan in its respective folder.

---

3 changes: 3 additions & 0 deletions internal/app/di.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions internal/app/di_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 62 additions & 20 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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", ""),
Expand Down
Loading
Loading