From 772beed4de65b46a14b0c1a4125a0670e17df2c7 Mon Sep 17 00:00:00 2001 From: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:12:01 +0100 Subject: [PATCH 1/6] Escape pool name when fetching pool ID Signed-off-by: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> --- pkg/scalers/azure_pipelines_scaler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 9b2dfa7cbb1..4e251a5e3a6 100644 --- a/pkg/scalers/azure_pipelines_scaler.go +++ b/pkg/scalers/azure_pipelines_scaler.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strconv" "strings" "time" @@ -265,7 +266,7 @@ func parseAzurePipelinesMetadata(ctx context.Context, config *ScalerConfig, http } func getPoolIDFromName(ctx context.Context, poolName string, metadata *azurePipelinesMetadata, httpClient *http.Client) (int, error) { - url := fmt.Sprintf("%s/_apis/distributedtask/pools?poolName=%s", metadata.organizationURL, poolName) + url := fmt.Sprintf("%s/_apis/distributedtask/pools?poolName=%s", metadata.organizationURL, url.QueryEscape(poolName)) body, err := getAzurePipelineRequest(ctx, url, metadata, httpClient) if err != nil { return -1, err From 4881b0a34e0ea77b799bbe7d7289eee1a9c4178f Mon Sep 17 00:00:00 2001 From: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:09:45 +0100 Subject: [PATCH 2/6] Change "url" in azure_pipelines_scaler to "urlString" Signed-off-by: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> --- pkg/scalers/azure_pipelines_scaler.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 4e251a5e3a6..c94aabf96f9 100644 --- a/pkg/scalers/azure_pipelines_scaler.go +++ b/pkg/scalers/azure_pipelines_scaler.go @@ -266,8 +266,8 @@ func parseAzurePipelinesMetadata(ctx context.Context, config *ScalerConfig, http } func getPoolIDFromName(ctx context.Context, poolName string, metadata *azurePipelinesMetadata, httpClient *http.Client) (int, error) { - url := fmt.Sprintf("%s/_apis/distributedtask/pools?poolName=%s", metadata.organizationURL, url.QueryEscape(poolName)) - body, err := getAzurePipelineRequest(ctx, url, metadata, httpClient) + urlString := fmt.Sprintf("%s/_apis/distributedtask/pools?poolName=%s", metadata.organizationURL, url.QueryEscape(poolName)) + body, err := getAzurePipelineRequest(ctx, urlString, metadata, httpClient) if err != nil { return -1, err } @@ -291,8 +291,8 @@ func getPoolIDFromName(ctx context.Context, poolName string, metadata *azurePipe } func validatePoolID(ctx context.Context, poolID string, metadata *azurePipelinesMetadata, httpClient *http.Client) (int, error) { - url := fmt.Sprintf("%s/_apis/distributedtask/pools?poolID=%s", metadata.organizationURL, poolID) - body, err := getAzurePipelineRequest(ctx, url, metadata, httpClient) + urlString := fmt.Sprintf("%s/_apis/distributedtask/pools?poolID=%s", metadata.organizationURL, poolID) + body, err := getAzurePipelineRequest(ctx, urlString, metadata, httpClient) if err != nil { return -1, fmt.Errorf("agent pool with id `%s` not found: %w", poolID, err) } @@ -306,8 +306,8 @@ func validatePoolID(ctx context.Context, poolID string, metadata *azurePipelines return result.ID, nil } -func getAzurePipelineRequest(ctx context.Context, url string, metadata *azurePipelinesMetadata, httpClient *http.Client) ([]byte, error) { - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) +func getAzurePipelineRequest(ctx context.Context, urlString string, metadata *azurePipelinesMetadata, httpClient *http.Client) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, "GET", urlString, nil) if err != nil { return []byte{}, err } @@ -326,7 +326,7 @@ func getAzurePipelineRequest(ctx context.Context, url string, metadata *azurePip r.Body.Close() if !(r.StatusCode >= 200 && r.StatusCode <= 299) { - return []byte{}, fmt.Errorf("the Azure DevOps REST API returned error. url: %s status: %d response: %s", url, r.StatusCode, string(b)) + return []byte{}, fmt.Errorf("the Azure DevOps REST API returned error. urlString: %s status: %d response: %s", urlString, r.StatusCode, string(b)) } return b, nil @@ -334,13 +334,13 @@ func getAzurePipelineRequest(ctx context.Context, url string, metadata *azurePip func (s *azurePipelinesScaler) GetAzurePipelinesQueueLength(ctx context.Context) (int64, error) { // HotFix Issue (#4387), $top changes the format of the returned JSON - var url string + var urlString string if s.metadata.parent != "" { - url = fmt.Sprintf("%s/_apis/distributedtask/pools/%d/jobrequests", s.metadata.organizationURL, s.metadata.poolID) + urlString = fmt.Sprintf("%s/_apis/distributedtask/pools/%d/jobrequests", s.metadata.organizationURL, s.metadata.poolID) } else { - url = fmt.Sprintf("%s/_apis/distributedtask/pools/%d/jobrequests?$top=%d", s.metadata.organizationURL, s.metadata.poolID, s.metadata.jobsToFetch) + urlString = fmt.Sprintf("%s/_apis/distributedtask/pools/%d/jobrequests?$top=%d", s.metadata.organizationURL, s.metadata.poolID, s.metadata.jobsToFetch) } - body, err := getAzurePipelineRequest(ctx, url, s.metadata, s.httpClient) + body, err := getAzurePipelineRequest(ctx, urlString, s.metadata, s.httpClient) if err != nil { return -1, err } From 1d1db030a0538fd4f5fd62bf72050b35eee3ba5d Mon Sep 17 00:00:00 2001 From: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 16:56:53 +0100 Subject: [PATCH 3/6] Add a test for poolName with spaces Signed-off-by: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> --- pkg/scalers/azure_pipelines_scaler_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/scalers/azure_pipelines_scaler_test.go b/pkg/scalers/azure_pipelines_scaler_test.go index 058063d4b31..5bae6756bb9 100644 --- a/pkg/scalers/azure_pipelines_scaler_test.go +++ b/pkg/scalers/azure_pipelines_scaler_test.go @@ -101,6 +101,8 @@ var testValidateAzurePipelinesPoolData = []validateAzurePipelinesPoolTestData{ {"poolName doesn't exist", map[string]string{"poolName": "sample"}, true, "poolName", http.StatusOK, `{"count":0,"value":[]}`}, // poolName is used if poolName and poolID are defined {"poolName is used if poolName and poolID are defined", map[string]string{"poolName": "sample", "poolID": "1"}, false, "poolName", http.StatusOK, `{"count":1,"value":[{"id":1}]}`}, + // poolName can have a space in it + {"poolName can have a space in it", map[string]string{"poolName": "sample pool name"}, false, "poolName", http.StatusOK, `{"count":1,"value":[{"id":1}]}`}, } func TestValidateAzurePipelinesPool(t *testing.T) { From ee6917e93c464ad6028d9efe72fbf5aa61f9f0df Mon Sep 17 00:00:00 2001 From: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:57:20 +0100 Subject: [PATCH 4/6] Add to CHANGELOG.md Signed-off-by: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dffb5458c4..a486f4931e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,7 @@ Here is an overview of all new **experimental** features: ### Fixes - **General**: Prevented stuck status due to timeouts during scalers generation ([#5083](https://github.com/kedacore/keda/issues/5083)) +- **Azure Pipelines**: Stop pool names with spaces causing HTTP 400 errors([#5107](https://github.com/kedacore/keda/issues/5107)) ### Deprecations From 9f8edaacfc126a70ebc041f048bd89e8d1a0b879 Mon Sep 17 00:00:00 2001 From: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 20:10:00 +0100 Subject: [PATCH 5/6] Fix spelling error Signed-off-by: Chunderbolt <59027738+chunderbolt@users.noreply.github.com> --- pkg/scalers/azure_pipelines_scaler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scalers/azure_pipelines_scaler_test.go b/pkg/scalers/azure_pipelines_scaler_test.go index 5bae6756bb9..99eb81140ef 100644 --- a/pkg/scalers/azure_pipelines_scaler_test.go +++ b/pkg/scalers/azure_pipelines_scaler_test.go @@ -111,7 +111,7 @@ func TestValidateAzurePipelinesPool(t *testing.T) { var apiStub = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _, ok := r.URL.Query()[testData.queryParam] if !ok { - t.Error("Worng QueryParam") + t.Error("Wrong QueryParam") } w.WriteHeader(testData.httpCode) _, _ = w.Write([]byte(testData.response)) From bc7c4d0e4a78a484b13c1a26de30de0609904472 Mon Sep 17 00:00:00 2001 From: Jorge Turrado Ferrero Date: Mon, 23 Oct 2023 22:19:19 +0200 Subject: [PATCH 6/6] Update CHANGELOG.md Signed-off-by: Jorge Turrado Ferrero --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a486f4931e2..a115afbcd5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,7 +70,7 @@ Here is an overview of all new **experimental** features: ### Fixes - **General**: Prevented stuck status due to timeouts during scalers generation ([#5083](https://github.com/kedacore/keda/issues/5083)) -- **Azure Pipelines**: Stop pool names with spaces causing HTTP 400 errors([#5107](https://github.com/kedacore/keda/issues/5107)) +- **Azure Pipelines**: No more HTTP 400 errors produced by poolName with spaces ([#5107](https://github.com/kedacore/keda/issues/5107)) ### Deprecations