From ada9afc74c99c7c8874436ec7fa0c80f6ba14452 Mon Sep 17 00:00:00 2001 From: Roman Khavronenko Date: Fri, 8 Sep 2023 09:32:48 +0200 Subject: [PATCH] vmalert: correctly add duplicated params to the query (#4955) Fix the bug when Group's `params` fields with multiple values were overriding each other instead of adding up. The bug was introduced in this commit https://github.com/VictoriaMetrics/VictoriaMetrics/commit/eccecdf177115297fa1dc4d42d38e23de9a9f2cb starting from v1.91.1 https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.91.1 https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4908 Signed-off-by: hagen1778 (cherry picked from commit 6351d07da8d83887d784728678d288c7d7e1b112) --- app/vmalert/datasource/vm.go | 11 +++++++++-- app/vmalert/datasource/vm_test.go | 11 +++++++++++ docs/CHANGELOG.md | 1 + 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go index 30f5ef1f2..42b45668e 100644 --- a/app/vmalert/datasource/vm.go +++ b/app/vmalert/datasource/vm.go @@ -90,8 +90,15 @@ func (s *VMStorage) ApplyParams(params QuerierParams) *VMStorage { s.extraParams = url.Values{} } for k, vl := range params.QueryParams { - for _, v := range vl { // custom query params are prior to default ones - s.extraParams.Set(k, v) + // custom query params are prior to default ones + if s.extraParams.Has(k) { + s.extraParams.Del(k) + } + for _, v := range vl { + // don't use .Set() instead of Del/Add since it is allowed + // for GET params to be duplicated + // see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4908 + s.extraParams.Add(k, v) } } } diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go index 69ff78477..b9c2183b0 100644 --- a/app/vmalert/datasource/vm_test.go +++ b/app/vmalert/datasource/vm_test.go @@ -489,6 +489,17 @@ func TestRequestParams(t *testing.T) { checkEqualString(t, exp, r.URL.RawQuery) }, }, + { + "allow duplicates in query params", + false, + storage.Clone().ApplyParams(QuerierParams{ + QueryParams: url.Values{"extra_labels": {"env=dev", "foo=bar"}}, + }), + func(t *testing.T, r *http.Request) { + exp := url.Values{"query": {query}, "round_digits": {"10"}, "extra_labels": {"env=dev", "foo=bar"}, "time": {timestamp.Format(time.RFC3339)}} + checkEqualString(t, exp.Encode(), r.URL.RawQuery) + }, + }, { "graphite extra params", false, diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7c9c1e56c..1a318a2df 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -20,6 +20,7 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vminsert enterprise](https://docs.victoriametrics.com/enterprise.html): properly parse `/insert/multitenant/*` urls, which have been broken since [v1.93.2](#v1932). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4947). * BUGFIX: properly build production armv5 binaries for `GOARCH=arm`. This has been broken after the upgrading of Go builder to Go1.21.0. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4965). * BUGFIX: [vmselect](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): return `503 Service Unavailable` status code when [partial responses](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#cluster-availability) are denied and some of `vmstorage` nodes are temporarily unavailable. Previously `422 Unprocessable Entiry` status code was mistakenly returned in this case, which could prevent from automatic recovery by re-sending the request to healthy cluster replica in another availability zone. +* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix the bug when Group's `params` fields with multiple values were overriding each other instead of adding up. The bug was introduced in [this commit](https://github.com/VictoriaMetrics/VictoriaMetrics/commit/eccecdf177115297fa1dc4d42d38e23de9a9f2cb) starting from [v1.91.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.91.1). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4908). ## [v1.87.8](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.87.8)