Repository: VictoriaMetrics/VictoriaMetrics
Pull request #3183 (2022-09-29 16:21:20)

lib/awsapi: fixes sign encoding
previously white spaces at filter were incorrectly encoded encoding tip was copied from aws signing lib
For example, the space character must be encoded as %20 (not using '+', as some encoding schemes do) https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3171


Patch:
diff --git a/app/vmalert/alerting.go b/app/vmalert/alerting.go
index 89b35deb3..db6b24f3e 100644
--- a/app/vmalert/alerting.go
+++ b/app/vmalert/alerting.go
@@ -165,11 +165,15 @@ func (ar *AlertingRule) logDebugf(at time.Time, a *notifier.Alert, format string
 }
 
 type labelSet struct {
-	// origin labels from series
-	// used for templating
+	// origin labels extracted from received time series
+	// plus extra labels (group labels, service labels like alertNameLabel).
+	// in case of conflicts, origin labels from time series preferred.
+	// used for templating annotations
 	origin map[string]string
-	// processed labels with additional data
-	// used as Alert labels
+	// processed labels includes origin labels
+	// plus extra labels (group labels, service labels like alertNameLabel).
+	// in case of conflicts, extra labels are preferred.
+	// used as labels attached to notifier.Alert and ALERTS series written to remote storage.
 	processed map[string]string
 }
 
@@ -177,7 +181,7 @@ type labelSet struct {
 // to labelSet which contains original and processed labels.
 func (ar *AlertingRule) toLabels(m datasource.Metric, qFn templates.QueryFn) (*labelSet, error) {
 	ls := &labelSet{
-		origin:    make(map[string]string, len(m.Labels)),
+		origin:    make(map[string]string),
 		processed: make(map[string]string),
 	}
 	for _, l := range m.Labels {
@@ -199,14 +203,23 @@ func (ar *AlertingRule) toLabels(m datasource.Metric, qFn templates.QueryFn) (*l
 	}
 	for k, v := range extraLabels {
 		ls.processed[k] = v
+		if _, ok := ls.origin[k]; !ok {
+			ls.origin[k] = v
+		}
 	}
 
 	// set additional labels to identify group and rule name
 	if ar.Name != "" {
 		ls.processed[alertNameLabel] = ar.Name
+		if _, ok := ls.origin[alertNameLabel]; !ok {
+			ls.origin[alertNameLabel] = ar.Name
+		}
 	}
 	if !*disableAlertGroupLabel && ar.GroupName != "" {
 		ls.processed[alertGroupNameLabel] = ar.GroupName
+		if _, ok := ls.origin[alertGroupNameLabel]; !ok {
+			ls.origin[alertGroupNameLabel] = ar.GroupName
+		}
 	}
 	return ls, nil
 }
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 5422e5e3c..bfa3a045d 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -34,6 +34,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 * FEATURE: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): add [sort_by_label_numeric](https://docs.victoriametrics.com/MetricsQL.html#sort_by_label_numeric) and [sort_by_label_numeric_desc](https://docs.victoriametrics.com/MetricsQL.html#sort_by_label_numeric_desc) functions for [numeric sort](https://www.gnu.org/software/coreutils/manual/html_node/Version-sort-is-not-the-same-as-numeric-sort.html) of input time series by the specified labels. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/2938).
 * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup.html) and [vmrestore](https://docs.victoriametrics.com/vmrestore.html): retry GCS operations for up to 3 minutes on temporary failures. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3147).
 
+* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly encode query params for aws signed requests, use `%20` instead of `+` as api requires. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3171).
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly calculate `rate_over_sum(m[d])` as `sum_over_time(m[d])/d`. Previously the `sum_over_time(m[d])` could be improperly divided by smaller than `d` time range. See [rate_over_sum() docs](https://docs.victoriametrics.com/MetricsQL.html#rate_over_sum) and [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3045).
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly calculate `increase(m[d])` over slow-changing counters with values smaller than 100. Previously [increase](https://docs.victoriametrics.com/MetricsQL.html#increase) could return unexpectedly big results in this case. See [the related issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/962) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3163).
 * BUGFIX: [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): properly calculate query results at `vmselect`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3067). The issue has been introduced in [v1.81.0](https://docs.victoriametrics.com/CHANGELOG.html#v1810).
diff --git a/lib/awsapi/sign.go b/lib/awsapi/sign.go
index d56890f12..88d182374 100644
--- a/lib/awsapi/sign.go
+++ b/lib/awsapi/sign.go
@@ -41,6 +41,9 @@ func signRequestWithTime(req *http.Request, service, region, payloadHash string,
 	datestamp := t.Format("20060102")
 	canonicalURL := uri.Path
 	canonicalQS := uri.Query().Encode()
+	// Replace "%20" with "+" according to AWS requirements.
+	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3171
+	canonicalQS = strings.ReplaceAll(canonicalQS, "+", "%20")
 
 	canonicalHeaders := fmt.Sprintf("host:%s\nx-amz-date:%s\n", uri.Host, amzdate)
 	signedHeaders := "host;x-amz-date"


Test patch:
diff --git a/app/vmalert/alerting_test.go b/app/vmalert/alerting_test.go
index a222b824e..89607116b 100644
--- a/app/vmalert/alerting_test.go
+++ b/app/vmalert/alerting_test.go
@@ -700,14 +700,26 @@ func TestAlertingRule_Template(t *testing.T) {
 		expAlerts map[uint64]*notifier.Alert
 	}{
 		{
-			newTestRuleWithLabels("common", "region", "east"),
+			&AlertingRule{
+				Name: "common",
+				Labels: map[string]string{
+					"region": "east",
+				},
+				Annotations: map[string]string{
+					"summary": `{{ $labels.alertname }}: Too high connection number for "{{ $labels.instance }}"`,
+				},
+				alerts: make(map[uint64]*notifier.Alert),
+				state:  newRuleState(),
+			},
 			[]datasource.Metric{
 				metricWithValueAndLabels(t, 1, "instance", "foo"),
 				metricWithValueAndLabels(t, 1, "instance", "bar"),
 			},
 			map[uint64]*notifier.Alert{
 				hash(map[string]string{alertNameLabel: "common", "region": "east", "instance": "foo"}): {
-					Annotations: map[string]string{},
+					Annotations: map[string]string{
+						"summary": `common: Too high connection number for "foo"`,
+					},
 					Labels: map[string]string{
 						alertNameLabel: "common",
 						"region":       "east",
@@ -715,7 +727,9 @@ func TestAlertingRule_Template(t *testing.T) {
 					},
 				},
 				hash(map[string]string{alertNameLabel: "common", "region": "east", "instance": "bar"}): {
-					Annotations: map[string]string{},
+					Annotations: map[string]string{
+						"summary": `common: Too high connection number for "bar"`,
+					},
 					Labels: map[string]string{
 						alertNameLabel: "common",
 						"region":       "east",
diff --git a/app/vmalert/config/testdata/rules/rules2-good.rules b/app/vmalert/config/testdata/rules/rules2-good.rules
index 615a085af..545a42d55 100644
--- a/app/vmalert/config/testdata/rules/rules2-good.rules
+++ b/app/vmalert/config/testdata/rules/rules2-good.rules
@@ -9,11 +9,12 @@ groups:
       denyPartialResponse: ["true"]
     rules:
       - alert: Conns
-        expr: sum(vm_tcplistener_conns) by(instance) > 1
+        expr: vm_tcplistener_conns > 0
         for: 3m
         debug: true
         annotations:
-          summary: Too high connection number for {{$labels.instance}}
+          labels: "Available labels: {{ $labels }}"
+          summary: Too high connection number for {{ $labels.instance }}
             {{ with printf "sum(vm_tcplistener_conns{instance=%q})" .Labels.instance | query }}
               {{ . | first | value }}
             {{ end }}