diff --git a/esignet-service/data/deployment.yaml b/esignet-service/data/deployment.yaml index 752e8ab76..f05938cdc 100644 --- a/esignet-service/data/deployment.yaml +++ b/esignet-service/data/deployment.yaml @@ -9,6 +9,9 @@ # read from this file; they're documented here for operators anyway. identifier: "${NAMESPACE}" port: 8080 +# Private Prometheus scrape port, not routed through the public ingress. +# Override with METRICS_PORT (env wins over this value). +metrics_port: 9090 issuer: "${MOSIP_ESIGNET_HOST}" data_dir: "${DATA_DIR}" @@ -45,6 +48,13 @@ redis: # REDIS_DIAL_TIMEOUT_SECS, REDIS_READ_TIMEOUT_SECS, REDIS_WRITE_TIMEOUT_SECS, # REDIS_POOL_TIMEOUT_SECS) is env-var-driven only and cannot be set here — # see the Redis doc comment in internal/config/redis.go. + # + # At startup the service logs one "redis pool config resolved" INFO line + # giving each setting's effective value and a "Source" field naming + # where it came from (env / default). An env var that is set but unusable + # (unparseable, or non-positive where a positive value is required) is + # reported as an "ignoring invalid config env var" WARN naming the rejected + # value and the value actually used — it is never discarded silently. db: dsn: "${DATABASE_URL}" @@ -52,7 +62,17 @@ db: # DB_CONN_MAX_LIFETIME_SECS, DB_CONN_MAX_IDLE_TIME_SECS) > these values > # compiled-in default (see loadDB in internal/config/db.go). Only an env # var of "0" opts *_lifetime_secs out to "no limit"; 0 here is treated as - # "not set". + # "not set", and a negative env var is rejected as invalid. + # + # The env var always wins over the values below — it is never silently + # shadowed by them. At startup the service logs one "db pool config + # resolved" INFO line giving each setting's effective value and a + # "Source" field naming where it came from (env / yaml / default), so + # "did my env var take effect?" is answered by the log rather than by + # guesswork. An env var that is set but unusable (unparseable, or + # non-positive where a positive value is required) is reported as an + # "ignoring invalid config env var" WARN naming the rejected value and the + # value actually used — it is never discarded silently. pool: max_open_conns: 25 max_idle_conns: 5 # kept well below max_open_conns: pgxpool eagerly diff --git a/esignet-service/internal/config/app.go b/esignet-service/internal/config/app.go index cec39f85a..c5118f93a 100644 --- a/esignet-service/internal/config/app.go +++ b/esignet-service/internal/config/app.go @@ -299,7 +299,12 @@ func applyDefaults(cfg *AppConfig) { cfg.Port = envIntOrConfigOrDefault("PORT", cfg.Port, defaultPort) cfg.Issuer = envOrConfigOrDefault("MOSIP_ESIGNET_HOST", cfg.Issuer, fmt.Sprintf("http://localhost:%d", cfg.Port)) cfg.DataDir = envOrConfigOrDefault("DATA_DIR", cfg.DataDir, defaultDataDir) - cfg.MetricsPort = envIntOrDefault("METRICS_PORT", defaultMetricsPort) + // Threads cfg.MetricsPort (the decoded `metrics_port:` value) through like + // cfg.Port above. Previously this read only the env var and the compiled + // default, so a metrics_port set in deployment.yaml parsed cleanly and was + // then silently discarded — the mirror image of the yaml-shadows-env trap + // in issue #2498, and the same "dead config" surprise for an operator. + cfg.MetricsPort = envIntOrConfigOrDefault("METRICS_PORT", cfg.MetricsPort, defaultMetricsPort) cfg.DB = loadDB(cfg.DB) cfg.PProfConfig = PProfConfig{ @@ -685,41 +690,122 @@ func envOrConfigOrDefault(key, fromYAML, fallback string) string { return fallback } -// configOrDefault is the shared yaml/fallback tail for envIntOrConfigOrDefault -// and envIntOrConfigOrDefaultAllowEnvZero: fromYAML wins if positive, -// otherwise fallback. A yaml value of 0 (or an omitted field, which decodes -// to the same zero value) can't be distinguished from "not configured", so -// neither variant can honor an explicit yaml 0 the way they can an explicit -// env var 0. -func configOrDefault(fromYAML, fallback int) int { +// configSource identifies which tier of the env > yaml > default precedence +// chain supplied a resolved setting's effective value. Reported alongside the +// value at startup so an operator can tell "my env var took effect" apart +// from "my env var was ignored and yaml won" without reading source. +type configSource string + +const ( + sourceEnv configSource = "env" + sourceYAML configSource = "yaml" + sourceDefault configSource = "default" +) + +// warnIgnoredEnvVar reports an env var that was set but could not be used, +// naming the rejected value alongside the value and source actually taking +// effect — so "why didn't my env var change anything" is answered by the log +// rather than by reading source. Mirrors envPositiveInt's existing warning. +func warnIgnoredEnvVar(key, raw string, using int, src configSource) { + applog.GetLogger().Warn(context.Background(), + "ignoring invalid config env var; falling back to lower-precedence value", + applog.String("key", key), + applog.String("value", raw), + applog.Int("usingValue", using), + applog.String("usingSource", string(src))) +} + +// resolvedSetting pairs a setting's effective value with the tier that +// supplied it, for reporting via logResolvedSettings. +type resolvedSetting struct { + name string + value int + src configSource +} + +// logResolvedSettings emits one INFO line reporting each setting's effective +// value alongside a "Source" field naming the tier it came from. The +// suffix convention lives here rather than at each call site so a dozen +// settings across db.go and redis.go can't drift apart. +func logResolvedSettings(msg string, settings ...resolvedSetting) { + fields := make([]applog.Field, 0, len(settings)*2) + for _, s := range settings { + fields = append(fields, + applog.Int(s.name, s.value), + applog.String(s.name+"Source", string(s.src))) + } + applog.GetLogger().Info(context.Background(), msg, fields...) +} + +// configOrDefaultSourced is the shared yaml/fallback tail for the resolvers +// below: fromYAML wins if positive, otherwise fallback. A yaml value of 0 (or +// an omitted field, which decodes to the same zero value) can't be +// distinguished from "not configured", so no variant can honor an explicit +// yaml 0 the way they can an explicit env var 0. +func configOrDefaultSourced(fromYAML, fallback int) (int, configSource) { if fromYAML > 0 { - return fromYAML + return fromYAML, sourceYAML } - return fallback + return fallback, sourceDefault +} + +// configOrDefault is configOrDefaultSourced without the source. +func configOrDefault(fromYAML, fallback int) int { + v, _ := configOrDefaultSourced(fromYAML, fallback) + return v } -// envIntOrConfigOrDefault is envOrConfigOrDefault for int settings. A -// zero/negative value at any tier is treated as "not set" and falls through -// to the next tier — this deliberately can't express an env-var-only "0 = -// no limit" opt-out; use envIntOrConfigOrDefaultAllowEnvZero for those. +// envIntOrConfigOrDefaultSourced is envOrConfigOrDefault for int settings, +// additionally reporting which tier supplied the value. A zero/negative value +// at any tier is treated as "not set" and falls through to the next tier — +// this deliberately can't express an env-var-only "0 = no limit" opt-out; use +// envIntOrConfigOrDefaultAllowEnvZeroSourced for those. +// +// An env var that is set but unusable (unparseable, or non-positive where +// positive is required) is warned about rather than silently discarded: that +// silence is the "dead config" trap this variant exists to close. +func envIntOrConfigOrDefaultSourced(key string, fromYAML, fallback int) (int, configSource) { + raw := strings.TrimSpace(os.Getenv(key)) + if raw == "" { + return configOrDefaultSourced(fromYAML, fallback) + } + if n, err := strconv.Atoi(raw); err == nil && n > 0 { + return n, sourceEnv + } + v, src := configOrDefaultSourced(fromYAML, fallback) + warnIgnoredEnvVar(key, raw, v, src) + return v, src +} + +// envIntOrConfigOrDefault is envIntOrConfigOrDefaultSourced without the source. func envIntOrConfigOrDefault(key string, fromYAML, fallback int) int { - if v := envIntOrDefault(key, 0); v > 0 { - return v + v, _ := envIntOrConfigOrDefaultSourced(key, fromYAML, fallback) + return v +} + +// envIntOrConfigOrDefaultAllowEnvZeroSourced is like +// envIntOrConfigOrDefaultSourced, but an explicitly-set env var of "0" is +// honored as-is rather than treated as "not set". Used for settings where 0 +// is a documented env-var-only opt-out (e.g. "0 = no limit" for a connection +// lifetime). Negative values remain invalid and are warned about. +func envIntOrConfigOrDefaultAllowEnvZeroSourced(key string, fromYAML, fallback int) (int, configSource) { + raw := strings.TrimSpace(os.Getenv(key)) + if raw == "" { + return configOrDefaultSourced(fromYAML, fallback) } - return configOrDefault(fromYAML, fallback) + if n, err := strconv.Atoi(raw); err == nil && n >= 0 { + return n, sourceEnv + } + v, src := configOrDefaultSourced(fromYAML, fallback) + warnIgnoredEnvVar(key, raw, v, src) + return v, src } -// envIntOrConfigOrDefaultAllowEnvZero is like envIntOrConfigOrDefault, but an -// explicitly-set env var of "0" is honored as-is rather than treated as -// "not set". Used for settings where 0 is a documented env-var-only opt-out -// (e.g. "0 = no limit" for a connection lifetime). +// envIntOrConfigOrDefaultAllowEnvZero is +// envIntOrConfigOrDefaultAllowEnvZeroSourced without the source. func envIntOrConfigOrDefaultAllowEnvZero(key string, fromYAML, fallback int) int { - if raw := os.Getenv(key); raw != "" { - if n, err := strconv.Atoi(raw); err == nil { - return n - } - } - return configOrDefault(fromYAML, fallback) + v, _ := envIntOrConfigOrDefaultAllowEnvZeroSourced(key, fromYAML, fallback) + return v } func envBool(key string) bool { diff --git a/esignet-service/internal/config/app_test.go b/esignet-service/internal/config/app_test.go index 2bdf9e05a..0f5da5c53 100644 --- a/esignet-service/internal/config/app_test.go +++ b/esignet-service/internal/config/app_test.go @@ -934,3 +934,125 @@ func (ts *AppConfigTestSuite) TestEnvBool() { }) } } + +// The env > yaml > default chain is the tuning surface operators actually +// use, so these lock down both the value it resolves to and the source it +// reports — the source is what the startup "… pool config resolved" log line +// turns into an answer for "did my env var take effect?" (issue #2498). +func TestEnvIntOrConfigOrDefaultSourced(t *testing.T) { + const key = "ESIGNET_TEST_SOURCED_INT" + + cases := []struct { + name string + env string + fromYAML int + fallback int + wantVal int + wantSrc configSource + }{ + {"env wins over yaml and default", "50", 15, 25, 50, sourceEnv}, + {"env wins with surrounding whitespace", " 50 ", 15, 25, 50, sourceEnv}, + {"yaml wins when env unset", "", 15, 25, 15, sourceYAML}, + {"default when neither set", "", 0, 25, 25, sourceDefault}, + // The "dead config" trap from issue #2498: each of these is an env var + // the operator believes they set. It must resolve to the lower tier + // *and report that tier*, never masquerade as an env-sourced value. + {"unparseable env falls back to yaml", "abc", 15, 25, 15, sourceYAML}, + {"unparseable env falls back to default", "abc", 0, 25, 25, sourceDefault}, + {"zero env falls back to yaml", "0", 15, 25, 15, sourceYAML}, + {"negative env falls back to yaml", "-5", 15, 25, 15, sourceYAML}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(key, tc.env) + + gotVal, gotSrc := envIntOrConfigOrDefaultSourced(key, tc.fromYAML, tc.fallback) + + require.Equal(t, tc.wantVal, gotVal) + require.Equal(t, tc.wantSrc, gotSrc) + // The source-less wrapper must stay in lockstep; ~40 existing call + // sites still go through it. + require.Equal(t, tc.wantVal, envIntOrConfigOrDefault(key, tc.fromYAML, tc.fallback)) + }) + } +} + +func TestEnvIntOrConfigOrDefaultAllowEnvZeroSourced(t *testing.T) { + const key = "ESIGNET_TEST_SOURCED_INT_ZERO" + + cases := []struct { + name string + env string + fromYAML int + fallback int + wantVal int + wantSrc configSource + }{ + // Unlike the variant above, an explicit "0" is a documented opt-out + // ("no limit"), not a "not set" signal. + {"explicit zero env is honored", "0", 1800, 1800, 0, sourceEnv}, + {"positive env wins", "600", 1800, 1800, 600, sourceEnv}, + {"yaml wins when env unset", "", 900, 1800, 900, sourceYAML}, + {"default when neither set", "", 0, 1800, 1800, sourceDefault}, + {"unparseable env falls back to yaml", "abc", 900, 1800, 900, sourceYAML}, + {"negative env falls back to default", "-5", 0, 1800, 1800, sourceDefault}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(key, tc.env) + + gotVal, gotSrc := envIntOrConfigOrDefaultAllowEnvZeroSourced(key, tc.fromYAML, tc.fallback) + + require.Equal(t, tc.wantVal, gotVal) + require.Equal(t, tc.wantSrc, gotSrc) + require.Equal(t, tc.wantVal, envIntOrConfigOrDefaultAllowEnvZero(key, tc.fromYAML, tc.fallback)) + }) + } +} + +func TestConfigOrDefaultSourced(t *testing.T) { + v, src := configOrDefaultSourced(15, 25) + require.Equal(t, 15, v) + require.Equal(t, sourceYAML, src) + + v, src = configOrDefaultSourced(0, 25) + require.Equal(t, 25, v) + require.Equal(t, sourceDefault, src) + + // A yaml 0 is indistinguishable from an omitted field, so it can never be + // an opt-out the way an explicit env "0" can. + require.Equal(t, 25, configOrDefault(0, 25)) + require.Equal(t, 15, configOrDefault(15, 25)) +} + +// metrics_port is a yaml-decodable field (AppConfig.MetricsPort), and +// KnownFields(true) means setting it in deployment.yaml parses cleanly — so a +// value set there must actually take effect. Sibling `port` on the line above +// honors yaml via envIntOrConfigOrDefault; metrics_port must not silently +// diverge from it (issue #2498, acceptance criterion 4). +func TestApplyDefaults_MetricsPortHonorsYAML(t *testing.T) { + t.Setenv("METRICS_PORT", "") + + cfg := &AppConfig{MetricsPort: 9091} + applyDefaults(cfg) + + require.Equal(t, 9091, cfg.MetricsPort) +} + +func TestApplyDefaults_MetricsPortEnvWinsOverYAML(t *testing.T) { + t.Setenv("METRICS_PORT", "9099") + + cfg := &AppConfig{MetricsPort: 9091} + applyDefaults(cfg) + + require.Equal(t, 9099, cfg.MetricsPort) +} + +func TestApplyDefaults_MetricsPortFallsBackToDefault(t *testing.T) { + t.Setenv("METRICS_PORT", "") + + cfg := &AppConfig{} + applyDefaults(cfg) + + require.Equal(t, defaultMetricsPort, cfg.MetricsPort) +} diff --git a/esignet-service/internal/config/db.go b/esignet-service/internal/config/db.go index 3d204bbfe..c51f946de 100644 --- a/esignet-service/internal/config/db.go +++ b/esignet-service/internal/config/db.go @@ -145,15 +145,29 @@ func resolveDBDSN(yamlDSN string) string { func loadDB(yamlDB DB) DB { dsn := resolveDBDSN(yamlDB.DSN) - maxOpen := clampPositiveInt32("DB_MAX_OPEN_CONNS", envIntOrConfigOrDefault("DB_MAX_OPEN_CONNS", yamlDB.Pool.MaxOpenConns, defaultDBMaxOpenConns)) - maxIdle := clampPositiveInt32("DB_MAX_IDLE_CONNS", envIntOrConfigOrDefault("DB_MAX_IDLE_CONNS", yamlDB.Pool.MaxIdleConns, defaultDBMaxIdleConns)) + maxOpenRaw, maxOpenSrc := envIntOrConfigOrDefaultSourced("DB_MAX_OPEN_CONNS", yamlDB.Pool.MaxOpenConns, defaultDBMaxOpenConns) + maxOpen := clampPositiveInt32("DB_MAX_OPEN_CONNS", maxOpenRaw) + maxIdleRaw, maxIdleSrc := envIntOrConfigOrDefaultSourced("DB_MAX_IDLE_CONNS", yamlDB.Pool.MaxIdleConns, defaultDBMaxIdleConns) + maxIdle := clampPositiveInt32("DB_MAX_IDLE_CONNS", maxIdleRaw) // An explicit env var of "0" means "no limit" — same convention as // database/sql and Redis. See effectiveMaxConnLifetime in Open() for why // pgxpool needs this translated rather than passed through. A yaml value // of 0 (or an omitted field, which decodes to the same zero value) can't // be distinguished from "not configured", so only the env var can opt out. - lifetimeSecs := clampDurationSecs("DB_CONN_MAX_LIFETIME_SECS", envIntOrConfigOrDefaultAllowEnvZero("DB_CONN_MAX_LIFETIME_SECS", yamlDB.Pool.ConnMaxLifetimeSecs, defaultDBConnMaxLifetimeSecs)) - idleSecs := clampDurationSecs("DB_CONN_MAX_IDLE_TIME_SECS", envIntOrConfigOrDefault("DB_CONN_MAX_IDLE_TIME_SECS", yamlDB.Pool.ConnMaxIdleTimeSecs, defaultDBConnMaxIdleTimeSecs)) + lifetimeRaw, lifetimeSrc := envIntOrConfigOrDefaultAllowEnvZeroSourced("DB_CONN_MAX_LIFETIME_SECS", yamlDB.Pool.ConnMaxLifetimeSecs, defaultDBConnMaxLifetimeSecs) + lifetimeSecs := clampDurationSecs("DB_CONN_MAX_LIFETIME_SECS", lifetimeRaw) + idleRaw, idleSrc := envIntOrConfigOrDefaultSourced("DB_CONN_MAX_IDLE_TIME_SECS", yamlDB.Pool.ConnMaxIdleTimeSecs, defaultDBConnMaxIdleTimeSecs) + idleSecs := clampDurationSecs("DB_CONN_MAX_IDLE_TIME_SECS", idleRaw) + + // Report the effective value *and* which tier supplied it. Logging only + // the values (as the "postgres connected" line in cmd/esignet/main.go + // does) leaves an operator unable to tell an env var that took effect + // from one that was ignored in favour of deployment.yaml. + logResolvedSettings("db pool config resolved", + resolvedSetting{"maxOpenConns", maxOpen, maxOpenSrc}, + resolvedSetting{"maxIdleConns", maxIdle, maxIdleSrc}, + resolvedSetting{"connMaxLifetimeSecs", lifetimeSecs, lifetimeSrc}, + resolvedSetting{"connMaxIdleTimeSecs", idleSecs, idleSrc}) return DB{ DSN: dsn, diff --git a/esignet-service/internal/config/db_test.go b/esignet-service/internal/config/db_test.go index b8d367489..03811a9d9 100644 --- a/esignet-service/internal/config/db_test.go +++ b/esignet-service/internal/config/db_test.go @@ -227,3 +227,49 @@ func TestEffectiveMaxConnLifetime(t *testing.T) { require.Equal(t, dbUnlimitedConnLifetime, effectiveMaxConnLifetime(-time.Second)) require.Equal(t, 30*time.Minute, effectiveMaxConnLifetime(30*time.Minute)) } + +// Regression lock for issue #2498: the shipped data/deployment.yaml ships a +// populated db.pool block, so every deployment has yaml values sitting ready +// to shadow an operator's env var. They must not win. +func TestLoadDB_EnvWinsOverPopulatedYAMLPool(t *testing.T) { + t.Setenv("DB_MAX_OPEN_CONNS", "50") + t.Setenv("DB_MAX_IDLE_CONNS", "9") + t.Setenv("DB_CONN_MAX_LIFETIME_SECS", "600") + t.Setenv("DB_CONN_MAX_IDLE_TIME_SECS", "120") + + // Mirrors the values shipped in data/deployment.yaml. + db := loadDB(DB{Pool: DBPool{ + MaxOpenConns: 25, + MaxIdleConns: 5, + ConnMaxLifetimeSecs: 1800, + ConnMaxIdleTimeSecs: 300, + }}) + + require.Equal(t, 50, db.Pool.MaxOpenConns) + require.Equal(t, 9, db.Pool.MaxIdleConns) + require.Equal(t, 600, db.Pool.ConnMaxLifetimeSecs) + require.Equal(t, 120, db.Pool.ConnMaxIdleTimeSecs) +} + +// An env var that is set but unusable falls back to yaml rather than taking +// effect — and, unlike before, says so via a WARN (see warnIgnoredEnvVar). +func TestLoadDB_InvalidEnvFallsBackToYAML(t *testing.T) { + t.Setenv("DB_MAX_OPEN_CONNS", "abc") + t.Setenv("DB_CONN_MAX_LIFETIME_SECS", "not-a-number") + + db := loadDB(DB{Pool: DBPool{MaxOpenConns: 15, ConnMaxLifetimeSecs: 900}}) + + require.Equal(t, 15, db.Pool.MaxOpenConns) + require.Equal(t, 900, db.Pool.ConnMaxLifetimeSecs) +} + +// A negative lifetime is not a second, undocumented route to "no limit" — only +// an explicit "0" opts out. This matches loadRedis, which already rejected +// negatives. +func TestLoadDB_NegativeLifetimeFallsBackToDefault(t *testing.T) { + t.Setenv("DB_CONN_MAX_LIFETIME_SECS", "-5") + + db := loadDB(DB{}) + + require.Equal(t, defaultDBConnMaxLifetimeSecs, db.Pool.ConnMaxLifetimeSecs) +} diff --git a/esignet-service/internal/config/redis.go b/esignet-service/internal/config/redis.go index a31964f7d..0dd135ba8 100644 --- a/esignet-service/internal/config/redis.go +++ b/esignet-service/internal/config/redis.go @@ -96,25 +96,41 @@ type Redis struct { // comment above). func loadRedis(yamlRedis Redis) Redis { // These fields are never read from yaml (yaml:"-", see the Redis doc - // comment above), so fromYAML is always 0 — envIntOrConfigOrDefault - // collapses to "env wins if positive, else the compiled default". - poolSize := envIntOrConfigOrDefault("REDIS_POOL_SIZE", 0, defaultRedisPoolSize) - minIdle := envIntOrConfigOrDefault("REDIS_MIN_IDLE_CONNS", 0, defaultRedisMinIdleConns) - idleTime := time.Duration(envIntOrConfigOrDefault("REDIS_CONN_MAX_IDLE_TIME_SECS", 0, defaultRedisConnMaxIdleTime)) * time.Second + // comment above), so fromYAML is always 0 — envIntOrConfigOrDefaultSourced + // collapses to "env wins if positive, else the compiled default", and the + // reported source is therefore only ever env or default, never yaml. + poolSize, poolSizeSrc := envIntOrConfigOrDefaultSourced("REDIS_POOL_SIZE", 0, defaultRedisPoolSize) + minIdle, minIdleSrc := envIntOrConfigOrDefaultSourced("REDIS_MIN_IDLE_CONNS", 0, defaultRedisMinIdleConns) + idleTimeSecs, idleTimeSrc := envIntOrConfigOrDefaultSourced("REDIS_CONN_MAX_IDLE_TIME_SECS", 0, defaultRedisConnMaxIdleTime) + idleTime := time.Duration(idleTimeSecs) * time.Second // Unlike the fields above, lifetimeSecs has a "0 = no limit" opt-out, so - // it can't use envIntOrConfigOrDefault (which treats <=0 at every tier as - // "not set") — an explicit env var of "0" must be honored as-is. - lifetimeSecs := envIntOrDefault("REDIS_CONN_MAX_LIFETIME_SECS", defaultRedisConnMaxLifetimeSecs) - if lifetimeSecs < 0 { - lifetimeSecs = defaultRedisConnMaxLifetimeSecs - } + // it needs the AllowEnvZero variant (the plain one treats <=0 at every + // tier as "not set") — an explicit env var of "0" must be honored as-is, + // while a negative value stays invalid and falls back to the default. + lifetimeSecs, lifetimeSrc := envIntOrConfigOrDefaultAllowEnvZeroSourced("REDIS_CONN_MAX_LIFETIME_SECS", 0, defaultRedisConnMaxLifetimeSecs) lifetime := time.Duration(lifetimeSecs) * time.Second // 0 = no limit - dialTimeout := time.Duration(envIntOrConfigOrDefault("REDIS_DIAL_TIMEOUT_SECS", 0, defaultRedisDialTimeoutSecs)) * time.Second - readTimeout := time.Duration(envIntOrConfigOrDefault("REDIS_READ_TIMEOUT_SECS", 0, defaultRedisReadTimeoutSecs)) * time.Second - writeTimeout := time.Duration(envIntOrConfigOrDefault("REDIS_WRITE_TIMEOUT_SECS", 0, defaultRedisWriteTimeoutSecs)) * time.Second - poolTimeout := time.Duration(envIntOrConfigOrDefault("REDIS_POOL_TIMEOUT_SECS", 0, defaultRedisPoolTimeoutSecs)) * time.Second + dialTimeoutSecs, dialTimeoutSrc := envIntOrConfigOrDefaultSourced("REDIS_DIAL_TIMEOUT_SECS", 0, defaultRedisDialTimeoutSecs) + readTimeoutSecs, readTimeoutSrc := envIntOrConfigOrDefaultSourced("REDIS_READ_TIMEOUT_SECS", 0, defaultRedisReadTimeoutSecs) + writeTimeoutSecs, writeTimeoutSrc := envIntOrConfigOrDefaultSourced("REDIS_WRITE_TIMEOUT_SECS", 0, defaultRedisWriteTimeoutSecs) + poolTimeoutSecs, poolTimeoutSrc := envIntOrConfigOrDefaultSourced("REDIS_POOL_TIMEOUT_SECS", 0, defaultRedisPoolTimeoutSecs) + dialTimeout := time.Duration(dialTimeoutSecs) * time.Second + readTimeout := time.Duration(readTimeoutSecs) * time.Second + writeTimeout := time.Duration(writeTimeoutSecs) * time.Second + poolTimeout := time.Duration(poolTimeoutSecs) * time.Second + + // See the matching "db pool config resolved" line in db.go: report which + // tier supplied each value, not just the value. + logResolvedSettings("redis pool config resolved", + resolvedSetting{"poolSize", poolSize, poolSizeSrc}, + resolvedSetting{"minIdleConns", minIdle, minIdleSrc}, + resolvedSetting{"connMaxIdleTimeSecs", idleTimeSecs, idleTimeSrc}, + resolvedSetting{"connMaxLifetimeSecs", lifetimeSecs, lifetimeSrc}, + resolvedSetting{"dialTimeoutSecs", dialTimeoutSecs, dialTimeoutSrc}, + resolvedSetting{"readTimeoutSecs", readTimeoutSecs, readTimeoutSrc}, + resolvedSetting{"writeTimeoutSecs", writeTimeoutSecs, writeTimeoutSrc}, + resolvedSetting{"poolTimeoutSecs", poolTimeoutSecs, poolTimeoutSrc}) keyPrefix := envOrConfigOrDefault("REDIS_KEY_PREFIX", yamlRedis.KeyPrefix, defaultRedisKeyPrefix) diff --git a/esignet-service/internal/config/redis_test.go b/esignet-service/internal/config/redis_test.go index f8cd482dc..0270c9b93 100644 --- a/esignet-service/internal/config/redis_test.go +++ b/esignet-service/internal/config/redis_test.go @@ -237,3 +237,23 @@ func TestRedisApplyPool(t *testing.T) { require.Equal(t, 4*time.Second, opts.PoolTimeout) require.NotNil(t, opts.TLSConfig) } + +// Redis pool fields are env-only (yaml:"-"), so an unusable env var falls back +// to the compiled default rather than being passed through. +func TestLoadRedis_InvalidEnvFallsBackToDefault(t *testing.T) { + t.Setenv("REDIS_POOL_SIZE", "abc") + t.Setenv("REDIS_CONN_MAX_LIFETIME_SECS", "not-a-number") + + r := loadRedis(Redis{}) + + require.Equal(t, defaultRedisPoolSize, r.PoolSize) + require.Equal(t, time.Duration(defaultRedisConnMaxLifetimeSecs)*time.Second, r.ConnMaxLifetime) +} + +func TestLoadRedis_NegativeLifetimeFallsBackToDefault(t *testing.T) { + t.Setenv("REDIS_CONN_MAX_LIFETIME_SECS", "-5") + + r := loadRedis(Redis{}) + + require.Equal(t, time.Duration(defaultRedisConnMaxLifetimeSecs)*time.Second, r.ConnMaxLifetime) +}