Skip to content

Commit 30315bc

Browse files
reyortiz3claude
reyortiz3
and
claude
committed
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 <noreply@anthropic.com>
1 parent d105775 commit 30315bc

13 files changed

Lines changed: 200 additions & 451 deletions

‎cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -525,28 +525,32 @@ type AuthServerStorageConfig struct {
525525
}
526526

527527
// RedisStorageConfig configures Redis connection for auth server storage.
528-
// Exactly one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) must be set.
528+
// Exactly one of addr or sentinelConfig must be set. Set clusterMode to true when
529+
// addr points to a Redis Cluster discovery endpoint (GCP Memorystore Cluster,
530+
// AWS ElastiCache cluster mode enabled).
529531
//
530-
// +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"
532+
// +kubebuilder:validation:XValidation:rule="(self.addr.size() > 0) != has(self.sentinelConfig)",message="exactly one of addr or sentinelConfig must be set"
533+
// +kubebuilder:validation:XValidation:rule="!self.clusterMode || self.addr.size() > 0",message="clusterMode requires addr to be set"
531534
//
532535
//nolint:lll // CEL validation rules exceed line length limit
533536
type RedisStorageConfig struct {
534-
// Addr is the Redis server address for standalone mode (e.g., "host:port").
535-
// Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present
536-
// a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig.
537+
// Addr is the Redis server address (host:port). Required for standalone and cluster modes.
538+
// Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier,
539+
// AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true).
540+
// Mutually exclusive with sentinelConfig.
537541
// +optional
538542
Addr string `json:"addr,omitempty"`
539543

540-
// SentinelConfig holds Redis Sentinel configuration.
541-
// Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig.
544+
// ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a
545+
// Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache
546+
// cluster mode enabled). Requires addr to be set.
542547
// +optional
543-
SentinelConfig *RedisSentinelConfig `json:"sentinelConfig,omitempty"`
548+
ClusterMode bool `json:"clusterMode,omitempty"`
544549

545-
// ClusterConfig holds Redis Cluster configuration.
546-
// Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster,
547-
// AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig.
550+
// SentinelConfig holds Redis Sentinel configuration.
551+
// Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr.
548552
// +optional
549-
ClusterConfig *RedisClusterConfig `json:"clusterConfig,omitempty"`
553+
SentinelConfig *RedisSentinelConfig `json:"sentinelConfig,omitempty"`
550554

551555
// ACLUserConfig configures Redis ACL user authentication.
552556
// +kubebuilder:validation:Required
@@ -607,16 +611,6 @@ type RedisSentinelConfig struct {
607611
DB int32 `json:"db,omitempty"`
608612
}
609613

610-
// RedisClusterConfig configures Redis Cluster connection.
611-
type RedisClusterConfig struct {
612-
// Addrs is the list of seed node host:port addresses for the Redis Cluster.
613-
// At least one address is required; go-redis discovers other nodes automatically.
614-
// +kubebuilder:validation:Required
615-
// +kubebuilder:validation:MinItems=1
616-
// +listType=atomic
617-
Addrs []string `json:"addrs"`
618-
}
619-
620614
// SentinelServiceRef references a Kubernetes Service for Sentinel discovery.
621615
type SentinelServiceRef struct {
622616
// Name of the Sentinel Service.

‎cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 0 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/thv-operator/pkg/controllerutil/authserver.go‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,11 @@ func buildStorageRunConfig(
544544
return nil, fmt.Errorf("redis config is required when storage type is redis")
545545
}
546546

547-
if err := validateRedisConnectionMode(redisConfig); err != nil {
548-
return nil, err
547+
if redisConfig.Addr != "" && redisConfig.SentinelConfig != nil {
548+
return nil, fmt.Errorf("addr and sentinelConfig are mutually exclusive for Redis storage")
549+
}
550+
if redisConfig.Addr == "" && redisConfig.SentinelConfig == nil {
551+
return nil, fmt.Errorf("one of addr (standalone or cluster) or sentinelConfig (Sentinel) is required for Redis storage")
549552
}
550553

551554
if redisConfig.ACLUserConfig == nil ||
@@ -565,6 +568,7 @@ func buildStorageRunConfig(
565568

566569
rc := &storage.RedisRunConfig{
567570
Addr: redisConfig.Addr,
571+
ClusterMode: redisConfig.ClusterMode,
568572
AuthType: storage.AuthTypeACLUser,
569573
ACLUserConfig: aclRunConfig,
570574
KeyPrefix: keyPrefix,
@@ -588,41 +592,12 @@ func buildStorageRunConfig(
588592
rc.SentinelTLS = convertRedisTLSConfig(redisConfig.SentinelTLS, true)
589593
}
590594

591-
if redisConfig.ClusterConfig != nil {
592-
rc.ClusterConfig = &storage.ClusterRunConfig{
593-
Addrs: redisConfig.ClusterConfig.Addrs,
594-
}
595-
}
596-
597595
return &storage.RunConfig{
598596
Type: string(storage.TypeRedis),
599597
RedisConfig: rc,
600598
}, nil
601599
}
602600

603-
// validateRedisConnectionMode checks that exactly one of addr, sentinelConfig,
604-
// or clusterConfig is set on the Redis storage config.
605-
func validateRedisConnectionMode(cfg *mcpv1beta1.RedisStorageConfig) error {
606-
modes := 0
607-
if cfg.Addr != "" {
608-
modes++
609-
}
610-
if cfg.SentinelConfig != nil {
611-
modes++
612-
}
613-
if cfg.ClusterConfig != nil {
614-
modes++
615-
}
616-
if modes > 1 {
617-
return fmt.Errorf("addr, sentinelConfig, and clusterConfig are mutually exclusive for Redis storage")
618-
}
619-
if modes == 0 {
620-
return fmt.Errorf("one of addr (standalone), sentinelConfig (Sentinel)," +
621-
" or clusterConfig (Cluster) is required for Redis storage")
622-
}
623-
return nil
624-
}
625-
626601
// convertRedisTLSConfig converts CRD RedisTLSConfig to RunConfig.
627602
// isSentinel determines which mount path to use for the CA cert file.
628603
func convertRedisTLSConfig(cfg *mcpv1beta1.RedisTLSConfig, isSentinel bool) *storage.RedisTLSRunConfig {

‎cmd/thv-operator/pkg/controllerutil/authserver_test.go‎

Lines changed: 7 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,7 +1674,7 @@ func TestBuildStorageRunConfig(t *testing.T) {
16741674
errContains: "redis config is required",
16751675
},
16761676
{
1677-
name: "Redis storage missing addr, sentinelConfig, and clusterConfig returns error",
1677+
name: "Redis storage missing addr and sentinelConfig returns error",
16781678
authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{
16791679
Issuer: "https://auth.example.com",
16801680
Storage: &mcpv1beta1.AuthServerStorageConfig{
@@ -1688,7 +1688,7 @@ func TestBuildStorageRunConfig(t *testing.T) {
16881688
},
16891689
},
16901690
wantErr: true,
1691-
errContains: "one of addr (standalone), sentinelConfig (Sentinel), or clusterConfig (Cluster) is required",
1691+
errContains: "one of addr (standalone or cluster) or sentinelConfig (Sentinel) is required",
16921692
},
16931693
{
16941694
name: "Redis storage with both addr and sentinelConfig returns error",
@@ -1713,60 +1713,14 @@ func TestBuildStorageRunConfig(t *testing.T) {
17131713
errContains: "mutually exclusive",
17141714
},
17151715
{
1716-
name: "Redis storage with addr and clusterConfig returns error",
1716+
name: "Redis cluster mode builds correctly",
17171717
authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{
17181718
Issuer: "https://auth.example.com",
17191719
Storage: &mcpv1beta1.AuthServerStorageConfig{
17201720
Type: mcpv1beta1.AuthServerStorageTypeRedis,
17211721
Redis: &mcpv1beta1.RedisStorageConfig{
1722-
Addr: "redis.example.com:6379",
1723-
ClusterConfig: &mcpv1beta1.RedisClusterConfig{
1724-
Addrs: []string{"node1.example.com:6379"},
1725-
},
1726-
ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{
1727-
UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"},
1728-
PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"},
1729-
},
1730-
},
1731-
},
1732-
},
1733-
wantErr: true,
1734-
errContains: "mutually exclusive",
1735-
},
1736-
{
1737-
name: "Redis storage with sentinelConfig and clusterConfig returns error",
1738-
authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{
1739-
Issuer: "https://auth.example.com",
1740-
Storage: &mcpv1beta1.AuthServerStorageConfig{
1741-
Type: mcpv1beta1.AuthServerStorageTypeRedis,
1742-
Redis: &mcpv1beta1.RedisStorageConfig{
1743-
SentinelConfig: &mcpv1beta1.RedisSentinelConfig{
1744-
MasterName: "mymaster",
1745-
SentinelAddrs: []string{"10.0.0.1:26379"},
1746-
},
1747-
ClusterConfig: &mcpv1beta1.RedisClusterConfig{
1748-
Addrs: []string{"node1.example.com:6379"},
1749-
},
1750-
ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{
1751-
UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "u"},
1752-
PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "s", Key: "p"},
1753-
},
1754-
},
1755-
},
1756-
},
1757-
wantErr: true,
1758-
errContains: "mutually exclusive",
1759-
},
1760-
{
1761-
name: "Redis cluster config builds correctly",
1762-
authConfig: &mcpv1beta1.EmbeddedAuthServerConfig{
1763-
Issuer: "https://auth.example.com",
1764-
Storage: &mcpv1beta1.AuthServerStorageConfig{
1765-
Type: mcpv1beta1.AuthServerStorageTypeRedis,
1766-
Redis: &mcpv1beta1.RedisStorageConfig{
1767-
ClusterConfig: &mcpv1beta1.RedisClusterConfig{
1768-
Addrs: []string{"node1.example.com:6379", "node2.example.com:6379"},
1769-
},
1722+
Addr: "discovery.example.com:6379",
1723+
ClusterMode: true,
17701724
ACLUserConfig: &mcpv1beta1.RedisACLUserConfig{
17711725
UsernameSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "username"},
17721726
PasswordSecretRef: &mcpv1beta1.SecretKeyRef{Name: "redis-secret", Key: "password"},
@@ -1778,10 +1732,8 @@ func TestBuildStorageRunConfig(t *testing.T) {
17781732
t.Helper()
17791733
assert.Equal(t, string(storage.TypeRedis), cfg.Type)
17801734
require.NotNil(t, cfg.RedisConfig)
1781-
require.NotNil(t, cfg.RedisConfig.ClusterConfig)
1782-
assert.Equal(t, []string{"node1.example.com:6379", "node2.example.com:6379"},
1783-
cfg.RedisConfig.ClusterConfig.Addrs)
1784-
assert.Empty(t, cfg.RedisConfig.Addr)
1735+
assert.Equal(t, "discovery.example.com:6379", cfg.RedisConfig.Addr)
1736+
assert.True(t, cfg.RedisConfig.ClusterMode)
17851737
assert.Nil(t, cfg.RedisConfig.SentinelConfig)
17861738
assert.Equal(t, storage.AuthTypeACLUser, cfg.RedisConfig.AuthType)
17871739
require.NotNil(t, cfg.RedisConfig.ACLUserConfig)

‎deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml‎

Lines changed: 28 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -316,28 +316,17 @@ spec:
316316
type: object
317317
addr:
318318
description: |-
319-
Addr is the Redis server address for standalone mode (e.g., "host:port").
320-
Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present
321-
a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig.
319+
Addr is the Redis server address (host:port). Required for standalone and cluster modes.
320+
Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier,
321+
AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true).
322+
Mutually exclusive with sentinelConfig.
322323
type: string
323-
clusterConfig:
324+
clusterMode:
324325
description: |-
325-
ClusterConfig holds Redis Cluster configuration.
326-
Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster,
327-
AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig.
328-
properties:
329-
addrs:
330-
description: |-
331-
Addrs is the list of seed node host:port addresses for the Redis Cluster.
332-
At least one address is required; go-redis discovers other nodes automatically.
333-
items:
334-
type: string
335-
minItems: 1
336-
type: array
337-
x-kubernetes-list-type: atomic
338-
required:
339-
- addrs
340-
type: object
326+
ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a
327+
Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache
328+
cluster mode enabled). Requires addr to be set.
329+
type: boolean
341330
dialTimeout:
342331
default: 5s
343332
description: |-
@@ -355,7 +344,7 @@ spec:
355344
sentinelConfig:
356345
description: |-
357346
SentinelConfig holds Redis Sentinel configuration.
358-
Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig.
347+
Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr.
359348
properties:
360349
db:
361350
default: 0
@@ -460,10 +449,10 @@ spec:
460449
- aclUserConfig
461450
type: object
462451
x-kubernetes-validations:
463-
- message: exactly one of addr (standalone), sentinelConfig
464-
(Sentinel), or clusterConfig (Cluster) must be set
465-
rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig)
466-
? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1'
452+
- message: exactly one of addr or sentinelConfig must be set
453+
rule: (self.addr.size() > 0) != has(self.sentinelConfig)
454+
- message: clusterMode requires addr to be set
455+
rule: '!self.clusterMode || self.addr.size() > 0'
467456
type:
468457
default: memory
469458
description: |-
@@ -1383,28 +1372,17 @@ spec:
13831372
type: object
13841373
addr:
13851374
description: |-
1386-
Addr is the Redis server address for standalone mode (e.g., "host:port").
1387-
Use for managed Redis services (GCP Memorystore, AWS ElastiCache) that present
1388-
a single endpoint and manage HA internally. Mutually exclusive with sentinelConfig and clusterConfig.
1375+
Addr is the Redis server address (host:port). Required for standalone and cluster modes.
1376+
Use for managed Redis services that expose a single endpoint (GCP Memorystore basic tier,
1377+
AWS ElastiCache without cluster mode, or cluster-mode services when clusterMode is true).
1378+
Mutually exclusive with sentinelConfig.
13891379
type: string
1390-
clusterConfig:
1380+
clusterMode:
13911381
description: |-
1392-
ClusterConfig holds Redis Cluster configuration.
1393-
Use for managed Redis services that use the Redis Cluster protocol (e.g., GCP Memorystore Cluster,
1394-
AWS ElastiCache Serverless). Mutually exclusive with addr and sentinelConfig.
1395-
properties:
1396-
addrs:
1397-
description: |-
1398-
Addrs is the list of seed node host:port addresses for the Redis Cluster.
1399-
At least one address is required; go-redis discovers other nodes automatically.
1400-
items:
1401-
type: string
1402-
minItems: 1
1403-
type: array
1404-
x-kubernetes-list-type: atomic
1405-
required:
1406-
- addrs
1407-
type: object
1382+
ClusterMode enables the Redis Cluster protocol. Set to true when addr points to a
1383+
Redis Cluster discovery endpoint (e.g., GCP Memorystore Cluster, AWS ElastiCache
1384+
cluster mode enabled). Requires addr to be set.
1385+
type: boolean
14081386
dialTimeout:
14091387
default: 5s
14101388
description: |-
@@ -1422,7 +1400,7 @@ spec:
14221400
sentinelConfig:
14231401
description: |-
14241402
SentinelConfig holds Redis Sentinel configuration.
1425-
Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr and clusterConfig.
1403+
Use for self-managed Redis with Sentinel-based HA. Mutually exclusive with addr.
14261404
properties:
14271405
db:
14281406
default: 0
@@ -1527,10 +1505,10 @@ spec:
15271505
- aclUserConfig
15281506
type: object
15291507
x-kubernetes-validations:
1530-
- message: exactly one of addr (standalone), sentinelConfig
1531-
(Sentinel), or clusterConfig (Cluster) must be set
1532-
rule: '(self.addr.size() > 0 ? 1 : 0) + (has(self.sentinelConfig)
1533-
? 1 : 0) + (has(self.clusterConfig) ? 1 : 0) == 1'
1508+
- message: exactly one of addr or sentinelConfig must be set
1509+
rule: (self.addr.size() > 0) != has(self.sentinelConfig)
1510+
- message: clusterMode requires addr to be set
1511+
rule: '!self.clusterMode || self.addr.size() > 0'
15341512
type:
15351513
default: memory
15361514
description: |-

0 commit comments

Comments
 (0)