Use shared toolhive-core redis client for session storage

pkg/runner/runner.go

@@ -17,6 +17,7 @@ import (
 
 	"golang.org/x/oauth2"
 
+	tcredis "github.com/stacklok/toolhive-core/redis"
 	"github.com/stacklok/toolhive/pkg/auth"
 	"github.com/stacklok/toolhive/pkg/auth/remote"
 	authsecrets "github.com/stacklok/toolhive/pkg/auth/secrets"
@@ -382,12 +383,11 @@ func (r *Runner) Run(ctx context.Context) error {
 		if keyPrefix == "" {
 			keyPrefix = "thv:proxy:session:"
 		}
-		storage, err := session.NewRedisStorage(ctx, session.RedisConfig{
-			Addr:      redisCfg.Address,
-			Password:  os.Getenv(session.RedisPasswordEnvVar),
-			DB:        int(redisCfg.DB),
-			KeyPrefix: keyPrefix,
-		}, effectiveSessionTTL)
+		storage, err := session.NewRedisStorage(ctx, tcredis.Config{
+			Addr:     redisCfg.Address,
+			Password: os.Getenv(session.RedisPasswordEnvVar),
+			DB:       int(redisCfg.DB),
+		}, keyPrefix, effectiveSessionTTL)
 		if err != nil {
 			return fmt.Errorf("failed to create Redis session storage: %w", err)
 		}

pkg/transport/session/manager.go

@@ -11,6 +11,8 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+
+	tcredis "github.com/stacklok/toolhive-core/redis"
 )
 
 const (
@@ -118,12 +120,20 @@ func NewManagerWithStorage(ttl time.Duration, factory Factory, storage Storage)
 // NewManagerWithRedis creates a session manager backed by Redis.
 // ctx is used for the initial Ping during construction and should carry any
 // deadline appropriate for the connection attempt (e.g. a startup timeout).
-// cfg supplies the Redis connection configuration; ttl is applied as both the
-// manager's cleanup interval and the Redis key TTL.
-// Returns an error if the Redis client cannot be constructed (e.g. invalid config or TLS error).
-// Callers that do not require Redis should use NewManager or NewTypedManager instead.
-func NewManagerWithRedis(ctx context.Context, ttl time.Duration, factory Factory, cfg RedisConfig) (*Manager, error) {
-	storage, err := NewRedisStorage(ctx, cfg, ttl)
+// cfg supplies the Redis connection configuration (delegated to the shared
+// toolhive-core redis package); keyPrefix namespaces the keys and must end
+// with ':'. ttl is applied as both the manager's cleanup interval and the
+// Redis key TTL. Returns an error if the Redis client cannot be constructed
+// (e.g. invalid config or TLS error). Callers that do not require Redis
+// should use NewManager or NewTypedManager instead.
+func NewManagerWithRedis(
+	ctx context.Context,
+	ttl time.Duration,
+	factory Factory,
+	cfg tcredis.Config,
+	keyPrefix string,
+) (*Manager, error) {
+	storage, err := NewRedisStorage(ctx, cfg, keyPrefix, ttl)
 	if err != nil {
 		return nil, fmt.Errorf("creating redis storage: %w", err)
 	}

pkg/transport/session/manager_redis_test.go

@@ -11,6 +11,8 @@ import (
 	"github.com/alicebob/miniredis/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	tcredis "github.com/stacklok/toolhive-core/redis"
 )
 
 func proxyFactory(id string) Session { return NewProxySession(id) }
@@ -23,10 +25,13 @@ func TestNewManagerWithRedis(t *testing.T) {
 		mr := miniredis.RunT(t)
 		defer mr.Close()
 
-		m, err := NewManagerWithRedis(context.Background(), time.Hour, proxyFactory, RedisConfig{
-			Addr:      mr.Addr(),
-			KeyPrefix: "test:mgr:",
-		})
+		m, err := NewManagerWithRedis(
+			context.Background(),
+			time.Hour,
+			proxyFactory,
+			tcredis.Config{Addr: mr.Addr()},
+			"test:mgr:",
+		)
 		require.NoError(t, err)
 		require.NotNil(t, m)
 		defer m.Stop()
@@ -35,23 +40,16 @@ func TestNewManagerWithRedis(t *testing.T) {
 		assert.True(t, isRedis, "storage should be *RedisStorage")
 	})
 
-	t.Run("invalid config returns error", func(t *testing.T) {
-		t.Parallel()
-		// Missing KeyPrefix → validateRedisConfig fails before Ping
-		m, err := NewManagerWithRedis(context.Background(), time.Hour, proxyFactory, RedisConfig{
-			Addr: "localhost:6379",
-		})
-		require.Error(t, err)
-		assert.Nil(t, m)
-	})
-
-	t.Run("invalid TLS CA cert returns error", func(t *testing.T) {
+	t.Run("missing key prefix returns error", func(t *testing.T) {
 		t.Parallel()
-		m, err := NewManagerWithRedis(context.Background(), time.Hour, proxyFactory, RedisConfig{
-			Addr:      "localhost:6379",
-			KeyPrefix: "test:mgr:",
-			TLS:       &RedisTLSConfig{CACert: []byte("not-valid-pem")},
-		})
+		// Empty keyPrefix fails session-invariant check before Ping.
+		m, err := NewManagerWithRedis(
+			context.Background(),
+			time.Hour,
+			proxyFactory,
+			tcredis.Config{Addr: "localhost:6379"},
+			"",
+		)
 		require.Error(t, err)
 		assert.Nil(t, m)
 	})
@@ -61,10 +59,13 @@ func TestNewManagerWithRedis(t *testing.T) {
 		mr := miniredis.RunT(t)
 		defer mr.Close()
 
-		m, err := NewManagerWithRedis(context.Background(), time.Hour, proxyFactory, RedisConfig{
-			Addr:      mr.Addr(),
-			KeyPrefix: "test:mgr:",
-		})
+		m, err := NewManagerWithRedis(
+			context.Background(),
+			time.Hour,
+			proxyFactory,
+			tcredis.Config{Addr: mr.Addr()},
+			"test:mgr:",
+		)
 		require.NoError(t, err)
 		defer m.Stop()
 
@@ -81,10 +82,13 @@ func TestNewManagerWithRedis(t *testing.T) {
 		mr := miniredis.RunT(t)
 		defer mr.Close()
 
-		m, err := NewManagerWithRedis(context.Background(), time.Hour, proxyFactory, RedisConfig{
-			Addr:      mr.Addr(),
-			KeyPrefix: "test:mgr:",
-		})
+		m, err := NewManagerWithRedis(
+			context.Background(),
+			time.Hour,
+			proxyFactory,
+			tcredis.Config{Addr: mr.Addr()},
+			"test:mgr:",
+		)
 		require.NoError(t, err)
 
 		require.NoError(t, m.AddWithID("bbbbbbbb-0002-0001-0001-000000000002"))

pkg/transport/session/redis_config.go

@@ -3,71 +3,8 @@
 
 package session
 
-import "time"
-
 // RedisPasswordEnvVar is the environment variable name for the Redis session storage password.
 // The operator injects this as a SecretKeyRef when sessionStorage.provider is "redis"
 // and passwordRef is set.
 // #nosec G101 -- This is an environment variable name, not a hardcoded credential
 const RedisPasswordEnvVar = "THV_SESSION_REDIS_PASSWORD"
-
-// Default timeouts for Redis operations.
-const (
-	DefaultDialTimeout  = 5 * time.Second
-	DefaultReadTimeout  = 3 * time.Second
-	DefaultWriteTimeout = 3 * time.Second
-)
-
-// RedisConfig configures the Redis storage backend for session storage.
-// Addr is used for standalone; SentinelConfig activates Sentinel mode (mutually exclusive).
-type RedisConfig struct {
-	// Addr is the Redis server address for standalone mode (e.g., "host:port").
-	Addr string
-
-	// SentinelConfig, when non-nil, activates Sentinel mode. Mutually exclusive with Addr.
-	SentinelConfig *SentinelConfig
-
-	// Username is the Redis ACL username (Redis 6.0+). When non-empty, both
-	// Username and Password are sent as ACL credentials (AUTH username password).
-	// Leave empty to authenticate as the default user (legacy AUTH password).
-	Username string
-
-	// Password is the Redis AUTH password. Used with Username for ACL auth,
-	// or alone for legacy AUTH with the default user.
-	Password string //nolint:gosec // G101: not a hardcoded credential
-
-	// DB is the Redis database index.
-	DB int
-
-	// KeyPrefix namespaces all session keys (e.g., "thv:vmcp:session:").
-	KeyPrefix string
-
-	// DialTimeout is the timeout for establishing a connection. Defaults to 5s.
-	DialTimeout time.Duration
-
-	// ReadTimeout is the timeout for read operations. Defaults to 3s.
-	ReadTimeout time.Duration
-
-	// WriteTimeout is the timeout for write operations. Defaults to 3s.
-	WriteTimeout time.Duration
-
-	// TLS configures TLS for Redis connections. nil means plaintext.
-	TLS *RedisTLSConfig
-}
-
-// SentinelConfig contains Redis Sentinel configuration.
-type SentinelConfig struct {
-	MasterName    string
-	SentinelAddrs []string
-}
-
-// RedisTLSConfig holds TLS configuration for Redis connections.
-// Presence of this struct enables TLS for the connection.
-type RedisTLSConfig struct {
-	// InsecureSkipVerify skips TLS certificate verification.
-	InsecureSkipVerify bool
-
-	// CACert is the PEM-encoded CA certificate for verifying the server.
-	// When nil, the system root CAs are used.
-	CACert []byte
-}

pkg/transport/session/session_data_storage_redis.go

@@ -11,6 +11,8 @@ import (
 	"time"
 
 	"github.com/redis/go-redis/v9"
+
+	tcredis "github.com/stacklok/toolhive-core/redis"
 )
 
 // RedisSessionDataStorage implements DataStorage backed by Redis/Valkey.
@@ -27,23 +29,31 @@ type RedisSessionDataStorage struct {
 	ttl       time.Duration
 }
 
-// NewRedisSessionDataStorage constructs a RedisSessionDataStorage.
-// cfg provides connection parameters; ttl is the sliding-window expiry applied
-// on every Create/Update and Load. The caller must call Close when done.
-func NewRedisSessionDataStorage(ctx context.Context, cfg RedisConfig, ttl time.Duration) (*RedisSessionDataStorage, error) {
-	if err := validateRedisConfig(&cfg); err != nil {
-		return nil, fmt.Errorf("invalid redis configuration: %w", err)
-	}
-	if ttl <= 0 {
-		return nil, fmt.Errorf("ttl must be a positive duration")
+// NewRedisSessionDataStorage constructs a RedisSessionDataStorage. Connection-mode
+// topology, timeouts, TLS, and credentials are configured through cfg; keyPrefix
+// is the per-tenant key prefix (e.g. "thv:vmcp:session:") and must end with ':'
+// to avoid key collisions. ttl is the sliding-window expiry applied on every
+// Create/Update and Load. The caller must call Close when done.
+//
+// Connection-mode validation, timeout defaults, client construction (standalone,
+// cluster, or sentinel), TLS plumbing, and connectivity verification are
+// delegated to the shared toolhive-core redis package.
+func NewRedisSessionDataStorage(
+	ctx context.Context,
+	cfg tcredis.Config,
+	keyPrefix string,
+	ttl time.Duration,
+) (*RedisSessionDataStorage, error) {
+	if err := validateSessionInvariants(keyPrefix, ttl); err != nil {
+		return nil, err
 	}
-	client, err := buildRedisClient(ctx, &cfg)
+	client, err := tcredis.NewClient(ctx, &cfg)
 	if err != nil {
 		return nil, err
 	}
 	return &RedisSessionDataStorage{
 		client:    client,
-		keyPrefix: cfg.KeyPrefix,
+		keyPrefix: keyPrefix,
 		ttl:       ttl,
 	}, nil
 }