redis: respect rediss:// (#8014)

Effectively revert #7978
as user could already enable TLS by using "rediss://" url scheme.

This would not work in case of a Shared Redis setup using Ring Client,
so ensure that we use the first shard address to setup the Ring Client
options. If the first shard address uses "rediss://", TLS will be
enabled and if it uses "redis://", TLS will be disabled.

enterprise/server/backends/redis_client/redis_client.go

@@ -13,23 +13,20 @@ 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)
 
 	// 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")
 	cacheShardedRedisPassword = flag.String("cache.redis.sharded.password", "", "Redis password", flag.Secret)
 
 	// 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 {

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
 }