fix(metricproviders): Check and handle invalid server URL. Fixed argoproj#1444 (argoproj#1534)

fix(metricproviders): Check and handle invalid server URL. Fixed argoproj#1444 (argoproj#1534)

Signed-off-by: Noam Gal <[email protected]>

Author: ATGardner

USERS.md

@@ -6,6 +6,7 @@ Organizations below are **officially** using Argo Rollouts. Please send a PR wit
 1. [Ambassador Labs](https://www.getambassador.io)
 1. [Ant Group](https://www.antgroup.com/)
 1. [Bucketplace](https://www.bucketplace.co.kr/)
+1. [Codefresh](https://codefresh.io/)
 1. [Databricks](https://github.com/databricks)
 1. [Devtron Labs](https://github.com/devtron-labs/devtron)
 1. [Intuit](https://www.intuit.com/)

metricproviders/datadog/datadog.go

@@ -70,7 +70,10 @@ func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alph
 		endpoint = p.config.Address + "/api/v1/query"
 	}
 
-	url, _ := url.Parse(endpoint)
+	url, err := url.Parse(endpoint)
+	if err != nil {
+		return metricutil.MarkMeasurementError(measurement, err)
+	}
 
 	now := unixNow()
 	var interval int64 = 300

metricproviders/datadog/datadog_test.go

@@ -27,6 +27,7 @@ func TestRunSuite(t *testing.T) {
 
 	// Test Cases
 	var tests = []struct {
+		serverURL               string
 		webServerStatus         int
 		webServerResponse       string
 		metric                  v1alpha1.Metric
@@ -172,62 +173,77 @@ func TestRunSuite(t *testing.T) {
 			expectedErrorMessage:    "Could not parse JSON body: json: cannot unmarshal string into Go struct field datadogResponse.Series of type []struct { Pointlist [][]float64 \"json:\\\"pointlist\\\"\" }",
 			useEnvVarForKeys:        false,
 		},
+
+		// Error if server address is faulty
+		{
+			serverURL:            "://wrong.schema",
+			metric:               v1alpha1.Metric{},
+			expectedPhase:        v1alpha1.AnalysisPhaseError,
+			expectedErrorMessage: "parse \"://wrong.schema/api/v1/query\": missing protocol scheme",
+			useEnvVarForKeys:     false,
+		},
 	}
 
 	// Run
 
 	for _, test := range tests {
-		// Server setup with response
-		server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		serverURL := test.serverURL
 
-			//Check query variables
-			actualQuery := req.URL.Query().Get("query")
-			actualFrom := req.URL.Query().Get("from")
-			actualTo := req.URL.Query().Get("to")
+		if serverURL == "" {
+			// Server setup with response
+			server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 
-			if actualQuery != "avg:kubernetes.cpu.user.total{*}" {
-				t.Errorf("\nquery expected avg:kubernetes.cpu.user.total{*} but got %s", actualQuery)
-			}
+				//Check query variables
+				actualQuery := req.URL.Query().Get("query")
+				actualFrom := req.URL.Query().Get("from")
+				actualTo := req.URL.Query().Get("to")
 
-			if from, err := strconv.ParseInt(actualFrom, 10, 64); err == nil && from != unixNow()-test.expectedIntervalSeconds {
-				t.Errorf("\nfrom %d expected be equal to %d", from, unixNow()-test.expectedIntervalSeconds)
-			} else if err != nil {
-				t.Errorf("\nfailed to parse from: %v", err)
-			}
+				if actualQuery != "avg:kubernetes.cpu.user.total{*}" {
+					t.Errorf("\nquery expected avg:kubernetes.cpu.user.total{*} but got %s", actualQuery)
+				}
 
-			if to, err := strconv.ParseInt(actualTo, 10, 64); err == nil && to != unixNow() {
-				t.Errorf("\nto %d was expected be equal to %d", to, unixNow())
-			} else if err != nil {
-				t.Errorf("\nfailed to parse to: %v", err)
-			}
+				if from, err := strconv.ParseInt(actualFrom, 10, 64); err == nil && from != unixNow()-test.expectedIntervalSeconds {
+					t.Errorf("\nfrom %d expected be equal to %d", from, unixNow()-test.expectedIntervalSeconds)
+				} else if err != nil {
+					t.Errorf("\nfailed to parse from: %v", err)
+				}
 
-			//Check headers
-			if req.Header.Get("Content-Type") != "application/json" {
-				t.Errorf("\nContent-Type header expected to be application/json but got %s", req.Header.Get("Content-Type"))
-			}
-			if req.Header.Get("DD-API-KEY") != expectedApiKey {
-				t.Errorf("\nDD-API-KEY header expected %s but got %s", expectedApiKey, req.Header.Get("DD-API-KEY"))
-			}
-			if req.Header.Get("DD-APPLICATION-KEY") != expectedAppKey {
-				t.Errorf("\nDD-APPLICATION-KEY header expected %s but got %s", expectedAppKey, req.Header.Get("DD-APPLICATION-KEY"))
-			}
+				if to, err := strconv.ParseInt(actualTo, 10, 64); err == nil && to != unixNow() {
+					t.Errorf("\nto %d was expected be equal to %d", to, unixNow())
+				} else if err != nil {
+					t.Errorf("\nfailed to parse to: %v", err)
+				}
 
-			// Return mock response
-			if test.webServerStatus < 200 || test.webServerStatus >= 300 {
-				http.Error(rw, test.webServerResponse, test.webServerStatus)
-			} else {
-				rw.Header().Set("Content-Type", "application/json")
-				io.WriteString(rw, test.webServerResponse)
-			}
-		}))
-		defer server.Close()
+				//Check headers
+				if req.Header.Get("Content-Type") != "application/json" {
+					t.Errorf("\nContent-Type header expected to be application/json but got %s", req.Header.Get("Content-Type"))
+				}
+				if req.Header.Get("DD-API-KEY") != expectedApiKey {
+					t.Errorf("\nDD-API-KEY header expected %s but got %s", expectedApiKey, req.Header.Get("DD-API-KEY"))
+				}
+				if req.Header.Get("DD-APPLICATION-KEY") != expectedAppKey {
+					t.Errorf("\nDD-APPLICATION-KEY header expected %s but got %s", expectedAppKey, req.Header.Get("DD-APPLICATION-KEY"))
+				}
+
+				// Return mock response
+				if test.webServerStatus < 200 || test.webServerStatus >= 300 {
+					http.Error(rw, test.webServerResponse, test.webServerStatus)
+				} else {
+					rw.Header().Set("Content-Type", "application/json")
+					io.WriteString(rw, test.webServerResponse)
+				}
+			}))
+			defer server.Close()
+
+			serverURL = server.URL
+		}
 
 		tokenSecret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: DatadogTokensSecretName,
 			},
 			Data: map[string][]byte{
-				"address": []byte(server.URL),
+				"address": []byte(serverURL),
 				"api-key": []byte(expectedApiKey),
 				"app-key": []byte(expectedAppKey),
 			},
@@ -236,7 +252,7 @@ func TestRunSuite(t *testing.T) {
 		if test.useEnvVarForKeys {
 			os.Setenv("DD_API_KEY", expectedApiKey)
 			os.Setenv("DD_APP_KEY", expectedAppKey)
-			os.Setenv("DD_ADDRESS", server.URL)
+			os.Setenv("DD_ADDRESS", serverURL)
 		} else {
 			os.Unsetenv("DD_API_KEY")
 			os.Unsetenv("DD_APP_KEY")