fix(mongodb): escape username and password (kedacore#3989)

Fixes kedacore#3992

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",

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))

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)

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,14 +77,20 @@ 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"},
 }
 
 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})