Add Overrides API component and rename old overrides to overrides-configs #6975

bogdan-st · 2025-08-15T18:41:17Z

Adds initial draft for the User Overrides API, described in https://cortexmetrics.io/docs/proposals/overrides-api/

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Signed-off-by: Bogdan Stancu <[email protected]>

friedrichg

I am liking it. I have do have a couple of suggestions

pkg/cortex/modules.go

docs/configuration/config-file-reference.md

Signed-off-by: Bogdan Stancu <[email protected]>

friedrichg

I gave it another pass. Thanks for your hard work!

friedrichg · 2025-09-02T11:59:05Z

docs/configuration/config-file-reference.md

    # CLI flag: -query-scheduler.grpc-client-config.connect-timeout
    [connect_timeout: <duration> | default = 5s]

+overrides:


You added a new target "overrides", that's good. No need to add new flags. Use the flags available: https://cortexmetrics.io/docs/configuration/configuration-file/#runtime_configuration_storage_config

friedrichg · 2025-09-02T11:59:56Z

pkg/overrides/overrides.go

+	if c.Backend == bucket.Filesystem {
+		return errors.New(ErrFilesystemBackendNotSupported)
+	}


Filesystem is supported by the runtime configuration overrides. The API should support it. Remove this.

friedrichg · 2025-09-02T12:03:54Z

pkg/overrides/overrides.go

+)
+
+// Config holds configuration for the overrides module
+type Config struct {


Remove this config. Use

cortex/pkg/util/runtimeconfig/manager.go

Line 31 in 2c07f00

type Config struct {

friedrichg · 2025-09-02T12:08:49Z

pkg/cortex/cortex.go

 	RuntimeConfig       runtimeconfig.Config                       `yaml:"runtime_config"`
 	MemberlistKV        memberlist.KVConfig                        `yaml:"memberlist"`
 	QueryScheduler      scheduler.Config                           `yaml:"query_scheduler"`
+	Overrides           overrides.Config                           `yaml:"overrides"`


Suggested change

Overrides overrides.Config `yaml:"overrides"`

friedrichg · 2025-09-02T12:09:33Z

pkg/cortex/cortex.go

 	c.RuntimeConfig.RegisterFlags(f)
 	c.MemberlistKV.RegisterFlags(f)
 	c.QueryScheduler.RegisterFlags(f)
+	c.Overrides.RegisterFlags(f)


Suggested change

c.Overrides.RegisterFlags(f)

friedrichg · 2025-09-02T12:17:53Z

docs/api/_index.md

 | [Tenant delete request](#tenant-delete-request) | Purger || `POST /purger/delete_tenant` |
 | [Tenant delete status](#tenant-delete-status) | Purger || `GET /purger/delete_tenant_status` |
+| [Get user overrides](#get-user-overrides) | Overrides || `GET /api/v1/user-overrides` |
+| [Set user overrides](#set-user-overrides) | Overrides || `PUT /api/v1/user-overrides` |


Suggested change

| [Set user overrides](#set-user-overrides) | Overrides || `PUT /api/v1/user-overrides` |

| [Set user overrides](#set-user-overrides) | Overrides || `POST /api/v1/user-overrides` |

friedrichg · 2025-09-02T12:25:38Z

pkg/overrides/api.go

+type RuntimeConfigFile struct {
+	Overrides     map[string]map[string]interface{} `yaml:"overrides"`
+	HardOverrides map[string]map[string]interface{} `yaml:"hard_overrides"`
+}


Suggested change

type RuntimeConfigFile struct {

Overrides map[string]map[string]interface{} `yaml:"overrides"`

HardOverrides map[string]map[string]interface{} `yaml:"hard_overrides"`

}

This overrides the other values saved in the runtime like ingester_limits

Use

cortex/pkg/cortex/runtime_config.go

Line 26 in 2c07f00

type RuntimeConfigValues struct {

instead

And make sure the API doesn't delete ingester_limits values and similar

friedrichg · 2025-09-02T12:30:53Z

pkg/overrides/limits.go

+var AllowedLimits = []string{
+	"max_global_series_per_user",
+	"max_global_series_per_metric",
+	"ingestion_rate",
+	"ingestion_burst_size",
+	"ruler_max_rules_per_rule_group",
+	"ruler_max_rule_groups_per_tenant",
+}


put this configuration in the runtime config
https://cortexmetrics.io/docs/configuration/configuration-file/#runtime_configuration_storage_config

friedrichg · 2025-09-02T12:33:59Z

pkg/overrides/overrides.go

+}
+
+// RegisterFlags registers the overrides module flags
+func (c *Config) RegisterFlags(f *flag.FlagSet) {


remove this

friedrichg · 2025-09-02T12:34:10Z

pkg/overrides/overrides.go

+}
+
+// Validate validates the configuration and returns an error if validation fails
+func (c *Config) Validate() error {


Remove this

friedrichg · 2025-09-02T12:46:34Z

Also add the overrides API to https://github.com/cortexproject/cortex/blob/master/docs/configuration/v1-guarantees.md

Signed-off-by: Bogdan Stancu <[email protected]>

araiu

I like this, long needed feature 🙇
Thank you!

dsabsay

Nice work. I left some questions and suggestions to think about.

I didn't review all of overrides_test.go but reviewed everything else.

dsabsay · 2025-09-12T21:29:59Z

integration/overrides_test.go

+		require.NoError(t, err)
+		defer resp.Body.Close()
+
+		assert.Equal(t, http.StatusOK, resp.StatusCode)


A 4xx of some kind is probably more appropriate in this case.

dsabsay · 2025-09-12T21:36:43Z

pkg/overrides/api.go

+)
+
+const (
+	// HTTP status codes


Why not use the existing http.StatusOK, etc. constants?

dsabsay · 2025-09-12T21:44:38Z

pkg/overrides/api.go

+
+	var config runtimeconfig.RuntimeConfigValues
+	if err := yaml.NewDecoder(reader).Decode(&config); err != nil {
+		return []string{}, nil // No allowed limits if config can't be decoded


This function returns an error but it is always nil.

I agree that a non-existing or invalid config file should be equivalent to having no allowed limits, but it could be expressed better. A couple options:

Remove the error from the return, but log each error.

Return the empty slice and an appropriate error. It is then the caller's responsibility to handle the error.

dsabsay · 2025-09-12T21:48:14Z

pkg/overrides/api.go

+
+	// Read overrides from bucket storage
+	overrides, err := a.getOverridesFromBucket(r.Context(), userID)
+	if err != nil {


This error should be logged.

Also, it might be safer not to return the raw error to the user. A 500 and maybe a general error message would be sufficient. The raw error could expose underlying detail (like storage configurations, etc.) that some providers might consider sensitive. Also, while unlikely, attackers can sometimes learn more than they should from such error messages. This applies anywhere we are returning an error message to the user.

dsabsay · 2025-09-12T21:52:00Z

pkg/overrides/api.go

+func (a *API) getOverridesFromBucket(ctx context.Context, userID string) (map[string]interface{}, error) {
+	reader, err := a.bucketClient.Get(ctx, a.runtimeConfigPath)
+	if err != nil {
+		return map[string]interface{}{}, nil


I don't know how widely this opinion is shared, but I'd prefer this error to be returned. Currently, as a user of getOverridesFromBucket() I can't distinguish between an empty configuration and a situation where there is a configuration but it couldn't be read for some reason (network error, etc.).

dsabsay · 2025-09-12T21:56:43Z

pkg/overrides/limits.go

+	var invalidLimits []string
+
+	for limitName := range overrides {
+		if !IsLimitAllowed(limitName, allowedLimits) {


Use slices.Contains().

dsabsay · 2025-09-12T21:57:23Z

pkg/overrides/limits.go

+
+// GetAllowedLimits returns the allowed limits from runtime config
+// If no allowed limits are configured, returns empty slice (no limits allowed)
+func GetAllowedLimits(allowedLimits []string) []string {


Why is this needed?

dsabsay · 2025-09-12T22:04:18Z

pkg/overrides/limits.go

+	// Read the runtime config to get hard limits
+	reader, err := a.bucketClient.Get(context.Background(), a.runtimeConfigPath)
+	if err != nil {
+		// If we can't read the config, skip hard limit validation


Right now I think this function is too lenient when the hard limits can't be read. If we corrupt (or don't set) the hard limits, users could give themselves unreasonable limits (e.g. 5 trillion active series).

Two options:

If hard limits can't be read, fail requests to update user overrides (this shouldn't really happen anyway).

Create a default hard limit, maybe configured via CLI, so if a hard limit isn't set for a tenant, it will default to something that will prevent completely unreasonable values.

Also, these scenarios might be good to have in the e2e tests.

dsabsay · 2025-09-12T22:06:14Z

pkg/overrides/limits.go

+// validateHardLimits checks if the provided overrides exceed any hard limits from the runtime config
+func (a *API) validateHardLimits(overrides map[string]interface{}, userID string) error {
+	// Read the runtime config to get hard limits
+	reader, err := a.bucketClient.Get(context.Background(), a.runtimeConfigPath)


How do cortex operators set these hard limits?

I think the same way overrides work today, manually in the runtime config file. This just adds the option for users to change a subset of limits within a set threshold through the api, cortex operator overrides stuff stays manual.

Signed-off-by: Bogdan Stancu <[email protected]>

pull-request-size bot added the size/XXL label Aug 15, 2025

dosubot bot added the type/feature label Aug 15, 2025

rename overrides to overridesConfig

aad1fee

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch 2 times, most recently from 29f5299 to 1dbe402 Compare August 15, 2025 18:55

initial overrides api draft

60d09fb

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch from 1dbe402 to 60d09fb Compare August 15, 2025 19:18

bogdan-st added 2 commits August 15, 2025 22:20

Merge branch 'master' into overrides_api

9e8166d

Signed-off-by: Bogdan Stancu <[email protected]>

fix docs and integration tests

f97b319

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st changed the title ~~Overrides API initial draft~~ Add Overrides API component and rename old overrides to overrides-configs Aug 17, 2025

friedrichg requested changes Aug 18, 2025

View reviewed changes

pkg/cortex/modules.go Show resolved Hide resolved

docs/configuration/config-file-reference.md Outdated Show resolved Hide resolved

remove from all target

608f1ab

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch 2 times, most recently from 90512a2 to 608f1ab Compare August 20, 2025 16:01

bogdan-st added 2 commits August 20, 2025 22:14

remove consul and unneeded flags from test

15f9f3c

Signed-off-by: Bogdan Stancu <[email protected]>

missed some

a2a7665

Signed-off-by: Bogdan Stancu <[email protected]>

friedrichg requested changes Sep 2, 2025

View reviewed changes

suggestions

5e21b22

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch 3 times, most recently from 86bd67e to 948fd7a Compare September 8, 2025 16:21

Merge branch 'master' into overrides_api

61632a1

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch from 948fd7a to 61632a1 Compare September 8, 2025 16:31

araiu approved these changes Sep 10, 2025

View reviewed changes

dsabsay reviewed Sep 12, 2025

View reviewed changes

sugestions from dsabsay

dff25e3

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch from 0346f20 to dff25e3 Compare September 20, 2025 19:47

lint

8e50e9a

Signed-off-by: Bogdan Stancu <[email protected]>

bogdan-st force-pushed the overrides_api branch from e1bc9be to 8e50e9a Compare September 20, 2025 20:14

bogdan-st added 2 commits September 20, 2025 23:35

lint2

06195e0

Signed-off-by: Bogdan Stancu <[email protected]>

modernize?

8ba408b

Signed-off-by: Bogdan Stancu <[email protected]>

	\| [Set user overrides](#set-user-overrides) \| Overrides \|\| `PUT /api/v1/user-overrides` \|
	\| [Set user overrides](#set-user-overrides) \| Overrides \|\| `POST /api/v1/user-overrides` \|

Add Overrides API component and rename old overrides to overrides-configs #6975

Are you sure you want to change the base?

Add Overrides API component and rename old overrides to overrides-configs #6975

Uh oh!

Conversation

bogdan-st commented Aug 15, 2025

Uh oh!

friedrichg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

friedrichg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedrichg commented Sep 2, 2025

Uh oh!

araiu left a comment

Choose a reason for hiding this comment

Uh oh!

dsabsay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!