diff --git a/enterprise/server/backends/redis_client/redis_client.go b/enterprise/server/backends/redis_client/redis_client.go index 95dfa255825..9a639920103 100644 --- a/enterprise/server/backends/redis_client/redis_client.go +++ b/enterprise/server/backends/redis_client/redis_client.go @@ -13,7 +13,6 @@ import ( var ( defaultRedisTarget = flag.String("app.default_redis_target", "", "A Redis target for storing remote shared state. To ease migration, the redis target from the remote execution config will be used if this value is not specified.", flag.Secret) - defaultRedisUseTLS = flag.Bool("app.default_redis.tls.enabled", false, "Use TLS when connecting to Redis.") defaultRedisShards = flag.Slice("app.default_sharded_redis.shards", []string{}, "Ordered list of Redis shard addresses.") defaultShardedRedisUsername = flag.String("app.default_sharded_redis.username", "", "Redis username") defaultShardedRedisPassword = flag.String("app.default_sharded_redis.password", "", "Redis password", flag.Secret) @@ -21,7 +20,6 @@ var ( // Cache Redis // TODO: We need to deprecate one of the redis targets here or distinguish them cacheRedisTargetFallback = flag.String("cache.redis_target", "", "A redis target for improved Caching/RBE performance. Target can be provided as either a redis connection URI or a host:port pair. URI schemas supported: redis[s]://[[USER][:PASSWORD]@][HOST][:PORT][/DATABASE] or unix://[[USER][:PASSWORD]@]SOCKET_PATH[?db=DATABASE] ** Enterprise only **", flag.Secret) - cacheRedisUseTLS = flag.Bool("cache.redis.tls.enabled", false, "Use TLS when connecting to Redis.") cacheRedisTarget = flag.String("cache.redis.redis_target", "", "A redis target for improved Caching/RBE performance. Target can be provided as either a redis connection URI or a host:port pair. URI schemas supported: redis[s]://[[USER][:PASSWORD]@][HOST][:PORT][/DATABASE] or unix://[[USER][:PASSWORD]@]SOCKET_PATH[?db=DATABASE] ** Enterprise only **", flag.Secret) cacheRedisShards = flag.Slice("cache.redis.sharded.shards", []string{}, "Ordered list of Redis shard addresses.") cacheShardedRedisUsername = flag.String("cache.redis.sharded.username", "", "Redis username") @@ -29,7 +27,6 @@ var ( // Remote Execution Redis remoteExecRedisTarget = flag.String("remote_execution.redis_target", "", "A Redis target for storing remote execution state. Falls back to app.default_redis_target if unspecified. Required for remote execution. To ease migration, the redis target from the cache config will be used if neither this value nor app.default_redis_target are specified.", flag.Secret) - remoteExecRedisUseTLS = flag.Bool("remote_execution.redis.tls.enabled", false, "Use TLS when connecting to Redis.") remoteExecRedisShards = flag.Slice("remote_execution.sharded_redis.shards", []string{}, "Ordered list of Redis shard addresses.") remoteExecShardedRedisUsername = flag.String("remote_execution.sharded_redis.username", "", "Redis username") remoteExecShardedRedisPassword = flag.String("remote_execution.sharded_redis.password", "", "Redis password", flag.Secret) @@ -42,31 +39,31 @@ type ShardedRedisConfig struct { } func defaultRedisClientOptsNoFallback() *redisutil.Opts { - if opts := redisutil.ShardsToOpts(*defaultRedisShards, *defaultRedisUseTLS, *defaultShardedRedisUsername, *defaultShardedRedisPassword); opts != nil { + if opts := redisutil.ShardsToOpts(*defaultRedisShards, *defaultShardedRedisUsername, *defaultShardedRedisPassword); opts != nil { return opts } - return redisutil.TargetToOpts(*defaultRedisTarget, *defaultRedisUseTLS) + return redisutil.TargetToOpts(*defaultRedisTarget) } func cacheRedisClientOptsNoFallback() *redisutil.Opts { // Prefer the client configs from Redis sub-config, is present. - if opts := redisutil.ShardsToOpts(*cacheRedisShards, *cacheRedisUseTLS, *cacheShardedRedisUsername, *cacheShardedRedisPassword); opts != nil { + if opts := redisutil.ShardsToOpts(*cacheRedisShards, *cacheShardedRedisUsername, *cacheShardedRedisPassword); opts != nil { return opts } - if opts := redisutil.TargetToOpts(*cacheRedisTarget, *cacheRedisUseTLS); opts != nil { + if opts := redisutil.TargetToOpts(*cacheRedisTarget); opts != nil { return opts } - return redisutil.TargetToOpts(*cacheRedisTargetFallback, *cacheRedisUseTLS) + return redisutil.TargetToOpts(*cacheRedisTargetFallback) } func remoteExecutionRedisClientOptsNoFallback() *redisutil.Opts { if !remote_execution_config.RemoteExecutionEnabled() { return nil } - if opts := redisutil.ShardsToOpts(*remoteExecRedisShards, *remoteExecRedisUseTLS, *remoteExecShardedRedisUsername, *remoteExecShardedRedisPassword); opts != nil { + if opts := redisutil.ShardsToOpts(*remoteExecRedisShards, *remoteExecShardedRedisUsername, *remoteExecShardedRedisPassword); opts != nil { return opts } - return redisutil.TargetToOpts(*remoteExecRedisTarget, *remoteExecRedisUseTLS) + return redisutil.TargetToOpts(*remoteExecRedisTarget) } func DefaultRedisClientOpts() *redisutil.Opts { diff --git a/enterprise/server/util/redisutil/redisutil.go b/enterprise/server/util/redisutil/redisutil.go index cbda5aea0fc..c2db78e3941 100644 --- a/enterprise/server/util/redisutil/redisutil.go +++ b/enterprise/server/util/redisutil/redisutil.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "flag" "fmt" + "net/url" "path/filepath" "runtime" "strings" @@ -68,43 +69,46 @@ func TargetToOptions(redisTarget string) *redis.Options { } } -func TargetToOpts(redisTarget string, useTLS bool) *Opts { +func TargetToOpts(redisTarget string) *Opts { if redisTarget == "" { return nil } libOpts := TargetToOptions(redisTarget) - var tlsConfig *tls.Config - if useTLS { - tlsConfig = &tls.Config{ - MinVersion: tls.VersionTLS12, - } - } return &Opts{ Addrs: []string{libOpts.Addr}, Network: libOpts.Network, Username: libOpts.Username, Password: libOpts.Password, DB: libOpts.DB, - TLSConfig: tlsConfig, + TLSConfig: libOpts.TLSConfig, } } -func ShardsToOpts(shards []string, useTLS bool, username, password string) *Opts { +func ShardsToOpts(shards []string, username, password string) *Opts { if len(shards) == 0 { return nil } - var tlsConfig *tls.Config - if useTLS { - tlsConfig = &tls.Config{ - MinVersion: tls.VersionTLS12, + var prevShardScheme string + for i, shard := range shards { + u, err := url.Parse(shard) + if err != nil { + log.Errorf("Failed to parse redis shard %q: %s, ignoring", shard, err) + continue } + if i > 0 { + if u.Scheme != prevShardScheme { + // TODO(sluongng): return an error instead of logging. + log.Warningf("All redis shards must use the same url scheme, but found %q and %q", prevShardScheme, u.Scheme) + break + } + } + prevShardScheme = u.Scheme } - return &Opts{ - Addrs: shards, - Username: username, - Password: password, - TLSConfig: tlsConfig, - } + opt := TargetToOpts(shards[0]) + opt.Addrs = shards + opt.Username = username + opt.Password = password + return opt } type HealthChecker struct { @@ -183,7 +187,15 @@ func (o *Opts) toRingOpts() (*redis.RingOptions, error) { TLSConfig: o.TLSConfig, } for i, addr := range o.Addrs { - opts.Addrs[fmt.Sprintf("shard%d", i)] = addr + if !isRedisURI(addr) { + // Assume tcp if no scheme is provided. + addr = "redis://" + addr + } + opt, err := redis.ParseURL(addr) + if err != nil { + return nil, status.FailedPreconditionErrorf("invalid redis shard address %q: %s", addr, err) + } + opts.Addrs[fmt.Sprintf("shard%d", i)] = opt.Addr } return opts, nil }