Fix scaledObject always matching when browserVersion is latest in tri…

…gger metadata (#4348)
kedacore · Mar 9, 2023 · 3088b4a · 3088b4a
1 parent 846a4d0
commit 3088b4a
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -96,6 +96,7 @@ Here is an overview of all new **experimental** features:
 - **NATS Jetstream Scaler:** Fix compatibility when cluster not on kubernetes ([#4101](https://github.com/kedacore/keda/issues/4101))
 - **Prometheus Metrics**: Expose Prometheus Metrics also when getting ScaledObject state ([#4075](https://github.com/kedacore/keda/issues/4075))
 - **Redis Scalers**: Fix panic produced by incorrect logger initialization ([#4197](https://github.com/kedacore/keda/issues/4197))
+- **Selenium Grid Scaler**: ScaledObject with a trigger whose metadata browserVersion is latest is always being triggered regardless of the browserVersion requested by the user ([#4347](https://github.com/kedacore/keda/issues/4347))
 
 ### Deprecations
 

diff --git a/pkg/scalers/selenium_grid_scaler.go b/pkg/scalers/selenium_grid_scaler.go
@@ -238,7 +238,7 @@ func getCountFromSeleniumResponse(b []byte, browserName string, browserVersion s
 				var platformNameMatches = capability.PlatformName == "" || strings.EqualFold(capability.PlatformName, platformName)
 				if strings.HasPrefix(capability.BrowserVersion, browserVersion) && platformNameMatches {
 					count++
-				} else if browserVersion == DefaultBrowserVersion && platformNameMatches {
+				} else if len(strings.TrimSpace(capability.BrowserVersion)) == 0 && browserVersion == DefaultBrowserVersion && platformNameMatches {
 					count++
 				}
 			}
@@ -255,7 +255,7 @@ func getCountFromSeleniumResponse(b []byte, browserName string, browserVersion s
 			if capability.BrowserName == sessionBrowserName {
 				if strings.HasPrefix(capability.BrowserVersion, browserVersion) && platformNameMatches {
 					count++
-				} else if browserVersion == DefaultBrowserVersion && platformNameMatches {
+				} else if len(strings.TrimSpace(capability.BrowserVersion)) == 0 && browserVersion == DefaultBrowserVersion && platformNameMatches {
 					count++
 				}
 			}

diff --git a/pkg/scalers/selenium_grid_scaler_test.go b/pkg/scalers/selenium_grid_scaler_test.go
@@ -112,14 +112,36 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "2 active sessions with matching browsername on 2 nodes and maxSession=4 should return count as 3 (rounded up from 2.5)",
+			name: "2 session queue with matching browsername and browserversion should return count as 1",
 			args: args{
 				b: []byte(`{
 					"data": {
 						"grid":{
 							"maxSession": 4,
 							"nodeCount": 2
 						},
+						"sessionsInfo": {
+							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\",\n \"browserVersion\": \"91.0\"\n}","{\n  \"browserName\": \"chrome\"\n}","{\n  \"browserName\": \"chrome\"\n}"]
+						}
+					}
+				}`),
+				browserName:        "chrome",
+				sessionBrowserName: "chrome",
+				browserVersion:     "latest",
+				platformName:       "linux",
+			},
+			want:    1,
+			wantErr: false,
+		},
+		{
+			name: "2 active sessions with matching browsername on 2 nodes and maxSession=4 should return count as 1 (rounded up from 0.75)",
+			args: args{
+				b: []byte(`{
+					"data": {
+						"grid":{
+							"maxSession": 4,
+							"nodeCount": 1
+						},
 						"sessionsInfo": {
 							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\",\n \"browserVersion\": \"91.0\"\n}","{\n  \"browserName\": \"chrome\"\n}","{\n  \"browserName\": \"chrome\"\n}"],
 							"sessions": [
@@ -139,14 +161,14 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 				}`),
 				browserName:        "chrome",
 				sessionBrowserName: "chrome",
-				browserVersion:     "latest",
+				browserVersion:     "91.0",
 				platformName:       "linux",
 			},
-			want:    3,
+			want:    1,
 			wantErr: false,
 		},
 		{
-			name: "2 active sessions with matching browsername on 1 node and maxSession=3 should return count as 2 (rounded up from 1.33)",
+			name: "2 active sessions with matching browsername on 1 node and maxSession=3 should return count as 1 (rounded up from 0.33)",
 			args: args{
 				b: []byte(`{
 					"data": {
@@ -176,20 +198,20 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 				browserVersion:     "latest",
 				platformName:       "linux",
 			},
-			want:    2,
+			want:    1,
 			wantErr: false,
 		},
 		{
 			name: "2 active sessions with matching browsername on 2 nodes should return count as 5",
 			args: args{
 				b: []byte(`{
 					"data": {
+						"grid":{
+							"maxSession": 2,
+							"nodeCount": 2
+						},
 						"sessionsInfo": {
-							"grid":{
-								"maxSession": 2,
-								"nodeCount": 2
-							},
-							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\"\n}","{\n  \"browserName\": \"chrome\"\n}","{\n  \"browserName\": \"chrome\"\n}"],
+							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\",\n \"browserVersion\": \"91.0\"\n}","{\n  \"browserName\": \"chrome\",\n \"browserVersion\": \"91.0\"\n}","{\n  \"browserName\": \"chrome\",\n \"browserVersion\": \"91.0\"\n}"],
 							"sessions": [
 								{
 									"id": "0f9c5a941aa4d755a54b84be1f6535b1",
@@ -207,7 +229,7 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 				}`),
 				browserName:        "chrome",
 				sessionBrowserName: "chrome",
-				browserVersion:     "latest",
+				browserVersion:     "91.0",
 				platformName:       "linux",
 			},
 			want:    5,
@@ -252,7 +274,7 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 							"nodeCount": 1
 						},
 						"sessionsInfo": {
-							"sessionQueueRequests": ["{\n  \"browserName\": \"MicrosoftEdge\"\n}","{\n  \"browserName\": \"MicrosoftEdge\"\n}"],
+							"sessionQueueRequests": ["{\n  \"browserName\": \"MicrosoftEdge\",\n \"browserVersion\": \"91.0\"\n}","{\n  \"browserName\": \"MicrosoftEdge\",\n \"browserVersion\": \"91.0\"\n}"],
 							"sessions": [
 								{
 									"id": "0f9c5a941aa4d755a54b84be1f6535b1",
@@ -265,7 +287,7 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 				}`),
 				browserName:        "MicrosoftEdge",
 				sessionBrowserName: "msedge",
-				browserVersion:     "latest",
+				browserVersion:     "91.0",
 				platformName:       "linux",
 			},
 			want:    3,
@@ -385,7 +407,7 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 							"nodeCount": 1
 						},
 						"sessionsInfo": {
-							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\",\n \"platformName\": \"linux\"\n}","{\n  \"browserName\": \"chrome\",\n \"platformName\": \"Windows 11\"\n}"],
+							"sessionQueueRequests": ["{\n  \"browserName\": \"chrome\",\n \"platformName\": \"linux\"\n}","{\n  \"browserName\": \"chrome\",\n \"platformName\": \"Windows 11\",\n \"browserVersion\": \"91.0\"\n}"],
 							"sessions": [
 								{
 									"id": "0f9c5a941aa4d755a54b84be1f6535b1",
@@ -403,7 +425,7 @@ func Test_getCountFromSeleniumResponse(t *testing.T) {
 				}`),
 				browserName:        "chrome",
 				sessionBrowserName: "chrome",
-				browserVersion:     "latest",
+				browserVersion:     "91.0",
 				platformName:       "Windows 11",
 			},
 			want:    2,