From 73454ef0d93f5f9f7375bd62f05f1113cf2b0d8d Mon Sep 17 00:00:00 2001 From: Roboshyim Date: Sun, 9 Aug 2026 07:55:35 +0000 Subject: [PATCH] refactor: load configuration from env struct tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every setting is now a struct field with an `env` tag parsed by caarlos0/env, replacing the hand-rolled getEnv/getEnvBool helpers and the per-field strconv blocks. Defaults live next to the field, the AMQP block uses a nested envPrefix struct, and the OTLP/Datadog fallbacks are expressed as expanding defaults instead of nested getEnv calls. What tags cannot express is split into normalize (values derived from other settings) and validate (range and cross-field checks). Load now returns an error instead of calling os.Exit, and unparseable or out-of-range values fail startup rather than silently falling back to the default, so a typo cannot run an instance with settings nobody chose. telemetry.Setup takes a config struct instead of five positional strings and no longer reads OTEL_TRACES_SAMPLER_RATIO itself, so the API reads no environment variable outside the config package. Also fixes .env.example, which documented SMTP_FROM — a variable nothing reads; the sender address is MAIL_FROM. Co-Authored-By: Claude Opus 5 (1M context) --- SELF_HOSTING.md | 9 + api/.env.example | 2 +- api/fixtures.go | 5 +- api/go.mod | 1 + api/go.sum | 2 + api/internal/config/config.go | 339 ++++++++++------------- api/internal/config/config_test.go | 270 ++++++++++++++++-- api/internal/telemetry/telemetry.go | 41 ++- api/internal/telemetry/telemetry_test.go | 31 --- api/migrate.go | 18 +- api/server.go | 7 +- api/telemetry.go | 20 ++ api/worker.go | 7 +- 13 files changed, 465 insertions(+), 287 deletions(-) create mode 100644 api/telemetry.go diff --git a/SELF_HOSTING.md b/SELF_HOSTING.md index 18faff52..7b6280c1 100644 --- a/SELF_HOSTING.md +++ b/SELF_HOSTING.md @@ -93,6 +93,12 @@ The Docker image runs three different commands: ## Environment Variables +All variables are read once at startup. A value that cannot be parsed (a +non-numeric count, an unparseable duration, a boolean that is not +`true`/`false`) or that is out of range makes the process exit with an error +instead of falling back to the default, so a typo cannot silently run the +instance with settings you did not choose. + ### Required | Variable | Description | @@ -202,6 +208,9 @@ Required only if you use the deployment tracking feature. | `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` | | Override for trace-specific endpoint | | `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` | | Override for log-specific endpoint | | `OTEL_SERVICE_NAME` | `shopmon` | Service name in traces | +| `OTEL_DEPLOYMENT_ENVIRONMENT` | `$DD_ENV` | Deployment environment reported as a resource attribute | +| `OTEL_SERVICE_VERSION` | `$DD_VERSION`, else the build revision | Service version reported as a resource attribute | +| `OTEL_TRACES_SAMPLER_RATIO` | `1` | Head sampling ratio, clamped to `[0, 1]` | ## Reverse Proxy Examples diff --git a/api/.env.example b/api/.env.example index db4dfad5..0dfe6275 100644 --- a/api/.env.example +++ b/api/.env.example @@ -7,7 +7,7 @@ SMTP_PORT=1025 SMTP_SECURE=false SMTP_USER= SMTP_PASS= -SMTP_FROM=noreply@shopmon.io +MAIL_FROM=noreply@shopmon.io # Sitespeed.io Service APP_SITESPEED_ENDPOINT=http://localhost:3001 diff --git a/api/fixtures.go b/api/fixtures.go index 9f452b5c..5cc7132c 100644 --- a/api/fixtures.go +++ b/api/fixtures.go @@ -58,7 +58,10 @@ type orgFixture struct { } func runFixtures(ctx context.Context, skipShop bool) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } pool, err := database.NewPool(ctx, cfg.DatabaseURL) if err != nil { diff --git a/api/go.mod b/api/go.mod index 268d4b57..c1bc2c6b 100644 --- a/api/go.mod +++ b/api/go.mod @@ -69,6 +69,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/sso v1.33.2 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.38.2 // indirect github.com/aws/aws-sdk-go-v2/service/sts v1.45.2 // indirect + github.com/caarlos0/env/v11 v11.4.1 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect diff --git a/api/go.sum b/api/go.sum index 810fe8cb..54542ade 100644 --- a/api/go.sum +++ b/api/go.sum @@ -60,6 +60,8 @@ github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs= github.com/bsm/ginkgo/v2 v2.12.0/go.mod h1:SwYbGRRDovPVboqFv0tPTcG1sN61LM1Z4ARdbAV9g4c= github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= github.com/bsm/gomega v1.27.10/go.mod h1:JyEr/xRbxbtgWNi8tIEVPUYZ5Dzef52k01W3YH0H+O0= +github.com/caarlos0/env/v11 v11.4.1 h1:fYwH0sWEsBSMPG7t4e/PEfTFzrWrpjyygXyUnWiSwEw= +github.com/caarlos0/env/v11 v11.4.1/go.mod h1:qupehSf/Y0TUTsxKywqRt/vJjN5nz6vauiYEUUr8P4U= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= diff --git a/api/internal/config/config.go b/api/internal/config/config.go index 1cab8c34..f6b9dd16 100644 --- a/api/internal/config/config.go +++ b/api/internal/config/config.go @@ -1,203 +1,199 @@ +// Package config loads the application configuration from the environment. +// +// Every setting is declared as a struct field with an `env` tag: the tag names +// the variable, `envDefault` carries the fallback, and `expand` lets a default +// reference another variable (used for the OTLP/Datadog aliases). Anything that +// cannot be expressed as a tag — values derived from other settings, or checks +// that span fields — lives in normalize and validate below. package config import ( - "log/slog" + "errors" + "fmt" "net" "net/url" - "os" "runtime/debug" - "strconv" "strings" "time" + env "github.com/caarlos0/env/v11" "github.com/joho/godotenv" ) type Config struct { - AppSecret string - DatabaseURL string - RedisURL string - FrontendURL string + AppSecret string `env:"APP_SECRET"` + DatabaseURL string `env:"DATABASE_URL" envDefault:"postgres://shopmon:shopmon@localhost:5432/shopmon"` + RedisURL string `env:"REDIS_URL" envDefault:"redis://localhost:6379"` + FrontendURL string `env:"FRONTEND_URL" envDefault:"http://localhost:3000"` // QueueTransport selects the background job backend: "postgres" (default, // jobs live in the app database) or "amqp" (RabbitMQ/LavinMQ broker). - QueueTransport string + QueueTransport string `env:"QUEUE_TRANSPORT" envDefault:"postgres"` // QueueAMQP is only read when QueueTransport is "amqp". - QueueAMQP QueueAMQPConfig + QueueAMQP QueueAMQPConfig `envPrefix:"QUEUE_AMQP_"` - MailDSN string - MailFrom string - SMTPReplyTo string + // MailDSN is the go-mailer SMTP DSN. When unset it is assembled from the + // legacy SMTP* fields below so existing deployments keep working. + MailDSN string `env:"MAIL_DSN"` + MailFrom string `env:"MAIL_FROM" envDefault:"noreply@shopmon.io"` - SitespeedEndpoint string - SitespeedPrefix string - SitespeedAPIKey string + SMTPHost string `env:"SMTP_HOST" envDefault:"localhost"` + SMTPPort string `env:"SMTP_PORT" envDefault:"1025"` + SMTPUser string `env:"SMTP_USER"` + SMTPPass string `env:"SMTP_PASS"` + SMTPSecure bool `env:"SMTP_SECURE"` + SMTPReplyTo string `env:"SMTP_REPLY_TO"` - S3Endpoint string - S3AccessKey string - S3SecretKey string - S3Bucket string - S3Region string + SitespeedEndpoint string `env:"APP_SITESPEED_ENDPOINT"` + SitespeedPrefix string `env:"APP_SITESPEED_PREFIX" envDefault:"local-"` + SitespeedAPIKey string `env:"APP_SITESPEED_API_KEY"` - GithubClientID string - GithubClientSecret string + S3Endpoint string `env:"APP_S3_ENDPOINT"` + S3AccessKey string `env:"APP_S3_ACCESS_KEY_ID"` + S3SecretKey string `env:"APP_S3_SECRET_ACCESS_KEY"` + S3Bucket string `env:"APP_S3_BUCKET" envDefault:"shopmon"` + S3Region string `env:"APP_S3_REGION" envDefault:"auto"` - PackagesAPIURL string - PackagesAPIToken string + GithubClientID string `env:"APP_OAUTH_GITHUB_CLIENT_ID"` + GithubClientSecret string `env:"APP_OAUTH_GITHUB_CLIENT_SECRET"` - DisableRegistration bool + PackagesAPIURL string `env:"PACKAGES_API_URL"` + PackagesAPIToken string `env:"PACKAGES_API_TOKEN"` + + DisableRegistration bool `env:"DISABLE_REGISTRATION"` // DeploymentScrapeDelay is how long to wait after a CLI deployment is // recorded before re-scraping the environment, giving post-deploy tasks // (theme compile, indexing, cache warming) time to settle. - DeploymentScrapeDelay time.Duration + DeploymentScrapeDelay time.Duration `env:"DEPLOYMENT_SCRAPE_DELAY" envDefault:"5m"` - ShopwareAPIURL string + ShopwareAPIURL string `env:"SHOPWARE_API_URL" envDefault:"https://api.shopware.com"` // ShopwareChangelogURL is the base URL of the Shopware release changelog API // (index.json + per-version JSON) crawled hourly by the worker. - ShopwareChangelogURL string - - OtelEnabled bool - OtelTraceEndpoint string - OtelLogEndpoint string - OtelServiceName string - OtelDeploymentEnv string - OtelServiceVersion string - + ShopwareChangelogURL string `env:"SHOPWARE_CHANGELOG_URL" envDefault:"https://releases.shopware.com/changelog"` + + // The OTLP signal endpoints fall back to the generic + // OTEL_EXPORTER_OTLP_ENDPOINT, and service env/version to the Datadog + // unified service tagging variables. + OtelTraceEndpoint string `env:"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT,expand" envDefault:"${OTEL_EXPORTER_OTLP_ENDPOINT}"` + OtelLogEndpoint string `env:"OTEL_EXPORTER_OTLP_LOGS_ENDPOINT,expand" envDefault:"${OTEL_EXPORTER_OTLP_ENDPOINT}"` + OtelServiceName string `env:"OTEL_SERVICE_NAME" envDefault:"shopmon"` + OtelDeploymentEnv string `env:"OTEL_DEPLOYMENT_ENVIRONMENT,expand" envDefault:"${DD_ENV}"` + OtelServiceVersion string `env:"OTEL_SERVICE_VERSION,expand" envDefault:"${DD_VERSION}"` + OtelSamplerRatio float64 `env:"OTEL_TRACES_SAMPLER_RATIO" envDefault:"1"` + // OtelEnabled is derived from OtelTraceEndpoint. + OtelEnabled bool + + // The WebAuthn relying party is derived from FrontendURL. WebAuthnRPID string WebAuthnRPName string WebAuthnRPOrigins []string - ListenAddr string - TrustedProxies []string + ListenAddr string `env:"LISTEN_ADDR" envDefault:":8080"` + TrustedProxies []string `env:"TRUSTED_PROXIES"` // AuthRateLimitMax is the number of auth requests allowed per IP per minute. // Raise it (e.g. for E2E tests) via AUTH_RATE_LIMIT_MAX. - AuthRateLimitMax int + AuthRateLimitMax int `env:"AUTH_RATE_LIMIT_MAX" envDefault:"20"` } // QueueAMQPConfig holds the broker settings for the AMQP job transport. type QueueAMQPConfig struct { - DSN string - Exchange string - Queue string + DSN string `env:"DSN" envDefault:"amqp://guest:guest@localhost:5672/"` + Exchange string `env:"EXCHANGE" envDefault:"shopmon"` + Queue string `env:"QUEUE" envDefault:"shopmon"` // PrefetchCount bounds unacknowledged deliveries per consumer. Keep it at or // above the worker concurrency so workers never idle waiting for messages. - PrefetchCount int + PrefetchCount int `env:"PREFETCH" envDefault:"10"` // DelayedExchange declares the exchange as x-delayed-message so delayed jobs // (post-deployment scrapes, sitespeed reruns) are held by the broker instead // of being delivered immediately. Needs LavinMQ (native) or the RabbitMQ // delayed-message plugin. - DelayedExchange bool + DelayedExchange bool `env:"DELAYED_EXCHANGE" envDefault:"true"` } -func loadDotEnv() { +// Load reads the configuration from .env (when present) and the environment. +// An unparseable or out-of-range value is an error rather than a silent +// fallback, so a typo cannot start the process with a setting the operator +// never asked for. +func Load() (*Config, error) { _ = godotenv.Load() -} -func Load() *Config { - loadDotEnv() - - cfg := &Config{ - AppSecret: getEnv("APP_SECRET", ""), - DatabaseURL: getEnv("DATABASE_URL", "postgres://shopmon:shopmon@localhost:5432/shopmon"), - RedisURL: getEnv("REDIS_URL", "redis://localhost:6379"), - FrontendURL: getEnv("FRONTEND_URL", "http://localhost:3000"), - - QueueTransport: strings.ToLower(strings.TrimSpace(getEnv("QUEUE_TRANSPORT", "postgres"))), - QueueAMQP: QueueAMQPConfig{ - DSN: getEnv("QUEUE_AMQP_DSN", "amqp://guest:guest@localhost:5672/"), - Exchange: getEnv("QUEUE_AMQP_EXCHANGE", "shopmon"), - Queue: getEnv("QUEUE_AMQP_QUEUE", "shopmon"), - DelayedExchange: getEnvBool("QUEUE_AMQP_DELAYED_EXCHANGE", true), - }, - - MailDSN: mailDSN(), - MailFrom: getEnv("MAIL_FROM", "noreply@shopmon.io"), - SMTPReplyTo: getEnv("SMTP_REPLY_TO", ""), - - SitespeedEndpoint: getEnv("APP_SITESPEED_ENDPOINT", ""), - SitespeedPrefix: getEnv("APP_SITESPEED_PREFIX", "local-"), - SitespeedAPIKey: getEnv("APP_SITESPEED_API_KEY", ""), - - S3Endpoint: getEnv("APP_S3_ENDPOINT", ""), - S3AccessKey: getEnv("APP_S3_ACCESS_KEY_ID", ""), - S3SecretKey: getEnv("APP_S3_SECRET_ACCESS_KEY", ""), - S3Bucket: getEnv("APP_S3_BUCKET", "shopmon"), - S3Region: getEnv("APP_S3_REGION", "auto"), - - GithubClientID: getEnv("APP_OAUTH_GITHUB_CLIENT_ID", ""), - GithubClientSecret: getEnv("APP_OAUTH_GITHUB_CLIENT_SECRET", ""), - - PackagesAPIURL: getEnv("PACKAGES_API_URL", ""), - PackagesAPIToken: getEnv("PACKAGES_API_TOKEN", ""), - - DisableRegistration: getEnv("DISABLE_REGISTRATION", "false") == "true", - - ShopwareAPIURL: getEnv("SHOPWARE_API_URL", "https://api.shopware.com"), - ShopwareChangelogURL: getEnv("SHOPWARE_CHANGELOG_URL", "https://releases.shopware.com/changelog"), - - OtelEnabled: getEnv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", getEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "")) != "", - OtelTraceEndpoint: getEnv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", getEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "")), - OtelLogEndpoint: getEnv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", getEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "")), - OtelServiceName: getEnv("OTEL_SERVICE_NAME", "shopmon"), - OtelDeploymentEnv: getEnv("OTEL_DEPLOYMENT_ENVIRONMENT", getEnv("DD_ENV", "")), - OtelServiceVersion: getEnv("OTEL_SERVICE_VERSION", getEnv("DD_VERSION", buildVersion())), - - ListenAddr: getEnv("LISTEN_ADDR", ":8080"), - TrustedProxies: parseCommaList(getEnv("TRUSTED_PROXIES", "")), + cfg := &Config{} + if err := env.Parse(cfg); err != nil { + return nil, fmt.Errorf("read environment: %w", err) } - // Parse the post-deployment scrape delay, falling back to 5m on an - // empty or invalid value. - cfg.DeploymentScrapeDelay = 5 * time.Minute - if raw := getEnv("DEPLOYMENT_SCRAPE_DELAY", ""); raw != "" { - if d, err := time.ParseDuration(raw); err == nil && d >= 0 { - cfg.DeploymentScrapeDelay = d - } else { - slog.Warn("invalid DEPLOYMENT_SCRAPE_DELAY, using default", "value", raw, "default", cfg.DeploymentScrapeDelay) - } + cfg.normalize() + + if err := cfg.validate(); err != nil { + return nil, err } - // Parse the auth rate-limit budget (requests per IP per minute), falling - // back to 20 on an empty or invalid value. - cfg.AuthRateLimitMax = 20 - if raw := getEnv("AUTH_RATE_LIMIT_MAX", ""); raw != "" { - if n, err := strconv.Atoi(raw); err == nil && n > 0 { - cfg.AuthRateLimitMax = n - } else { - slog.Warn("invalid AUTH_RATE_LIMIT_MAX, using default", "value", raw, "default", cfg.AuthRateLimitMax) - } + return cfg, nil +} + +// normalize fills in the settings that are derived from other settings and +// cleans up the ones the struct tags cannot express on their own. +func (c *Config) normalize() { + c.QueueTransport = strings.ToLower(strings.TrimSpace(c.QueueTransport)) + c.TrustedProxies = trimList(c.TrustedProxies) + + if c.MailDSN == "" { + c.MailDSN = c.smtpDSN() } - // Parse the AMQP consumer prefetch, falling back to 10 (the worker - // concurrency) on an empty or invalid value. - cfg.QueueAMQP.PrefetchCount = 10 - if raw := getEnv("QUEUE_AMQP_PREFETCH", ""); raw != "" { - if n, err := strconv.Atoi(raw); err == nil && n > 0 { - cfg.QueueAMQP.PrefetchCount = n - } else { - slog.Warn("invalid QUEUE_AMQP_PREFETCH, using default", "value", raw, "default", cfg.QueueAMQP.PrefetchCount) - } + if c.OtelServiceVersion == "" { + c.OtelServiceVersion = buildVersion() } + c.OtelEnabled = c.OtelTraceEndpoint != "" + c.OtelSamplerRatio = min(max(c.OtelSamplerRatio, 0), 1) - // Derive WebAuthn config from FrontendURL - if parsed, err := url.Parse(cfg.FrontendURL); err == nil { - cfg.WebAuthnRPID = parsed.Hostname() - cfg.WebAuthnRPName = "Shopmon" - cfg.WebAuthnRPOrigins = []string{cfg.FrontendURL} + if parsed, err := url.Parse(c.FrontendURL); err == nil { + c.WebAuthnRPID = parsed.Hostname() + c.WebAuthnRPName = "Shopmon" + c.WebAuthnRPOrigins = []string{c.FrontendURL} } +} - // Validate APP_SECRET length for AES encryption - if cfg.AppSecret != "" { - keyLen := len(cfg.AppSecret) - if keyLen != 16 && keyLen != 24 && keyLen != 32 { - slog.Error("invalid APP_SECRET length: must be exactly 16, 24, or 32 bytes for AES encryption", "length", keyLen) - os.Exit(1) - } +func (c *Config) validate() error { + var errs []error + + // AES-128/192/256 need a key of exactly 16, 24 or 32 bytes. An empty secret + // stays allowed: commands that never touch encrypted data (migrate) run + // without one. + if n := len(c.AppSecret); n != 0 && n != 16 && n != 24 && n != 32 { + errs = append(errs, fmt.Errorf("APP_SECRET must be exactly 16, 24 or 32 bytes for AES encryption, got %d", n)) + } + if c.DeploymentScrapeDelay < 0 { + errs = append(errs, fmt.Errorf("DEPLOYMENT_SCRAPE_DELAY must not be negative, got %s", c.DeploymentScrapeDelay)) + } + if c.AuthRateLimitMax <= 0 { + errs = append(errs, fmt.Errorf("AUTH_RATE_LIMIT_MAX must be greater than 0, got %d", c.AuthRateLimitMax)) + } + if c.QueueAMQP.PrefetchCount <= 0 { + errs = append(errs, fmt.Errorf("QUEUE_AMQP_PREFETCH must be greater than 0, got %d", c.QueueAMQP.PrefetchCount)) } - return cfg + return errors.Join(errs...) +} + +// smtpDSN assembles a go-mailer SMTP DSN from the legacy SMTP* settings. +// SMTPSecure selects the smtps scheme (implicit TLS). +func (c *Config) smtpDSN() string { + scheme := "smtp" + if c.SMTPSecure { + scheme = "smtps" + } + + u := url.URL{ + Scheme: scheme, + Host: net.JoinHostPort(c.SMTPHost, c.SMTPPort), + } + if c.SMTPUser != "" { + u.User = url.UserPassword(c.SMTPUser, c.SMTPPass) + } + return u.String() } // buildVersion returns the VCS revision the binary was built from, embedded by @@ -232,68 +228,17 @@ func buildVersion() string { return revision } -// mailDSN returns the go-mailer SMTP DSN. It prefers the MAIL_DSN env var; when -// unset it assembles a DSN from the legacy SMTP_* vars so existing deployments -// keep working. SMTP_SECURE=true selects the smtps scheme (implicit TLS). -func mailDSN() string { - if dsn := getEnv("MAIL_DSN", ""); dsn != "" { - return dsn - } - - scheme := "smtp" - if getEnv("SMTP_SECURE", "false") == "true" { - scheme = "smtps" - } - - host := getEnv("SMTP_HOST", "localhost") - port := getEnv("SMTP_PORT", "1025") - - u := url.URL{ - Scheme: scheme, - Host: net.JoinHostPort(host, port), - } - if user := getEnv("SMTP_USER", ""); user != "" { - u.User = url.UserPassword(user, getEnv("SMTP_PASS", "")) - } - return u.String() -} - -// getEnvBool parses a boolean env var, accepting every representation -// strconv.ParseBool does (1, t, T, TRUE, true, True and their false -// counterparts). An unset or unparseable value keeps the fallback and warns, -// so a typo cannot silently flip a flag that defaults to on. -func getEnvBool(key string, fallback bool) bool { - raw := getEnv(key, "") - if raw == "" { - return fallback - } - - value, err := strconv.ParseBool(raw) - if err != nil { - slog.Warn("invalid boolean value, using default", "key", key, "value", raw, "default", fallback) - return fallback - } - return value -} - -func getEnv(key, fallback string) string { - if v := os.Getenv(key); v != "" { - return v +// trimList trims each entry of a comma-separated list and drops empty ones, so +// "a, b," yields ["a", "b"]. +func trimList(values []string) []string { + result := make([]string, 0, len(values)) + for _, v := range values { + if v = strings.TrimSpace(v); v != "" { + result = append(result, v) + } } - return fallback -} - -func parseCommaList(s string) []string { - if s == "" { + if len(result) == 0 { return nil } - parts := strings.Split(s, ",") - result := make([]string, 0, len(parts)) - for _, p := range parts { - p = strings.TrimSpace(p) - if p != "" { - result = append(result, p) - } - } return result } diff --git a/api/internal/config/config_test.go b/api/internal/config/config_test.go index 5bcb802c..b4f395f5 100644 --- a/api/internal/config/config_test.go +++ b/api/internal/config/config_test.go @@ -5,30 +5,34 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDeploymentScrapeDelay(t *testing.T) { tests := []struct { - name string - env string - want time.Duration + name string + env string + want time.Duration + wantErr bool }{ {name: "default when unset", env: "", want: 5 * time.Minute}, {name: "custom duration", env: "2m", want: 2 * time.Minute}, {name: "zero disables delay", env: "0s", want: 0}, - {name: "invalid falls back to default", env: "not-a-duration", want: 5 * time.Minute}, - {name: "negative falls back to default", env: "-1m", want: 5 * time.Minute}, + {name: "invalid is rejected", env: "not-a-duration", wantErr: true}, + {name: "negative is rejected", env: "-1m", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.env == "" { - t.Setenv("DEPLOYMENT_SCRAPE_DELAY", "") - } else { - t.Setenv("DEPLOYMENT_SCRAPE_DELAY", tt.env) + t.Setenv("DEPLOYMENT_SCRAPE_DELAY", tt.env) + + cfg, err := Load() + if tt.wantErr { + assert.Error(t, err) + return } - cfg := Load() + require.NoError(t, err) assert.Equal(t, tt.want, cfg.DeploymentScrapeDelay) }) } @@ -46,7 +50,8 @@ func TestQueueTransportDefaultsToPostgres(t *testing.T) { t.Setenv(key, "") } - cfg := Load() + cfg, err := Load() + require.NoError(t, err) assert.Equal(t, "postgres", cfg.QueueTransport) assert.Equal(t, "amqp://guest:guest@localhost:5672/", cfg.QueueAMQP.DSN) @@ -71,40 +76,52 @@ func TestQueueTransportIsNormalized(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Setenv("QUEUE_TRANSPORT", tt.env) - assert.Equal(t, tt.want, Load().QueueTransport) + cfg, err := Load() + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.QueueTransport) }) } } func TestQueueAMQPPrefetch(t *testing.T) { tests := []struct { - name string - env string - want int + name string + env string + want int + wantErr bool }{ {name: "default when unset", env: "", want: 10}, {name: "custom count", env: "50", want: 50}, - {name: "invalid falls back to default", env: "many", want: 10}, - {name: "zero falls back to default", env: "0", want: 10}, - {name: "negative falls back to default", env: "-1", want: 10}, + {name: "invalid is rejected", env: "many", wantErr: true}, + {name: "zero is rejected", env: "0", wantErr: true}, + {name: "negative is rejected", env: "-1", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv("QUEUE_AMQP_PREFETCH", tt.env) - assert.Equal(t, tt.want, Load().QueueAMQP.PrefetchCount) + cfg, err := Load() + if tt.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.QueueAMQP.PrefetchCount) }) } } func TestQueueAMQPDelayedExchange(t *testing.T) { // Delayed delivery is load-bearing (post-deployment scrapes, sitespeed - // reruns), so only an explicit, parseable false may turn it off. + // reruns), so it may only be turned off by an explicit, parseable false. + // Anything else fails the load instead of silently picking a side. tests := []struct { - name string - env string - want bool + name string + env string + want bool + wantErr bool }{ {name: "default when unset", env: "", want: true}, {name: "explicit false", env: "false", want: false}, @@ -113,15 +130,216 @@ func TestQueueAMQPDelayedExchange(t *testing.T) { {name: "uppercase TRUE", env: "TRUE", want: true}, {name: "titlecase True", env: "True", want: true}, {name: "one", env: "1", want: true}, - {name: "unparseable keeps delayed delivery on", env: "yes", want: true}, - {name: "typo keeps delayed delivery on", env: "flase", want: true}, + {name: "unparseable is rejected", env: "yes", wantErr: true}, + {name: "typo is rejected", env: "flase", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Setenv("QUEUE_AMQP_DELAYED_EXCHANGE", tt.env) - assert.Equal(t, tt.want, Load().QueueAMQP.DelayedExchange) + cfg, err := Load() + if tt.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.QueueAMQP.DelayedExchange) + }) + } +} + +func TestAuthRateLimitMax(t *testing.T) { + tests := []struct { + name string + env string + want int + wantErr bool + }{ + {name: "default when unset", env: "", want: 20}, + {name: "custom budget", env: "500", want: 500}, + {name: "invalid is rejected", env: "lots", wantErr: true}, + {name: "zero is rejected", env: "0", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("AUTH_RATE_LIMIT_MAX", tt.env) + + cfg, err := Load() + if tt.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.AuthRateLimitMax) }) } } + +func TestAppSecretLength(t *testing.T) { + tests := []struct { + name string + env string + wantErr bool + }{ + {name: "empty is allowed", env: ""}, + {name: "16 bytes", env: "0123456789abcdef"}, + {name: "24 bytes", env: "0123456789abcdef01234567"}, + {name: "32 bytes", env: "0123456789abcdef0123456789abcdef"}, + {name: "too short is rejected", env: "short", wantErr: true}, + {name: "odd length is rejected", env: "0123456789abcdef0", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("APP_SECRET", tt.env) + + _, err := Load() + if tt.wantErr { + assert.ErrorContains(t, err, "APP_SECRET") + return + } + + assert.NoError(t, err) + }) + } +} + +func TestMailDSN(t *testing.T) { + t.Run("MAIL_DSN wins", func(t *testing.T) { + t.Setenv("MAIL_DSN", "smtp://custom:2525") + t.Setenv("SMTP_HOST", "ignored") + + cfg, err := Load() + require.NoError(t, err) + assert.Equal(t, "smtp://custom:2525", cfg.MailDSN) + }) + + tests := []struct { + name string + host string + port string + user string + pass string + secure string + want string + }{ + {name: "defaults to local mailpit", want: "smtp://localhost:1025"}, + {name: "host and port", host: "mail.example.com", port: "587", want: "smtp://mail.example.com:587"}, + {name: "credentials", host: "mail.example.com", port: "587", user: "u", pass: "p", want: "smtp://u:p@mail.example.com:587"}, + {name: "secure selects smtps", host: "mail.example.com", port: "465", secure: "true", want: "smtps://mail.example.com:465"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("MAIL_DSN", "") + t.Setenv("SMTP_HOST", tt.host) + t.Setenv("SMTP_PORT", tt.port) + t.Setenv("SMTP_USER", tt.user) + t.Setenv("SMTP_PASS", tt.pass) + t.Setenv("SMTP_SECURE", tt.secure) + + cfg, err := Load() + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.MailDSN) + }) + } +} + +func TestOtelEndpointsFallBackToGenericEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector:4318") + t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "") + t.Setenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT", "") + + cfg, err := Load() + require.NoError(t, err) + + assert.Equal(t, "http://collector:4318", cfg.OtelTraceEndpoint) + assert.Equal(t, "http://collector:4318", cfg.OtelLogEndpoint) + assert.True(t, cfg.OtelEnabled) + + t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces:4318") + + cfg, err = Load() + require.NoError(t, err) + + assert.Equal(t, "http://traces:4318", cfg.OtelTraceEndpoint) + assert.Equal(t, "http://collector:4318", cfg.OtelLogEndpoint) +} + +func TestOtelDisabledWithoutEndpoint(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "") + + cfg, err := Load() + require.NoError(t, err) + + assert.Empty(t, cfg.OtelTraceEndpoint) + assert.False(t, cfg.OtelEnabled) +} + +func TestOtelSamplerRatioIsClamped(t *testing.T) { + tests := []struct { + name string + env string + want float64 + wantErr bool + }{ + {name: "default samples everything", env: "", want: 1}, + {name: "half", env: "0.5", want: 0.5}, + {name: "above one clamps", env: "2.5", want: 1}, + {name: "below zero clamps", env: "-1", want: 0}, + {name: "invalid is rejected", env: "not-a-number", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("OTEL_TRACES_SAMPLER_RATIO", tt.env) + + cfg, err := Load() + if tt.wantErr { + assert.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.OtelSamplerRatio) + }) + } +} + +func TestTrustedProxiesAreTrimmed(t *testing.T) { + tests := []struct { + name string + env string + want []string + }{ + {name: "unset", env: "", want: nil}, + {name: "single", env: "10.0.0.1", want: []string{"10.0.0.1"}}, + {name: "trimmed and empties dropped", env: " 10.0.0.1 , ,10.0.0.2,", want: []string{"10.0.0.1", "10.0.0.2"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TRUSTED_PROXIES", tt.env) + + cfg, err := Load() + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.TrustedProxies) + }) + } +} + +func TestWebAuthnDerivedFromFrontendURL(t *testing.T) { + t.Setenv("FRONTEND_URL", "https://shopmon.example.com") + + cfg, err := Load() + require.NoError(t, err) + + assert.Equal(t, "shopmon.example.com", cfg.WebAuthnRPID) + assert.Equal(t, "Shopmon", cfg.WebAuthnRPName) + assert.Equal(t, []string{"https://shopmon.example.com"}, cfg.WebAuthnRPOrigins) +} diff --git a/api/internal/telemetry/telemetry.go b/api/internal/telemetry/telemetry.go index 138c75df..c7ac82f9 100644 --- a/api/internal/telemetry/telemetry.go +++ b/api/internal/telemetry/telemetry.go @@ -7,7 +7,6 @@ import ( "log/slog" "net/url" "os" - "strconv" "time" "go.opentelemetry.io/contrib/bridges/otelslog" @@ -22,11 +21,25 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.26.0" ) +// Config describes the exporters and the resource attributes they report. +type Config struct { + ServiceName string + Version string + DeploymentEnv string + TraceEndpoint string + LogEndpoint string + // SamplerRatio is the head sampling ratio in [0, 1]; 1 samples everything. + SamplerRatio float64 +} + // Setup initializes OpenTelemetry tracing and logging with OTLP HTTP exporters. // It sets slog's default logger to a handler that sends logs via OTLP and also // writes to stderr. Returns a shutdown function that should be called on application exit. -// If endpoint is empty, telemetry is disabled and a no-op shutdown is returned. -func Setup(ctx context.Context, serviceName, version, deploymentEnv, traceEndpoint, logEndpoint string) (shutdown func(context.Context) error) { +// If both endpoints are empty, telemetry is disabled and a no-op shutdown is returned. +func Setup(ctx context.Context, cfg Config) (shutdown func(context.Context) error) { + serviceName, version, deploymentEnv := cfg.ServiceName, cfg.Version, cfg.DeploymentEnv + traceEndpoint, logEndpoint := cfg.TraceEndpoint, cfg.LogEndpoint + if traceEndpoint == "" && logEndpoint == "" { return func(context.Context) error { return nil } } @@ -67,7 +80,7 @@ func Setup(ctx context.Context, serviceName, version, deploymentEnv, traceEndpoi tp := sdktrace.NewTracerProvider( sdktrace.WithBatcher(traceExporter), sdktrace.WithResource(res), - sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(parseSamplerRatio()))), + sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(cfg.SamplerRatio))), ) otel.SetTracerProvider(tp) otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator( @@ -110,26 +123,6 @@ func Setup(ctx context.Context, serviceName, version, deploymentEnv, traceEndpoi } } -// parseSamplerRatio reads OTEL_TRACES_SAMPLER_RATIO and returns it clamped to -// [0, 1]. It defaults to 1.0 (sample everything) when unset or unparseable. -func parseSamplerRatio() float64 { - raw := os.Getenv("OTEL_TRACES_SAMPLER_RATIO") - if raw == "" { - return 1.0 - } - ratio, err := strconv.ParseFloat(raw, 64) - if err != nil { - return 1.0 - } - if ratio < 0 { - return 0 - } - if ratio > 1 { - return 1 - } - return ratio -} - // ensurePath appends defaultPath to the endpoint URL if it has no path set. func ensurePath(endpoint, defaultPath string) string { u, err := url.Parse(endpoint) diff --git a/api/internal/telemetry/telemetry_test.go b/api/internal/telemetry/telemetry_test.go index 424fef69..0c2a27c3 100644 --- a/api/internal/telemetry/telemetry_test.go +++ b/api/internal/telemetry/telemetry_test.go @@ -4,11 +4,9 @@ import ( "context" "errors" "log/slog" - "os" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestEnsurePath(t *testing.T) { @@ -51,35 +49,6 @@ func TestEnsurePath(t *testing.T) { } } -func TestParseSamplerRatio(t *testing.T) { - tests := []struct { - name string - set bool - env string - want float64 - }{ - {name: "unset defaults to 1.0", set: false, want: 1.0}, - {name: "empty defaults to 1.0", set: true, env: "", want: 1.0}, - {name: "half", set: true, env: "0.5", want: 0.5}, - {name: "one", set: true, env: "1.0", want: 1.0}, - {name: "zero", set: true, env: "0.0", want: 0.0}, - {name: "above one clamps", set: true, env: "2.5", want: 1.0}, - {name: "below zero clamps", set: true, env: "-1", want: 0.0}, - {name: "invalid defaults to 1.0", set: true, env: "not-a-number", want: 1.0}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.set { - t.Setenv("OTEL_TRACES_SAMPLER_RATIO", tt.env) - } else { - require.NoError(t, os.Unsetenv("OTEL_TRACES_SAMPLER_RATIO")) - } - assert.Equal(t, tt.want, parseSamplerRatio()) - }) - } -} - type fakeHandler struct { calls int enabled bool diff --git a/api/migrate.go b/api/migrate.go index 697d0e60..5b3773af 100644 --- a/api/migrate.go +++ b/api/migrate.go @@ -45,7 +45,11 @@ func migrateCmd() *cobra.Command { Use: "up", Short: "Run all pending migrations", RunE: func(cmd *cobra.Command, args []string) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } + m, err := newMigrate(cfg.DatabaseURL) if err != nil { return err @@ -65,7 +69,11 @@ func migrateCmd() *cobra.Command { Use: "down", Short: "Rollback the last migration", RunE: func(cmd *cobra.Command, args []string) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } + m, err := newMigrate(cfg.DatabaseURL) if err != nil { return err @@ -85,7 +93,11 @@ func migrateCmd() *cobra.Command { Use: "status", Short: "Show current migration version", RunE: func(cmd *cobra.Command, args []string) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } + m, err := newMigrate(cfg.DatabaseURL) if err != nil { return err diff --git a/api/server.go b/api/server.go index 14ea7063..6752edb7 100644 --- a/api/server.go +++ b/api/server.go @@ -76,13 +76,16 @@ func serverCmd() *cobra.Command { } func runServer(cmd *cobra.Command, args []string) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) defer stop() // OpenTelemetry - otelShutdown := telemetry.Setup(ctx, cfg.OtelServiceName, cfg.OtelServiceVersion, cfg.OtelDeploymentEnv, cfg.OtelTraceEndpoint, cfg.OtelLogEndpoint) + otelShutdown := telemetry.Setup(ctx, telemetryConfig(cfg, cfg.OtelServiceName)) defer func() { if err := otelShutdown(context.Background()); err != nil { slog.Error("otel shutdown error", "error", err) diff --git a/api/telemetry.go b/api/telemetry.go new file mode 100644 index 00000000..4e8eef3c --- /dev/null +++ b/api/telemetry.go @@ -0,0 +1,20 @@ +package main + +import ( + "github.com/friendsofshopware/shopmon/api/internal/config" + "github.com/friendsofshopware/shopmon/api/internal/telemetry" +) + +// telemetryConfig translates the OTel environment configuration into the +// telemetry setup config. The service name is passed in because the worker +// reports under its own name. +func telemetryConfig(cfg *config.Config, serviceName string) telemetry.Config { + return telemetry.Config{ + ServiceName: serviceName, + Version: cfg.OtelServiceVersion, + DeploymentEnv: cfg.OtelDeploymentEnv, + TraceEndpoint: cfg.OtelTraceEndpoint, + LogEndpoint: cfg.OtelLogEndpoint, + SamplerRatio: cfg.OtelSamplerRatio, + } +} diff --git a/api/worker.go b/api/worker.go index f0fdb40a..54e4906b 100644 --- a/api/worker.go +++ b/api/worker.go @@ -33,12 +33,15 @@ func workerCmd() *cobra.Command { } func runWorker(cmd *cobra.Command, args []string) error { - cfg := config.Load() + cfg, err := config.Load() + if err != nil { + return err + } ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) defer stop() - otelShutdown := telemetry.Setup(ctx, cfg.OtelServiceName+"-worker", cfg.OtelServiceVersion, cfg.OtelDeploymentEnv, cfg.OtelTraceEndpoint, cfg.OtelLogEndpoint) + otelShutdown := telemetry.Setup(ctx, telemetryConfig(cfg, cfg.OtelServiceName+"-worker")) defer func() { if err := otelShutdown(context.Background()); err != nil { slog.Error("otel shutdown error", "error", err)