From 4f84471bc9d25523d0ea4eafe3eebf695afa8dc3 Mon Sep 17 00:00:00 2001 From: chunderbolt <59027738+chunderbolt@users.noreply.github.com> Date: Mon, 23 Oct 2023 22:15:29 +0100 Subject: [PATCH] fix: URL encode poolName in azure-pipelines (#5120) Co-authored-by: Jorge Turrado Ferrero --- CHANGELOG.md | 1 + pkg/scalers/azure_pipelines_scaler.go | 23 +++++++++++----------- pkg/scalers/azure_pipelines_scaler_test.go | 4 +++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36accd6deb1..612767ea480 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,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**: No more HTTP 400 errors produced by poolName with spaces ([#5107](https://github.com/kedacore/keda/issues/5107)) ### Deprecations diff --git a/pkg/scalers/azure_pipelines_scaler.go b/pkg/scalers/azure_pipelines_scaler.go index 9b2dfa7cbb1..c94aabf96f9 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,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, 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 } @@ -290,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) } @@ -305,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 } @@ -325,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 @@ -333,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 } diff --git a/pkg/scalers/azure_pipelines_scaler_test.go b/pkg/scalers/azure_pipelines_scaler_test.go index 058063d4b31..99eb81140ef 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) { @@ -109,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))