diff --git a/cmd/thv-operator/app/app.go b/cmd/thv-operator/app/app.go index 418b49632d..364921228b 100644 --- a/cmd/thv-operator/app/app.go +++ b/cmd/thv-operator/app/app.go @@ -81,6 +81,11 @@ func Run() { // Bridge to slog for consistency with the rest of the ToolHive codebase. ctrl.SetLogger(logr.FromSlogHandler(slog.Default().Handler())) + if err := ctrlutil.ValidateDefaultRedisConfig(); err != nil { + setupLog.Error(err, "invalid global Redis configuration") + os.Exit(1) + } + podNamespace, _ := os.LookupEnv("POD_NAMESPACE") options := ctrl.Options{ diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go index b87b1dddd5..b2e8fa3a6f 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go @@ -221,6 +221,7 @@ func populateScalingConfigForRemoteProxy(runConfig *runner.RunConfig, proxy *mcp } runConfig.ScalingConfig.SessionRedis = &runner.SessionRedisConfig{ Address: def.Addr, + TLS: ctrlutil.SessionRedisTLSConfig(nil), } } } diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 83c4c4baa7..c5d15ff4cf 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -1437,7 +1437,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer( } // Mount the session storage Redis CA bundle when TLS references one. - sessionTLSVolumes, sessionTLSMounts := ctrlutil.SessionRedisTLSVolumes(m.Spec.SessionStorage) + sessionTLSVolumes, sessionTLSMounts := ctrlutil.SessionRedisTLSVolumesWithDefault(m.Spec.SessionStorage) volumes = append(volumes, sessionTLSVolumes...) volumeMounts = append(volumeMounts, sessionTLSMounts...) @@ -2094,7 +2094,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate( return true } - wantSessionTLSVolumes, _ := ctrlutil.SessionRedisTLSVolumes(mcpServer.Spec.SessionStorage) + wantSessionTLSVolumes, _ := ctrlutil.SessionRedisTLSVolumesWithDefault(mcpServer.Spec.SessionStorage) if ctrlutil.SessionRedisTLSVolumeNeedsUpdate(deployment.Spec.Template.Spec.Volumes, wantSessionTLSVolumes) { return true } diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig.go b/cmd/thv-operator/controllers/mcpserver_runconfig.go index 04e237d900..ab26d11a9f 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig.go @@ -325,6 +325,7 @@ func populateScalingConfig(runConfig *runner.RunConfig, m *mcpv1beta1.MCPServer) } else if defaultRedis != nil { runConfig.ScalingConfig.SessionRedis = &runner.SessionRedisConfig{ Address: defaultRedis.Addr, + TLS: ctrlutil.SessionRedisTLSConfig(nil), } } } diff --git a/cmd/thv-operator/controllers/session_redis_tls_test.go b/cmd/thv-operator/controllers/session_redis_tls_test.go index f7ee8d0300..9613c42cc2 100644 --- a/cmd/thv-operator/controllers/session_redis_tls_test.go +++ b/cmd/thv-operator/controllers/session_redis_tls_test.go @@ -18,6 +18,8 @@ import ( "github.com/stacklok/toolhive/cmd/thv-operator/internal/testutil" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" "github.com/stacklok/toolhive/pkg/container/kubernetes" + "github.com/stacklok/toolhive/pkg/redisconfig" + "github.com/stacklok/toolhive/pkg/runner" "github.com/stacklok/toolhive/pkg/vmcp/workloads" ) @@ -40,7 +42,43 @@ func sessionTLSStorage(caSecret string) *mcpv1beta1.SessionStorageConfig { // referenced Secret is detected as drift even though the mounted path (and so // the runtime config) is unchanged. func TestSessionRedisTLSCAMountedAndDriftDetected(t *testing.T) { - t.Parallel() + t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_NAME", "") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_KEY", "") + tests := []struct { + name string + global bool + memory bool + }{ + {name: "explicit Redis TLS"}, + {name: "global Redis TLS", global: true}, + {name: "global Redis TLS with explicit memory", global: true, memory: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", "") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", "") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "") + testSessionRedisTLSCAMountedAndDriftDetected(t, tt.global, tt.memory) + }) + } +} + +func testSessionRedisTLSCAMountedAndDriftDetected(t *testing.T, global, memory bool) { + t.Helper() + storage := sessionTLSStorage + if global { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "global-redis:6380") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", "true") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "ca.crt") + storage = func(caSecret string) *mcpv1beta1.SessionStorageConfig { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", caSecret) + if memory { + return &mcpv1beta1.SessionStorageConfig{Provider: "memory"} + } + return nil + } + } type harness struct { // build returns the desired Deployment for a workload using caSecret. @@ -52,16 +90,16 @@ func TestSessionRedisTLSCAMountedAndDriftDetected(t *testing.T) { mcpServer := func(caSecret string) *mcpv1beta1.MCPServer { return v1beta1test.NewMCPServer("tls-mcp", "default", v1beta1test.WithTransport("streamable-http"), - v1beta1test.WithSessionStorage(sessionTLSStorage(caSecret))) + v1beta1test.WithSessionStorage(storage(caSecret))) } remoteProxy := func(caSecret string) *mcpv1beta1.MCPRemoteProxy { return v1beta1test.NewMCPRemoteProxy("tls-proxy", "default", - v1beta1test.WithRemoteProxySessionStorage(sessionTLSStorage(caSecret))) + v1beta1test.WithRemoteProxySessionStorage(storage(caSecret))) } virtualServer := func(caSecret string) *mcpv1beta1.VirtualMCPServer { return v1beta1test.NewVirtualMCPServer("tls-vmcp", "default", v1beta1test.WithVMCPGroupRef("test-group"), - v1beta1test.WithVMCPSessionStorage(sessionTLSStorage(caSecret))) + v1beta1test.WithVMCPSessionStorage(storage(caSecret))) } kinds := map[string]harness{ @@ -115,8 +153,6 @@ func TestSessionRedisTLSCAMountedAndDriftDetected(t *testing.T) { for name, h := range kinds { t.Run(name, func(t *testing.T) { - t.Parallel() - dep := h.build(t, "redis-ca") require.NotNil(t, dep) @@ -126,6 +162,11 @@ func TestSessionRedisTLSCAMountedAndDriftDetected(t *testing.T) { volume = &dep.Spec.Template.Spec.Volumes[i] } } + if memory && name == "MCPRemoteProxy" { + assert.Nil(t, volume, "explicit memory suppresses the remote proxy global fallback") + assert.False(t, h.needsUpdate(t, dep, "redis-ca")) + return + } require.NotNil(t, volume, "the session storage CA Secret must be mounted") require.NotNil(t, volume.Secret) assert.Equal(t, "redis-ca", volume.Secret.SecretName) @@ -143,6 +184,12 @@ func TestSessionRedisTLSCAMountedAndDriftDetected(t *testing.T) { "a freshly built Deployment must not be flagged as drifted") assert.True(t, h.needsUpdate(t, dep, "rotated-redis-ca"), "switching the referenced CA Secret must trigger a Deployment update") + if global { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "rotated.pem") + assert.True(t, h.needsUpdate(t, dep, "redis-ca"), + "switching the global CA Secret key must trigger a Deployment update") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "ca.crt") + } }) } } @@ -184,3 +231,68 @@ func newRemoteProxyTLSReconciler(t *testing.T, proxy *mcpv1beta1.MCPRemoteProxy) PlatformDetector: ctrlutil.NewSharedPlatformDetector(), } } + +func TestGlobalRedisTLSRunConfigSelection(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "global-redis:6380") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", "true") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "ca.crt") + + caTLS := &redisconfig.TLSConfig{CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"} + tests := []struct { + name string + storage *mcpv1beta1.SessionStorageConfig + globalCA string + wantAddr string + wantTLS *redisconfig.TLSConfig + }{ + {name: "global system roots", wantAddr: "global-redis:6380", wantTLS: &redisconfig.TLSConfig{}}, + {name: "global private CA", globalCA: "global-ca", wantAddr: "global-redis:6380", wantTLS: caTLS}, + {name: "explicit plaintext overrides global TLS", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "redis", Address: "local-redis:6379"}, wantAddr: "local-redis:6379"}, + {name: "explicit system roots override global CA", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "redis", Address: "local-redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{}}, + wantAddr: "local-redis:6380", wantTLS: &redisconfig.TLSConfig{}}, + {name: "explicit private CA overrides global CA", globalCA: "global-ca", storage: sessionTLSStorage("local-ca"), + wantAddr: "redis:6380", wantTLS: caTLS}, + {name: "memory retains each resource existing precedence", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "memory"}, wantAddr: "global-redis:6380", wantTLS: caTLS}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", tt.globalCA) + key := "" + if tt.globalCA != "" { + key = "ca.crt" + } + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", key) + before := tt.storage.DeepCopy() + for _, remote := range []bool{false, true} { + name := "MCPServer" + if remote { + name = "MCPRemoteProxy" + } + t.Run(name, func(t *testing.T) { + cfg := &runner.RunConfig{} + if remote { + proxy := v1beta1test.NewMCPRemoteProxy("proxy", "default", v1beta1test.WithRemoteProxySessionStorage(tt.storage)) + populateScalingConfigForRemoteProxy(cfg, proxy) + } else { + server := v1beta1test.NewMCPServer("server", "default", v1beta1test.WithSessionStorage(tt.storage)) + populateScalingConfig(cfg, server) + } + if remote && tt.storage != nil && tt.storage.Provider == "memory" { + if cfg.ScalingConfig != nil { + assert.Nil(t, cfg.ScalingConfig.SessionRedis) + } + return + } + require.NotNil(t, cfg.ScalingConfig) + require.NotNil(t, cfg.ScalingConfig.SessionRedis) + assert.Equal(t, tt.wantAddr, cfg.ScalingConfig.SessionRedis.Address) + assert.Equal(t, tt.wantTLS, cfg.ScalingConfig.SessionRedis.TLS) + }) + } + assert.Equal(t, before, tt.storage, "global fallback must not mutate spec.sessionStorage") + }) + } +} diff --git a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go index 5947422ca8..9bf74efc0c 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_deployment.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_deployment.go @@ -278,7 +278,7 @@ func (r *VirtualMCPServerReconciler) buildPodVolumesForVmcp( volumeMounts = append(volumeMounts, authServerMounts...) } - sessionTLSVolumes, sessionTLSMounts := ctrlutil.SessionRedisTLSVolumes(vmcp.Spec.SessionStorage) + sessionTLSVolumes, sessionTLSMounts := ctrlutil.SessionRedisTLSVolumesWithDefault(vmcp.Spec.SessionStorage) volumes = append(volumes, sessionTLSVolumes...) volumeMounts = append(volumeMounts, sessionTLSMounts...) diff --git a/cmd/thv-operator/pkg/controllerutil/defaultredis.go b/cmd/thv-operator/pkg/controllerutil/defaultredis.go index ff247f7d1e..233a58989c 100644 --- a/cmd/thv-operator/pkg/controllerutil/defaultredis.go +++ b/cmd/thv-operator/pkg/controllerutil/defaultredis.go @@ -3,7 +3,13 @@ package controllerutil -import "os" +import ( + "fmt" + "os" + "strconv" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" +) // DefaultRedisConfig holds global Redis defaults read from the operator's own // environment variables. These are injected by the Helm chart when @@ -17,14 +23,54 @@ type DefaultRedisConfig struct { // SecretKey is the key within SecretName that holds the password. // Defaults to "redis-password" when SecretName is set and no key is provided. SecretKey string + // TLS configures verified transport security for the global Redis fallback. + // A referenced CA Secret must exist in each workload's namespace. + TLS *mcpv1beta1.RedisTLSConfig } // ReadDefaultRedisConfig returns the global Redis defaults from the operator's -// environment, or nil when TOOLHIVE_DEFAULT_REDIS_ADDR is not set. +// environment, or nil when TOOLHIVE_DEFAULT_REDIS_ADDR is not set. Invalid TLS +// settings panic rather than silently falling back to plaintext; the operator +// calls ValidateDefaultRedisConfig at startup to report these errors gracefully. func ReadDefaultRedisConfig() *DefaultRedisConfig { + cfg, err := readDefaultRedisConfig() + if err != nil { + panic(err) + } + return cfg +} + +// ValidateDefaultRedisConfig validates the global Redis environment before the +// operator starts its controllers. It validates references, not Secret contents; +// CA Secrets are projected separately into each workload's namespace. +func ValidateDefaultRedisConfig() error { + _, err := readDefaultRedisConfig() + return err +} + +func readDefaultRedisConfig() (*DefaultRedisConfig, error) { + enabled := false + if raw := os.Getenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED"); raw != "" { + parsed, err := strconv.ParseBool(raw) + if err != nil { + return nil, fmt.Errorf("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED must be a boolean: %w", err) + } + enabled = parsed + } + caName := os.Getenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME") + caKey := os.Getenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY") + if (caName == "") != (caKey == "") { + return nil, fmt.Errorf("default Redis TLS CA Secret name and key must both be set") + } + if caName != "" && !enabled { + return nil, fmt.Errorf("default Redis TLS CA Secret requires TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED=true") + } addr := os.Getenv("TOOLHIVE_DEFAULT_REDIS_ADDR") if addr == "" { - return nil + if enabled { + return nil, fmt.Errorf("default Redis TLS requires TOOLHIVE_DEFAULT_REDIS_ADDR") + } + return nil, nil } cfg := &DefaultRedisConfig{ Addr: addr, @@ -34,5 +80,11 @@ func ReadDefaultRedisConfig() *DefaultRedisConfig { if cfg.SecretName != "" && cfg.SecretKey == "" { cfg.SecretKey = "redis-password" } - return cfg + if enabled { + cfg.TLS = &mcpv1beta1.RedisTLSConfig{} + if caName != "" { + cfg.TLS.CACertSecretRef = &mcpv1beta1.SecretKeyRef{Name: caName, Key: caKey} + } + } + return cfg, nil } diff --git a/cmd/thv-operator/pkg/controllerutil/defaultredis_test.go b/cmd/thv-operator/pkg/controllerutil/defaultredis_test.go index dd4497df0c..d7a9c4733b 100644 --- a/cmd/thv-operator/pkg/controllerutil/defaultredis_test.go +++ b/cmd/thv-operator/pkg/controllerutil/defaultredis_test.go @@ -9,38 +9,77 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" ) -func TestReadDefaultRedisConfig_NotSet(t *testing.T) { - t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_NAME", "") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_KEY", "") - - got := ctrlutil.ReadDefaultRedisConfig() - assert.Nil(t, got) +func TestReadDefaultRedisConfig(t *testing.T) { + tests := []struct { + name string + addr string + passwordSecret string + passwordKey string + enabled string + caSecret string + caKey string + want *ctrlutil.DefaultRedisConfig + }{ + {name: "no address leaves global Redis disabled"}, + {name: "legacy address only stays plaintext", addr: "redis:6379", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6379"}}, + {name: "explicitly disabled TLS stays plaintext", addr: "redis:6379", enabled: "false", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6379"}}, + {name: "existing password key default is preserved", addr: "redis:6379", passwordSecret: "redis-creds", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6379", SecretName: "redis-creds", SecretKey: "redis-password"}}, + {name: "configured password key is preserved", addr: "redis:6379", passwordSecret: "redis-creds", passwordKey: "custom", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6379", SecretName: "redis-creds", SecretKey: "custom"}}, + {name: "enabled TLS uses system roots", addr: "redis:6380", enabled: "true", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{}}}, + {name: "private CA and password secrets stay independent", addr: "redis:6380", enabled: "true", + passwordSecret: "redis-creds", caSecret: "redis-ca", caKey: "bundle.pem", + want: &ctrlutil.DefaultRedisConfig{Addr: "redis:6380", SecretName: "redis-creds", SecretKey: "redis-password", + TLS: &mcpv1beta1.RedisTLSConfig{CACertSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-ca", Key: "bundle.pem"}}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", tt.addr) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", tt.enabled) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", tt.caSecret) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", tt.caKey) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_NAME", tt.passwordSecret) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_KEY", tt.passwordKey) + require.NoError(t, ctrlutil.ValidateDefaultRedisConfig()) + assert.Equal(t, tt.want, ctrlutil.ReadDefaultRedisConfig()) + }) + } } -func TestReadDefaultRedisConfig_AddrOnly(t *testing.T) { - t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "myredis:6379") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_NAME", "") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_KEY", "") - - got := ctrlutil.ReadDefaultRedisConfig() - require.NotNil(t, got) - assert.Equal(t, "myredis:6379", got.Addr) - assert.Empty(t, got.SecretName) - assert.Empty(t, got.SecretKey) -} - -func TestReadDefaultRedisConfig_WithSecret(t *testing.T) { - t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "myredis:6379") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_NAME", "redis-creds") - t.Setenv("TOOLHIVE_DEFAULT_REDIS_SECRET_KEY", "my-password") - - got := ctrlutil.ReadDefaultRedisConfig() - require.NotNil(t, got) - assert.Equal(t, "myredis:6379", got.Addr) - assert.Equal(t, "redis-creds", got.SecretName) - assert.Equal(t, "my-password", got.SecretKey) +func TestValidateDefaultRedisConfigRejectsInvalidTLS(t *testing.T) { + tests := []struct { + name string + addr string + enabled string + caSecret string + caKey string + }{ + {name: "malformed enable flag", addr: "redis:6380", enabled: "invalid"}, + {name: "malformed enable flag without address", enabled: "invalid"}, + {name: "TLS enabled without Redis address", enabled: "true"}, + {name: "CA secret without key", addr: "redis:6380", enabled: "true", caSecret: "redis-ca"}, + {name: "CA key without secret", addr: "redis:6380", enabled: "true", caKey: "ca.crt"}, + {name: "CA configured with TLS disabled", addr: "redis:6379", enabled: "false", caSecret: "redis-ca", caKey: "ca.crt"}, + {name: "CA configured with TLS omitted", addr: "redis:6379", caSecret: "redis-ca", caKey: "ca.crt"}, + {name: "CA configured without address", enabled: "true", caSecret: "redis-ca", caKey: "ca.crt"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", tt.addr) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", tt.enabled) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", tt.caSecret) + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", tt.caKey) + require.Error(t, ctrlutil.ValidateDefaultRedisConfig()) + assert.Panics(t, func() { ctrlutil.ReadDefaultRedisConfig() }, + "invalid global TLS must never silently produce a plaintext configuration") + }) + } } diff --git a/cmd/thv-operator/pkg/controllerutil/session_redis_tls.go b/cmd/thv-operator/pkg/controllerutil/session_redis_tls.go index d5d5b2e034..54847eb5c6 100644 --- a/cmd/thv-operator/pkg/controllerutil/session_redis_tls.go +++ b/cmd/thv-operator/pkg/controllerutil/session_redis_tls.go @@ -30,8 +30,9 @@ const ( // SessionRedisTLSConfig returns the runtime TLS settings for Redis session // storage, or nil when session storage is not Redis or TLS is not configured -// (plaintext). When a CA Secret is referenced, CACertFile points at the file -// mounted by SessionRedisTLSVolumes. +// (plaintext). A nil session storage config selects the operator-wide default. +// Explicit Redis settings never inherit global TLS. When a CA Secret is +// referenced, CACertFile points at the file mounted by SessionRedisTLSVolumes. func SessionRedisTLSConfig(ss *mcpv1beta1.SessionStorageConfig) *redisconfig.TLSConfig { tlsCfg := sessionRedisTLS(ss) if tlsCfg == nil { @@ -46,7 +47,8 @@ func SessionRedisTLSConfig(ss *mcpv1beta1.SessionStorageConfig) *redisconfig.TLS // SessionRedisTLSVolumes returns the volume and mount that project the // session storage Redis CA Secret into the container, or nil slices when no -// CA Secret is referenced. +// CA Secret is referenced. A nil session storage config selects the global default; +// explicit non-Redis storage does not use it. func SessionRedisTLSVolumes(ss *mcpv1beta1.SessionStorageConfig) ([]corev1.Volume, []corev1.VolumeMount) { tlsCfg := sessionRedisTLS(ss) if tlsCfg == nil || tlsCfg.CACertSecretRef == nil { @@ -57,6 +59,17 @@ func SessionRedisTLSVolumes(ss *mcpv1beta1.SessionStorageConfig) ([]corev1.Volum return []corev1.Volume{vol}, []corev1.VolumeMount{mount} } +// SessionRedisTLSVolumesWithDefault returns CA projections for workloads whose +// existing storage selection falls back to global Redis for non-Redis providers. +// MCPServer and VirtualMCPServer use this policy; MCPRemoteProxy only falls back +// when session storage is absent and must use SessionRedisTLSVolumes instead. +func SessionRedisTLSVolumesWithDefault(ss *mcpv1beta1.SessionStorageConfig) ([]corev1.Volume, []corev1.VolumeMount) { + if ss != nil && ss.Provider != mcpv1beta1.SessionStorageProviderRedis { + return SessionRedisTLSVolumes(nil) + } + return SessionRedisTLSVolumes(ss) +} + // SessionRedisTLSVolumeNeedsUpdate reports whether the live pod volumes project // a different session storage CA Secret than the desired pod volumes. The // RunConfig checksum already triggers a rollout when TLS is enabled or @@ -81,9 +94,16 @@ func SessionRedisTLSVolumeNeedsUpdate(live, desired []corev1.Volume) bool { } // sessionRedisTLS returns the TLS block of a Redis session storage config, or -// nil when session storage is not Redis or TLS is not configured. +// nil when session storage is not Redis or TLS is not configured. A nil +// session storage config selects the global default Redis TLS settings. func sessionRedisTLS(ss *mcpv1beta1.SessionStorageConfig) *mcpv1beta1.RedisTLSConfig { - if ss == nil || ss.Provider != mcpv1beta1.SessionStorageProviderRedis { + if ss == nil { + if def := ReadDefaultRedisConfig(); def != nil { + return def.TLS + } + return nil + } + if ss.Provider != mcpv1beta1.SessionStorageProviderRedis { return nil } return ss.TLS diff --git a/cmd/thv-operator/pkg/controllerutil/session_redis_tls_test.go b/cmd/thv-operator/pkg/controllerutil/session_redis_tls_test.go index 6854270cd2..5c2a250094 100644 --- a/cmd/thv-operator/pkg/controllerutil/session_redis_tls_test.go +++ b/cmd/thv-operator/pkg/controllerutil/session_redis_tls_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" k8sptr "k8s.io/utils/ptr" @@ -171,3 +172,68 @@ func caSecretVolume(secret, key string) corev1.Volume { }, } } + +// Global defaults apply only when the caller selected the global Redis backend. +// Explicit Redis blocks own their TLS policy, including omitted TLS (plaintext). +func TestSessionRedisTLSGlobalSelection(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "global-redis:6380") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", "true") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", "global-ca") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "global.pem") + + tests := []struct { + name string + storage *mcpv1beta1.SessionStorageConfig + wantTLS *redisconfig.TLSConfig + wantCA string + wantKey string + }{ + {name: "omitted storage inherits global CA", wantTLS: &redisconfig.TLSConfig{ + CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"}, wantCA: "global-ca", wantKey: "global.pem"}, + {name: "explicit plaintext does not inherit global TLS", storage: &mcpv1beta1.SessionStorageConfig{ + Provider: "redis", Address: "local-redis:6379"}}, + {name: "explicit system roots do not inherit global CA", storage: &mcpv1beta1.SessionStorageConfig{ + Provider: "redis", Address: "local-redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{}}, wantTLS: &redisconfig.TLSConfig{}}, + {name: "explicit private CA overrides global CA", storage: &mcpv1beta1.SessionStorageConfig{ + Provider: "redis", Address: "local-redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{ + CACertSecretRef: &mcpv1beta1.SecretKeyRef{Name: "local-ca", Key: "local.pem"}}}, + wantTLS: &redisconfig.TLSConfig{CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"}, + wantCA: "local-ca", wantKey: "local.pem"}, + {name: "explicit memory does not select global defaults", storage: &mcpv1beta1.SessionStorageConfig{Provider: "memory"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", "global-ca") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "global.pem") + before := tt.storage.DeepCopy() + assert.Equal(t, tt.wantTLS, SessionRedisTLSConfig(tt.storage)) + volumes, mounts := SessionRedisTLSVolumes(tt.storage) + if tt.wantCA == "" { + assert.Empty(t, volumes) + assert.Empty(t, mounts) + } else { + require.Len(t, volumes, 1) + assert.Equal(t, caSecretVolume(tt.wantCA, tt.wantKey), volumes[0]) + require.Len(t, mounts, 1) + assert.Equal(t, tt.wantTLS.CACertFile, mounts[0].MountPath) + } + assert.Equal(t, before, tt.storage, "selecting global TLS must not mutate the workload spec") + }) + } + + memory := &mcpv1beta1.SessionStorageConfig{Provider: "memory"} + volumes, mounts := SessionRedisTLSVolumesWithDefault(memory) + require.Len(t, volumes, 1) + assert.Equal(t, caSecretVolume("global-ca", "global.pem"), volumes[0]) + require.Len(t, mounts, 1) + assert.Equal(t, "/etc/toolhive/session-redis-tls/ca.crt", mounts[0].MountPath, + "MCPServer and VirtualMCPServer keep their existing non-Redis global fallback") + assert.Equal(t, &mcpv1beta1.SessionStorageConfig{Provider: "memory"}, memory) + + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", "") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", "") + assert.Equal(t, &redisconfig.TLSConfig{}, SessionRedisTLSConfig(nil)) + volumes, mounts = SessionRedisTLSVolumes(nil) + assert.Empty(t, volumes, "system roots need no CA Secret projection") + assert.Empty(t, mounts) +} diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter.go b/cmd/thv-operator/pkg/vmcpconfig/converter.go index 0961730bed..34bfb04868 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter.go @@ -528,7 +528,7 @@ func convertSessionStorage(vmcp *mcpv1beta1.VirtualMCPServer) *vmcpconfig.Sessio Address: vmcp.Spec.SessionStorage.Address, DB: vmcp.Spec.SessionStorage.DB, KeyPrefix: vmcp.Spec.SessionStorage.KeyPrefix, - // spec.sessionStorage.tls is the only TLS source (CEL rejects + // Explicit spec.sessionStorage.tls overrides global defaults (CEL rejects // spec.config.sessionStorage.tls); CACertFile points at the CA bundle // the deployment builder mounts from caCertSecretRef. TLS: controllerutil.SessionRedisTLSConfig(vmcp.Spec.SessionStorage), @@ -539,6 +539,7 @@ func convertSessionStorage(vmcp *mcpv1beta1.VirtualMCPServer) *vmcpconfig.Sessio return &vmcpconfig.SessionStorageConfig{ Provider: mcpv1beta1.SessionStorageProviderRedis, Address: def.Addr, + TLS: controllerutil.SessionRedisTLSConfig(nil), } } return nil diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go index 941b741f8b..a78edbfc71 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter_test.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter_test.go @@ -2336,3 +2336,49 @@ func TestConverter_PassthroughHeaders(t *testing.T) { }) } } + +func TestConvertSessionStorage_GlobalTLS(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_ADDR", "global-redis:6380") + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED", "true") + tests := []struct { + name string + storage *mcpv1beta1.SessionStorageConfig + globalCA string + wantAddr string + wantTLS *redisconfig.TLSConfig + }{ + {name: "global system roots", wantAddr: "global-redis:6380", wantTLS: &redisconfig.TLSConfig{}}, + {name: "global private CA", globalCA: "global-ca", wantAddr: "global-redis:6380", + wantTLS: &redisconfig.TLSConfig{CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"}}, + {name: "explicit plaintext does not inherit TLS", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "redis", Address: "local-redis:6379"}, wantAddr: "local-redis:6379"}, + {name: "explicit system roots do not inherit CA", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "redis", Address: "local-redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{}}, + wantAddr: "local-redis:6380", wantTLS: &redisconfig.TLSConfig{}}, + {name: "explicit CA overrides global CA", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "redis", Address: "local-redis:6380", TLS: &mcpv1beta1.RedisTLSConfig{ + CACertSecretRef: &mcpv1beta1.SecretKeyRef{Name: "local-ca", Key: "local.pem"}}}, + wantAddr: "local-redis:6380", wantTLS: &redisconfig.TLSConfig{CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"}}, + {name: "memory retains existing global fallback", globalCA: "global-ca", + storage: &mcpv1beta1.SessionStorageConfig{Provider: "memory"}, wantAddr: "global-redis:6380", + wantTLS: &redisconfig.TLSConfig{CACertFile: "/etc/toolhive/session-redis-tls/ca.crt"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME", tt.globalCA) + key := "" + if tt.globalCA != "" { + key = "ca.crt" + } + t.Setenv("TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY", key) + before := tt.storage.DeepCopy() + server := v1beta1test.NewVirtualMCPServer("vmcp", "default", v1beta1test.WithVMCPSessionStorage(tt.storage)) + got := convertSessionStorage(server) + require.NotNil(t, got) + assert.Equal(t, "redis", got.Provider) + assert.Equal(t, tt.wantAddr, got.Address) + assert.Equal(t, tt.wantTLS, got.TLS) + assert.Equal(t, before, server.Spec.SessionStorage) + }) + } +} diff --git a/deploy/charts/operator/README.md b/deploy/charts/operator/README.md index 16b5a49765..ab21bc6d45 100644 --- a/deploy/charts/operator/README.md +++ b/deploy/charts/operator/README.md @@ -46,7 +46,7 @@ The command removes all the Kubernetes components associated with the chart and |-----|------|---------|-------------| | fullnameOverride | string | `"toolhive-operator"` | Override the generated name of the operator Deployment and related resources. | | nameOverride | string | `""` | Override the chart name used in resource labels and generated names. | -| operator | object | `{"affinity":{},"autoscaling":{"enabled":false,"maxReplicas":100,"minReplicas":1,"targetCPUUtilizationPercentage":80},"containerSecurityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"defaultImagePullSecrets":[],"defaultRedis":{"addr":"","existingSecret":"","existingSecretKey":""},"env":[],"features":{"experimental":false,"storageVersionMigrator":true},"gc":{"gogc":75,"gomemlimit":"110MiB"},"image":"ghcr.io/stacklok/toolhive/operator:v0.54.0","imageDiscovery":{"enabled":false,"resources":{}},"imagePullPolicy":"IfNotPresent","imagePullSecrets":[],"leaderElectionRole":{"binding":{"name":"toolhive-operator-leader-election-rolebinding"},"name":"toolhive-operator-leader-election-role","rules":[{"apiGroups":[""],"resources":["configmaps"],"verbs":["get","list","watch","create","update","patch","delete"]},{"apiGroups":["coordination.k8s.io"],"resources":["leases"],"verbs":["get","list","watch","create","update","patch","delete"]},{"apiGroups":["events.k8s.io"],"resources":["events"],"verbs":["create","patch"]}]},"livenessProbe":{"httpGet":{"path":"/healthz","port":"health"},"initialDelaySeconds":15,"periodSeconds":20},"nodeSelector":{},"podAnnotations":{},"podLabels":{},"podSecurityContext":{"runAsNonRoot":true},"ports":[{"containerPort":8080,"name":"metrics","protocol":"TCP"},{"containerPort":8081,"name":"health","protocol":"TCP"}],"proxyHost":"0.0.0.0","rbac":{"allowedNamespaces":[],"scope":"cluster"},"readinessProbe":{"httpGet":{"path":"/readyz","port":"health"},"initialDelaySeconds":5,"periodSeconds":10},"replicaCount":1,"resources":{"limits":{"cpu":"500m","memory":"128Mi"},"requests":{"cpu":"10m","memory":"64Mi"}},"serviceAccount":{"annotations":{},"automountServiceAccountToken":true,"create":true,"labels":{},"name":"toolhive-operator"},"tolerations":[],"toolhiveRunnerImage":"ghcr.io/stacklok/toolhive/proxyrunner:v0.54.0","vmcpImage":"ghcr.io/stacklok/toolhive/vmcp:v0.54.0","volumeMounts":[],"volumes":[]}` | Settings for the operator Deployment and related resources. | +| operator | object | `{"affinity":{},"autoscaling":{"enabled":false,"maxReplicas":100,"minReplicas":1,"targetCPUUtilizationPercentage":80},"containerSecurityContext":{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}},"defaultImagePullSecrets":[],"defaultRedis":{"addr":"","existingSecret":"","existingSecretKey":"","tls":{"caCertSecretRef":{"key":"ca.crt","name":""},"enabled":false}},"env":[],"features":{"experimental":false,"storageVersionMigrator":true},"gc":{"gogc":75,"gomemlimit":"110MiB"},"image":"ghcr.io/stacklok/toolhive/operator:v0.54.0","imageDiscovery":{"enabled":false,"resources":{}},"imagePullPolicy":"IfNotPresent","imagePullSecrets":[],"leaderElectionRole":{"binding":{"name":"toolhive-operator-leader-election-rolebinding"},"name":"toolhive-operator-leader-election-role","rules":[{"apiGroups":[""],"resources":["configmaps"],"verbs":["get","list","watch","create","update","patch","delete"]},{"apiGroups":["coordination.k8s.io"],"resources":["leases"],"verbs":["get","list","watch","create","update","patch","delete"]},{"apiGroups":["events.k8s.io"],"resources":["events"],"verbs":["create","patch"]}]},"livenessProbe":{"httpGet":{"path":"/healthz","port":"health"},"initialDelaySeconds":15,"periodSeconds":20},"nodeSelector":{},"podAnnotations":{},"podLabels":{},"podSecurityContext":{"runAsNonRoot":true},"ports":[{"containerPort":8080,"name":"metrics","protocol":"TCP"},{"containerPort":8081,"name":"health","protocol":"TCP"}],"proxyHost":"0.0.0.0","rbac":{"allowedNamespaces":[],"scope":"cluster"},"readinessProbe":{"httpGet":{"path":"/readyz","port":"health"},"initialDelaySeconds":5,"periodSeconds":10},"replicaCount":1,"resources":{"limits":{"cpu":"500m","memory":"128Mi"},"requests":{"cpu":"10m","memory":"64Mi"}},"serviceAccount":{"annotations":{},"automountServiceAccountToken":true,"create":true,"labels":{},"name":"toolhive-operator"},"tolerations":[],"toolhiveRunnerImage":"ghcr.io/stacklok/toolhive/proxyrunner:v0.54.0","vmcpImage":"ghcr.io/stacklok/toolhive/vmcp:v0.54.0","volumeMounts":[],"volumes":[]}` | Settings for the operator Deployment and related resources. | | operator.affinity | object | `{}` | Affinity rules for scheduling the operator pod. | | operator.autoscaling | object | `{"enabled":false,"maxReplicas":100,"minReplicas":1,"targetCPUUtilizationPercentage":80}` | Horizontal Pod Autoscaler settings for the operator Deployment. | | operator.autoscaling.enabled | bool | `false` | Create a Horizontal Pod Autoscaler for the operator Deployment. When enabled, the chart omits `replicaCount` from the Deployment. | @@ -55,10 +55,15 @@ The command removes all the Kubernetes components associated with the chart and | operator.autoscaling.targetCPUUtilizationPercentage | int | `80` | Target average CPU utilization as a percentage of requested CPU for autoscaling. | | operator.containerSecurityContext | object | `{"allowPrivilegeEscalation":false,"capabilities":{"drop":["ALL"]},"readOnlyRootFilesystem":true,"runAsNonRoot":true,"runAsUser":1000,"seccompProfile":{"type":"RuntimeDefault"}}` | Security context for the operator container and image-discovery containers. | | operator.defaultImagePullSecrets | list | `[]` | Default image pull Secrets for workloads created by the operator. Accepts Secret names or objects with a `name` field. Entries are merged with each resource's `imagePullSecrets`, with resource-level entries taking precedence for duplicate names. Secrets must exist in each workload's namespace. | -| operator.defaultRedis | object | `{"addr":"","existingSecret":"","existingSecretKey":""}` | Default Redis or Valkey session storage settings for workloads without `spec.sessionStorage` configured. | +| operator.defaultRedis | object | `{"addr":"","existingSecret":"","existingSecretKey":"","tls":{"caCertSecretRef":{"key":"ca.crt","name":""},"enabled":false}}` | Default Redis or Valkey session storage settings for workloads without `spec.sessionStorage` configured. | | operator.defaultRedis.addr | string | `""` | Redis or Valkey address in `host:port` format. When empty, uses `global.redis.host` and `global.redis.port` (default `6379`) from a parent chart. No default session storage is configured when neither address is set. | | operator.defaultRedis.existingSecret | string | `""` | Name of an existing Secret containing the Redis password. When empty, uses `global.redis.existingSecret`. The Secret must exist in each workload's namespace. Used only when a Redis address is configured. | | operator.defaultRedis.existingSecretKey | string | `""` | Key containing the password in `existingSecret`. When empty, uses `global.redis.existingSecretKey`, then `redis-password`. | +| operator.defaultRedis.tls | object | `{"caCertSecretRef":{"key":"ca.crt","name":""},"enabled":false}` | TLS settings for the default Redis or Valkey connection. TLS is enabled only when explicitly requested and a Redis address is configured. | +| operator.defaultRedis.tls.caCertSecretRef | object | `{"key":"ca.crt","name":""}` | Optional CA certificate for verifying the Redis server. The Secret must exist in each workload's namespace, not the operator namespace. | +| operator.defaultRedis.tls.caCertSecretRef.key | string | `"ca.crt"` | Key containing the PEM-encoded CA certificate in the Secret. | +| operator.defaultRedis.tls.caCertSecretRef.name | string | `""` | Name of an existing Secret containing a PEM-encoded CA certificate. Leave empty to use system roots. | +| operator.defaultRedis.tls.enabled | bool | `false` | Verify the Redis server certificate using system roots or the optional CA certificate below. Requires a configured Redis address. | | operator.env | list | `[]` | Additional environment variables for the operator container. Chart-managed variables take precedence for duplicate names. Set `TOOLHIVE_SKIP_UPDATE_CHECK` to `"true"` to disable periodic update checks and the usage metrics collected with them. | | operator.features.experimental | bool | `false` | Enable experimental operator features. | | operator.features.storageVersionMigrator | bool | `true` | Enable the storage version migration controller to rewrite resources in the current storage version and remove obsolete entries from CRD `status.storedVersions`. Applies to ToolHive CRDs with the `toolhive.stacklok.dev/auto-migrate-storage-version` label set to `"true"`. Requires `operator.rbac.scope=cluster`; set this to `false` for namespace-scoped installations. | diff --git a/deploy/charts/operator/templates/deployment.yaml b/deploy/charts/operator/templates/deployment.yaml index c83ba99db0..140b300ea6 100644 --- a/deploy/charts/operator/templates/deployment.yaml +++ b/deploy/charts/operator/templates/deployment.yaml @@ -123,6 +123,54 @@ spec: {{- $rdAddr = printf "%s:%v" $gh $gp }} {{- end }} {{- end }} + {{- $rdTLS := .Values.operator.defaultRedis.tls }} + {{- if kindIs "invalid" $rdTLS }} + {{- $rdTLS = dict }} + {{- end }} + {{- if not (kindIs "map" $rdTLS) }} + {{- fail "operator.defaultRedis.tls must be an object" }} + {{- end }} + {{- range $field := keys $rdTLS }} + {{- if not (has $field (list "enabled" "caCertSecretRef")) }} + {{- fail (printf "operator.defaultRedis.tls.%s is unsupported" $field) }} + {{- end }} + {{- end }} + {{- $tlsEnabled := get $rdTLS "enabled" }} + {{- if and (hasKey $rdTLS "enabled") (not (kindIs "bool" $tlsEnabled)) }} + {{- fail "operator.defaultRedis.tls.enabled must be a boolean" }} + {{- end }} + {{- $caRef := get $rdTLS "caCertSecretRef" }} + {{- if or (not (hasKey $rdTLS "caCertSecretRef")) (kindIs "invalid" $caRef) }} + {{- $caRef = dict }} + {{- end }} + {{- if not (kindIs "map" $caRef) }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef must be an object" }} + {{- end }} + {{- range $field := keys $caRef }} + {{- if not (has $field (list "name" "key")) }} + {{- fail (printf "operator.defaultRedis.tls.caCertSecretRef.%s is unsupported" $field) }} + {{- end }} + {{- end }} + {{- $caName := get $caRef "name" }} + {{- $caKey := get $caRef "key" | default "ca.crt" }} + {{- if and (hasKey $caRef "name") (not (kindIs "string" $caName)) }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef.name must be a string" }} + {{- end }} + {{- if and (hasKey $caRef "key") (not (kindIs "string" (get $caRef "key"))) }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef.key must be a string" }} + {{- end }} + {{- if and $caName (not $tlsEnabled) }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef.name requires operator.defaultRedis.tls.enabled=true" }} + {{- end }} + {{- if and $caName (not (get $caRef "key")) }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef.key must be non-empty when name is set" }} + {{- end }} + {{- if and (not $caName) (ne $caKey "ca.crt") }} + {{- fail "operator.defaultRedis.tls.caCertSecretRef.key requires a Secret name" }} + {{- end }} + {{- if and $tlsEnabled (not $rdAddr) }} + {{- fail "operator.defaultRedis.tls.enabled requires operator.defaultRedis.addr or global.redis.host" }} + {{- end }} {{- if $rdAddr }} - name: TOOLHIVE_DEFAULT_REDIS_ADDR value: {{ $rdAddr | quote }} @@ -134,6 +182,16 @@ spec: - name: TOOLHIVE_DEFAULT_REDIS_SECRET_KEY value: {{ $rdKey | quote }} {{- end }} + {{- if $tlsEnabled }} + - name: TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED + value: "true" + {{- if $caName }} + - name: TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME + value: {{ $caName | quote }} + - name: TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY + value: {{ $caKey | quote }} + {{- end }} + {{- end }} {{- end }} livenessProbe: {{- toYaml .Values.operator.livenessProbe | nindent 12 }} diff --git a/deploy/charts/operator/tests/default_redis_test.yaml b/deploy/charts/operator/tests/default_redis_test.yaml index 18dfe7a449..fd570248dd 100644 --- a/deploy/charts/operator/tests/default_redis_test.yaml +++ b/deploy/charts/operator/tests/default_redis_test.yaml @@ -10,6 +10,10 @@ tests: path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_ADDR")]' - notExists: path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_SECRET_NAME")]' + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED")]' + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME")]' - it: injects TOOLHIVE_DEFAULT_REDIS_ADDR from operator.defaultRedis.addr set: @@ -73,3 +77,157 @@ tests: asserts: - notExists: path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_SECRET_NAME")]' + + - it: leaves TLS disabled for an address on port 6380 + set: + operator.defaultRedis.addr: "myredis:6380" + operator.defaultRedis.tls.enabled: false + asserts: + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED")]' + + - it: tolerates an omitted TLS block from older values + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls: null + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_ADDR, value: "myredis:6379" } + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED")]' + + - it: enables verified TLS with system roots + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED, value: "true" } + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME")]' + - notExists: + path: 'spec.template.spec.containers[0].env[?(@.name=="TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY")]' + + - it: enables verified TLS with a custom CA certificate Secret + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + operator.defaultRedis.tls.caCertSecretRef.name: "redis-ca" + operator.defaultRedis.tls.caCertSecretRef.key: "root.pem" + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED, value: "true" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME, value: "redis-ca" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY, value: "root.pem" } + + - it: uses the default CA key when only a Secret name is configured + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + operator.defaultRedis.tls.caCertSecretRef.name: "redis-ca" + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY, value: "ca.crt" } + + - it: enables TLS with the address inherited from the parent chart + set: + global.redis.host: "global-redis.internal" + global.redis.port: 6380 + operator.defaultRedis.tls.enabled: true + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_ADDR, value: "global-redis.internal:6380" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED, value: "true" } + + - it: keeps the explicit address and password Secret precedence with TLS + set: + operator.defaultRedis.addr: "myredis:6380" + operator.defaultRedis.existingSecret: "local-creds" + operator.defaultRedis.existingSecretKey: "local-password" + operator.defaultRedis.tls.enabled: true + global.redis.host: "global-redis.internal" + global.redis.existingSecret: "global-creds" + global.redis.existingSecretKey: "global-password" + asserts: + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_ADDR, value: "myredis:6380" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_SECRET_NAME, value: "local-creds" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_SECRET_KEY, value: "local-password" } + - contains: + path: 'spec.template.spec.containers[0].env' + content: { name: TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED, value: "true" } + + - it: rejects a non-boolean TLS flag + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: "true" + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.enabled must be a boolean' + + - it: rejects a non-object TLS block + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls: false + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls must be an object' + + - it: rejects TLS without a Redis address + set: + operator.defaultRedis.tls.enabled: true + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.enabled requires operator.defaultRedis.addr or global.redis.host' + + - it: rejects an empty CA Secret key + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + operator.defaultRedis.tls.caCertSecretRef.name: "redis-ca" + operator.defaultRedis.tls.caCertSecretRef.key: "" + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.caCertSecretRef.key must be non-empty when name is set' + + - it: rejects a CA Secret when TLS is disabled + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.caCertSecretRef.name: "redis-ca" + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.caCertSecretRef.name requires operator.defaultRedis.tls.enabled=true' + + - it: rejects a CA key configured without a Secret name + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + operator.defaultRedis.tls.caCertSecretRef.key: "root.pem" + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.caCertSecretRef.key requires a Secret name' + + - it: rejects unsupported insecure TLS options + set: + operator.defaultRedis.addr: "myredis:6379" + operator.defaultRedis.tls.enabled: true + operator.defaultRedis.tls.insecureSkipVerify: true + asserts: + - failedTemplate: + errorPattern: 'operator.defaultRedis.tls.insecureSkipVerify is unsupported' diff --git a/deploy/charts/operator/values.yaml b/deploy/charts/operator/values.yaml index bd769e580d..5b74e6f368 100644 --- a/deploy/charts/operator/values.yaml +++ b/deploy/charts/operator/values.yaml @@ -46,6 +46,20 @@ operator: # -- Key containing the password in `existingSecret`. When empty, uses # `global.redis.existingSecretKey`, then `redis-password`. existingSecretKey: "" + # -- TLS settings for the default Redis or Valkey connection. TLS is + # enabled only when explicitly requested and a Redis address is configured. + tls: + # -- Verify the Redis server certificate using system roots or the + # optional CA certificate below. Requires a configured Redis address. + enabled: false + # -- Optional CA certificate for verifying the Redis server. The Secret + # must exist in each workload's namespace, not the operator namespace. + caCertSecretRef: + # -- Name of an existing Secret containing a PEM-encoded CA certificate. + # Leave empty to use system roots. + name: "" + # -- Key containing the PEM-encoded CA certificate in the Secret. + key: ca.crt # -- Container image for the operator. image: ghcr.io/stacklok/toolhive/operator:v0.54.0 diff --git a/docs/arch/09-operator-architecture.md b/docs/arch/09-operator-architecture.md index 8e2e3748ae..36fd49ed23 100644 --- a/docs/arch/09-operator-architecture.md +++ b/docs/arch/09-operator-architecture.md @@ -559,6 +559,28 @@ The operator sets `Container.Args` (not `Command`) on the proxy-runner Deploymen **Implementation**: `cmd/thv-proxyrunner/app/commands.go` (cobra wiring), `cmd/thv-proxyrunner/app/run.go` (`run` command logic) +## Operator-wide Redis session storage + +`operator.defaultRedis` in the Helm chart supplies an address and optional password +Secret reference for workloads using the operator's Redis session storage fallback. +Enable `operator.defaultRedis.tls.enabled` for verified TLS; an optional +`operator.defaultRedis.tls.caCertSecretRef` selects a private CA. The password and +CA Secrets must exist in each workload's namespace. + +The chart injects connection settings and Secret references into the operator's +environment. The operator validates the TLS settings at startup, renders the +existing session TLS configuration, and projects the CA into the proxy runner or +vMCP container. It does not load the CA into its own process or transfer it between +namespaces. The existing configuration checksum and CA-volume comparisons trigger +Deployment updates when the selected TLS configuration or CA reference changes. +Changing CA contents under the same reference requires restarting workload pods. + +Per-workload Redis configuration takes precedence over global address, password, +and TLS settings. Disabled global TLS preserves plaintext behavior. Provider +selection retains the existing differences between resource kinds; see +[Redis transport security](13-vmcp-scalability.md#transport-security) for the +configuration example, environment variables, and compatibility details. + ## Design Principles **From**: `cmd/thv-operator/DESIGN.md` diff --git a/docs/arch/13-vmcp-scalability.md b/docs/arch/13-vmcp-scalability.md index 6d15e05340..66aee7cc99 100644 --- a/docs/arch/13-vmcp-scalability.md +++ b/docs/arch/13-vmcp-scalability.md @@ -136,9 +136,41 @@ logs one startup WARN carrying a `store` attribute, whether or not a password is configured. vMCP additionally logs its existing no-authentication WARN when `THV_SESSION_REDIS_PASSWORD` is unset; the two cover different gaps. -The operator-wide `defaultRedis` fallback (`TOOLHIVE_DEFAULT_REDIS_ADDR`) does not -carry TLS settings yet, so workloads that rely on it always log the plaintext WARN. -To encrypt their connection, set `spec.sessionStorage` with `tls` on the workload. +The operator-wide Redis fallback supports verified TLS through Helm: + +```yaml +operator: + defaultRedis: + addr: redis.example.com:6380 + tls: + enabled: true + caCertSecretRef: + name: redis-ca + key: ca.crt +``` + +Omit `caCertSecretRef` to verify against system roots. The CA Secret must exist +in every workload namespace that uses the fallback, alongside any configured +password Secret. The operator passes references to the workloads; it does not +copy Secret contents between namespaces or connect to Redis itself. + +The chart sets `TOOLHIVE_DEFAULT_REDIS_TLS_ENABLED` and, for a private CA, +`TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_NAME` and +`TOOLHIVE_DEFAULT_REDIS_TLS_CA_SECRET_KEY`. Invalid settings stop operator startup. +CA projection, certificate validation, rate-limiter TLS, and restart requirements +are the same as for per-workload session storage. A change to operator defaults +is applied as workloads reconcile after the operator restarts. + +Explicit per-workload Redis settings override the entire global connection, +including TLS; omitting TLS on an explicit Redis connection keeps that connection +plaintext. The default remains plaintext when global TLS is disabled. Global TLS +always verifies server certificates; it has no `insecureSkipVerify` option. + +This change preserves existing provider selection: MCPRemoteProxy suppresses the +fallback for any explicit `sessionStorage`, while MCPServer and VirtualMCPServer +can still select global Redis when `provider: memory` is explicit. Their existing +password-selection behavior is unchanged. Configure an explicit Redis connection +when overriding global defaults, and avoid relying on this provider inconsistency. ## File descriptor limits