From d1057759b791f0294633717fff1bc5765bbe7a51 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 1 May 2026 11:04:55 -0400 Subject: [PATCH 1/4] Add Redis Cluster mode support to auth server storage Managed Redis services like GCP Memorystore Cluster and AWS ElastiCache Serverless use the Redis Cluster protocol rather than standalone or Sentinel connections, making it impossible to use them without this third mode. - Add ClusterConfig/ClusterRunConfig types to storage and runner layers - Wire cluster client creation via redis.NewClusterClient in NewRedisStorage - Update validateConfig, convertRedisRunConfig, and buildStorageRunConfig to enforce three-way mutual exclusion (addr / sentinel / cluster) - Add RedisClusterConfig CRD type with MinItems=1 validation; update CEL rule to require exactly one of the three modes - Regenerate deepcopy, CRD YAML, and Helm chart templates - Add unit tests for all new validation and happy-path code paths Closes #5010 Co-Authored-By: Claude Sonnet 4.6 --- .../v1beta1/mcpexternalauthconfig_types.go | 24 +++++- .../api/v1beta1/zz_generated.deepcopy.go | 25 ++++++ .../pkg/controllerutil/authserver.go | 37 ++++++-- .../pkg/controllerutil/authserver_test.go | 84 ++++++++++++++++++- ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 58 ++++++++++--- ...olhive.stacklok.dev_virtualmcpservers.yaml | 58 ++++++++++--- ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 58 ++++++++++--- ...olhive.stacklok.dev_virtualmcpservers.yaml | 58 ++++++++++--- pkg/authserver/runner/embeddedauthserver.go | 27 ++++-- .../runner/embeddedauthserver_test.go | 65 +++++++++++++- pkg/authserver/storage/config.go | 18 +++- pkg/authserver/storage/redis.go | 70 +++++++++++++--- pkg/authserver/storage/redis_test.go | 52 +++++++++++- 13 files changed, 554 insertions(+), 80 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index fb24749204..a595f64f7b 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -525,23 +525,29 @@ type AuthServerStorageConfig struct { } // RedisStorageConfig configures Redis connection for auth server storage. -// Exactly one of addr (standalone) or sentinelConfig (Sentinel) must be set. +// Exactly one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) must be set. // -// +kubebuilder:validation:XValidation:rule="(self.addr.size() > 0) != has(self.sentinelConfig)",message="exactly one of addr (standalone) or sentinelConfig (Sentinel) must be set" +// +kubebuilder:validation:XValidation:rule="(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1",message="exactly one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) must be set" // //nolint:lll // CEL validation rules exceed line length limit type RedisStorageConfig struct { // Addr is the Redis server address for standalone mode (e.g., "host:port"). // Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - // a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + // a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. // +optional Addr string `json:"addr,omitempty"` // SentinelConfig holds Redis Sentinel configuration. - // Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + // Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. // +optional SentinelConfig *RedisSentinelConfig `json:"sentinelConfig,omitempty"` + // ClusterConfig holds Redis Cluster configuration. + // Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + // AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + // +optional + ClusterConfig *RedisClusterConfig `json:"clusterConfig,omitempty"` + // ACLUserConfig configures Redis ACL user authentication. // +kubebuilder:validation:Required ACLUserConfig *RedisACLUserConfig `json:"aclUserConfig"` @@ -601,6 +607,16 @@ type RedisSentinelConfig struct { DB int32 `json:"db,omitempty"` } +// RedisClusterConfig configures Redis Cluster connection. +type RedisClusterConfig struct { + // Addrs is the list of seed node host:port addresses for the Redis Cluster. + // At least one address is required; go-redis discovers other nodes automatically. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinItems=1 + // +listType=atomic + Addrs []string `json:"addrs"` +} + // SentinelServiceRef references a Kubernetes Service for Sentinel discovery. type SentinelServiceRef struct { // Name of the Sentinel Service. diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index 04da44756d..fa6dfc5445 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -2264,6 +2264,26 @@ func (in *RedisACLUserConfig) DeepCopy() *RedisACLUserConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RedisClusterConfig) DeepCopyInto(out *RedisClusterConfig) { + *out = *in + if in.Addrs != nil { + in, out := &in.Addrs, &out.Addrs + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RedisClusterConfig. +func (in *RedisClusterConfig) DeepCopy() *RedisClusterConfig { + if in == nil { + return nil + } + out := new(RedisClusterConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RedisSentinelConfig) DeepCopyInto(out *RedisSentinelConfig) { *out = *in @@ -2297,6 +2317,11 @@ func (in *RedisStorageConfig) DeepCopyInto(out *RedisStorageConfig) { *out = new(RedisSentinelConfig) (*in).DeepCopyInto(*out) } + if in.ClusterConfig != nil { + in, out := &in.ClusterConfig, &out.ClusterConfig + *out = new(RedisClusterConfig) + (*in).DeepCopyInto(*out) + } if in.ACLUserConfig != nil { in, out := &in.ACLUserConfig, &out.ACLUserConfig *out = new(RedisACLUserConfig) diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index bccf0df3bb..44a59ac4a0 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -544,12 +544,8 @@ func buildStorageRunConfig( return nil, fmt.Errorf("redis config is required when storage type is redis") } - if redisConfig.Addr == "" && redisConfig.SentinelConfig == nil { - return nil, fmt.Errorf("either addr (standalone) or sentinel config is required for Redis storage") - } - - if redisConfig.Addr != "" && redisConfig.SentinelConfig != nil { - return nil, fmt.Errorf("addr and sentinel config are mutually exclusive for Redis storage") + if err := validateRedisConnectionMode(redisConfig); err != nil { + return nil, err } if redisConfig.ACLUserConfig == nil || @@ -592,12 +588,41 @@ func buildStorageRunConfig( rc.SentinelTLS = convertRedisTLSConfig(redisConfig.SentinelTLS, true) } + if redisConfig.ClusterConfig != nil { + rc.ClusterConfig = &storage.ClusterRunConfig{ + Addrs: redisConfig.ClusterConfig.Addrs, + } + } + return &storage.RunConfig{ Type: string(storage.TypeRedis), RedisConfig: rc, }, nil } +// validateRedisConnectionMode checks that exactly one of addr, sentinelConfig, +// or clusterConfig is set on the Redis storage config. +func validateRedisConnectionMode(cfg *mcpv1beta1.RedisStorageConfig) error { + modes := 0 + if cfg.Addr != "" { + modes++ + } + if cfg.SentinelConfig != nil { + modes++ + } + if cfg.ClusterConfig != nil { + modes++ + } + if modes > 1 { + return fmt.Errorf("addr, sentinelConfig, and clusterConfig are mutually exclusive for Redis storage") + } + if modes == 0 { + return fmt.Errorf("one of addr (standalone), sentinelConfig (Sentinel)," + + " or clusterConfig (Cluster) is required for Redis storage") + } + return nil +} + // convertRedisTLSConfig converts CRD RedisTLSConfig to RunConfig. // isSentinel determines which mount path to use for the CA cert file. func convertRedisTLSConfig(cfg *mcpv1beta1.RedisTLSConfig, isSentinel bool) *storage.RedisTLSRunConfig { diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index 7add4da7af..a6323c11a6 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -1674,7 +1674,7 @@ func TestBuildStorageRunConfig(t *testing.T) { errContains: "redis config is required", }, { - name: "Redis storage missing both addr and sentinelConfig returns error", + name: "Redis storage missing addr, sentinelConfig, and clusterConfig returns error", authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ Issuer: "https://auth.example.com", Storage: &mcpv1beta1.AuthServerStorageConfig{ @@ -1688,7 +1688,7 @@ func TestBuildStorageRunConfig(t *testing.T) { }, }, wantErr: true, - errContains: "either addr (standalone) or sentinel config is required", + errContains: "one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) is required", }, { name: "Redis storage with both addr and sentinelConfig returns error", @@ -1710,7 +1710,85 @@ func TestBuildStorageRunConfig(t *testing.T) { }, }, wantErr: true, - errContains: "addr and sentinel config are mutually exclusive", + errContains: "mutually exclusive", + }, + { + name: "Redis storage with addr and clusterConfig returns error", + authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + Storage: &mcpv1beta1.AuthServerStorageConfig{ + Type: mcpv1beta1.AuthServerStorageTypeRedis, + Redis: &mcpv1beta1.RedisStorageConfig{ + Addr: "redis.example.com:6379", + ClusterConfig: &mcpv1beta1.RedisClusterConfig{ + Addrs: []string{"node1.example.com:6379"}, + }, + ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ + UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"}, + PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"}, + }, + }, + }, + }, + wantErr: true, + errContains: "mutually exclusive", + }, + { + name: "Redis storage with sentinelConfig and clusterConfig returns error", + authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + Storage: &mcpv1beta1.AuthServerStorageConfig{ + Type: mcpv1beta1.AuthServerStorageTypeRedis, + Redis: &mcpv1beta1.RedisStorageConfig{ + SentinelConfig: &mcpv1beta1.RedisSentinelConfig{ + MasterName: "mymaster", + SentinelAddrs: []string{"10.0.0.1:26379"}, + }, + ClusterConfig: &mcpv1beta1.RedisClusterConfig{ + Addrs: []string{"node1.example.com:6379"}, + }, + ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ + UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"}, + PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"}, + }, + }, + }, + }, + wantErr: true, + errContains: "mutually exclusive", + }, + { + name: "Redis cluster config builds correctly", + authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + Storage: &mcpv1beta1.AuthServerStorageConfig{ + Type: mcpv1beta1.AuthServerStorageTypeRedis, + Redis: &mcpv1beta1.RedisStorageConfig{ + ClusterConfig: &mcpv1beta1.RedisClusterConfig{ + Addrs: []string{"node1.example.com:6379", "node2.example.com:6379"}, + }, + ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ + UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "username"}, + PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "password"}, + }, + }, + }, + }, + checkFunc: func(t *testing.T, cfg *storage.RunConfig) { + t.Helper() + assert.Equal(t, string(storage.TypeRedis), cfg.Type) + require.NotNil(t, cfg.RedisConfig) + require.NotNil(t, cfg.RedisConfig.ClusterConfig) + assert.Equal(t, []string{"node1.example.com:6379", "node2.example.com:6379"}, + cfg.RedisConfig.ClusterConfig.Addrs) + assert.Empty(t, cfg.RedisConfig.Addr) + assert.Nil(t, cfg.RedisConfig.SentinelConfig) + assert.Equal(t, storage.AuthTypeACLUser, cfg.RedisConfig.AuthType) + require.NotNil(t, cfg.RedisConfig.ACLUserConfig) + assert.Equal(t, authrunner.RedisUsernameEnvVar, cfg.RedisConfig.ACLUserConfig.UsernameEnvVar) + assert.Equal(t, authrunner.RedisPasswordEnvVar, cfg.RedisConfig.ACLUserConfig.PasswordEnvVar) + assert.Equal(t, "thv:auth:{default:test-server}:", cfg.RedisConfig.KeyPrefix) + }, }, { name: "Redis storage with standalone addr builds correctly", diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 6e4f0d6f65..7fe151dfc5 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -318,8 +318,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -337,7 +355,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -442,9 +460,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- @@ -1366,8 +1385,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -1385,7 +1422,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -1490,9 +1527,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index a51fe4b5bd..e44a444548 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -191,8 +191,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -210,7 +228,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -315,9 +333,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- @@ -2687,8 +2706,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -2706,7 +2743,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -2811,9 +2848,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 4ea0c1cb07..5ba354a41b 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -321,8 +321,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -340,7 +358,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -445,9 +463,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- @@ -1369,8 +1388,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -1388,7 +1425,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -1493,9 +1530,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 6078670479..0391680dcd 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -194,8 +194,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -213,7 +231,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -318,9 +336,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- @@ -2690,8 +2709,26 @@ spec: description: |- Addr is the Redis server address for standalone mode (e.g., "host:port"). Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. + a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. type: string + clusterConfig: + description: |- + ClusterConfig holds Redis Cluster configuration. + Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, + AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + properties: + addrs: + description: |- + Addrs is the list of seed node host:port addresses for the Redis Cluster. + At least one address is required; go-redis discovers other nodes automatically. + items: + type: string + minItems: 1 + type: array + x-kubernetes-list-type: atomic + required: + - addrs + type: object dialTimeout: default: 5s description: |- @@ -2709,7 +2746,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. properties: db: default: 0 @@ -2814,9 +2851,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone) or sentinelConfig - (Sentinel) must be set - rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: exactly one of addr (standalone), sentinelConfig + (Sentinel), or clusterConfig (Cluster) must be set + rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) + ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' type: default: memory description: |- diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index dcc884e09d..cac3cf30f9 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -481,25 +481,40 @@ func convertRedisRunConfig(rc *storage.RedisRunConfig) (*storage.RedisConfig, er return nil, fmt.Errorf("redis config is required when storage type is redis") } - if rc.Addr != "" && rc.SentinelConfig != nil { - return nil, fmt.Errorf("addr and sentinel_config are mutually exclusive") + modes := 0 + if rc.Addr != "" { + modes++ + } + if rc.SentinelConfig != nil { + modes++ + } + if rc.ClusterConfig != nil { + modes++ } - if rc.Addr == "" && rc.SentinelConfig == nil { - return nil, fmt.Errorf("one of addr (standalone) or sentinel_config (sentinel) is required") + if modes > 1 { + return nil, fmt.Errorf("addr, sentinel_config, and cluster_config are mutually exclusive; exactly one must be set") + } + if modes == 0 { + return nil, fmt.Errorf("one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") } cfg := &storage.RedisConfig{ KeyPrefix: rc.KeyPrefix, } - if rc.Addr != "" { + switch { + case rc.Addr != "": cfg.Addr = rc.Addr - } else { + case rc.SentinelConfig != nil: cfg.SentinelConfig = &storage.SentinelConfig{ MasterName: rc.SentinelConfig.MasterName, SentinelAddrs: rc.SentinelConfig.SentinelAddrs, DB: rc.SentinelConfig.DB, } + case rc.ClusterConfig != nil: + cfg.ClusterConfig = &storage.ClusterConfig{ + Addrs: rc.ClusterConfig.Addrs, + } } aclCfg, err := convertRedisACLConfig(rc.ACLUserConfig) diff --git a/pkg/authserver/runner/embeddedauthserver_test.go b/pkg/authserver/runner/embeddedauthserver_test.go index faec066857..f8fbf19c18 100644 --- a/pkg/authserver/runner/embeddedauthserver_test.go +++ b/pkg/authserver/runner/embeddedauthserver_test.go @@ -1098,7 +1098,7 @@ func TestCreateStorage(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "one of addr (standalone) or sentinel_config (sentinel) is required") + assert.Contains(t, err.Error(), "one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") }) } @@ -1122,7 +1122,7 @@ func TestConvertRedisRunConfig(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "one of addr (standalone) or sentinel_config (sentinel) is required") + assert.Contains(t, err.Error(), "one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") }) t.Run("missing ACL user config returns error", func(t *testing.T) { @@ -1185,6 +1185,41 @@ func TestConvertRedisRunConfig(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "one of addr") }) + + t.Run("cluster config with addr also set returns error", func(t *testing.T) { + t.Parallel() + _, err := convertRedisRunConfig(&storage.RedisRunConfig{ + Addr: "redis.example.com:6379", + ClusterConfig: &storage.ClusterRunConfig{ + Addrs: []string{"node1:6379", "node2:6379"}, + }, + ACLUserConfig: &storage.ACLUserRunConfig{ + PasswordEnvVar: "PASS", + }, + KeyPrefix: "thv:", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") + }) + + t.Run("cluster config with sentinel also set returns error", func(t *testing.T) { + t.Parallel() + _, err := convertRedisRunConfig(&storage.RedisRunConfig{ + SentinelConfig: &storage.SentinelRunConfig{ + MasterName: "mymaster", + SentinelAddrs: []string{"sentinel:26379"}, + }, + ClusterConfig: &storage.ClusterRunConfig{ + Addrs: []string{"node1:6379"}, + }, + ACLUserConfig: &storage.ACLUserRunConfig{ + PasswordEnvVar: "PASS", + }, + KeyPrefix: "thv:", + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "mutually exclusive") + }) } // TestConvertRedisRunConfig_WithEnvVars tests convertRedisRunConfig with environment variables. @@ -1299,6 +1334,32 @@ func TestConvertRedisRunConfig_WithEnvVars(t *testing.T) { assert.Empty(t, cfg.ACLUserConfig.Username) assert.Equal(t, "mypass", cfg.ACLUserConfig.Password) }) + + t.Run("cluster config resolves correctly", func(t *testing.T) { + t.Setenv("TEST_REDIS_USER_CLUSTER", "clusteruser") + t.Setenv("TEST_REDIS_PASS_CLUSTER", "clusterpass") + + cfg, err := convertRedisRunConfig(&storage.RedisRunConfig{ + ClusterConfig: &storage.ClusterRunConfig{ + Addrs: []string{"node1.example.com:6379", "node2.example.com:6379"}, + }, + ACLUserConfig: &storage.ACLUserRunConfig{ + UsernameEnvVar: "TEST_REDIS_USER_CLUSTER", + PasswordEnvVar: "TEST_REDIS_PASS_CLUSTER", + }, + KeyPrefix: "thv:auth:ns:name:", + }) + require.NoError(t, err) + require.NotNil(t, cfg) + require.NotNil(t, cfg.ClusterConfig) + assert.Equal(t, []string{"node1.example.com:6379", "node2.example.com:6379"}, cfg.ClusterConfig.Addrs) + assert.Empty(t, cfg.Addr) + assert.Nil(t, cfg.SentinelConfig) + require.NotNil(t, cfg.ACLUserConfig) + assert.Equal(t, "clusteruser", cfg.ACLUserConfig.Username) + assert.Equal(t, "clusterpass", cfg.ACLUserConfig.Password) + assert.Equal(t, "thv:auth:ns:name:", cfg.KeyPrefix) + }) } // stubServer is a minimal authserver.Server implementation for testing RegisterHandlers. diff --git a/pkg/authserver/storage/config.go b/pkg/authserver/storage/config.go index 2f066b0a98..c1f8c67d12 100644 --- a/pkg/authserver/storage/config.go +++ b/pkg/authserver/storage/config.go @@ -66,17 +66,27 @@ type RunConfig struct { RedisConfig *RedisRunConfig `json:"redis_config,omitempty" yaml:"redis_config,omitempty"` } +// ClusterRunConfig contains Redis Cluster configuration for cross-process serialization. +type ClusterRunConfig struct { + // Addrs is the list of seed node addresses (host:port) for the Redis Cluster. + Addrs []string `json:"addrs" yaml:"addrs"` +} + // RedisRunConfig is the serializable Redis configuration for RunConfig. -// Exactly one of Addr (standalone) or SentinelConfig (Sentinel) must be set. +// Exactly one of Addr (standalone), SentinelConfig (Sentinel), or ClusterConfig (Cluster) must be set. type RedisRunConfig struct { // Addr is the Redis server address for standalone mode (e.g., "host:port"). - // Mutually exclusive with SentinelConfig. + // Mutually exclusive with SentinelConfig and ClusterConfig. Addr string `json:"addr,omitempty" yaml:"addr,omitempty"` // SentinelConfig contains Sentinel-specific configuration. - // Mutually exclusive with Addr. + // Mutually exclusive with Addr and ClusterConfig. SentinelConfig *SentinelRunConfig `json:"sentinel_config,omitempty" yaml:"sentinel_config,omitempty"` + // ClusterConfig contains Redis Cluster-specific configuration. + // Mutually exclusive with Addr and SentinelConfig. + ClusterConfig *ClusterRunConfig `json:"cluster_config,omitempty" yaml:"cluster_config,omitempty"` + // AuthType must be "aclUser" - only ACL user authentication is supported. AuthType string `json:"auth_type" yaml:"auth_type"` @@ -95,7 +105,7 @@ type RedisRunConfig struct { // WriteTimeout is the timeout for write operations (e.g., "3s"). WriteTimeout string `json:"write_timeout,omitempty" yaml:"write_timeout,omitempty"` - // TLS configures TLS for Redis/Valkey master connections. + // TLS configures TLS for Redis/Valkey master or cluster node connections. TLS *RedisTLSRunConfig `json:"tls,omitempty" yaml:"tls,omitempty"` // SentinelTLS configures TLS for Sentinel connections. Only applies when SentinelConfig is set. diff --git a/pkg/authserver/storage/redis.go b/pkg/authserver/storage/redis.go index 9b968be27c..9e508cdd22 100644 --- a/pkg/authserver/storage/redis.go +++ b/pkg/authserver/storage/redis.go @@ -50,15 +50,24 @@ func warnOnCleanupErr(err error, operation, key string) { } } +// ClusterConfig contains Redis Cluster connection configuration. +type ClusterConfig struct { + // Addrs is the list of seed node addresses (host:port) for the Redis Cluster. + Addrs []string +} + // RedisConfig holds Redis connection configuration for runtime use. type RedisConfig struct { // Addr is the Redis server address for standalone mode (e.g., "host:port"). - // Mutually exclusive with SentinelConfig. + // Mutually exclusive with SentinelConfig and ClusterConfig. Addr string - // SentinelConfig is required for Sentinel mode. Mutually exclusive with Addr. + // SentinelConfig is required for Sentinel mode. Mutually exclusive with Addr and ClusterConfig. SentinelConfig *SentinelConfig + // ClusterConfig is required for Cluster mode. Mutually exclusive with Addr and SentinelConfig. + ClusterConfig *ClusterConfig + // ACLUserConfig is required - ACL user authentication only. ACLUserConfig *ACLUserConfig @@ -70,8 +79,8 @@ type RedisConfig struct { ReadTimeout time.Duration WriteTimeout time.Duration - // TLS configures TLS for connections to the Redis/Valkey master. - // When nil, master connections are plaintext. + // TLS configures TLS for connections to the Redis/Valkey master or cluster nodes. + // When nil, connections are plaintext. TLS *RedisTLSConfig // SentinelTLS configures TLS for connections to Sentinel instances. @@ -105,9 +114,9 @@ type ACLUserConfig struct { } // RedisStorage implements the Storage interface backed by Redis. -// Supports standalone mode (single endpoint) and Sentinel failover mode. -// It provides distributed storage for OAuth2 tokens, authorization codes, -// user data, and pending authorizations, enabling horizontal scaling. +// Supports standalone mode (single endpoint), Sentinel failover mode, and +// Cluster mode. It provides distributed storage for OAuth2 tokens, authorization +// codes, user data, and pending authorizations, enabling horizontal scaling. type RedisStorage struct { client redis.UniversalClient keyPrefix string @@ -226,7 +235,8 @@ func NewRedisStorage(ctx context.Context, cfg RedisConfig) (*RedisStorage, error var client redis.UniversalClient - if cfg.SentinelConfig != nil { + switch { + case cfg.SentinelConfig != nil: opts := &redis.FailoverOptions{ MasterName: cfg.SentinelConfig.MasterName, SentinelAddrs: cfg.SentinelConfig.SentinelAddrs, @@ -244,7 +254,26 @@ func NewRedisStorage(ctx context.Context, cfg RedisConfig) (*RedisStorage, error } client = redis.NewFailoverClient(opts) - } else { + + case cfg.ClusterConfig != nil: + tlsCfg, err := buildTLSConfig(cfg.TLS) + if err != nil { + return nil, fmt.Errorf("TLS config: %w", err) + } + + opts := &redis.ClusterOptions{ + Addrs: cfg.ClusterConfig.Addrs, + Username: cfg.ACLUserConfig.Username, + Password: cfg.ACLUserConfig.Password, + DialTimeout: cfg.DialTimeout, + ReadTimeout: cfg.ReadTimeout, + WriteTimeout: cfg.WriteTimeout, + TLSConfig: tlsCfg, + } + + client = redis.NewClusterClient(opts) + + default: masterTLS, err := buildTLSConfig(cfg.TLS) if err != nil { return nil, fmt.Errorf("master TLS config: %w", err) @@ -293,11 +322,21 @@ func defaultSessionFactory(subject, idpSessionID, clientID string) fosite.Sessio } func validateConfig(cfg *RedisConfig) error { - if cfg.Addr != "" && cfg.SentinelConfig != nil { - return errors.New("addr and sentinel configuration are mutually exclusive") + modes := 0 + if cfg.Addr != "" { + modes++ } - if cfg.Addr == "" && cfg.SentinelConfig == nil { - return errors.New("one of addr (standalone) or sentinel configuration is required") + if cfg.SentinelConfig != nil { + modes++ + } + if cfg.ClusterConfig != nil { + modes++ + } + if modes > 1 { + return errors.New("addr, sentinel, and cluster configuration are mutually exclusive; exactly one must be set") + } + if modes == 0 { + return errors.New("one of addr (standalone), sentinel configuration, or cluster configuration is required") } if cfg.SentinelConfig != nil { if cfg.SentinelConfig.MasterName == "" { @@ -307,6 +346,11 @@ func validateConfig(cfg *RedisConfig) error { return errors.New("at least one sentinel address is required") } } + if cfg.ClusterConfig != nil { + if len(cfg.ClusterConfig.Addrs) == 0 { + return errors.New("at least one cluster address is required") + } + } if cfg.ACLUserConfig == nil { return errors.New("ACL user configuration is required") } diff --git a/pkg/authserver/storage/redis_test.go b/pkg/authserver/storage/redis_test.go index 25fb3d6e32..a43dda251b 100644 --- a/pkg/authserver/storage/redis_test.go +++ b/pkg/authserver/storage/redis_test.go @@ -103,7 +103,7 @@ func TestRedisConfig_Validation(t *testing.T) { { name: "neither addr nor sentinel config", cfg: RedisConfig{ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:"}, - wantErr: "one of addr (standalone) or sentinel configuration is required", + wantErr: "one of addr (standalone), sentinel configuration, or cluster configuration is required", }, { name: "addr and sentinel config both set", @@ -113,7 +113,32 @@ func TestRedisConfig_Validation(t *testing.T) { ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:", }, - wantErr: "addr and sentinel configuration are mutually exclusive", + wantErr: "mutually exclusive", + }, + { + name: "cluster config and addr both set", + cfg: RedisConfig{ + Addr: "localhost:6379", + ClusterConfig: &ClusterConfig{Addrs: []string{"localhost:6379"}}, + ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, + KeyPrefix: "test:", + }, + wantErr: "mutually exclusive", + }, + { + name: "cluster config and sentinel both set", + cfg: RedisConfig{ + SentinelConfig: &SentinelConfig{MasterName: "m", SentinelAddrs: []string{"localhost:26379"}}, + ClusterConfig: &ClusterConfig{Addrs: []string{"localhost:6379"}}, + ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, + KeyPrefix: "test:", + }, + wantErr: "mutually exclusive", + }, + { + name: "cluster config with no addrs", + cfg: RedisConfig{ClusterConfig: &ClusterConfig{}, ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:"}, + wantErr: "at least one cluster address is required", }, { name: "missing sentinel master name", @@ -222,6 +247,29 @@ func TestNewRedisStorage_Standalone_WithMiniredis(t *testing.T) { require.NoError(t, s.Health(ctx)) } +func TestNewRedisStorage_Cluster_ConnectionFailure(t *testing.T) { + t.Parallel() + + cfg := RedisConfig{ + ClusterConfig: &ClusterConfig{ + Addrs: []string{"localhost:19998"}, + }, + ACLUserConfig: &ACLUserConfig{ + Username: "user", + Password: "pass", + }, + KeyPrefix: "test:", + DialTimeout: 100 * time.Millisecond, + } + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + + _, err := NewRedisStorage(ctx, cfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to connect to redis") +} + // --- Client Tests --- func TestRedisStorage_Client(t *testing.T) { From 30315bcccb4eba16d88ef66f12066ccf246d2cd8 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 1 May 2026 12:01:38 -0400 Subject: [PATCH 2/4] Simplify Redis Cluster mode to addr + clusterMode bool ClusterConfig{Addrs []string} was a misleading abstraction: GCP Memorystore Cluster and AWS ElastiCache cluster mode both expose a single discovery endpoint, so the slice always held exactly one address and was redundant with the existing Addr field. Replace ClusterConfig with a ClusterMode bool flag that is set alongside the existing Addr field. go-redis NewClusterClient still receives the single endpoint as []string{addr} and auto-discovers the full cluster topology from there. Co-Authored-By: Claude Sonnet 4.6 --- .../v1beta1/mcpexternalauthconfig_types.go | 38 ++++----- .../api/v1beta1/zz_generated.deepcopy.go | 25 ------ .../pkg/controllerutil/authserver.go | 37 ++------- .../pkg/controllerutil/authserver_test.go | 62 ++------------- ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 78 +++++++------------ ...olhive.stacklok.dev_virtualmcpservers.yaml | 78 +++++++------------ ...e.stacklok.dev_mcpexternalauthconfigs.yaml | 78 +++++++------------ ...olhive.stacklok.dev_virtualmcpservers.yaml | 78 +++++++------------ pkg/authserver/runner/embeddedauthserver.go | 32 +++----- .../runner/embeddedauthserver_test.go | 40 +++------- pkg/authserver/storage/config.go | 22 ++---- pkg/authserver/storage/redis.go | 50 +++++------- pkg/authserver/storage/redis_test.go | 33 ++++---- 13 files changed, 200 insertions(+), 451 deletions(-) diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index a595f64f7b..61ef7a55f7 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -525,28 +525,32 @@ type AuthServerStorageConfig struct { } // RedisStorageConfig configures Redis connection for auth server storage. -// Exactly one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) must be set. +// Exactly one of addr or sentinelConfig must be set. Set clusterMode to true when +// addr points to a Redis Cluster discovery endpoint (GCP Memorystore Cluster, +// AWS ElastiCache cluster mode enabled). // -// +kubebuilder:validation:XValidation:rule="(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1",message="exactly one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) must be set" +// +kubebuilder:validation:XValidation:rule="(self.addr.size() > 0) != has(self.sentinelConfig)",message="exactly one of addr or sentinelConfig must be set" +// +kubebuilder:validation:XValidation:rule="!self.clusterMode || self.addr.size() > 0",message="clusterMode requires addr to be set" // //nolint:lll // CEL validation rules exceed line length limit type RedisStorageConfig struct { - // Addr is the Redis server address for standalone mode (e.g., "host:port"). - // Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - // a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + // Addr is the Redis server address (host:port). Required for standalone and cluster modes. + // Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + // AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + // Mutually exclusive with sentinelConfig. // +optional Addr string `json:"addr,omitempty"` - // SentinelConfig holds Redis Sentinel configuration. - // Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + // ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + // Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + // cluster mode enabled). Requires addr to be set. // +optional - SentinelConfig *RedisSentinelConfig `json:"sentinelConfig,omitempty"` + ClusterMode bool `json:"clusterMode,omitempty"` - // ClusterConfig holds Redis Cluster configuration. - // Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - // AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. + // SentinelConfig holds Redis Sentinel configuration. + // Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. // +optional - ClusterConfig *RedisClusterConfig `json:"clusterConfig,omitempty"` + SentinelConfig *RedisSentinelConfig `json:"sentinelConfig,omitempty"` // ACLUserConfig configures Redis ACL user authentication. // +kubebuilder:validation:Required @@ -607,16 +611,6 @@ type RedisSentinelConfig struct { DB int32 `json:"db,omitempty"` } -// RedisClusterConfig configures Redis Cluster connection. -type RedisClusterConfig struct { - // Addrs is the list of seed node host:port addresses for the Redis Cluster. - // At least one address is required; go-redis discovers other nodes automatically. - // +kubebuilder:validation:Required - // +kubebuilder:validation:MinItems=1 - // +listType=atomic - Addrs []string `json:"addrs"` -} - // SentinelServiceRef references a Kubernetes Service for Sentinel discovery. type SentinelServiceRef struct { // Name of the Sentinel Service. diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index fa6dfc5445..04da44756d 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -2264,26 +2264,6 @@ func (in *RedisACLUserConfig) DeepCopy() *RedisACLUserConfig { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RedisClusterConfig) DeepCopyInto(out *RedisClusterConfig) { - *out = *in - if in.Addrs != nil { - in, out := &in.Addrs, &out.Addrs - *out = make([]string, len(*in)) - copy(*out, *in) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RedisClusterConfig. -func (in *RedisClusterConfig) DeepCopy() *RedisClusterConfig { - if in == nil { - return nil - } - out := new(RedisClusterConfig) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RedisSentinelConfig) DeepCopyInto(out *RedisSentinelConfig) { *out = *in @@ -2317,11 +2297,6 @@ func (in *RedisStorageConfig) DeepCopyInto(out *RedisStorageConfig) { *out = new(RedisSentinelConfig) (*in).DeepCopyInto(*out) } - if in.ClusterConfig != nil { - in, out := &in.ClusterConfig, &out.ClusterConfig - *out = new(RedisClusterConfig) - (*in).DeepCopyInto(*out) - } if in.ACLUserConfig != nil { in, out := &in.ACLUserConfig, &out.ACLUserConfig *out = new(RedisACLUserConfig) diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 44a59ac4a0..5b366c36ee 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -544,8 +544,11 @@ func buildStorageRunConfig( return nil, fmt.Errorf("redis config is required when storage type is redis") } - if err := validateRedisConnectionMode(redisConfig); err != nil { - return nil, err + if redisConfig.Addr != "" && redisConfig.SentinelConfig != nil { + return nil, fmt.Errorf("addr and sentinelConfig are mutually exclusive for Redis storage") + } + if redisConfig.Addr == "" && redisConfig.SentinelConfig == nil { + return nil, fmt.Errorf("one of addr (standalone or cluster) or sentinelConfig (Sentinel) is required for Redis storage") } if redisConfig.ACLUserConfig == nil || @@ -565,6 +568,7 @@ func buildStorageRunConfig( rc := &storage.RedisRunConfig{ Addr: redisConfig.Addr, + ClusterMode: redisConfig.ClusterMode, AuthType: storage.AuthTypeACLUser, ACLUserConfig: aclRunConfig, KeyPrefix: keyPrefix, @@ -588,41 +592,12 @@ func buildStorageRunConfig( rc.SentinelTLS = convertRedisTLSConfig(redisConfig.SentinelTLS, true) } - if redisConfig.ClusterConfig != nil { - rc.ClusterConfig = &storage.ClusterRunConfig{ - Addrs: redisConfig.ClusterConfig.Addrs, - } - } - return &storage.RunConfig{ Type: string(storage.TypeRedis), RedisConfig: rc, }, nil } -// validateRedisConnectionMode checks that exactly one of addr, sentinelConfig, -// or clusterConfig is set on the Redis storage config. -func validateRedisConnectionMode(cfg *mcpv1beta1.RedisStorageConfig) error { - modes := 0 - if cfg.Addr != "" { - modes++ - } - if cfg.SentinelConfig != nil { - modes++ - } - if cfg.ClusterConfig != nil { - modes++ - } - if modes > 1 { - return fmt.Errorf("addr, sentinelConfig, and clusterConfig are mutually exclusive for Redis storage") - } - if modes == 0 { - return fmt.Errorf("one of addr (standalone), sentinelConfig (Sentinel)," + - " or clusterConfig (Cluster) is required for Redis storage") - } - return nil -} - // convertRedisTLSConfig converts CRD RedisTLSConfig to RunConfig. // isSentinel determines which mount path to use for the CA cert file. func convertRedisTLSConfig(cfg *mcpv1beta1.RedisTLSConfig, isSentinel bool) *storage.RedisTLSRunConfig { diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index a6323c11a6..2def8c7118 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -1674,7 +1674,7 @@ func TestBuildStorageRunConfig(t *testing.T) { errContains: "redis config is required", }, { - name: "Redis storage missing addr, sentinelConfig, and clusterConfig returns error", + name: "Redis storage missing addr and sentinelConfig returns error", authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ Issuer: "https://auth.example.com", Storage: &mcpv1beta1.AuthServerStorageConfig{ @@ -1688,7 +1688,7 @@ func TestBuildStorageRunConfig(t *testing.T) { }, }, wantErr: true, - errContains: "one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) is required", + errContains: "one of addr (standalone or cluster) or sentinelConfig (Sentinel) is required", }, { name: "Redis storage with both addr and sentinelConfig returns error", @@ -1713,60 +1713,14 @@ func TestBuildStorageRunConfig(t *testing.T) { errContains: "mutually exclusive", }, { - name: "Redis storage with addr and clusterConfig returns error", + name: "Redis cluster mode builds correctly", authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ Issuer: "https://auth.example.com", Storage: &mcpv1beta1.AuthServerStorageConfig{ Type: mcpv1beta1.AuthServerStorageTypeRedis, Redis: &mcpv1beta1.RedisStorageConfig{ - Addr: "redis.example.com:6379", - ClusterConfig: &mcpv1beta1.RedisClusterConfig{ - Addrs: []string{"node1.example.com:6379"}, - }, - ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ - UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"}, - PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"}, - }, - }, - }, - }, - wantErr: true, - errContains: "mutually exclusive", - }, - { - name: "Redis storage with sentinelConfig and clusterConfig returns error", - authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ - Issuer: "https://auth.example.com", - Storage: &mcpv1beta1.AuthServerStorageConfig{ - Type: mcpv1beta1.AuthServerStorageTypeRedis, - Redis: &mcpv1beta1.RedisStorageConfig{ - SentinelConfig: &mcpv1beta1.RedisSentinelConfig{ - MasterName: "mymaster", - SentinelAddrs: []string{"10.0.0.1:26379"}, - }, - ClusterConfig: &mcpv1beta1.RedisClusterConfig{ - Addrs: []string{"node1.example.com:6379"}, - }, - ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ - UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"}, - PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"}, - }, - }, - }, - }, - wantErr: true, - errContains: "mutually exclusive", - }, - { - name: "Redis cluster config builds correctly", - authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{ - Issuer: "https://auth.example.com", - Storage: &mcpv1beta1.AuthServerStorageConfig{ - Type: mcpv1beta1.AuthServerStorageTypeRedis, - Redis: &mcpv1beta1.RedisStorageConfig{ - ClusterConfig: &mcpv1beta1.RedisClusterConfig{ - Addrs: []string{"node1.example.com:6379", "node2.example.com:6379"}, - }, + Addr: "discovery.example.com:6379", + ClusterMode: true, ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{ UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "username"}, PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "password"}, @@ -1778,10 +1732,8 @@ func TestBuildStorageRunConfig(t *testing.T) { t.Helper() assert.Equal(t, string(storage.TypeRedis), cfg.Type) require.NotNil(t, cfg.RedisConfig) - require.NotNil(t, cfg.RedisConfig.ClusterConfig) - assert.Equal(t, []string{"node1.example.com:6379", "node2.example.com:6379"}, - cfg.RedisConfig.ClusterConfig.Addrs) - assert.Empty(t, cfg.RedisConfig.Addr) + assert.Equal(t, "discovery.example.com:6379", cfg.RedisConfig.Addr) + assert.True(t, cfg.RedisConfig.ClusterMode) assert.Nil(t, cfg.RedisConfig.SentinelConfig) assert.Equal(t, storage.AuthTypeACLUser, cfg.RedisConfig.AuthType) require.NotNil(t, cfg.RedisConfig.ACLUserConfig) diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 7fe151dfc5..3c79c0376f 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -316,28 +316,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -355,7 +344,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -460,10 +449,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- @@ -1383,28 +1372,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -1422,7 +1400,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -1527,10 +1505,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index e44a444548..3bf7d667b9 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -189,28 +189,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -228,7 +217,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -333,10 +322,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- @@ -2704,28 +2693,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -2743,7 +2721,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -2848,10 +2826,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 5ba354a41b..d797872318 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -319,28 +319,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -358,7 +347,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -463,10 +452,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- @@ -1386,28 +1375,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -1425,7 +1403,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -1530,10 +1508,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 0391680dcd..21321d0c9e 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -192,28 +192,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -231,7 +220,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -336,10 +325,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- @@ -2707,28 +2696,17 @@ spec: type: object addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). - Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present - a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig. + Addr is the Redis server address (host:port). Required for standalone and cluster modes. + Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier, + AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true). + Mutually exclusive with sentinelConfig. type: string - clusterConfig: + clusterMode: description: |- - ClusterConfig holds Redis Cluster configuration. - Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster, - AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig. - properties: - addrs: - description: |- - Addrs is the list of seed node host:port addresses for the Redis Cluster. - At least one address is required; go-redis discovers other nodes automatically. - items: - type: string - minItems: 1 - type: array - x-kubernetes-list-type: atomic - required: - - addrs - type: object + ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a + Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache + cluster mode enabled). Requires addr to be set. + type: boolean dialTimeout: default: 5s description: |- @@ -2746,7 +2724,7 @@ spec: sentinelConfig: description: |- SentinelConfig holds Redis Sentinel configuration. - Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig. + Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. properties: db: default: 0 @@ -2851,10 +2829,10 @@ spec: - aclUserConfig type: object x-kubernetes-validations: - - message: exactly one of addr (standalone), sentinelConfig - (Sentinel), or clusterConfig (Cluster) must be set - rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig) - ? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1' + - message: exactly one of addr or sentinelConfig must be set + rule: (self.addr.size() > 0) != has(self.sentinelConfig) + - message: clusterMode requires addr to be set + rule: '!self.clusterMode || self.addr.size() > 0' type: default: memory description: |- diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index cac3cf30f9..88167c976c 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -481,40 +481,28 @@ func convertRedisRunConfig(rc *storage.RedisRunConfig) (*storage.RedisConfig, er return nil, fmt.Errorf("redis config is required when storage type is redis") } - modes := 0 - if rc.Addr != "" { - modes++ + if rc.Addr != "" && rc.SentinelConfig != nil { + return nil, fmt.Errorf("addr and sentinel_config are mutually exclusive; exactly one must be set") } - if rc.SentinelConfig != nil { - modes++ - } - if rc.ClusterConfig != nil { - modes++ - } - if modes > 1 { - return nil, fmt.Errorf("addr, sentinel_config, and cluster_config are mutually exclusive; exactly one must be set") + if rc.Addr == "" && rc.SentinelConfig == nil { + return nil, fmt.Errorf("one of addr (standalone or cluster) or sentinel_config (sentinel) is required") } - if modes == 0 { - return nil, fmt.Errorf("one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") + if rc.ClusterMode && rc.SentinelConfig != nil { + return nil, fmt.Errorf("cluster mode cannot be used with sentinel configuration") } cfg := &storage.RedisConfig{ - KeyPrefix: rc.KeyPrefix, + Addr: rc.Addr, + ClusterMode: rc.ClusterMode, + KeyPrefix: rc.KeyPrefix, } - switch { - case rc.Addr != "": - cfg.Addr = rc.Addr - case rc.SentinelConfig != nil: + if rc.SentinelConfig != nil { cfg.SentinelConfig = &storage.SentinelConfig{ MasterName: rc.SentinelConfig.MasterName, SentinelAddrs: rc.SentinelConfig.SentinelAddrs, DB: rc.SentinelConfig.DB, } - case rc.ClusterConfig != nil: - cfg.ClusterConfig = &storage.ClusterConfig{ - Addrs: rc.ClusterConfig.Addrs, - } } aclCfg, err := convertRedisACLConfig(rc.ACLUserConfig) diff --git a/pkg/authserver/runner/embeddedauthserver_test.go b/pkg/authserver/runner/embeddedauthserver_test.go index f8fbf19c18..7e83fb558a 100644 --- a/pkg/authserver/runner/embeddedauthserver_test.go +++ b/pkg/authserver/runner/embeddedauthserver_test.go @@ -1098,7 +1098,7 @@ func TestCreateStorage(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") + assert.Contains(t, err.Error(), "one of addr (standalone or cluster) or sentinel_config (sentinel) is required") }) } @@ -1122,7 +1122,7 @@ func TestConvertRedisRunConfig(t *testing.T) { }, }) require.Error(t, err) - assert.Contains(t, err.Error(), "one of addr (standalone), sentinel_config (sentinel), or cluster_config (cluster) is required") + assert.Contains(t, err.Error(), "one of addr (standalone or cluster) or sentinel_config (sentinel) is required") }) t.Run("missing ACL user config returns error", func(t *testing.T) { @@ -1186,39 +1186,21 @@ func TestConvertRedisRunConfig(t *testing.T) { assert.Contains(t, err.Error(), "one of addr") }) - t.Run("cluster config with addr also set returns error", func(t *testing.T) { - t.Parallel() - _, err := convertRedisRunConfig(&storage.RedisRunConfig{ - Addr: "redis.example.com:6379", - ClusterConfig: &storage.ClusterRunConfig{ - Addrs: []string{"node1:6379", "node2:6379"}, - }, - ACLUserConfig: &storage.ACLUserRunConfig{ - PasswordEnvVar: "PASS", - }, - KeyPrefix: "thv:", - }) - require.Error(t, err) - assert.Contains(t, err.Error(), "mutually exclusive") - }) - - t.Run("cluster config with sentinel also set returns error", func(t *testing.T) { + t.Run("cluster mode with sentinel also set returns error", func(t *testing.T) { t.Parallel() _, err := convertRedisRunConfig(&storage.RedisRunConfig{ + ClusterMode: true, SentinelConfig: &storage.SentinelRunConfig{ MasterName: "mymaster", SentinelAddrs: []string{"sentinel:26379"}, }, - ClusterConfig: &storage.ClusterRunConfig{ - Addrs: []string{"node1:6379"}, - }, ACLUserConfig: &storage.ACLUserRunConfig{ PasswordEnvVar: "PASS", }, KeyPrefix: "thv:", }) require.Error(t, err) - assert.Contains(t, err.Error(), "mutually exclusive") + assert.Contains(t, err.Error(), "cluster mode cannot be used with sentinel") }) } @@ -1335,14 +1317,13 @@ func TestConvertRedisRunConfig_WithEnvVars(t *testing.T) { assert.Equal(t, "mypass", cfg.ACLUserConfig.Password) }) - t.Run("cluster config resolves correctly", func(t *testing.T) { + t.Run("cluster mode resolves correctly", func(t *testing.T) { t.Setenv("TEST_REDIS_USER_CLUSTER", "clusteruser") t.Setenv("TEST_REDIS_PASS_CLUSTER", "clusterpass") cfg, err := convertRedisRunConfig(&storage.RedisRunConfig{ - ClusterConfig: &storage.ClusterRunConfig{ - Addrs: []string{"node1.example.com:6379", "node2.example.com:6379"}, - }, + Addr: "discovery.example.com:6379", + ClusterMode: true, ACLUserConfig: &storage.ACLUserRunConfig{ UsernameEnvVar: "TEST_REDIS_USER_CLUSTER", PasswordEnvVar: "TEST_REDIS_PASS_CLUSTER", @@ -1351,9 +1332,8 @@ func TestConvertRedisRunConfig_WithEnvVars(t *testing.T) { }) require.NoError(t, err) require.NotNil(t, cfg) - require.NotNil(t, cfg.ClusterConfig) - assert.Equal(t, []string{"node1.example.com:6379", "node2.example.com:6379"}, cfg.ClusterConfig.Addrs) - assert.Empty(t, cfg.Addr) + assert.Equal(t, "discovery.example.com:6379", cfg.Addr) + assert.True(t, cfg.ClusterMode) assert.Nil(t, cfg.SentinelConfig) require.NotNil(t, cfg.ACLUserConfig) assert.Equal(t, "clusteruser", cfg.ACLUserConfig.Username) diff --git a/pkg/authserver/storage/config.go b/pkg/authserver/storage/config.go index c1f8c67d12..281be69866 100644 --- a/pkg/authserver/storage/config.go +++ b/pkg/authserver/storage/config.go @@ -66,27 +66,21 @@ type RunConfig struct { RedisConfig *RedisRunConfig `json:"redis_config,omitempty" yaml:"redis_config,omitempty"` } -// ClusterRunConfig contains Redis Cluster configuration for cross-process serialization. -type ClusterRunConfig struct { - // Addrs is the list of seed node addresses (host:port) for the Redis Cluster. - Addrs []string `json:"addrs" yaml:"addrs"` -} - // RedisRunConfig is the serializable Redis configuration for RunConfig. -// Exactly one of Addr (standalone), SentinelConfig (Sentinel), or ClusterConfig (Cluster) must be set. +// Exactly one of Addr (standalone/cluster) or SentinelConfig must be set. +// Set ClusterMode to true when Addr points to a Redis Cluster discovery endpoint. type RedisRunConfig struct { - // Addr is the Redis server address for standalone mode (e.g., "host:port"). - // Mutually exclusive with SentinelConfig and ClusterConfig. + // Addr is the Redis server address (host:port). Required for standalone and cluster modes. + // Mutually exclusive with SentinelConfig. Addr string `json:"addr,omitempty" yaml:"addr,omitempty"` + // ClusterMode enables the Redis Cluster protocol. Requires Addr to be set. + ClusterMode bool `json:"cluster_mode,omitempty" yaml:"cluster_mode,omitempty"` + // SentinelConfig contains Sentinel-specific configuration. - // Mutually exclusive with Addr and ClusterConfig. + // Mutually exclusive with Addr. SentinelConfig *SentinelRunConfig `json:"sentinel_config,omitempty" yaml:"sentinel_config,omitempty"` - // ClusterConfig contains Redis Cluster-specific configuration. - // Mutually exclusive with Addr and SentinelConfig. - ClusterConfig *ClusterRunConfig `json:"cluster_config,omitempty" yaml:"cluster_config,omitempty"` - // AuthType must be "aclUser" - only ACL user authentication is supported. AuthType string `json:"auth_type" yaml:"auth_type"` diff --git a/pkg/authserver/storage/redis.go b/pkg/authserver/storage/redis.go index 9e508cdd22..6b3fe4f19a 100644 --- a/pkg/authserver/storage/redis.go +++ b/pkg/authserver/storage/redis.go @@ -50,23 +50,18 @@ func warnOnCleanupErr(err error, operation, key string) { } } -// ClusterConfig contains Redis Cluster connection configuration. -type ClusterConfig struct { - // Addrs is the list of seed node addresses (host:port) for the Redis Cluster. - Addrs []string -} - // RedisConfig holds Redis connection configuration for runtime use. type RedisConfig struct { - // Addr is the Redis server address for standalone mode (e.g., "host:port"). - // Mutually exclusive with SentinelConfig and ClusterConfig. + // Addr is the Redis server address (host:port). Required for standalone and cluster modes. + // Mutually exclusive with SentinelConfig. Addr string - // SentinelConfig is required for Sentinel mode. Mutually exclusive with Addr and ClusterConfig. - SentinelConfig *SentinelConfig + // ClusterMode enables Redis Cluster protocol. Requires Addr to be set. + // Use for managed cluster-mode services (GCP Memorystore Cluster, AWS ElastiCache cluster mode). + ClusterMode bool - // ClusterConfig is required for Cluster mode. Mutually exclusive with Addr and SentinelConfig. - ClusterConfig *ClusterConfig + // SentinelConfig is required for Sentinel mode. Mutually exclusive with Addr. + SentinelConfig *SentinelConfig // ACLUserConfig is required - ACL user authentication only. ACLUserConfig *ACLUserConfig @@ -215,7 +210,7 @@ func configureTLSDialer(opts *redis.FailoverOptions, masterCfg, sentinelCfg *Red } // NewRedisStorage creates Redis-backed storage. -// Supports standalone mode (Addr set) and Sentinel failover mode (SentinelConfig set). +// Supports standalone mode (Addr), cluster mode (Addr + ClusterMode), and Sentinel mode (SentinelConfig). // Returns error if configuration validation fails or connection cannot be established. func NewRedisStorage(ctx context.Context, cfg RedisConfig) (*RedisStorage, error) { if err := validateConfig(&cfg); err != nil { @@ -255,14 +250,14 @@ func NewRedisStorage(ctx context.Context, cfg RedisConfig) (*RedisStorage, error client = redis.NewFailoverClient(opts) - case cfg.ClusterConfig != nil: + case cfg.ClusterMode: tlsCfg, err := buildTLSConfig(cfg.TLS) if err != nil { return nil, fmt.Errorf("TLS config: %w", err) } opts := &redis.ClusterOptions{ - Addrs: cfg.ClusterConfig.Addrs, + Addrs: []string{cfg.Addr}, Username: cfg.ACLUserConfig.Username, Password: cfg.ACLUserConfig.Password, DialTimeout: cfg.DialTimeout, @@ -322,21 +317,17 @@ func defaultSessionFactory(subject, idpSessionID, clientID string) fosite.Sessio } func validateConfig(cfg *RedisConfig) error { - modes := 0 - if cfg.Addr != "" { - modes++ + if cfg.ClusterMode && cfg.SentinelConfig != nil { + return errors.New("cluster mode cannot be used with sentinel configuration") } - if cfg.SentinelConfig != nil { - modes++ - } - if cfg.ClusterConfig != nil { - modes++ + if cfg.Addr != "" && cfg.SentinelConfig != nil { + return errors.New("addr and sentinel configuration are mutually exclusive; exactly one must be set") } - if modes > 1 { - return errors.New("addr, sentinel, and cluster configuration are mutually exclusive; exactly one must be set") + if cfg.ClusterMode && cfg.Addr == "" { + return errors.New("cluster mode requires addr to be set") } - if modes == 0 { - return errors.New("one of addr (standalone), sentinel configuration, or cluster configuration is required") + if cfg.Addr == "" && cfg.SentinelConfig == nil { + return errors.New("one of addr (standalone or cluster) or sentinel configuration is required") } if cfg.SentinelConfig != nil { if cfg.SentinelConfig.MasterName == "" { @@ -346,11 +337,6 @@ func validateConfig(cfg *RedisConfig) error { return errors.New("at least one sentinel address is required") } } - if cfg.ClusterConfig != nil { - if len(cfg.ClusterConfig.Addrs) == 0 { - return errors.New("at least one cluster address is required") - } - } if cfg.ACLUserConfig == nil { return errors.New("ACL user configuration is required") } diff --git a/pkg/authserver/storage/redis_test.go b/pkg/authserver/storage/redis_test.go index a43dda251b..bb7a75a6bf 100644 --- a/pkg/authserver/storage/redis_test.go +++ b/pkg/authserver/storage/redis_test.go @@ -103,7 +103,7 @@ func TestRedisConfig_Validation(t *testing.T) { { name: "neither addr nor sentinel config", cfg: RedisConfig{ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:"}, - wantErr: "one of addr (standalone), sentinel configuration, or cluster configuration is required", + wantErr: "one of addr (standalone or cluster) or sentinel configuration is required", }, { name: "addr and sentinel config both set", @@ -116,29 +116,23 @@ func TestRedisConfig_Validation(t *testing.T) { wantErr: "mutually exclusive", }, { - name: "cluster config and addr both set", - cfg: RedisConfig{ - Addr: "localhost:6379", - ClusterConfig: &ClusterConfig{Addrs: []string{"localhost:6379"}}, - ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, - KeyPrefix: "test:", - }, - wantErr: "mutually exclusive", - }, - { - name: "cluster config and sentinel both set", + name: "cluster mode with sentinel config", cfg: RedisConfig{ + ClusterMode: true, SentinelConfig: &SentinelConfig{MasterName: "m", SentinelAddrs: []string{"localhost:26379"}}, - ClusterConfig: &ClusterConfig{Addrs: []string{"localhost:6379"}}, ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:", }, - wantErr: "mutually exclusive", + wantErr: "cluster mode cannot be used with sentinel", }, { - name: "cluster config with no addrs", - cfg: RedisConfig{ClusterConfig: &ClusterConfig{}, ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, KeyPrefix: "test:"}, - wantErr: "at least one cluster address is required", + name: "cluster mode without addr", + cfg: RedisConfig{ + ClusterMode: true, + ACLUserConfig: &ACLUserConfig{Username: "u", Password: "p"}, + KeyPrefix: "test:", + }, + wantErr: "cluster mode requires addr", }, { name: "missing sentinel master name", @@ -251,9 +245,8 @@ func TestNewRedisStorage_Cluster_ConnectionFailure(t *testing.T) { t.Parallel() cfg := RedisConfig{ - ClusterConfig: &ClusterConfig{ - Addrs: []string{"localhost:19998"}, - }, + Addr: "localhost:19998", + ClusterMode: true, ACLUserConfig: &ACLUserConfig{ Username: "user", Password: "pass", From 428a24b83f5c0be9740bf0cef0b919d10e91b5e7 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 1 May 2026 12:18:00 -0400 Subject: [PATCH 3/4] Regenerate Swagger docs for cluster_mode field Co-Authored-By: Claude Sonnet 4.6 --- docs/server/docs.go | 6 +++++- docs/server/swagger.json | 6 +++++- docs/server/swagger.yaml | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/server/docs.go b/docs/server/docs.go index a108f6087f..1cd3a245da 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -769,13 +769,17 @@ const docTemplate = `{ "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver_storage.ACLUserRunConfig" }, "addr": { - "description": "Addr is the Redis server address for standalone mode (e.g., \"host:port\").\nMutually exclusive with SentinelConfig.", + "description": "Addr is the Redis server address (host:port). Required for standalone and cluster modes.\nMutually exclusive with SentinelConfig.", "type": "string" }, "auth_type": { "description": "AuthType must be \"aclUser\" - only ACL user authentication is supported.", "type": "string" }, + "cluster_mode": { + "description": "ClusterMode enables the Redis Cluster protocol. Requires Addr to be set.", + "type": "boolean" + }, "dial_timeout": { "description": "DialTimeout is the timeout for establishing connections (e.g., \"5s\").", "type": "string" diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 8a5d8df9b5..2fa20abe2d 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -762,13 +762,17 @@ "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_authserver_storage.ACLUserRunConfig" }, "addr": { - "description": "Addr is the Redis server address for standalone mode (e.g., \"host:port\").\nMutually exclusive with SentinelConfig.", + "description": "Addr is the Redis server address (host:port). Required for standalone and cluster modes.\nMutually exclusive with SentinelConfig.", "type": "string" }, "auth_type": { "description": "AuthType must be \"aclUser\" - only ACL user authentication is supported.", "type": "string" }, + "cluster_mode": { + "description": "ClusterMode enables the Redis Cluster protocol. Requires Addr to be set.", + "type": "boolean" + }, "dial_timeout": { "description": "DialTimeout is the timeout for establishing connections (e.g., \"5s\").", "type": "string" diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index bdc1cac8a0..46fa4b74a4 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -820,13 +820,17 @@ components: $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_authserver_storage.ACLUserRunConfig' addr: description: |- - Addr is the Redis server address for standalone mode (e.g., "host:port"). + Addr is the Redis server address (host:port). Required for standalone and cluster modes. Mutually exclusive with SentinelConfig. type: string auth_type: description: AuthType must be "aclUser" - only ACL user authentication is supported. type: string + cluster_mode: + description: ClusterMode enables the Redis Cluster protocol. Requires Addr + to be set. + type: boolean dial_timeout: description: DialTimeout is the timeout for establishing connections (e.g., "5s"). From ce18ffdc012a59bcf558e4f227d9a54423b16a98 Mon Sep 17 00:00:00 2001 From: Reynier Ortiz Vega Date: Fri, 1 May 2026 15:27:14 -0400 Subject: [PATCH 4/4] Regenerate CRD API reference docs for clusterMode field Co-Authored-By: Claude Sonnet 4.6 --- docs/operator/crd-api.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index b6e0c50cda..95461bc0d1 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -2761,7 +2761,9 @@ _Appears in:_ RedisStorageConfig configures Redis connection for auth server storage. -Exactly one of addr (standalone) or sentinelConfig (Sentinel) must be set. +Exactly one of addr or sentinelConfig must be set. Set clusterMode to true when +addr points to a Redis Cluster discovery endpoint (GCP Memorystore Cluster, +AWS ElastiCache cluster mode enabled). @@ -2770,7 +2772,8 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | -| `addr` _string_ | Addr is the Redis server address for standalone mode (e.g., "host:port").
Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present
a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig. | | Optional: \{\}
| +| `addr` _string_ | Addr is the Redis server address (host:port). Required for standalone and cluster modes.
Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier,
AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true).
Mutually exclusive with sentinelConfig. | | Optional: \{\}
| +| `clusterMode` _boolean_ | ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a
Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache
cluster mode enabled). Requires addr to be set. | | Optional: \{\}
| | `sentinelConfig` _[api.v1beta1.RedisSentinelConfig](#apiv1beta1redissentinelconfig)_ | SentinelConfig holds Redis Sentinel configuration.
Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr. | | Optional: \{\}
| | `aclUserConfig` _[api.v1beta1.RedisACLUserConfig](#apiv1beta1redisacluserconfig)_ | ACLUserConfig configures Redis ACL user authentication. | | Required: \{\}
| | `dialTimeout` _string_ | DialTimeout is the timeout for establishing connections.
Format: Go duration string (e.g., "5s", "1m"). | 5s | Pattern: `^([0-9]+(\.[0-9]+)?(ns\|us\|µs\|ms\|s\|m\|h))+$`
Optional: \{\}
|