Validate tenant limits in user-overrides endpoint

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [ENHANCEMENT] Ruler: Add rule information to query parameters sent to the Query Frontend when `-ruler.frontend-address` is configured, so the Query Frontend query stats logs (`-frontend.enabled-ruler-query-stats`) can identify the evaluated rule. In this mode, the Ruler query stats log (`-ruler.query-stats-enabled`) omits the fetched statistics fields, which are always zero. #6539
 * [ENHANCEMENT] gRPC: When `-auth.enabled=true`, a gRPC request without the `X-Scope-OrgID` metadata is now rejected with the `UNAUTHENTICATED` status code instead of `UNKNOWN`. #7873
 * [ENHANCEMENT] Upgrade promql-engine and Thanos to latest. #7889
+* [ENHANCEMENT] Overrides API: Validate the merged tenant limits in `POST /api/v1/user-overrides` with the same checks used when loading the runtime config, and reject invalid values with 400. #7897
 * [BUGFIX] Update prometheus alertmanager version to v0.34.1 to apply upstream inhibit rule bug fixes. #7877
 * [BUGFIX] Tenant Federation: Fix regex tenant federation not working in single binary mode, and failing with `multiple org IDs present` when a matched tenant ID contains regex metacharacters and also matches other tenants as a regex. #7869
 * [BUGFIX] Query Frontend: Fix the data selection time window computed for an instant query sent without a `time` parameter. The default evaluation time was passed to `util.ParseTimeParam` in milliseconds, but that helper formats the default and parses it as seconds, so the value overflowed `UnixNano()` and wrapped to a timestamp decades away from `now`. This made the `data_select_min_time` and `data_select_max_time` query stats wrong, and stopped the `time_window` query attribute of query priority and query rejection from matching such queries correctly. #7880

docs/api/_index.md

@@ -981,6 +981,8 @@ The following limits can be modified via the API:
 
 Overrides are validated against hard limits defined in the runtime configuration file. If a requested override exceeds the hard limit for the tenant, the request will be rejected with a 400 status code.
 
+The merged limits of the tenant are also validated with the same checks used when loading the runtime configuration. If the validation fails, the request will be rejected with a 400 status code.
+
 ## Store-gateway
 
 ### Store-gateway ring status

docs/guides/overrides.md

@@ -203,6 +203,7 @@ Returns success with no body.
 **Response (400 Bad Request):**
 - Invalid limit names (not in `api_allowed_limits`)
 - Values exceeding hard limits
+- Values failing limits validation (see [Limits Validation](#limits-validation))
 - Invalid JSON format
 
 ### Delete User Overrides
@@ -280,6 +281,10 @@ Limit names correspond to fields in the [`limits_config`](../configuration/confi
 - `max_label_name_length`
 - `max_label_value_length`
 
+### Limits Validation
+
+Before writing to the runtime configuration file, the merged limits of the tenant are validated with the same checks that Cortex components run when they load the runtime configuration. For example, setting `max_global_series_per_user` is rejected when `-distributor.shard-by-all-labels` is disabled.
+
 ## Hard Limits
 
 Hard limits provide per-tenant upper bounds for override values. They prevent tenants from setting limits that exceed their allocated capacity.

integration/overrides_test.go

@@ -174,6 +174,44 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
 		assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
 	})
 
+	t.Run("POST overrides failing limits validation", func(t *testing.T) {
+		// max_global_series_per_user requires shard-by-all-labels, which is disabled by default.
+		invalidOverrides := map[string]interface{}{
+			"max_global_series_per_user": 10000,
+		}
+		requestBody, err := json.Marshal(invalidOverrides)
+		require.NoError(t, err)
+
+		req, err := http.NewRequest("POST", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
+		require.NoError(t, err)
+		req.Header.Set("X-Scope-OrgID", "user1")
+		req.Header.Set("Content-Type", "application/json")
+
+		resp, err := http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
+
+		// The runtime config must be left unchanged.
+		req, err = http.NewRequest("GET", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", nil)
+		require.NoError(t, err)
+		req.Header.Set("X-Scope-OrgID", "user1")
+
+		resp, err = http.DefaultClient.Do(req)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+		var savedOverrides map[string]interface{}
+		err = json.NewDecoder(resp.Body).Decode(&savedOverrides)
+		require.NoError(t, err)
+
+		assert.Equal(t, float64(5000), savedOverrides["ingestion_rate"])
+		assert.Equal(t, float64(0), savedOverrides["max_global_series_per_user"])
+	})
+
 	t.Run("DELETE overrides", func(t *testing.T) {
 		req, err := http.NewRequest("DELETE", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", nil)
 		require.NoError(t, err)
@@ -223,7 +261,11 @@ func TestOverridesAPIHardLimits(t *testing.T) {
 	}
 	uploadRuntimeConfig(t, minio, runtimeConfig)
 
-	cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-hard-limits", defaultArgsOverrides(minio), "")
+	// max_global_series_per_user is rejected by the limits validation unless shard-by-all-labels is enabled.
+	flags := e2e.MergeFlags(defaultArgsOverrides(minio), map[string]string{
+		"-distributor.shard-by-all-labels": "true",
+	})
+	cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-hard-limits", flags, "")
 	require.NoError(t, s.StartAndWaitReady(cortexSvc))
 
 	t.Run("POST overrides within hard limits", func(t *testing.T) {

pkg/cortex/modules.go

@@ -218,7 +218,7 @@ func (t *Cortex) initOverridesConfig() (services.Service, error) {
 
 func (t *Cortex) initOverrides() (services.Service, error) {
 
-	overridesAPI, err := overrides.New(t.Cfg.RuntimeConfig, util_log.Logger, prometheus.DefaultRegisterer)
+	overridesAPI, err := overrides.New(t.Cfg.RuntimeConfig, runtimeConfigLoader{cfg: t.Cfg}.validateTenantLimits, util_log.Logger, prometheus.DefaultRegisterer)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create overrides API: %w", err)
 	}

pkg/cortex/runtime_config.go

@@ -77,10 +77,7 @@ func (l runtimeConfigLoader) load(r io.Reader) (any, error) {
 			// refer to https://github.com/cortexproject/cortex/issues/6741#issuecomment-3067244929
 			if overrides != nil {
 				for userID, ul := range overrides.TenantLimits {
-					if err := ul.Validate(l.cfg.NameValidationScheme, l.cfg.Distributor.ShardByAllLabels, l.cfg.Ingester.ActiveSeriesMetricsEnabled, l.cfg.Distributor.HATrackerConfig.UpdateTimeout, l.cfg.Distributor.HATrackerConfig.UpdateTimeoutJitterMax); err != nil {
-						return nil, err
-					}
-					if err := ul.ValidateQueryLimits(userID, l.cfg.BlocksStorage.TSDB.CloseIdleTSDBTimeout); err != nil {
+					if err := l.validateTenantLimits(userID, ul); err != nil {
 						return nil, err
 					}
 				}
@@ -91,6 +88,13 @@ func (l runtimeConfigLoader) load(r io.Reader) (any, error) {
 	return overrides, nil
 }
 
+func (l runtimeConfigLoader) validateTenantLimits(userID string, ul *validation.Limits) error {
+	if err := ul.Validate(l.cfg.NameValidationScheme, l.cfg.Distributor.ShardByAllLabels, l.cfg.Ingester.ActiveSeriesMetricsEnabled, l.cfg.Distributor.HATrackerConfig.UpdateTimeout, l.cfg.Distributor.HATrackerConfig.UpdateTimeoutJitterMax); err != nil {
+		return err
+	}
+	return ul.ValidateQueryLimits(userID, l.cfg.BlocksStorage.TSDB.CloseIdleTSDBTimeout)
+}
+
 func multiClientRuntimeConfigChannel(manager *runtimeconfig.Manager) func() <-chan kv.MultiRuntimeConfig {
 	if manager == nil {
 		return nil

pkg/overrides/api.go

@@ -26,6 +26,8 @@ const (
 	ErrRuntimeConfig = "runtime config read error"
 )
 
+var errInvalidOverrides = errors.New("invalid overrides")
+
 // getAllowedLimitsFromBucket reads allowed limits from the runtime config file
 func (a *API) getAllowedLimitsFromBucket(ctx context.Context) ([]string, error) {
 	reader, err := a.bucketClient.Get(ctx, a.runtimeConfigPath)
@@ -117,6 +119,11 @@ func (a *API) SetOverrides(w http.ResponseWriter, r *http.Request) {
 
 	// Write overrides to bucket storage
 	if err := a.setOverridesToBucket(r.Context(), userID, overrides); err != nil {
+		if errors.Is(err, errInvalidOverrides) {
+			level.Error(a.logger).Log("msg", "limits validation failed", "userID", userID, "err", err)
+			http.Error(w, "Invalid overrides", http.StatusBadRequest)
+			return
+		}
 		level.Error(a.logger).Log("msg", "failed to set overrides to bucket", "userID", userID, "err", err)
 		http.Error(w, "Internal server error", http.StatusInternalServerError)
 		return
@@ -248,6 +255,13 @@ func (a *API) setOverridesToBucket(ctx context.Context, userID string, overrides
 		return fmt.Errorf("invalid overrides format: %w", err)
 	}
 
+	// Reject limits that would make the runtime config fail to load in other components.
+	if a.validateLimits != nil {
+		if err := a.validateLimits(userID, &limits); err != nil {
+			return fmt.Errorf("%w: %w", errInvalidOverrides, err)
+		}
+	}
+
 	config.TenantLimits[userID] = &limits
 
 	data, err := yaml.Marshal(config)

pkg/overrides/overrides.go

@@ -12,31 +12,37 @@ import (
 	"github.com/cortexproject/cortex/pkg/storage/bucket"
 	"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
 	"github.com/cortexproject/cortex/pkg/util/services"
+	"github.com/cortexproject/cortex/pkg/util/validation"
 )
 
 const (
 	ErrInvalidOverridesConfiguration = "invalid overrides configuration"
 	ErrFailedToCreateBucketClient    = "failed to create bucket client for overrides"
 )
 
+// LimitsValidator validates the merged limits of a tenant before they are written to the runtime config.
+type LimitsValidator func(userID string, limits *validation.Limits) error
+
 type API struct {
 	services.Service
 	cfg               runtimeconfig.Config
 	logger            log.Logger
 	registerer        prometheus.Registerer
 	bucketClient      objstore.Bucket
 	runtimeConfigPath string
+	validateLimits    LimitsValidator
 }
 
-func New(cfg runtimeconfig.Config, logger log.Logger, registerer prometheus.Registerer) (*API, error) {
+func New(cfg runtimeconfig.Config, validateLimits LimitsValidator, logger log.Logger, registerer prometheus.Registerer) (*API, error) {
 	if err := cfg.StorageConfig.Validate(); err != nil {
 		return nil, fmt.Errorf("%s: %w", ErrInvalidOverridesConfiguration, err)
 	}
 
 	api := &API{
-		cfg:        cfg,
-		logger:     logger,
-		registerer: registerer,
+		cfg:            cfg,
+		logger:         logger,
+		registerer:     registerer,
+		validateLimits: validateLimits,
 	}
 	api.Service = services.NewBasicService(api.starting, api.running, api.stopping)
 	return api, nil

pkg/overrides/overrides_test.go

@@ -3,11 +3,13 @@ package overrides
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -22,6 +24,7 @@ import (
 	"github.com/cortexproject/cortex/pkg/util/log"
 	"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
 	"github.com/cortexproject/cortex/pkg/util/services"
+	"github.com/cortexproject/cortex/pkg/util/validation"
 )
 
 func TestConfig_Validate(t *testing.T) {
@@ -111,7 +114,7 @@ func TestNew(t *testing.T) {
 
 	for testName, testData := range tests {
 		t.Run(testName, func(t *testing.T) {
-			api, err := New(testData.cfg, log.Logger, prometheus.DefaultRegisterer)
+			api, err := New(testData.cfg, nil, log.Logger, prometheus.DefaultRegisterer)
 
 			if testData.expectError {
 				assert.Error(t, err)
@@ -140,7 +143,7 @@ func TestOverridesModuleServiceInterface(t *testing.T) {
 	}
 	// Set default values before validation
 	flagext.DefaultValues(&cfg.StorageConfig.S3)
-	api, err := New(cfg, log.Logger, prometheus.DefaultRegisterer)
+	api, err := New(cfg, nil, log.Logger, prometheus.DefaultRegisterer)
 	require.NoError(t, err)
 	require.NotNil(t, api)
 
@@ -417,7 +420,7 @@ api_allowed_limits:
 			}
 			// Set default values before validation
 			flagext.DefaultValues(&cfg.StorageConfig.S3)
-			api, err := New(cfg, log.Logger, prometheus.DefaultRegisterer)
+			api, err := New(cfg, nil, log.Logger, prometheus.DefaultRegisterer)
 			require.NoError(t, err)
 			require.NotNil(t, api)
 
@@ -515,7 +518,7 @@ func TestAPITenantExtraction(t *testing.T) {
 			}
 			// Set default values before validation
 			flagext.DefaultValues(&cfg.StorageConfig.S3)
-			api, err := New(cfg, log.Logger, prometheus.DefaultRegisterer)
+			api, err := New(cfg, nil, log.Logger, prometheus.DefaultRegisterer)
 			require.NoError(t, err)
 			require.NotNil(t, api)
 
@@ -612,7 +615,7 @@ api_allowed_limits:
 			}
 			// Set default values before validation
 			flagext.DefaultValues(&cfg.StorageConfig.S3)
-			api, err := New(cfg, log.Logger, prometheus.DefaultRegisterer)
+			api, err := New(cfg, nil, log.Logger, prometheus.DefaultRegisterer)
 			require.NoError(t, err)
 			require.NotNil(t, api)
 
@@ -652,3 +655,68 @@ api_allowed_limits:
 		})
 	}
 }
+
+func TestSetOverridesLimitsValidation(t *testing.T) {
+	const runtimeConfig = `overrides:
+  user1:
+    ingestion_rate: 1000
+    max_global_series_per_user: 100000
+api_allowed_limits:
+  - ingestion_rate
+  - max_global_series_per_user`
+
+	tests := map[string]struct {
+		requestBody    string
+		validateLimits LimitsValidator
+		expectedStatus int
+		expectedBody   string
+		expectUpload   bool
+	}{
+		"validator receives merged limits": {
+			requestBody: `{"ingestion_rate": 2000}`,
+			validateLimits: func(userID string, limits *validation.Limits) error {
+				assert.Equal(t, "user1", userID)
+				assert.Equal(t, float64(2000), limits.IngestionRate)
+				assert.Equal(t, 100000, limits.MaxGlobalSeriesPerUser)
+				return nil
+			},
+			expectedStatus: http.StatusOK,
+			expectUpload:   true,
+		},
+		"validator error rejects the request": {
+			requestBody: `{"max_global_series_per_user": 200000}`,
+			validateLimits: func(string, *validation.Limits) error {
+				return errors.New("max_global_series_per_user is not supported")
+			},
+			expectedStatus: http.StatusBadRequest,
+			expectedBody:   "Invalid overrides",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			mockBucket := &bucket.ClientMock{}
+			mockBucket.MockGet("runtime.yaml", runtimeConfig, nil)
+			mockBucket.MockUpload("runtime.yaml", nil)
+
+			api, err := New(runtimeconfig.Config{StorageConfig: bucket.Config{Backend: bucket.Filesystem}}, tc.validateLimits, log.Logger, prometheus.NewRegistry())
+			require.NoError(t, err)
+			api.bucketClient = mockBucket
+			api.runtimeConfigPath = "runtime.yaml"
+
+			req := httptest.NewRequest(http.MethodPost, "/api/v1/user-overrides", strings.NewReader(tc.requestBody))
+			req.Header.Set("X-Scope-OrgID", "user1")
+			recorder := httptest.NewRecorder()
+
+			api.SetOverrides(recorder, req)
+
+			assert.Equal(t, tc.expectedStatus, recorder.Code)
+			assert.Contains(t, recorder.Body.String(), tc.expectedBody)
+			if tc.expectUpload {
+				mockBucket.AssertCalled(t, "Upload", mock.Anything, "runtime.yaml", mock.Anything)
+			} else {
+				mockBucket.AssertNotCalled(t, "Upload", mock.Anything, "runtime.yaml", mock.Anything)
+			}
+		})
+	}
+}