fix(redis): fix redis parameter parsing and add unit tests

Signed-off-by: Andrei Aaron <[email protected]>

go.mod

@@ -56,6 +56,7 @@ require (
 	github.com/sigstore/cosign/v2 v2.4.1
 	github.com/sigstore/sigstore v1.8.12
 	github.com/smartystreets/goconvey v1.8.1
+	github.com/spf13/cast v1.7.0
 	github.com/spf13/cobra v1.8.1
 	github.com/spf13/viper v1.19.0
 	github.com/stretchr/testify v1.10.0
@@ -430,7 +431,6 @@ require (
 	github.com/sourcegraph/conc v0.3.0 // indirect
 	github.com/spdx/tools-golang v0.5.5 // indirect
 	github.com/spf13/afero v1.11.0 // indirect
-	github.com/spf13/cast v1.7.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/spiffe/go-spiffe/v2 v2.3.0 // indirect
 	github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6 // indirect

pkg/common/redis.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/redis/go-redis/v9"
+	"github.com/spf13/cast"
 
 	"zotregistry.dev/zot/errors"
 	"zotregistry.dev/zot/pkg/log"
@@ -15,8 +16,6 @@ func GetRedisClient(redisConfig map[string]interface{}, log log.Logger) (redis.U
 	// go-redis supports connecting via the redis uri specification (more convenient than parameter parsing)
 	// Note failover/Sentinel cannot be configured via URL parsing at the moment
 	if val, ok := redisConfig["url"]; ok {
-		log.Info().Interface("redisConfig", redisConfig).Msg("parsing redis url")
-
 		str, ok := val.(string)
 		if !ok {
 			return nil, fmt.Errorf("%w: cachedriver %s has invalid value for url", errors.ErrBadConfig, redisConfig)
@@ -41,167 +40,240 @@ func GetRedisClient(redisConfig map[string]interface{}, log log.Logger) (redis.U
 	}
 
 	// URL configuration not provided by the user, we need to initialize UniversalOptions based on the provided parameters
-	opts, err := ParseRedisUniversalOptions(redisConfig, log)
-	if err != nil {
-		return nil, err
-	}
+	opts := ParseRedisUniversalOptions(redisConfig, log)
 
 	return redis.NewUniversalClient(opts), nil
 }
 
 func ParseRedisUniversalOptions(redisConfig map[string]interface{}, //nolint: gocyclo
 	log log.Logger,
-) (*redis.UniversalOptions, error) {
+) *redis.UniversalOptions {
 	opts := redis.UniversalOptions{}
 
-	log.Info().Interface("redisConfig", redisConfig).Msg("parsing redis universal options")
+	sanitizedConfig := map[string]interface{}{}
+	for k, v := range redisConfig {
+		if k == "password" || k == "sentinel_password" {
+			sanitizedConfig[k] = "******"
+			continue
+		}
+
+		sanitizedConfig[k] = v
+	}
+
+	log.Info().Interface("redisConfig", sanitizedConfig).Msg("parsing redis universal options")
 
-	if val, ok := getParamaterValue[[]string](redisConfig, "addr", log); ok {
+	if val, ok := getStringSlice(redisConfig, "addr", false, log); ok {
 		opts.Addrs = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "client_name", log); ok {
+	if val, ok := getString(redisConfig, "client_name", false, log); ok {
 		opts.ClientName = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "db", log); ok {
+	if val, ok := getInt(redisConfig, "db", false, log); ok {
 		opts.DB = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "protocol", log); ok {
+	if val, ok := getInt(redisConfig, "protocol", false, log); ok {
 		opts.Protocol = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "username", log); ok {
+	if val, ok := getString(redisConfig, "username", false, log); ok {
 		opts.Username = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "password", log); ok {
+	if val, ok := getString(redisConfig, "password", true, log); ok {
 		opts.Password = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "sentinel_username", log); ok {
+	if val, ok := getString(redisConfig, "sentinel_username", false, log); ok {
 		opts.SentinelUsername = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "sentinel_password", log); ok {
+	if val, ok := getString(redisConfig, "sentinel_password", true, log); ok {
 		opts.SentinelPassword = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "max_retries", log); ok {
+	if val, ok := getInt(redisConfig, "max_retries", false, log); ok {
 		opts.MaxRetries = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "min_retry_backoff", log); ok {
+	if val, ok := getDuration(redisConfig, "min_retry_backoff", false, log); ok {
 		opts.MinRetryBackoff = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "max_retry_backoff", log); ok {
+	if val, ok := getDuration(redisConfig, "max_retry_backoff", false, log); ok {
 		opts.MaxRetryBackoff = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "dial_timeout", log); ok {
+	if val, ok := getDuration(redisConfig, "dial_timeout", false, log); ok {
 		opts.DialTimeout = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "read_timeout", log); ok {
+	if val, ok := getDuration(redisConfig, "read_timeout", false, log); ok {
 		opts.ReadTimeout = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "write_timeout", log); ok {
+	if val, ok := getDuration(redisConfig, "write_timeout", false, log); ok {
 		opts.WriteTimeout = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "context_timeout_enabled", log); ok {
+	if val, ok := getBool(redisConfig, "context_timeout_enabled", false, log); ok {
 		opts.ContextTimeoutEnabled = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "pool_fifo", log); ok {
+	if val, ok := getBool(redisConfig, "pool_fifo", false, log); ok {
 		opts.PoolFIFO = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "pool_size", log); ok {
+	if val, ok := getInt(redisConfig, "pool_size", false, log); ok {
 		opts.PoolSize = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "pool_timeout", log); ok {
+	if val, ok := getDuration(redisConfig, "pool_timeout", false, log); ok {
 		opts.PoolTimeout = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "min_idle_conns", log); ok {
+	if val, ok := getInt(redisConfig, "min_idle_conns", false, log); ok {
 		opts.MinIdleConns = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "max_idle_conns", log); ok {
+	if val, ok := getInt(redisConfig, "max_idle_conns", false, log); ok {
 		opts.MaxIdleConns = val
 	}
 
-	if val, ok := getParamaterValue[int](redisConfig, "max_active_conns", log); ok {
+	if val, ok := getInt(redisConfig, "max_active_conns", false, log); ok {
 		opts.MaxActiveConns = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "conn_max_idle_time", log); ok {
+	if val, ok := getDuration(redisConfig, "conn_max_idle_time", false, log); ok {
 		opts.ConnMaxIdleTime = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "conn_max_lifetime", log); ok {
+	if val, ok := getDuration(redisConfig, "conn_max_lifetime", false, log); ok {
 		opts.ConnMaxLifetime = val
 	}
 
-	if val, ok := getParamaterValue[time.Duration](redisConfig, "connmaxlifetime", log); ok {
-		opts.ConnMaxLifetime = val
-	}
-
-	if val, ok := getParamaterValue[int](redisConfig, "max_redirects", log); ok {
+	if val, ok := getInt(redisConfig, "max_redirects", false, log); ok {
 		opts.MaxRedirects = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "read_only", log); ok {
+	if val, ok := getBool(redisConfig, "read_only", false, log); ok {
 		opts.ReadOnly = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "route_by_latency", log); ok {
+	if val, ok := getBool(redisConfig, "route_by_latency", false, log); ok {
 		opts.RouteByLatency = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "route_randomly", log); ok {
+	if val, ok := getBool(redisConfig, "route_randomly", false, log); ok {
 		opts.RouteRandomly = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "master_name", log); ok {
+	if val, ok := getString(redisConfig, "master_name", false, log); ok {
 		opts.MasterName = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "disable_identity", log); ok {
+	if val, ok := getBool(redisConfig, "disable_identity", false, log); ok {
 		opts.DisableIndentity = val
 	}
 
-	if val, ok := getParamaterValue[string](redisConfig, "identity_suffix", log); ok {
+	if val, ok := getString(redisConfig, "identity_suffix", false, log); ok {
 		opts.IdentitySuffix = val
 	}
 
-	if val, ok := getParamaterValue[bool](redisConfig, "unstable_resp3", log); ok {
+	if val, ok := getBool(redisConfig, "unstable_resp3", false, log); ok {
 		opts.UnstableResp3 = val
 	}
 
-	log.Info().Interface("RedisUniversalOptions", opts).Msg("finished parsing redis universal options")
+	log.Info().Msg("finished parsing redis universal options")
 
-	return &opts, nil
+	return &opts
 }
 
-func getParamaterValue[T any](dict map[string]interface{}, key string, log log.Logger) (T, bool) {
-	var ret T
+func logCastWarning(key string, value interface{}, hideValue bool, log log.Logger) {
+	if hideValue {
+		log.Warn().Str("key", key).Msg("failed to cast parameter to intended type")
+	} else {
+		log.Warn().Str("key", key).Interface("value", value).Msg("failed to cast parameter to intended type")
+	}
+}
 
+func getBool(dict map[string]interface{}, key string, hideValue bool, log log.Logger) (bool, bool) {
 	value, ok := dict[key]
 	if !ok {
-		return ret, false
+		return false, false
 	}
 
-	ret, ok = value.(T)
+	ret, err := cast.ToBoolE(value)
+	if err != nil {
+		logCastWarning(key, value, hideValue, log)
+
+		return false, false
+	}
+
+	return ret, true
+}
+
+func getInt(dict map[string]interface{}, key string, hideValue bool, log log.Logger) (int, bool) {
+	value, ok := dict[key]
 	if !ok {
-		log.Warn().Str("key", key).Interface("value", value).Msg("failed to cast parameter to intended type")
+		return 0, false
+	}
+
+	ret, err := cast.ToIntE(value)
+	if err != nil {
+		logCastWarning(key, value, hideValue, log)
+
+		return 0, false
+	}
+
+	return ret, true
+}
+
+func getString(dict map[string]interface{}, key string, hideValue bool, log log.Logger) (string, bool) {
+	value, ok := dict[key]
+	if !ok {
+		return "", false
+	}
+
+	ret, err := cast.ToStringE(value)
+	if err != nil {
+		logCastWarning(key, value, hideValue, log)
+
+		return "", false
+	}
+
+	return ret, true
+}
+
+func getStringSlice(dict map[string]interface{}, key string, hideValue bool, log log.Logger) ([]string, bool) {
+	value, ok := dict[key]
+	if !ok {
+		return []string{}, false
+	}
+
+	ret, err := cast.ToStringSliceE(value)
+	if err != nil {
+		logCastWarning(key, value, hideValue, log)
+
+		return []string{}, false
+	}
+
+	return ret, true
+}
+
+func getDuration(dict map[string]interface{}, key string, hideValue bool, log log.Logger) (time.Duration, bool) {
+	value, ok := dict[key]
+	if !ok {
+		return 0, false
+	}
+
+	ret, err := cast.ToDurationE(value)
+	if err != nil {
+		logCastWarning(key, value, hideValue, log)
 
-		return ret, false
+		return 0, false
 	}
 
 	return ret, true