From c7e830ab8ac69c559cfcb2aa4de5cf62272dca14 Mon Sep 17 00:00:00 2001 From: Jorge Turrado Ferrero Date: Fri, 9 Dec 2022 11:42:36 +0100 Subject: [PATCH] fix(mongodb): escape username and password (#3989) Fixes https://github.com/kedacore/keda/issues/3992 Signed-off-by: Pedro Tanaka --- BUILD.md | 10 ++++++-- CHANGELOG.md | 1 + pkg/scalers/mongo_scaler.go | 25 +++++++++++-------- pkg/scalers/mongo_scaler_test.go | 41 +++++++++++++++++++++++++++----- 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/BUILD.md b/BUILD.md index 5c7d28ca3ff..26a96382646 100644 --- a/BUILD.md +++ b/BUILD.md @@ -138,7 +138,10 @@ Follow these instructions if you want to debug the KEDA operator using VS Code. "request": "launch", "mode": "debug", "program": "${workspaceFolder}/main.go", - "env": {"WATCH_NAMESPACE": ""} + "env": { + "WATCH_NAMESPACE": "", + "KEDA_CLUSTER_OBJECT_NAMESPACE": "keda" + } } ] } @@ -171,7 +174,10 @@ Follow these instructions if you want to debug the KEDA metrics server using VS "request": "launch", "mode": "auto", "program": "${workspaceFolder}/adapter/main.go", - "env": {"WATCH_NAMESPACE": ""}, + "env": { + "WATCH_NAMESPACE": "", + "KEDA_CLUSTER_OBJECT_NAMESPACE": "keda" + }, "args": [ "--authentication-kubeconfig=PATH_TO_YOUR_KUBECONFIG", "--authentication-skip-lookup", diff --git a/CHANGELOG.md b/CHANGELOG.md index ed3bed53365..c1e17c214b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md - **Azure Blob Scaler** Store forgotten logger ([#3811](https://github.com/kedacore/keda/issues/3811)) - **Datadog Scaler** The last data point of some specific query is always null ([#3906](https://github.com/kedacore/keda/issues/3906)) - **GCP Stackdriver Scalar:** Update Stackdriver client to handle detecting double and int64 value types ([#3777](https://github.com/kedacore/keda/issues/3777)) +- **MongoDB Scaler:** Username/password can contain `:/?#[]@` ([#3992](https://github.com/kedacore/keda/issues/3992)) - **New Relic Scaler** Store forgotten logger ([#3945](https://github.com/kedacore/keda/issues/3945)) - **Prometheus Scaler:** Treat Inf the same as Null result ([#3644](https://github.com/kedacore/keda/issues/3644)) - **NATS Jetstream:** Correctly count messages that should be redelivered (waiting for ack) towards keda value ([#3787](https://github.com/kedacore/keda/issues/3787)) diff --git a/pkg/scalers/mongo_scaler.go b/pkg/scalers/mongo_scaler.go index bba3ca9ebbd..82034a20a45 100644 --- a/pkg/scalers/mongo_scaler.go +++ b/pkg/scalers/mongo_scaler.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "net" + "net/url" "strconv" "time" @@ -130,7 +132,7 @@ func parseMongoDBMetadata(config *ScalerConfig) (*mongoDBMetadata, string, error if val, ok := config.TriggerMetadata["queryValue"]; ok { queryValue, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, "", fmt.Errorf("failed to convert %v to int, because of %v", queryValue, err.Error()) + return nil, "", fmt.Errorf("failed to convert %v to int, because of %v", val, err.Error()) } meta.queryValue = queryValue } else { @@ -141,15 +143,16 @@ func parseMongoDBMetadata(config *ScalerConfig) (*mongoDBMetadata, string, error if val, ok := config.TriggerMetadata["activationQueryValue"]; ok { activationQueryValue, err := strconv.ParseInt(val, 10, 64) if err != nil { - return nil, "", fmt.Errorf("failed to convert %v to int, because of %v", activationQueryValue, err.Error()) + return nil, "", fmt.Errorf("failed to convert %v to int, because of %v", val, err.Error()) } meta.activationQueryValue = activationQueryValue } - meta.dbName, err = GetFromAuthOrMeta(config, "dbName") + dbName, err := GetFromAuthOrMeta(config, "dbName") if err != nil { return nil, "", err } + meta.dbName = dbName // Resolve connectionString switch { @@ -159,20 +162,23 @@ func parseMongoDBMetadata(config *ScalerConfig) (*mongoDBMetadata, string, error meta.connectionString = config.ResolvedEnv[config.TriggerMetadata["connectionStringFromEnv"]] default: meta.connectionString = "" - meta.host, err = GetFromAuthOrMeta(config, "host") + host, err := GetFromAuthOrMeta(config, "host") if err != nil { return nil, "", err } + meta.host = host - meta.port, err = GetFromAuthOrMeta(config, "port") + port, err := GetFromAuthOrMeta(config, "port") if err != nil { return nil, "", err } + meta.port = port - meta.username, err = GetFromAuthOrMeta(config, "username") + username, err := GetFromAuthOrMeta(config, "username") if err != nil { return nil, "", err } + meta.username = username if config.AuthParams["password"] != "" { meta.password = config.AuthParams["password"] @@ -188,9 +194,8 @@ func parseMongoDBMetadata(config *ScalerConfig) (*mongoDBMetadata, string, error connStr = meta.connectionString } else { // Build connection str - addr := fmt.Sprintf("%s:%s", meta.host, meta.port) - auth := fmt.Sprintf("%s:%s", meta.username, meta.password) - connStr = "mongodb://" + auth + "@" + addr + "/" + meta.dbName + addr := net.JoinHostPort(meta.host, meta.port) + connStr = fmt.Sprintf("mongodb://%s:%s@%s/%s", url.QueryEscape(meta.username), url.QueryEscape(meta.password), addr, meta.dbName) } if val, ok := config.TriggerMetadata["metricName"]; ok { @@ -245,7 +250,7 @@ func (s *mongoDBScaler) getQueryResult(ctx context.Context) (int64, error) { } // GetMetrics query from mongoDB,and return to external metrics -func (s *mongoDBScaler) GetMetrics(ctx context.Context, metricName string, metricSelector labels.Selector) ([]external_metrics.ExternalMetricValue, error) { +func (s *mongoDBScaler) GetMetrics(ctx context.Context, metricName string, _ labels.Selector) ([]external_metrics.ExternalMetricValue, error) { num, err := s.getQueryResult(ctx) if err != nil { return []external_metrics.ExternalMetricValue{}, fmt.Errorf("failed to inspect momgoDB, because of %v", err) diff --git a/pkg/scalers/mongo_scaler_test.go b/pkg/scalers/mongo_scaler_test.go index 303ccf9a3a6..1550ede6fbb 100644 --- a/pkg/scalers/mongo_scaler_test.go +++ b/pkg/scalers/mongo_scaler_test.go @@ -5,12 +5,13 @@ import ( "testing" "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" "go.mongodb.org/mongo-driver/mongo" ) var testMongoDBResolvedEnv = map[string]string{ - "MongoDB_CONN_STR": "test_conn_str", - "MongoDB_PASSWORD": "test", + "MongoDB_CONN_STR": "mongodb://mongodb0.example.com:27017", + "MongoDB_PASSWORD": "test@password", } type parseMongoDBMetadataTestData struct { @@ -20,6 +21,11 @@ type parseMongoDBMetadataTestData struct { raisesError bool } +type mongoDBConnectionStringTestData struct { + metadataTestData *parseMongoDBMetadataTestData + connectionString string +} + type mongoDBMetricIdentifier struct { metadataTestData *parseMongoDBMetadataTestData scalerIndex int @@ -36,22 +42,29 @@ var testMONGODBMetadata = []parseMongoDBMetadataTestData{ }, // connectionStringFromEnv { - metadata: map[string]string{"query": `{"name":"John"}`, "collection": "demo", "queryValue": "12", "connectionStringFromEnv": "Mongo_CONN_STR", "dbName": "test"}, + metadata: map[string]string{"query": `{"name":"John"}`, "collection": "demo", "queryValue": "12", "connectionStringFromEnv": "MongoDB_CONN_STR", "dbName": "test"}, authParams: map[string]string{}, resolvedEnv: testMongoDBResolvedEnv, raisesError: false, }, // with metric name { - metadata: map[string]string{"query": `{"name":"John"}`, "metricName": "hpa", "collection": "demo", "queryValue": "12", "connectionStringFromEnv": "Mongo_CONN_STR", "dbName": "test"}, + metadata: map[string]string{"query": `{"name":"John"}`, "metricName": "hpa", "collection": "demo", "queryValue": "12", "connectionStringFromEnv": "MongoDB_CONN_STR", "dbName": "test"}, authParams: map[string]string{}, resolvedEnv: testMongoDBResolvedEnv, raisesError: false, }, + // from passwordFromEnv + { + metadata: map[string]string{"query": `{"name":"John"}`, "metricName": "hpa", "collection": "demo", "queryValue": "12", "passwordFromEnv": "MongoDB_PASSWORD"}, + authParams: map[string]string{"dbName": "test", "host": "localshot", "port": "1234", "username": "sample"}, + resolvedEnv: testMongoDBResolvedEnv, + raisesError: false, + }, // from trigger auth { metadata: map[string]string{"query": `{"name":"John"}`, "metricName": "hpa", "collection": "demo", "queryValue": "12"}, - authParams: map[string]string{"dbName": "test", "host": "localshot", "port": "1234", "username": "sample", "password": "secure"}, + authParams: map[string]string{"dbName": "test", "host": "localshot", "port": "1234", "username": "sample", "password": "sec@ure"}, resolvedEnv: testMongoDBResolvedEnv, raisesError: false, }, @@ -64,6 +77,12 @@ var testMONGODBMetadata = []parseMongoDBMetadataTestData{ }, } +var mongoDBConnectionStringTestDatas = []mongoDBConnectionStringTestData{ + {metadataTestData: &testMONGODBMetadata[2], connectionString: "mongodb://mongodb0.example.com:27017"}, + {metadataTestData: &testMONGODBMetadata[3], connectionString: "mongodb://sample:test%40password@localshot:1234/test"}, + {metadataTestData: &testMONGODBMetadata[4], connectionString: "mongodb://sample:sec%40ure@localshot:1234/test"}, +} + var mongoDBMetricIdentifiers = []mongoDBMetricIdentifier{ {metadataTestData: &testMONGODBMetadata[2], scalerIndex: 0, name: "s0-mongodb-hpa"}, {metadataTestData: &testMONGODBMetadata[2], scalerIndex: 1, name: "s1-mongodb-hpa"}, @@ -71,7 +90,7 @@ var mongoDBMetricIdentifiers = []mongoDBMetricIdentifier{ func TestParseMongoDBMetadata(t *testing.T) { for _, testData := range testMONGODBMetadata { - _, _, err := parseMongoDBMetadata(&ScalerConfig{TriggerMetadata: testData.metadata, AuthParams: testData.authParams}) + _, _, err := parseMongoDBMetadata(&ScalerConfig{ResolvedEnv: testData.resolvedEnv, TriggerMetadata: testData.metadata, AuthParams: testData.authParams}) if err != nil && !testData.raisesError { t.Error("Expected success but got error:", err) } @@ -81,6 +100,16 @@ func TestParseMongoDBMetadata(t *testing.T) { } } +func TestParseMongoDBConnectionString(t *testing.T) { + for _, testData := range mongoDBConnectionStringTestDatas { + _, connStr, err := parseMongoDBMetadata(&ScalerConfig{ResolvedEnv: testData.metadataTestData.resolvedEnv, TriggerMetadata: testData.metadataTestData.metadata, AuthParams: testData.metadataTestData.authParams}) + if err != nil { + t.Error("Expected success but got error:", err) + } + assert.Equal(t, testData.connectionString, connStr) + } +} + func TestMongoDBGetMetricSpecForScaling(t *testing.T) { for _, testData := range mongoDBMetricIdentifiers { meta, _, err := parseMongoDBMetadata(&ScalerConfig{ResolvedEnv: testData.metadataTestData.resolvedEnv, AuthParams: testData.metadataTestData.authParams, TriggerMetadata: testData.metadataTestData.metadata, ScalerIndex: testData.scalerIndex})