Skip to content

Commit

Permalink
refactor: Remove unused variables in IBM MQ scaler
Browse files Browse the repository at this point in the history 
Signed-off-by: Rick Brouwer <[email protected]>
  • Loading branch information
@rickbrouwer
rickbrouwer committed Aug 8, 2024
1 parent a03ed32 commit 995b612
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 48 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ Here is an overview of all new **experimental** features:

### Deprecations

- IBM MQ Scaler: Remove unused variables in IBM MQ scaler ([#6033](https://github.com/kedacore/keda/issues/6033))

You can find all deprecations in [this overview](https://github.com/kedacore/keda/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Abreaking-change) and [join the discussion here](https://github.com/kedacore/keda/discussions/categories/deprecations).

New deprecation(s):
Expand Down
55 changes: 26 additions & 29 deletions pkg/scalers/ibmmq_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
// Default variables and settings
const (
defaultTargetQueueDepth = 20
defaultTLSDisabled = false
)

// IBMMQScaler assigns struct data pointer to metadata variable
Expand All @@ -38,13 +37,11 @@ type IBMMQScaler struct {
// IBMMQMetadata Metadata used by KEDA to query IBM MQ queue depth and scale
type IBMMQMetadata struct {
host string
queueManager string
queueName string
username string
password string
queueDepth int64
activationQueueDepth int64
tlsDisabled bool
triggerIndex int

// TLS
Expand Down Expand Up @@ -78,12 +75,14 @@ func NewIBMMQScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
return nil, fmt.Errorf("error getting scaler metric type: %w", err)
}

meta, err := parseIBMMQMetadata(config)
logger := InitializeLogger(config, "ibm_mq_scaler")

meta, err := parseIBMMQMetadata(config, logger)
if err != nil {
return nil, fmt.Errorf("error parsing IBM MQ metadata: %w", err)
}

httpClient := kedautil.CreateHTTPClient(config.GlobalHTTPTimeout, meta.tlsDisabled)
httpClient := kedautil.CreateHTTPClient(config.GlobalHTTPTimeout, meta.unsafeSsl)

// Configure TLS if cert and key are specified
if meta.cert != "" && meta.key != "" {
Expand All @@ -99,7 +98,7 @@ func NewIBMMQScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
metadata: meta,
defaultHTTPTimeout: config.GlobalHTTPTimeout,
httpClient: httpClient,
logger: InitializeLogger(config, "ibm_mq_scaler"),
logger: logger,
}, nil
}

Expand All @@ -112,7 +111,7 @@ func (s *IBMMQScaler) Close(context.Context) error {
}

// parseIBMMQMetadata checks the existence of and validates the MQ connection data provided
func parseIBMMQMetadata(config *scalersconfig.ScalerConfig) (*IBMMQMetadata, error) {
func parseIBMMQMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (*IBMMQMetadata, error) {
meta := IBMMQMetadata{}

if val, ok := config.TriggerMetadata["host"]; ok {
Expand All @@ -125,12 +124,6 @@ func parseIBMMQMetadata(config *scalersconfig.ScalerConfig) (*IBMMQMetadata, err
return nil, fmt.Errorf("no host URI given")
}

if val, ok := config.TriggerMetadata["queueManager"]; ok {
meta.queueManager = val
} else {
return nil, fmt.Errorf("no queue manager given")
}

if val, ok := config.TriggerMetadata["queueName"]; ok {
meta.queueName = val
} else {
Expand All @@ -144,7 +137,6 @@ func parseIBMMQMetadata(config *scalersconfig.ScalerConfig) (*IBMMQMetadata, err
}
meta.queueDepth = queueDepth
} else {
fmt.Println("No target depth defined - setting default")
meta.queueDepth = defaultTargetQueueDepth
}

Expand All @@ -157,15 +149,29 @@ func parseIBMMQMetadata(config *scalersconfig.ScalerConfig) (*IBMMQMetadata, err
meta.activationQueueDepth = activationQueueDepth
}

if val, ok := config.TriggerMetadata["tls"]; ok {
tlsDisabled, err := strconv.ParseBool(val)
// TODO: Refactor code because 'tls' is DEPRECATED and will be removed in v2.17
// Check if both "tls" and "unsafeSsl" are specified
tlsVal, tlsOk := config.TriggerMetadata["tls"]
unsafeSslVal, unsafeSslOk := config.TriggerMetadata["unsafeSsl"]

switch {
case tlsOk && unsafeSslOk:
return nil, fmt.Errorf("'tls' and 'unsafeSsl' are both specified. Please use only 'unsafeSsl'")
case tlsOk:
logger.Info("The 'tls' setting is DEPRECATED and will be removed in v2.17 - Use 'unsafeSsl' instead")
unsafeSsl, err := strconv.ParseBool(tlsVal)
if err != nil {
return nil, fmt.Errorf("invalid tls setting: %w", err)
}
meta.tlsDisabled = tlsDisabled
} else {
fmt.Println("No tls setting defined - setting default")
meta.tlsDisabled = defaultTLSDisabled
meta.unsafeSsl = unsafeSsl
case unsafeSslOk:
unsafeSsl, err := strconv.ParseBool(unsafeSslVal)
if err != nil {
return nil, fmt.Errorf("invalid unsafeSsl setting: %w", err)
}
meta.unsafeSsl = unsafeSsl
default:
meta.unsafeSsl = false
}

if val, ok := config.AuthParams["username"]; ok && val != "" {
Expand All @@ -190,15 +196,6 @@ func parseIBMMQMetadata(config *scalersconfig.ScalerConfig) (*IBMMQMetadata, err
meta.key = config.AuthParams["key"]
meta.keyPassword = config.AuthParams["keyPassword"]

meta.unsafeSsl = false
if val, ok := config.TriggerMetadata["unsafeSsl"]; ok {
boolVal, err := strconv.ParseBool(val)
if err != nil {
return nil, fmt.Errorf("failed to parse unsafeSsl value. Must be either true or false")
}
meta.unsafeSsl = boolVal
}

meta.triggerIndex = config.TriggerIndex
return &meta, nil
}
Expand Down
33 changes: 16 additions & 17 deletions pkg/scalers/ibmmq_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"

"github.com/kedacore/keda/v2/pkg/scalers/scalersconfig"
Expand Down Expand Up @@ -50,37 +51,35 @@ var testIBMMQMetadata = []parseIBMMQMetadataTestData{
// Nothing passed
{map[string]string{}, true, map[string]string{}},
// Properly formed metadata
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Invalid queueDepth using a string
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "AA"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "AA"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Invalid activationQueueDepth using a string
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "1", "activationQueueDepth": "AA"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "1", "activationQueueDepth": "AA"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// No host provided
{map[string]string{"queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Missing queueManager
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Missing queueName
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Invalid URL
{map[string]string{"host": testInvalidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testInvalidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Properly formed authParams Basic Auth
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
// Properly formed authParams Basic Auth and TLS
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123", "ca": "cavalue", "cert": "certvalue", "key": "keyvalue"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, false, map[string]string{"username": "testUsername", "password": "Pass123", "ca": "cavalue", "cert": "certvalue", "key": "keyvalue"}},
// No username provided
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"password": "Pass123"}},
// No password provided
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10"}, true, map[string]string{"username": "testUsername"}},
// Wrong input unsafeSsl
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue", "queueDepth": "10", "unsafeSsl": "random"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue", "queueDepth": "10", "unsafeSsl": "random"}, true, map[string]string{"username": "testUsername", "password": "Pass123"}},
}

// Test MQ Connection metadata is parsed correctly
// should error on missing required field
// and verify that the password field is handled correctly.
func TestIBMMQParseMetadata(t *testing.T) {
for _, testData := range testIBMMQMetadata {
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadata, AuthParams: testData.authParams})
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadata, AuthParams: testData.authParams}, logr.Discard())
if err != nil && !testData.isError {
t.Error("Expected success but got error", err)
fmt.Println(testData)
Expand All @@ -98,13 +97,13 @@ func TestIBMMQParseMetadata(t *testing.T) {

// Test case for TestParseDefaultQueueDepth test
var testDefaultQueueDepth = []parseIBMMQMetadataTestData{
{map[string]string{"host": testValidMQQueueURL, "queueManager": "testQueueManager", "queueName": "testQueue"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
{map[string]string{"host": testValidMQQueueURL, "queueName": "testQueue"}, false, map[string]string{"username": "testUsername", "password": "Pass123"}},
}

// Test that DefaultQueueDepth is set when queueDepth is not provided
func TestParseDefaultQueueDepth(t *testing.T) {
for _, testData := range testDefaultQueueDepth {
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadata, AuthParams: testData.authParams})
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadata, AuthParams: testData.authParams}, logr.Discard())
switch {
case err != nil && !testData.isError:
t.Error("Expected success but got error", err)
Expand All @@ -119,7 +118,7 @@ func TestParseDefaultQueueDepth(t *testing.T) {
// Create a scaler and check if metrics method is available
func TestIBMMQGetMetricSpecForScaling(t *testing.T) {
for _, testData := range IBMMQMetricIdentifiers {
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadataTestData.metadata, AuthParams: testData.metadataTestData.authParams, TriggerIndex: testData.triggerIndex})
metadata, err := parseIBMMQMetadata(&scalersconfig.ScalerConfig{ResolvedEnv: sampleIBMMQResolvedEnv, TriggerMetadata: testData.metadataTestData.metadata, AuthParams: testData.metadataTestData.authParams, TriggerIndex: testData.triggerIndex}, logr.Discard())
httpTimeout := 100 * time.Millisecond

if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions tests/scalers/ibmmq/ibmmq_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,8 @@ spec:
queueDepth: "3"
activationQueueDepth: "{{.ActivationQueueDepth}}"
host: {{.MqscAdminRestEndpoint}}
queueManager: {{.QueueManagerName}}
queueName: {{.QueueName}}
tls: "true"
unsafeSsl: "true"
usernameFromEnv: ""
passwordFromEnv: ""
authenticationRef:
Expand Down

0 comments on commit 995b612

Please sign in to comment.