Enforce /api/v1/series API query length check at Querier (#6018)

* enforce /api/v1/series API query length check at Querier

Signed-off-by: Ben Ye <[email protected]>

* changelog

Signed-off-by: Ben Ye <[email protected]>

---------

Signed-off-by: Ben Ye <[email protected]>

Author: yeya24

CHANGELOG.md

@@ -18,6 +18,7 @@
 * [ENHANCEMENT] Upgrade go to 1.21.11 #6014
 * [BUGFIX] Configsdb: Fix endline issue in db password. #5920
 * [BUGFIX] Ingester: Fix `user` and `type` labels for the `cortex_ingester_tsdb_head_samples_appended_total` TSDB metric. #5952
+* [BUGFIX] Querier: Enforce max query length check for `/api/v1/series` API even though `ignoreMaxQueryLength` is set to true. #6018
 
 ## 1.17.1 2024-05-20
 

pkg/querier/querier.go

@@ -375,20 +375,21 @@ func (q querier) Select(ctx context.Context, sortSeries bool, sp *storage.Select
 	// so we make sure changes are reflected back to hints.
 	sp.Start = startMs
 	sp.End = endMs
+	getSeries := sp.Func == "series"
 
 	// For series queries without specifying the start time, we prefer to
 	// only query ingesters and not to query maxQueryLength to avoid OOM kill.
-	if sp.Func == "series" && startMs == 0 {
+	if getSeries && startMs == 0 {
 		return metadataQuerier.Select(ctx, true, sp, matchers...)
 	}
 
 	startTime := model.Time(startMs)
 	endTime := model.Time(endMs)
 
-	// Validate query time range. This validation should be done only for instant / range queries and
-	// NOT for metadata queries (series, labels) because the query-frontend doesn't support splitting
-	// of such queries.
-	if !q.ignoreMaxQueryLength {
+	// Validate query time range. This validation for instant / range queries can be done either at Query Frontend
+	// or here at Querier. When the check is done at Query Frontend, we still want to enforce the max query length
+	// check for /api/v1/series request since there is no specific tripperware for series.
+	if !q.ignoreMaxQueryLength || getSeries {
 		if maxQueryLength := q.limits.MaxQueryLength(userID); maxQueryLength > 0 && endTime.Sub(startTime) > maxQueryLength {
 			limitErr := validation.LimitError(fmt.Sprintf(validation.ErrQueryTooLong, endTime.Sub(startTime), maxQueryLength))
 			return storage.ErrSeriesSet(limitErr)

pkg/querier/querier_test.go

@@ -892,6 +892,42 @@ func TestQuerier_ValidateQueryTimeRange_MaxQueryLength(t *testing.T) {
 	}
 }
 
+func TestQuerier_ValidateQueryTimeRange_MaxQueryLength_Series(t *testing.T) {
+	t.Parallel()
+	const maxQueryLength = 30 * 24 * time.Hour
+
+	//parallel testing causes data race
+	var cfg Config
+	flagext.DefaultValues(&cfg)
+	// Disable active query tracker to avoid mmap error.
+	cfg.ActiveQueryTrackerDir = ""
+	// Ignore max query length check at Querier but it still enforces it for Series.
+	cfg.IgnoreMaxQueryLength = true
+
+	limits := DefaultLimitsConfig()
+	limits.MaxQueryLength = model.Duration(maxQueryLength)
+	overrides, err := validation.NewOverrides(limits, nil)
+	require.NoError(t, err)
+
+	chunkStore := &emptyChunkStore{}
+	distributor := &emptyDistributor{}
+
+	queryables := []QueryableWithFilter{UseAlwaysQueryable(NewMockStoreQueryable(cfg, chunkStore))}
+	queryable, _, _ := New(cfg, overrides, distributor, queryables, nil, log.NewNopLogger())
+
+	ctx := user.InjectOrgID(context.Background(), "test")
+	now := time.Now()
+	end := now.Add(-time.Minute)
+	start := end.Add(-maxQueryLength - time.Hour)
+	minT := util.TimeToMillis(start)
+	maxT := util.TimeToMillis(end)
+	q, err := queryable.Querier(minT, maxT)
+	require.NoError(t, err)
+	ss := q.Select(ctx, false, &storage.SelectHints{Func: "series", Start: minT, End: maxT})
+	require.False(t, ss.Next())
+	require.True(t, strings.Contains(ss.Err().Error(), "the query time range exceeds the limit (query length: 721h0m0s, limit: 720h0m0s)"))
+}
+
 func TestQuerier_ValidateQueryTimeRange_MaxQueryLookback(t *testing.T) {
 	t.Parallel()
 	const (