-
Notifications
You must be signed in to change notification settings - Fork 198
Wire Redis Backend Selection into session.Manager (RC-7) #4208
Description
Description
Add a factory constructor to pkg/transport/session/manager.go that selects RedisStorage when a RedisConfig is provided and falls back to LocalStorage otherwise. This wires the Redis storage backend implemented in RC-6 into the session manager lifecycle, enabling the manager to be constructed with an externalized Redis backend without changing the signatures of any existing constructors (NewManager, NewTypedManager).
This task is the second layer of the THV-0047 horizontal scaling foundation. Once complete, the vMCP server can construct a Redis-backed session manager by passing a RedisConfig, and all existing callers that do not supply a config continue to use in-memory LocalStorage without modification.
Context
pkg/transport/session/manager.go currently exposes three constructors: NewManager, NewTypedManager, and NewManagerWithStorage. The first two always use NewLocalStorage() as the storage backend. NewManagerWithStorage is a lower-level extension point that accepts any Storage implementation. RC-7 adds a higher-level convenience constructor — NewManagerWithRedis or similar — that internally constructs a RedisStorage from a RedisConfig and calls NewManagerWithStorage, keeping existing callers untouched.
Parent epic: stacklok/stacklok-epics#262
Dependencies: #266 (RC-6: Redis Storage Backend for transport/session)
Blocks: TASK-003 (RC-8: Persist vMCP Session Metadata to Redis on Creation)
Acceptance Criteria
- A new exported constructor (e.g.
NewManagerWithRedis) exists inpkg/transport/session/manager.gothat accepts aRedisConfig(or a pointer) and aFactoryand returns(*Manager, error) - When
RedisConfigis non-nil (or the pointer is non-nil), the constructor callsNewRedisStorage(from RC-6) and passes the resultingRedisStoragetoNewManagerWithStorage - The TTL passed to
NewManagerWithStorageis also used as the Redis key TTL so that session expiry in Redis matches the manager's cleanup interval -
NewManagersignature is unchanged and continues to useNewLocalStorageinternally -
NewTypedManagersignature is unchanged and continues to useNewLocalStorageinternally -
NewManagerWithStoragesignature is unchanged (still the general extension point for custom storage) - All existing callers of
NewManager,NewTypedManager, andNewManagerWithStoragecompile and pass tests without modification - Unit tests cover: Redis config present →
RedisStorageis used;nilor absent config →LocalStorageis used - Unit tests verify the constructor returns an error when
NewRedisStoragefails (e.g. invalid config) - All tests pass (
go test ./pkg/transport/session/...) - Code reviewed and approved
Technical Approach
Recommended Implementation
Add one new function to pkg/transport/session/manager.go:
NewManagerWithRedis(ttl time.Duration, factory Factory, cfg RedisConfig) (*Manager, error)
Inside, call NewRedisStorage(cfg, ttl) — the constructor defined in TASK-001. If NewRedisStorage returns an error (bad config, TLS cert load failure), propagate it. Otherwise pass the RedisStorage to NewManagerWithStorage and return the manager.
Because NewManagerWithStorage already starts the cleanup goroutine, no additional lifecycle code is needed. For Redis-backed storage the cleanup goroutine calls DeleteExpired which is a no-op (Redis TTL handles expiry), so there is no performance concern.
The call site that needs to opt in to Redis (pkg/vmcp/server/server.go, line 382) will be updated in a later task. This task only adds the constructor; the server wiring is deferred.
Patterns & Frameworks
- Follow the existing constructor pattern in
pkg/transport/session/manager.go: construct aManagerstruct literal, startgo m.cleanupRoutine(), return - Use
NewManagerWithStorageas the sole internal extension point — do not duplicate the manager struct initialization - Return
(*Manager, error)from the new constructor to propagateNewRedisStorageerrors; all existing constructors return*Manager(no error) becauseNewLocalStorageis infallible - Test helper pattern: follow
pkg/authserver/storage/redis_test.go'snewTestRedisStorage/withRedisStoragepattern usinggithub.com/alicebob/miniredis/v2
Code Pointers
pkg/transport/session/manager.go— file to modify; addNewManagerWithRedisafterNewManagerWithStorage(line 104); thettlfield is the value to forward toNewRedisStorageas the key TTLpkg/transport/session/storage_redis.go— new file from TASK-001; providesNewRedisStorage(cfg RedisConfig, ttl time.Duration) (*RedisStorage, error)which this constructor callspkg/transport/session/redis_config.go— new file from TASK-001; provides theRedisConfigtype accepted by the new constructorpkg/transport/session/storage_local.go— shows whatNewLocalStoragereturns for comparison; the fallback path that existing callers rely onpkg/transport/session/storage_test.go— existing test patterns for storage and manager; new tests forNewManagerWithRedisshould follow the same table-driven,t.Parallel()stylepkg/authserver/storage/redis_test.go—newTestRedisStorage/withRedisStoragehelper pattern usingminiredis.RunT(t); adapt for session-layer testspkg/vmcp/server/server.goline 382 — currentNewManagercall that will eventually be replaced; this task does NOT change this call site
Component Interfaces
// NewManagerWithRedis creates a session manager backed by Redis.
// 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 TLS config).
// Callers that do not require Redis should continue to use NewManager or NewTypedManager.
func NewManagerWithRedis(ttl time.Duration, factory Factory, cfg RedisConfig) (*Manager, error) {
storage, err := NewRedisStorage(cfg, ttl)
if err != nil {
return nil, fmt.Errorf("creating redis storage: %w", err)
}
return NewManagerWithStorage(ttl, factory, storage), nil
}No changes to existing signatures:
// Unchanged — existing callers continue to compile without modification.
func NewManager(ttl time.Duration, factory interface{}) *Manager
func NewTypedManager(ttl time.Duration, sessionType SessionType) *Manager
func NewManagerWithStorage(ttl time.Duration, factory Factory, storage Storage) *ManagerTesting Strategy
Use github.com/alicebob/miniredis/v2 (miniredis.RunT(t)) for all Redis-backed tests. No live Redis connection is required.
Unit Tests
-
NewManagerWithRedis— valid config pointing at a miniredis instance → returns non-nil*Managerand nil error -
NewManagerWithRedis—RedisConfigwith an unreachable address → construction should succeed (go-redis defers connection); verify that the returned manager is non-nil (lazy connection is the go-redis default) -
NewManagerWithRedis—RedisConfigwith invalid TLS PEM inCACert→ returns a non-nil error -
NewManagerWithRedis— round-trip: store a session viamanager.AddWithID, retrieve it viamanager.Get; verify the session can be found (exercises the Redis path end-to-end with miniredis) -
NewManager— verify it still returns a manager backed byLocalStorage(type assertionm.storage.(*LocalStorage)succeeds) -
NewTypedManager— verify it still returns a manager backed byLocalStorage
Integration Tests
- None required at this layer; unit tests with miniredis provide sufficient coverage
Edge Cases
- Calling
Stop()on a Redis-backed manager closes the underlying Redis client (go-redisclient.Close()is called viaRedisStorage.Close()) -
DeleteExpiredcalled by the cleanup goroutine on a Redis-backed manager is a no-op and does not return an error
Out of Scope
- Implementing
RedisStorageitself (TASK-001 / RC-6) - Changing the call site in
pkg/vmcp/server/server.goto useNewManagerWithRedis(deferred to a later wiring task, or done as part of TASK-003+) - Any changes to
pkg/vmcppackages - Redis Cluster support
- Operational concerns: Redis provisioning, Kubernetes configuration
References
- RFC THV-0047: RFC: Horizontal Scaling for vMCP and Proxy Runner toolhive-rfcs#47
- Parent epic: stacklok/stacklok-epics#262
- Upstream dependency (RC-6): stacklok/stacklok-epics#266
Storageinterface:pkg/transport/session/storage.goNewManagerWithStorageextension point:pkg/transport/session/manager.go(line 104)- Authserver Redis test helper pattern:
pkg/authserver/storage/redis_test.go