stacklok / toolhive / 26125515811

66%

Build Type

push

github

Committed by web-flow

Commit Message

fix(operator): inject THV_SESSION_REDIS_PASSWORD for MCPServer (#5286)

* fix(operator): inject THV_SESSION_REDIS_PASSWORD for MCPServer

MCPServer.spec.sessionStorage.passwordRef is accepted by the CRD
schema and the runner reads THV_SESSION_REDIS_PASSWORD from
pkg/transport/session/redis_config.go, but the operator never bridged
the two: populateScalingConfig in mcpserver_runconfig.go copies only
Address/DB/KeyPrefix into RunConfig, and no env var is added to the
proxy runner container. Result: any MCPServer that points its session
storage at a password-protected Redis fails on connect with
"NOAUTH Authentication required".

PR #4368 (which introduced the runconfig wiring) explicitly noted in
its summary that "the Redis password is intentionally excluded and
injected as a pod env var instead". The env-var injection landed for
VirtualMCPServer in #4215 / buildRedisPasswordEnvVar but the parallel
MCPServer change was never made.

This change closes that gap by mirroring the VirtualMCPServer pattern:

- Add buildMCPServerRedisPasswordEnvVar in mcpserver_controller.go
  which returns []corev1.EnvVar with a single SecretKeyRef-backed
  THV_SESSION_REDIS_PASSWORD entry when sessionStorage.provider ==
  "redis" and passwordRef is set; returns nil otherwise.
- Append the helper's output to the proxy container's env slice in
  the MCPServer reconciler, immediately before user-supplied
  resourceOverrides.proxyDeployment.env so user overrides can still
  win on name collisions.
- Add TestBuildMCPServerRedisPasswordEnvVar mirroring
  TestBuildRedisPasswordEnvVar for VirtualMCPServer (nil storage,
  memory provider, redis without passwordRef, redis with passwordRef).
- No CRD changes — the schema already supports passwordRef.

Tested:
- go build ./cmd/thv-operator/... succeeds
- go test ./cmd/thv-operator/controllers/ all green (3.7s)

* ci: retrigger after infrastructure-canceled run

The previous Tests / Test Go Code job (76080809592) was killed by
GitHub Act... (continued)

Coverage Stats

19 of 19 new or added lines in 1 file covered. (100.0%)

11 existing lines in 3 files now uncovered.

64621 of 98697 relevant lines covered (65.47%)

62.26 hits per line