fix: Support unsafeSsl and enable ssl verification as default (#4006)

Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]>
kedacore · Dec 16, 2022 · 6c49e26 · 6c49e26
1 parent eddae01
commit 6c49e26
Show file tree

Hide file tree

Showing 5 changed files with 240 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,7 @@ Here is an overview of all new **experimental** features:
 
 - **General**: Properly retrieve and close scalers cache ([#4011](https://github.com/kedacore/keda/issues/4011))
 - **Azure Key Vault:** Raise an error if authentication mechanism not provided ([#4010](https://github.com/kedacore/keda/issues/4010))
+- **Redis Scalers**: Support `unsafeSsl` and enable ssl verification as default ([#4005](https://github.com/kedacore/keda/issues/4005))
 
 ### Deprecations
 

diff --git a/pkg/scalers/redis_scaler.go b/pkg/scalers/redis_scaler.go
@@ -43,6 +43,7 @@ type redisConnectionInfo struct {
 	hosts            []string
 	ports            []string
 	enableTLS        bool
+	unsafeSsl        bool
 }
 
 type redisMetadata struct {
@@ -95,6 +96,7 @@ func NewRedisScaler(ctx context.Context, isClustered, isSentinel bool, config *S
 	if err != nil {
 		return nil, fmt.Errorf("error parsing redis metadata: %s", err)
 	}
+
 	return createRedisScaler(ctx, meta, luaScript, metricType, logger)
 }
 
@@ -183,6 +185,24 @@ func parseRedisMetadata(config *ScalerConfig, parserFn redisAddressParser) (*red
 		connectionInfo: connInfo,
 	}
 
+	meta.connectionInfo.enableTLS = defaultEnableTLS
+	if val, ok := config.TriggerMetadata["enableTLS"]; ok {
+		tls, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("enableTLS parsing error %s", err.Error())
+		}
+		meta.connectionInfo.enableTLS = tls
+	}
+
+	meta.connectionInfo.unsafeSsl = false
+	if val, ok := config.TriggerMetadata["unsafeSsl"]; ok {
+		parsedVal, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing unsafeSsl: %s", err)
+		}
+		meta.connectionInfo.unsafeSsl = parsedVal
+	}
+
 	meta.listLength = defaultListLength
 	if val, ok := config.TriggerMetadata["listLength"]; ok {
 		listLength, err := strconv.ParseInt(val, 10, 64)
@@ -304,15 +324,6 @@ func parseRedisAddress(metadata, resolvedEnv, authParams map[string]string) (red
 		info.password = resolvedEnv[metadata["passwordFromEnv"]]
 	}
 
-	info.enableTLS = defaultEnableTLS
-	if val, ok := metadata["enableTLS"]; ok {
-		tls, err := strconv.ParseBool(val)
-		if err != nil {
-			return info, fmt.Errorf("enableTLS parsing error %s", err.Error())
-		}
-		info.enableTLS = tls
-	}
-
 	return info, nil
 }
 
@@ -382,15 +393,6 @@ func parseRedisClusterAddress(metadata, resolvedEnv, authParams map[string]strin
 		info.password = resolvedEnv[metadata["passwordFromEnv"]]
 	}
 
-	info.enableTLS = defaultEnableTLS
-	if val, ok := metadata["enableTLS"]; ok {
-		tls, err := strconv.ParseBool(val)
-		if err != nil {
-			return info, fmt.Errorf("enableTLS parsing error %s", err.Error())
-		}
-		info.enableTLS = tls
-	}
-
 	return info, nil
 }
 
@@ -439,15 +441,6 @@ func parseRedisSentinelAddress(metadata, resolvedEnv, authParams map[string]stri
 		info.sentinelMaster = resolvedEnv[metadata["sentinelMasterFromEnv"]]
 	}
 
-	info.enableTLS = defaultEnableTLS
-	if val, ok := metadata["enableTLS"]; ok {
-		tls, err := strconv.ParseBool(val)
-		if err != nil {
-			return info, fmt.Errorf("enableTLS parsing error %s", err.Error())
-		}
-		info.enableTLS = tls
-	}
-
 	return info, nil
 }
 
@@ -459,7 +452,7 @@ func getRedisClusterClient(ctx context.Context, info redisConnectionInfo) (*redi
 	}
 	if info.enableTLS {
 		options.TLSConfig = &tls.Config{
-			InsecureSkipVerify: info.enableTLS,
+			InsecureSkipVerify: info.unsafeSsl,
 		}
 	}
 
@@ -483,7 +476,7 @@ func getRedisSentinelClient(ctx context.Context, info redisConnectionInfo, dbInd
 	}
 	if info.enableTLS {
 		options.TLSConfig = &tls.Config{
-			InsecureSkipVerify: info.enableTLS,
+			InsecureSkipVerify: info.unsafeSsl,
 		}
 	}
 
@@ -504,7 +497,7 @@ func getRedisClient(ctx context.Context, info redisConnectionInfo, dbIndex int)
 	}
 	if info.enableTLS {
 		options.TLSConfig = &tls.Config{
-			InsecureSkipVerify: info.enableTLS,
+			InsecureSkipVerify: info.unsafeSsl,
 		}
 	}
 

diff --git a/pkg/scalers/redis_scaler_test.go b/pkg/scalers/redis_scaler_test.go
@@ -292,6 +292,47 @@ func TestParseRedisClusterMetadata(t *testing.T) {
 			},
 			wantErr: nil,
 		},
+		{
+			name: "tls enabled without setting unsafeSsl",
+			metadata: map[string]string{
+				"listName":  "mylist",
+				"enableTLS": "true",
+			},
+			authParams: map[string]string{
+				"addresses": ":7001, :7002",
+			},
+			wantMeta: &redisMetadata{
+				listLength: 5,
+				listName:   "mylist",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{":7001", ":7002"},
+					enableTLS: true,
+					unsafeSsl: false,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "tls enabled with unsafeSsl true",
+			metadata: map[string]string{
+				"listName":  "mylist",
+				"enableTLS": "true",
+				"unsafeSsl": "true",
+			},
+			authParams: map[string]string{
+				"addresses": ":7001, :7002",
+			},
+			wantMeta: &redisMetadata{
+				listLength: 5,
+				listName:   "mylist",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{":7001", ":7002"},
+					enableTLS: true,
+					unsafeSsl: true,
+				},
+			},
+			wantErr: nil,
+		},
 	}
 
 	for _, testCase := range cases {
@@ -697,6 +738,47 @@ func TestParseRedisSentinelMetadata(t *testing.T) {
 			},
 			wantErr: nil,
 		},
+		{
+			name: "tls enabled without setting unsafeSsl",
+			metadata: map[string]string{
+				"listName":  "mylist",
+				"enableTLS": "true",
+			},
+			authParams: map[string]string{
+				"addresses": ":7001, :7002",
+			},
+			wantMeta: &redisMetadata{
+				listLength: 5,
+				listName:   "mylist",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{":7001", ":7002"},
+					enableTLS: true,
+					unsafeSsl: false,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "tls enabled with unsafeSsl true",
+			metadata: map[string]string{
+				"listName":  "mylist",
+				"enableTLS": "true",
+				"unsafeSsl": "true",
+			},
+			authParams: map[string]string{
+				"addresses": ":7001, :7002",
+			},
+			wantMeta: &redisMetadata{
+				listLength: 5,
+				listName:   "mylist",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{":7001", ":7002"},
+					enableTLS: true,
+					unsafeSsl: true,
+				},
+			},
+			wantErr: nil,
+		},
 	}
 
 	for _, testCase := range cases {

diff --git a/pkg/scalers/redis_streams_scaler.go b/pkg/scalers/redis_streams_scaler.go
@@ -157,6 +157,25 @@ func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser)
 	meta := redisStreamsMetadata{
 		connectionInfo: connInfo,
 	}
+
+	meta.connectionInfo.enableTLS = defaultEnableTLS
+	if val, ok := config.TriggerMetadata["enableTLS"]; ok {
+		tls, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("enableTLS parsing error %s", err.Error())
+		}
+		meta.connectionInfo.enableTLS = tls
+	}
+
+	meta.connectionInfo.unsafeSsl = false
+	if val, ok := config.TriggerMetadata["unsafeSsl"]; ok {
+		parsedVal, err := strconv.ParseBool(val)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing unsafeSsl: %s", err)
+		}
+		meta.connectionInfo.unsafeSsl = parsedVal
+	}
+
 	meta.targetPendingEntriesCount = defaultTargetPendingEntriesCount
 
 	if val, ok := config.TriggerMetadata[pendingEntriesCountMetadata]; ok {
@@ -189,6 +208,7 @@ func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser)
 		}
 		meta.databaseIndex = int(dbIndex)
 	}
+
 	meta.scalerIndex = config.ScalerIndex
 	return &meta, nil
 }

diff --git a/pkg/scalers/redis_streams_scaler_test.go b/pkg/scalers/redis_streams_scaler_test.go
@@ -376,6 +376,63 @@ func TestParseRedisClusterStreamsMetadata(t *testing.T) {
 			},
 			wantErr: nil,
 		},
+		{
+			name: "tls enabled without setting unsafeSsl",
+			metadata: map[string]string{
+				"hosts":               "a, b, c",
+				"ports":               "1, 2, 3",
+				"stream":              "my-stream",
+				"pendingEntriesCount": "10",
+				"consumerGroup":       "consumer1",
+				"enableTLS":           "true",
+			},
+			authParams: map[string]string{
+				"password": "password",
+			},
+			wantMeta: &redisStreamsMetadata{
+				streamName:                "my-stream",
+				targetPendingEntriesCount: 10,
+				consumerGroupName:         "consumer1",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{"a:1", "b:2", "c:3"},
+					hosts:     []string{"a", "b", "c"},
+					ports:     []string{"1", "2", "3"},
+					password:  "password",
+					enableTLS: true,
+					unsafeSsl: false,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "tls enabled with unsafeSsl true",
+			metadata: map[string]string{
+				"hosts":               "a, b, c",
+				"ports":               "1, 2, 3",
+				"stream":              "my-stream",
+				"pendingEntriesCount": "10",
+				"consumerGroup":       "consumer1",
+				"enableTLS":           "true",
+				"unsafeSsl":           "true",
+			},
+			authParams: map[string]string{
+				"password": "password",
+			},
+			wantMeta: &redisStreamsMetadata{
+				streamName:                "my-stream",
+				targetPendingEntriesCount: 10,
+				consumerGroupName:         "consumer1",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{"a:1", "b:2", "c:3"},
+					hosts:     []string{"a", "b", "c"},
+					ports:     []string{"1", "2", "3"},
+					password:  "password",
+					enableTLS: true,
+					unsafeSsl: true,
+				},
+			},
+			wantErr: nil,
+		},
 	}
 
 	for _, testCase := range cases {
@@ -815,6 +872,63 @@ func TestParseRedisSentinelStreamsMetadata(t *testing.T) {
 			},
 			wantErr: nil,
 		},
+		{
+			name: "tls enabled without setting unsafeSsl",
+			metadata: map[string]string{
+				"hosts":               "a, b, c",
+				"ports":               "1, 2, 3",
+				"stream":              "my-stream",
+				"pendingEntriesCount": "10",
+				"consumerGroup":       "consumer1",
+				"enableTLS":           "true",
+			},
+			authParams: map[string]string{
+				"password": "password",
+			},
+			wantMeta: &redisStreamsMetadata{
+				streamName:                "my-stream",
+				targetPendingEntriesCount: 10,
+				consumerGroupName:         "consumer1",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{"a:1", "b:2", "c:3"},
+					hosts:     []string{"a", "b", "c"},
+					ports:     []string{"1", "2", "3"},
+					password:  "password",
+					enableTLS: true,
+					unsafeSsl: false,
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "tls enabled with unsafeSsl true",
+			metadata: map[string]string{
+				"hosts":               "a, b, c",
+				"ports":               "1, 2, 3",
+				"stream":              "my-stream",
+				"pendingEntriesCount": "10",
+				"consumerGroup":       "consumer1",
+				"enableTLS":           "true",
+				"unsafeSsl":           "true",
+			},
+			authParams: map[string]string{
+				"password": "password",
+			},
+			wantMeta: &redisStreamsMetadata{
+				streamName:                "my-stream",
+				targetPendingEntriesCount: 10,
+				consumerGroupName:         "consumer1",
+				connectionInfo: redisConnectionInfo{
+					addresses: []string{"a:1", "b:2", "c:3"},
+					hosts:     []string{"a", "b", "c"},
+					ports:     []string{"1", "2", "3"},
+					password:  "password",
+					enableTLS: true,
+					unsafeSsl: true,
+				},
+			},
+			wantErr: nil,
+		},
 	}
 
 	for _, testCase := range cases {