Fix store debug matchers panic on regex matcher (#7903)

* fix store debug matchers panic on regex Signed-off-by: Ben Ye <[email protected]> * add test Signed-off-by: Ben Ye <[email protected]> * changelog Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]>
thanos-io · Nov 13, 2024 · f9da21e · f9da21e
1 parent bfbabbb
commit f9da21e
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re
 - [#7832](https://github.com/thanos-io/thanos/pull/7832) Query Frontend: Fix cache keys for dynamic split intervals.
 - [#7885](https://github.com/thanos-io/thanos/pull/7885) Store: Return chunks to the pool after completing a Series call.
 - [#7893](https://github.com/thanos-io/thanos/pull/7893) Sidecar: Fix retrieval of external labels for Prometheus v3.0.0.
+- [#7903](https://github.com/thanos-io/thanos/pull/7903) Query: Fix panic on regex store matchers.
 
 ### Added
 - [#7763](https://github.com/thanos-io/thanos/pull/7763) Ruler: use native histograms for client latency metrics.

diff --git a/pkg/api/query/v1_test.go b/pkg/api/query/v1_test.go
@@ -1722,6 +1722,16 @@ func TestParseStoreDebugMatchersParam(t *testing.T) {
 				labels.MustNewMatcher(labels.MatchEqual, "cluster", "test"),
 			}},
 		},
+		{
+			storeMatchers: `{__address__=~"localhost:.*"}`,
+			fail:          false,
+			result:        [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchRegexp, "__address__", "localhost:.*")}},
+		},
+		{
+			storeMatchers: `{__address__!~"localhost:.*"}`,
+			fail:          false,
+			result:        [][]*labels.Matcher{{labels.MustNewMatcher(labels.MatchNotRegexp, "__address__", "localhost:.*")}},
+		},
 	} {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			api := QueryAPI{
@@ -1736,11 +1746,24 @@ func TestParseStoreDebugMatchersParam(t *testing.T) {
 
 			storeMatchers, err := api.parseStoreDebugMatchersParam(r)
 			if !tc.fail {
-				testutil.Equals(t, tc.result, storeMatchers)
+				testutil.Equals(t, len(tc.result), len(storeMatchers))
+				for i, m := range tc.result {
+					testutil.Equals(t, len(m), len(storeMatchers[i]))
+					for j, n := range m {
+						testutil.Equals(t, n.String(), storeMatchers[i][j].String())
+					}
+				}
 				testutil.Equals(t, (*baseAPI.ApiError)(nil), err)
 			} else {
 				testutil.NotOk(t, err)
 			}
+
+			// We don't care about results but checking for panic.
+			for _, matchers := range storeMatchers {
+				for _, matcher := range matchers {
+					_ = matcher.Matches("")
+				}
+			}
 		})
 	}
 }

diff --git a/pkg/extpromql/parser.go b/pkg/extpromql/parser.go
@@ -43,16 +43,7 @@ func ParseMetricSelector(input string) ([]*labels.Matcher, error) {
 		return nil, fmt.Errorf("expected type *parser.VectorSelector, got %T", expr)
 	}
 
-	matchers := make([]*labels.Matcher, len(vs.LabelMatchers))
-	for i, lm := range vs.LabelMatchers {
-		matchers[i] = &labels.Matcher{
-			Type:  lm.Type,
-			Name:  lm.Name,
-			Value: lm.Value,
-		}
-	}
-
-	return matchers, nil
+	return vs.LabelMatchers, nil
 }
 
 func isEmptyNameMatcherErr(err error) bool {