fix(metrics-operator): fix url encoding in DT metrics queries (#1893)

Signed-off-by: Florian Bacher <[email protected]> Co-authored-by: Giovanni Liva <[email protected]>
keptn · Aug 21, 2023 · 5437df9 · 5437df9
1 parent 06defd3
commit 5437df9
Show file tree

Hide file tree

Showing 4 changed files with 316 additions and 227 deletions.
diff --git a/metrics-operator/controllers/common/providers/dynatrace/common.go b/metrics-operator/controllers/common/providers/dynatrace/common.go
@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/url"
+	"strings"
 
 	metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3"
 	corev1 "k8s.io/api/core/v1"
@@ -37,3 +39,22 @@ func getDTSecret(ctx context.Context, provider metricsapi.KeptnMetricsProvider,
 	}
 	return string(token), nil
 }
+
+func urlEncodeQuery(query string) string {
+	params := strings.Split(query, "&")
+
+	result := ""
+
+	for i, param := range params {
+		keyAndValue := strings.Split(param, "=")
+		if len(keyAndValue) == 2 {
+			encodedKeyAndValue := keyAndValue[0] + "=" + url.QueryEscape(keyAndValue[1])
+			if i > 0 {
+				result += "&"
+			}
+			result += encodedKeyAndValue
+		}
+	}
+
+	return result
+}
diff --git a/metrics-operator/controllers/common/providers/dynatrace/common_test.go b/metrics-operator/controllers/common/providers/dynatrace/common_test.go
@@ -67,3 +67,42 @@ func TestGetSecret_NoTokenData(t *testing.T) {
 	require.ErrorIs(t, e, ErrSecretKeyRefNotDefined)
 	require.Empty(t, r)
 }
+
+func Test_urlEncodeQuery(t *testing.T) {
+	type args struct {
+		query string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "encode single parameter",
+			args: args{
+				query: "metricSelector=my:metric-selector",
+			},
+			want: "metricSelector=my%3Ametric-selector",
+		},
+		{
+			name: "encode multiple parameters",
+			args: args{
+				query: "metricSelector=my:metric-selector&from=now-2h",
+			},
+			want: "metricSelector=my%3Ametric-selector&from=now-2h",
+		},
+		{
+			name: "omit wrongly formatted input",
+			args: args{
+				query: "metricSelector=my:metric-selector&from",
+			},
+			want: "metricSelector=my%3Ametric-selector",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := urlEncodeQuery(tt.args.query)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
diff --git a/metrics-operator/controllers/common/providers/dynatrace/dynatrace.go b/metrics-operator/controllers/common/providers/dynatrace/dynatrace.go
@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"reflect"
 	"strings"
 	"time"
@@ -42,14 +41,17 @@ type DynatraceData struct {
 // EvaluateQuery fetches the SLI values from dynatrace provider
 func (d *KeptnDynatraceProvider) EvaluateQuery(ctx context.Context, metric metricsapi.KeptnMetric, provider metricsapi.KeptnMetricsProvider) (string, []byte, error) {
 	baseURL := d.normalizeAPIURL(provider.Spec.TargetServer)
-	query := url.QueryEscape(metric.Spec.Query)
+
 	var qURL string
 	if metric.Spec.Range != nil {
-		qURL = baseURL + "v2/metrics/query?metricSelector=" + query + "&from=now-" + metric.Spec.Range.Interval
+		qURL = "metricSelector=" + metric.Spec.Query + "&from=now-" + metric.Spec.Range.Interval
 	} else {
-		qURL = baseURL + "v2/metrics/query?metricSelector=" + query
+		qURL = "metricSelector=" + metric.Spec.Query
 	}
 
+	qURL = urlEncodeQuery(qURL)
+	qURL = baseURL + "v2/metrics/query?" + qURL
+
 	d.Log.Info("Running query: " + qURL)
 	ctx, cancel := context.WithTimeout(ctx, 20*time.Second)
 	defer cancel()
@@ -66,8 +68,10 @@ func (d *KeptnDynatraceProvider) EvaluateQueryForStep(ctx context.Context, metri
 		return nil, nil, fmt.Errorf("spec.range is not defined!")
 	}
 	baseURL := d.normalizeAPIURL(provider.Spec.TargetServer)
-	query := url.QueryEscape(metric.Spec.Query)
-	qURL := baseURL + "v2/metrics/query?metricSelector=" + query + "&from=now-" + metric.Spec.Range.Interval + "&resolution=" + metric.Spec.Range.Step
+	qURL := "metricSelector=" + metric.Spec.Query + "&from=now-" + metric.Spec.Range.Interval + "&resolution=" + metric.Spec.Range.Step
+
+	qURL = urlEncodeQuery(qURL)
+	qURL = baseURL + "v2/metrics/query?" + qURL
 
 	d.Log.Info("Running query: " + qURL)
 	ctx, cancel := context.WithTimeout(ctx, 20*time.Second)
@@ -161,7 +165,7 @@ func (d *KeptnDynatraceProvider) getResultSlice(result *DynatraceResponse) []str
 		for _, points := range r.Data {
 			for _, v := range points.Values {
 				if v != nil {
-					totalValues = totalValues + 1
+					totalValues++
 				}
 			}
 		}