Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Dec 16, 2022
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2ca973
fix: Support unsafeSsl and enable ssl verification as default
JorTurFer Dec 12, 2022
7a44b8b
Merge branch 'main' into fix-redis
JorTurFer Dec 15, 2022
cc14f54
Merge branch 'main' into fix-redis
JorTurFer Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ Here is an overview of all new **experimental** features:
### Fixes

- **General**: Properly retrieve and close scalers cache ([#4011](https://github.com/kedacore/keda/issues/4011))
- **Redis Scalers**: Support `unsafeSsl` and enable ssl verification as default ([#4005](https://github.com/kedacore/keda/issues/4005))

### Deprecations

Expand Down
53 changes: 23 additions & 30 deletions pkg/scalers/redis_scaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type redisConnectionInfo struct {
hosts []string
ports []string
enableTLS bool
unsafeSsl bool
}

type redisMetadata struct {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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,
}
JorTurFer marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -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,
}
JorTurFer marked this conversation as resolved.
Show resolved Hide resolved
}

Expand All @@ -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,
}
JorTurFer marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
82 changes: 82 additions & 0 deletions pkg/scalers/redis_scaler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 20 additions & 0 deletions pkg/scalers/redis_streams_scaler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -189,6 +208,7 @@ func parseRedisStreamsMetadata(config *ScalerConfig, parseFn redisAddressParser)
}
meta.databaseIndex = int(dbIndex)
}

meta.scalerIndex = config.ScalerIndex
return &meta, nil
}
Expand Down
114 changes: 114 additions & 0 deletions pkg/scalers/redis_streams_scaler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down