Extract Redis initialization out of runServeCmd (#46830)

Extracts the Redis pool and the cached_mysql / mysqlredis datastore
wrappers out of `runServeCmd` and into a new `cmd/fleet/redis.go`. Same
pattern as the prior extractions on this issue (#44929, #45343, #45583,
#46166, #46421, #46517, #46742). Continues the path toward `serve.go`
>60% coverage per the discussion on #33370.

Three functions come out of the inline block:

- `initRedis` — builds the Redis pool, wraps the datastore with
`cached_mysql.New`, and applies `mysqlredis.New` with the
license-enforced host limit and host-cache options. Returns the pool,
the fully wrapped `fleet.Datastore`, and the outermost
`*mysqlredis.Datastore` (a few callers need the concrete type).
- `buildRedisPoolConfig` — translates `config.RedisConfig` into the
`redis.PoolConfig`, including the `redis://` scheme strip.
- `validateRedisConfig` — encodes the host-cache invariant:
`HostCacheEnabled` requires `HostCacheTTL > 0`. Returns an error so the
caller (or in this case `initRedis` via `initFatal`) can refuse boot
without that decision being buried inside a pure builder.

Behavior is preserved — `runServeCmd` calls these in the same order with
the same arguments, the host-cache validation still aborts startup when
violated, and the full `cmd/fleet` suite passes against MySQL + Redis.
`initRedis` returns early after `initFatal` so it's safe when the
caller's `initFatal` doesn't terminate (the case in tests). Following
the precedent established on #46742, the caller also has a loud
`initFatal` + `return` guard against a nil pool (covers the same nilaway
flow we hit on the datastore slice).

On test scope: `TestValidateRedisConfig` covers all four combinations of
`HostCacheEnabled` and `HostCacheTTL` — that's the real
boot/refuse-to-boot decision. `TestBuildRedisPoolConfigStripsScheme`
pins the `redis://` scheme-strip contract for Render-style URIs. I
didn't add a `buildRedisPoolConfig` field-mapping matrix or an
`initRedis` happy-path unit test: the former would just re-state the
struct literal, and the latter needs a real Redis pool (the smoke boot
exercises it end-to-end instead).

This completes the four named init-block extractions on this issue. If
further coverage gains are needed beyond what these have already moved,
the next conversation is whether to test `runServeCmd` directly via the
injected `initFatal`.

**Related issue:** Refs #33370

# Checklist for submitter

- [x] Added/updated automated tests
- Changes file: not applicable — internal refactor with no user-visible
behavior change

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Refactor**
* Consolidated Redis initialization and datastore wrapping into a
dedicated helper; startup now validates the Redis pool and handles
initialization failures explicitly.

* **Tests**
* Added unit tests for Redis address handling and host-cache TTL
validation to ensure config behavior is enforced.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

cmd/fleet/redis.go

@@ -0,0 +1,120 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"log/slog"
+	"strings"
+
+	"github.com/fleetdm/fleet/v4/server/config"
+	"github.com/fleetdm/fleet/v4/server/datastore/cached_mysql"
+	"github.com/fleetdm/fleet/v4/server/datastore/mysqlredis"
+	"github.com/fleetdm/fleet/v4/server/datastore/redis"
+	"github.com/fleetdm/fleet/v4/server/fleet"
+)
+
+// buildRedisPoolConfig translates the Fleet Redis config into the redis
+// package's PoolConfig. The address has its "redis://" scheme stripped so
+// providers that publish a full URI (e.g. Render's managed Redis) work
+// without a separate config knob.
+func buildRedisPoolConfig(cfg config.RedisConfig) redis.PoolConfig {
+	return redis.PoolConfig{
+		// Strip the Redis URI scheme if it's present. Scheme docs are at:
+		// https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
+		// In the future, we could support the full Redis URI if needed
+		// (including username, password, database, etc.)
+		Server:                    strings.TrimPrefix(cfg.Address, "redis://"),
+		Username:                  cfg.Username,
+		Password:                  cfg.Password,
+		Database:                  cfg.Database,
+		UseTLS:                    cfg.UseTLS,
+		Region:                    cfg.Region,
+		CacheName:                 cfg.CacheName,
+		StsAssumeRoleArn:          cfg.StsAssumeRoleArn,
+		StsExternalID:             cfg.StsExternalID,
+		ConnTimeout:               cfg.ConnectTimeout,
+		KeepAlive:                 cfg.KeepAlive,
+		ConnectRetryAttempts:      cfg.ConnectRetryAttempts,
+		ClusterFollowRedirections: cfg.ClusterFollowRedirections,
+		ClusterReadFromReplica:    cfg.ClusterReadFromReplica,
+		TLSCert:                   cfg.TLSCert,
+		TLSKey:                    cfg.TLSKey,
+		TLSCA:                     cfg.TLSCA,
+		TLSServerName:             cfg.TLSServerName,
+		TLSHandshakeTimeout:       cfg.TLSHandshakeTimeout,
+		MaxIdleConns:              cfg.MaxIdleConns,
+		MaxOpenConns:              cfg.MaxOpenConns,
+		ConnMaxLifetime:           cfg.ConnMaxLifetime,
+		IdleTimeout:               cfg.IdleTimeout,
+		ConnWaitTimeout:           cfg.ConnWaitTimeout,
+		WriteTimeout:              cfg.WriteTimeout,
+		ReadTimeout:               cfg.ReadTimeout,
+	}
+}
+
+// validateRedisConfig returns a non-nil error when the Redis host-cache
+// configuration is inconsistent (enabled without a positive TTL). It
+// encodes the boot/refuse-to-boot rule for the cache configuration so the
+// decision can be unit-tested without spinning up Redis.
+func validateRedisConfig(cfg config.RedisConfig) error {
+	if cfg.HostCacheEnabled && cfg.HostCacheTTL <= 0 {
+		return fmt.Errorf("redis.host_cache_ttl must be > 0 when redis.host_cache_enabled is true (got %s)", cfg.HostCacheTTL)
+	}
+	return nil
+}
+
+// initRedis brings up the Redis pool and the two datastore wrappers that
+// depend on it: cached_mysql (in-memory caching layer over the datastore)
+// and mysqlredis (Redis-backed host lookup and license-enforced host
+// limit). Failures go through initFatal. Returns nil values on the
+// failure path so the function is safe when initFatal does not terminate
+// (e.g., tests using a recorder).
+//
+// The returned fleet.Datastore is the fully wrapped chain (mysqlredis →
+// cached_mysql → input ds); the returned *mysqlredis.Datastore is the
+// outermost wrapper, which a few callers need by concrete type.
+func initRedis(
+	ctx context.Context,
+	cfg config.FleetConfig,
+	license *fleet.LicenseInfo,
+	ds fleet.Datastore,
+	logger *slog.Logger,
+	initFatal func(err error, msg string),
+) (fleet.RedisPool, fleet.Datastore, *mysqlredis.Datastore) {
+	if license == nil {
+		initFatal(errors.New("license was nil"), "initialize Redis")
+		return nil, nil, nil
+	}
+
+	// Validate cheap local config before dialing Redis: surfaces a
+	// host-cache config error as itself, not as a connectivity failure,
+	// and avoids opening a pool that would be discarded if initFatal is
+	// swapped (e.g., a test recorder) and execution continues.
+	if err := validateRedisConfig(cfg.Redis); err != nil {
+		initFatal(err, "validate host cache configuration")
+		return nil, nil, nil
+	}
+
+	redisPool, err := redis.NewPool(buildRedisPoolConfig(cfg.Redis))
+	if err != nil {
+		initFatal(err, "initialize Redis")
+		return nil, nil, nil
+	}
+	logger.InfoContext(ctx, "redis initialized", "component", "redis", "mode", redisPool.Mode())
+
+	wrappedDS := cached_mysql.New(ds)
+
+	var dsOpts []mysqlredis.Option
+	if license.DeviceCount > 0 && cfg.License.EnforceHostLimit {
+		dsOpts = append(dsOpts, mysqlredis.WithEnforcedHostLimit(license.DeviceCount))
+	}
+	if cfg.Redis.HostCacheEnabled {
+		dsOpts = append(dsOpts, mysqlredis.WithHostCache(cfg.Redis.HostCacheTTL))
+		logger.InfoContext(ctx, "host lookup redis cache enabled",
+			"component", "mysqlredis", "ttl", cfg.Redis.HostCacheTTL)
+	}
+
+	redisWrapperDS := mysqlredis.New(wrappedDS, redisPool, dsOpts...)
+	return redisPool, redisWrapperDS, redisWrapperDS
+}

cmd/fleet/redis_test.go

@@ -0,0 +1,71 @@
+package main
+
+import (
+	"testing"
+	"time"
+
+	"github.com/fleetdm/fleet/v4/server/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateRedisConfig(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		cfg     config.RedisConfig
+		wantErr bool
+		wantSub string
+	}{
+		{
+			name: "host cache disabled with zero ttl is ok",
+			cfg:  config.RedisConfig{HostCacheEnabled: false, HostCacheTTL: 0},
+		},
+		{
+			name: "host cache disabled with negative ttl is ok",
+			cfg:  config.RedisConfig{HostCacheEnabled: false, HostCacheTTL: -1 * time.Second},
+		},
+		{
+			name: "host cache enabled with positive ttl is ok",
+			cfg:  config.RedisConfig{HostCacheEnabled: true, HostCacheTTL: 5 * time.Minute},
+		},
+		{
+			name:    "host cache enabled with zero ttl is rejected",
+			cfg:     config.RedisConfig{HostCacheEnabled: true, HostCacheTTL: 0},
+			wantErr: true,
+			wantSub: "host_cache_ttl must be > 0",
+		},
+		{
+			name:    "host cache enabled with negative ttl is rejected",
+			cfg:     config.RedisConfig{HostCacheEnabled: true, HostCacheTTL: -1 * time.Second},
+			wantErr: true,
+			wantSub: "host_cache_ttl must be > 0",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateRedisConfig(tc.cfg)
+			if tc.wantErr {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.wantSub)
+				return
+			}
+			assert.NoError(t, err)
+		})
+	}
+}
+
+func TestBuildRedisPoolConfigStripsScheme(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{name: "scheme stripped", input: "redis://example.com:6379", want: "example.com:6379"},
+		{name: "no scheme passes through", input: "example.com:6379", want: "example.com:6379"},
+		{name: "empty passes through", input: "", want: ""},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got := buildRedisPoolConfig(config.RedisConfig{Address: tc.input})
+			assert.Equal(t, tc.want, got.Server)
+		})
+	}
+}

cmd/fleet/serve.go

@@ -52,7 +52,6 @@ import (
 	"github.com/fleetdm/fleet/v4/server/contexts/installersize"
 	licensectx "github.com/fleetdm/fleet/v4/server/contexts/license"
 	"github.com/fleetdm/fleet/v4/server/cron"
-	"github.com/fleetdm/fleet/v4/server/datastore/cached_mysql"
 	"github.com/fleetdm/fleet/v4/server/datastore/failing"
 	"github.com/fleetdm/fleet/v4/server/datastore/filesystem"
 	"github.com/fleetdm/fleet/v4/server/datastore/mysql"
@@ -276,61 +275,13 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
 		}
 	}
 
-	// Strip the Redis URI scheme if it's present. Scheme docs are at: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml
-	// This allows us to use Render's Redis service in render.yaml, including the free tier.
-	// In the future, we could support the full Redis URI if needed (including username, password, database, etc.)
-	redisAddress := strings.TrimPrefix(config.Redis.Address, "redis://")
-	redisPool, err := redis.NewPool(redis.PoolConfig{
-		Server:                    redisAddress,
-		Username:                  config.Redis.Username,
-		Password:                  config.Redis.Password,
-		Database:                  config.Redis.Database,
-		UseTLS:                    config.Redis.UseTLS,
-		Region:                    config.Redis.Region,
-		CacheName:                 config.Redis.CacheName,
-		StsAssumeRoleArn:          config.Redis.StsAssumeRoleArn,
-		StsExternalID:             config.Redis.StsExternalID,
-		ConnTimeout:               config.Redis.ConnectTimeout,
-		KeepAlive:                 config.Redis.KeepAlive,
-		ConnectRetryAttempts:      config.Redis.ConnectRetryAttempts,
-		ClusterFollowRedirections: config.Redis.ClusterFollowRedirections,
-		ClusterReadFromReplica:    config.Redis.ClusterReadFromReplica,
-		TLSCert:                   config.Redis.TLSCert,
-		TLSKey:                    config.Redis.TLSKey,
-		TLSCA:                     config.Redis.TLSCA,
-		TLSServerName:             config.Redis.TLSServerName,
-		TLSHandshakeTimeout:       config.Redis.TLSHandshakeTimeout,
-		MaxIdleConns:              config.Redis.MaxIdleConns,
-		MaxOpenConns:              config.Redis.MaxOpenConns,
-		ConnMaxLifetime:           config.Redis.ConnMaxLifetime,
-		IdleTimeout:               config.Redis.IdleTimeout,
-		ConnWaitTimeout:           config.Redis.ConnWaitTimeout,
-		WriteTimeout:              config.Redis.WriteTimeout,
-		ReadTimeout:               config.Redis.ReadTimeout,
-	})
-	if err != nil {
-		initFatal(err, "initialize Redis")
+	var redisPool fleet.RedisPool
+	var redisWrapperDS *mysqlredis.Datastore
+	redisPool, ds, redisWrapperDS = initRedis(cmd.Context(), config, license, ds, logger, initFatal)
+	if redisPool == nil {
+		initFatal(errors.New("redis pool was nil after initialization"), "initialize Redis")
+		return
 	}
-	logger.InfoContext(cmd.Context(), "redis initialized", "component", "redis", "mode", redisPool.Mode())
-
-	ds = cached_mysql.New(ds)
-	var dsOpts []mysqlredis.Option
-	if license.DeviceCount > 0 && config.License.EnforceHostLimit {
-		dsOpts = append(dsOpts, mysqlredis.WithEnforcedHostLimit(license.DeviceCount))
-	}
-	if config.Redis.HostCacheEnabled {
-		if config.Redis.HostCacheTTL <= 0 {
-			initFatal(
-				fmt.Errorf("redis.host_cache_ttl must be > 0 when redis.host_cache_enabled is true (got %s)", config.Redis.HostCacheTTL),
-				"validate host cache configuration",
-			)
-		}
-		dsOpts = append(dsOpts, mysqlredis.WithHostCache(config.Redis.HostCacheTTL))
-		logger.InfoContext(cmd.Context(), "host lookup redis cache enabled",
-			"component", "mysqlredis", "ttl", config.Redis.HostCacheTTL)
-	}
-	redisWrapperDS := mysqlredis.New(ds, redisPool, dsOpts...)
-	ds = redisWrapperDS
 
 	resultStore := pubsub.NewRedisQueryResults(redisPool, config.Redis.DuplicateResults,
 		logger.With("component", "query-results"),