From 3fb694b102004916d8462f0e6697bd2abb116824 Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Sun, 26 Nov 2023 21:34:29 +0000 Subject: [PATCH] Minor changes from code review Signed-off-by: dttung2905 --- CHANGELOG.md | 2 +- pkg/scalers/azure_data_explorer_scaler.go | 23 +++++++----------- .../azure_data_explorer_scaler_test.go | 24 ++++++++----------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ce67698bb3..fc7af2f6c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,7 +81,6 @@ Here is an overview of all new **experimental** features: ### Deprecations -- **General**: Clean up previously deprecated code for 2.13 release ([#5051](https://github.com/kedacore/keda/issues/5051)) 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). @@ -92,6 +91,7 @@ New deprecation(s): ### Breaking Changes - **General**: TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX)) +- **General**: Clean up previously deprecated code in Azure Data Explorer Scaler about clientSecret for 2.13 release ([#5051](https://github.com/kedacore/keda/issues/5051)) ### Other diff --git a/pkg/scalers/azure_data_explorer_scaler.go b/pkg/scalers/azure_data_explorer_scaler.go index 23f0a773303..e7fe2ef901c 100644 --- a/pkg/scalers/azure_data_explorer_scaler.go +++ b/pkg/scalers/azure_data_explorer_scaler.go @@ -19,7 +19,6 @@ package scalers import ( "context" "fmt" - "os" "strconv" "github.com/Azure/azure-kusto-go/kusto" @@ -160,22 +159,16 @@ func parseAzureDataExplorerAuthParams(config *ScalerConfig, logger logr.Logger) } metadata.ClientID = clientID - clientSecretEnVar, err := getParameterFromConfig(config, "clientSecretFromEnv", true) - if err != nil { - return nil, err + var clientSecret string + if val, ok := config.AuthParams["clientSecret"]; ok && val != "" { + clientSecret = val + } else if val, ok = config.TriggerMetadata["clientSecretFromEnv"]; ok && val != "" { + clientSecret = val + } else { + return nil, fmt.Errorf("error parsing metadata. Details: clientSecret was not found in metadata. Check your ScaledObject configuration") } - clientSecretFromEnv := os.Getenv(clientSecretEnVar) + metadata.ClientSecret = clientSecret - var clientSecretFromTrigger string - if val, ok := config.TriggerMetadata["clientSecret"]; ok { - clientSecretFromTrigger = val - } - - if clientSecretFromTrigger != "" { - metadata.ClientSecret = clientSecretFromTrigger - } else if clientSecretFromEnv != "" { - metadata.ClientSecret = clientSecretFromEnv - } default: return nil, fmt.Errorf("error parsing auth params") } diff --git a/pkg/scalers/azure_data_explorer_scaler_test.go b/pkg/scalers/azure_data_explorer_scaler_test.go index b80e8854652..f3ce13b7b59 100644 --- a/pkg/scalers/azure_data_explorer_scaler_test.go +++ b/pkg/scalers/azure_data_explorer_scaler_test.go @@ -19,7 +19,6 @@ package scalers import ( "context" "fmt" - "os" "testing" "github.com/go-logr/logr" @@ -86,6 +85,8 @@ var testDataExplorerMetadataWithClientAndSecret = []parseDataExplorerMetadataTes "cloud": "private"}, true}, // All parameters set - pass {map[string]string{"tenantId": azureTenantID, "clientId": aadAppClientID, "clientSecretFromEnv": aadAppSecret, "endpoint": dataExplorerEndpoint, "databaseName": databaseName, "query": dataExplorerQuery, "threshold": dataExplorerThreshold}, false}, + // False because we should not get clientSecret from TriggerMetadata + {map[string]string{"tenantId": azureTenantID, "clientId": aadAppClientID, "clientSecret": aadAppSecret, "endpoint": dataExplorerEndpoint, "databaseName": databaseName, "query": dataExplorerQuery, "threshold": dataExplorerThreshold}, true}, } var testDataExplorerMetadataWithPodIdentity = []parseDataExplorerMetadataTestData{ @@ -102,16 +103,14 @@ var testDataExplorerMetadataWithPodIdentity = []parseDataExplorerMetadataTestDat } var testDataExplorerMetricIdentifiers = []dataExplorerMetricIdentifier{ - {&testDataExplorerMetadataWithClientAndSecret[len(testDataExplorerMetadataWithClientAndSecret)-1], 0, GenerateMetricNameWithIndex(0, kedautil.NormalizeString(fmt.Sprintf("%s-%s", adxName, databaseName)))}, + {&testDataExplorerMetadataWithClientAndSecret[len(testDataExplorerMetadataWithClientAndSecret)-2], 0, GenerateMetricNameWithIndex(0, kedautil.NormalizeString(fmt.Sprintf("%s-%s", adxName, databaseName)))}, {&testDataExplorerMetadataWithPodIdentity[len(testDataExplorerMetadataWithPodIdentity)-1], 1, GenerateMetricNameWithIndex(1, kedautil.NormalizeString(fmt.Sprintf("%s-%s", adxName, databaseName)))}, } func TestDataExplorerParseMetadata(t *testing.T) { // Auth through clientId, clientSecret and tenantId - for _, testData := range testDataExplorerMetadataWithClientAndSecret { - // Setting the key and value of the environment variable to be the same - _ = os.Setenv(testData.metadata["clientSecretFromEnv"], testData.metadata["clientSecretFromEnv"]) - meta, err := parseAzureDataExplorerMetadata( + for id, testData := range testDataExplorerMetadataWithClientAndSecret { + _, err := parseAzureDataExplorerMetadata( &ScalerConfig{ ResolvedEnv: dataExplorerResolvedEnv, TriggerMetadata: testData.metadata, @@ -120,13 +119,10 @@ func TestDataExplorerParseMetadata(t *testing.T) { logr.Discard()) if err != nil && !testData.isError { - t.Error("Expected success but got error", err) + t.Errorf("Test case %d: expected success but got error %v", id, err) } if testData.isError && err == nil { - t.Error("Expected error but got success") - } - if !testData.isError && err == nil && meta.ClientSecret != testData.metadata["clientSecretFromEnv"] { - t.Errorf("Expected clientSecret to be %s but got %s", testData.metadata["clientSecretFromEnv"], meta.ClientSecret) + t.Errorf("Test case %d: expected error but got success", id) } } @@ -166,7 +162,7 @@ func TestDataExplorerParseMetadata(t *testing.T) { } func TestDataExplorerGetMetricSpecForScaling(t *testing.T) { - for _, testData := range testDataExplorerMetricIdentifiers { + for id, testData := range testDataExplorerMetricIdentifiers { meta, err := parseAzureDataExplorerMetadata( &ScalerConfig{ ResolvedEnv: dataExplorerResolvedEnv, @@ -176,7 +172,7 @@ func TestDataExplorerGetMetricSpecForScaling(t *testing.T) { ScalerIndex: testData.scalerIndex}, logr.Discard()) if err != nil { - t.Error("Failed to parse metadata:", err) + t.Errorf("Test case %d: failed to parse metadata: %v", id, err) } mockDataExplorerScaler := azureDataExplorerScaler{ @@ -190,7 +186,7 @@ func TestDataExplorerGetMetricSpecForScaling(t *testing.T) { metricSpec := mockDataExplorerScaler.GetMetricSpecForScaling(context.Background()) metricName := metricSpec[0].External.Metric.Name if metricName != testData.name { - t.Error("Wrong External metric source name:", metricName) + t.Errorf("Test case %d: wrong External metric source name: %v", id, metricName) } } }