From e2ca973c18181373fb1ddbf1da6cbc5279d02a3f Mon Sep 17 00:00:00 2001 From: Jorge Turrado Date: Mon, 12 Dec 2022 19:20:05 +0100 Subject: [PATCH] fix: Support unsafeSsl and enable ssl verification as default Signed-off-by: Jorge Turrado --- CHANGELOG.md | 2 +- pkg/scalers/redis_scaler.go | 53 +++++------ pkg/scalers/redis_scaler_test.go | 82 ++++++++++++++++ pkg/scalers/redis_streams_scaler.go | 20 ++++ pkg/scalers/redis_streams_scaler_test.go | 114 +++++++++++++++++++++++ 5 files changed, 240 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 595b9f6b28a..10df6820e3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,7 +59,7 @@ Here is an overview of all new **experimental** features: ### Fixes -- **General**: TODO ([#TODO](https://github.com/kedacore/keda/issues/TODO)) +- **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 index f2cb4dc293d..7f37733eef9 100644 --- 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 index 1760c958ac7..6fcbb76d556 100644 --- 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 index fad5782c1ae..799426cb672 100644 --- 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 index aedad198d5a..43151884c6c 100644 --- 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 {