suggestions

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

Author: bogdan-st

integration/overrides_test.go

@@ -52,13 +52,13 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
 	flags := map[string]string{
 		"-target": "overrides",
 
-		"-overrides.runtime-config-file":  "runtime.yaml",
-		"-overrides.backend":              "s3",
-		"-overrides.s3.access-key-id":     e2edb.MinioAccessKey,
-		"-overrides.s3.secret-access-key": e2edb.MinioSecretKey,
-		"-overrides.s3.bucket-name":       "cortex",
-		"-overrides.s3.endpoint":          minio.NetworkHTTPEndpoint(),
-		"-overrides.s3.insecure":          "true",
+		"-runtime-config.file":                 "runtime.yaml",
+		"-runtime-config.backend":              "s3",
+		"-runtime-config.s3.access-key-id":     e2edb.MinioAccessKey,
+		"-runtime-config.s3.secret-access-key": e2edb.MinioSecretKey,
+		"-runtime-config.s3.bucket-name":       "cortex",
+		"-runtime-config.s3.endpoint":          minio.NetworkHTTPEndpoint(),
+		"-runtime-config.s3.insecure":          "true",
 	}
 
 	cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides", flags, "")
@@ -100,15 +100,15 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
 		assert.Empty(t, overrides)
 	})
 
-	t.Run("PUT overrides for new user", func(t *testing.T) {
+	t.Run("POST overrides for new user", func(t *testing.T) {
 		newOverrides := map[string]interface{}{
 			"ingestion_rate":       6000,
 			"ingestion_burst_size": 7000,
 		}
 		requestBody, err := json.Marshal(newOverrides)
 		require.NoError(t, err)
 
-		req, err := http.NewRequest("PUT", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
+		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", "user3")
 		req.Header.Set("Content-Type", "application/json")
@@ -137,14 +137,14 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
 		assert.Equal(t, float64(7000), savedOverrides["ingestion_burst_size"])
 	})
 
-	t.Run("PUT overrides with invalid limit", func(t *testing.T) {
+	t.Run("POST overrides with invalid limit", func(t *testing.T) {
 		invalidOverrides := map[string]interface{}{
 			"invalid_limit": 5000,
 		}
 		requestBody, err := json.Marshal(invalidOverrides)
 		require.NoError(t, err)
 
-		req, err := http.NewRequest("PUT", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader(requestBody))
+		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", "user4")
 		req.Header.Set("Content-Type", "application/json")
@@ -156,8 +156,8 @@ func TestOverridesAPIWithRunningCortex(t *testing.T) {
 		assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
 	})
 
-	t.Run("PUT overrides with invalid JSON", func(t *testing.T) {
-		req, err := http.NewRequest("PUT", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader([]byte("invalid json")))
+	t.Run("POST overrides with invalid JSON", func(t *testing.T) {
+		req, err := http.NewRequest("POST", "http://"+cortexSvc.HTTPEndpoint()+"/api/v1/user-overrides", bytes.NewReader([]byte("invalid json")))
 		require.NoError(t, err)
 		req.Header.Set("X-Scope-OrgID", "user5")
 		req.Header.Set("Content-Type", "application/json")
@@ -208,16 +208,34 @@ func TestOverridesAPITenantExtraction(t *testing.T) {
 	minio := e2edb.NewMinio(9010, "cortex")
 	require.NoError(t, s.StartAndWaitReady(minio))
 
+	// Upload an empty runtime config file to S3
+	runtimeConfig := map[string]interface{}{
+		"overrides": map[string]interface{}{},
+	}
+	runtimeConfigData, err := yaml.Marshal(runtimeConfig)
+	require.NoError(t, err)
+
+	s3Client, err := s3.NewBucketWithConfig(nil, s3.Config{
+		Endpoint:  minio.HTTPEndpoint(),
+		Insecure:  true,
+		Bucket:    "cortex",
+		AccessKey: e2edb.MinioAccessKey,
+		SecretKey: e2edb.MinioSecretKey,
+	}, "overrides-test-tenant", nil)
+	require.NoError(t, err)
+
+	require.NoError(t, s3Client.Upload(context.Background(), "runtime.yaml", bytes.NewReader(runtimeConfigData)))
+
 	flags := map[string]string{
 		"-target": "overrides",
 
-		"-overrides.runtime-config-file":  "runtime.yaml",
-		"-overrides.backend":              "s3",
-		"-overrides.s3.access-key-id":     e2edb.MinioAccessKey,
-		"-overrides.s3.secret-access-key": e2edb.MinioSecretKey,
-		"-overrides.s3.bucket-name":       "cortex",
-		"-overrides.s3.endpoint":          minio.NetworkHTTPEndpoint(),
-		"-overrides.s3.insecure":          "true",
+		"-runtime-config.file":                 "runtime.yaml",
+		"-runtime-config.backend":              "s3",
+		"-runtime-config.s3.access-key-id":     e2edb.MinioAccessKey,
+		"-runtime-config.s3.secret-access-key": e2edb.MinioSecretKey,
+		"-runtime-config.s3.bucket-name":       "cortex",
+		"-runtime-config.s3.endpoint":          minio.NetworkHTTPEndpoint(),
+		"-runtime-config.s3.insecure":          "true",
 	}
 
 	cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-tenant", flags, "")
@@ -248,44 +266,3 @@ func TestOverridesAPITenantExtraction(t *testing.T) {
 
 	require.NoError(t, s.Stop(cortexSvc))
 }
-
-func TestOverridesAPIFilesystemBackendRejected(t *testing.T) {
-	s, err := e2e.NewScenario(networkName)
-	require.NoError(t, err)
-	defer s.Close()
-
-	t.Run("filesystem backend should be rejected", func(t *testing.T) {
-		flags := map[string]string{
-			"-target":                        "overrides",
-			"-overrides.runtime-config-file": "runtime.yaml",
-			"-overrides.backend":             "filesystem",
-		}
-
-		cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-filesystem", flags, "")
-
-		err = s.StartAndWaitReady(cortexSvc)
-		if err == nil {
-			t.Error("Expected Cortex to fail to start with filesystem backend, but it started successfully")
-			require.NoError(t, s.Stop(cortexSvc))
-		} else {
-			t.Logf("Expected failure with filesystem backend: %v", err)
-		}
-	})
-
-	t.Run("no backend specified should be rejected", func(t *testing.T) {
-		flags := map[string]string{
-			"-target":                        "overrides",
-			"-overrides.runtime-config-file": "runtime.yaml",
-		}
-
-		cortexSvc := e2ecortex.NewSingleBinary("cortex-overrides-no-backend", flags, "")
-
-		err = s.StartAndWaitReady(cortexSvc)
-		if err == nil {
-			t.Error("Expected Cortex to fail to start with no backend specified, but it started successfully")
-			require.NoError(t, s.Stop(cortexSvc))
-		} else {
-			t.Logf("Expected failure with no backend specified: %v", err)
-		}
-	})
-}

pkg/api/api.go

@@ -390,7 +390,7 @@ func (a *API) RegisterRulerAPI(r *ruler.API) {
 func (a *API) RegisterOverrides(o *overrides.API) {
 	// Register individual overrides API routes with the main API
 	a.RegisterRoute("/api/v1/user-overrides", http.HandlerFunc(o.GetOverrides), true, "GET")
-	a.RegisterRoute("/api/v1/user-overrides", http.HandlerFunc(o.SetOverrides), true, "PUT")
+	a.RegisterRoute("/api/v1/user-overrides", http.HandlerFunc(o.SetOverrides), true, "POST")
 	a.RegisterRoute("/api/v1/user-overrides", http.HandlerFunc(o.DeleteOverrides), true, "DELETE")
 
 	// Add link to the index page

pkg/cortex/cortex.go

@@ -127,7 +127,6 @@ type Config struct {
 	RuntimeConfig       runtimeconfig.Config                       `yaml:"runtime_config"`
 	MemberlistKV        memberlist.KVConfig                        `yaml:"memberlist"`
 	QueryScheduler      scheduler.Config                           `yaml:"query_scheduler"`
-	Overrides           overrides.Config                           `yaml:"overrides"`
 
 	Tracing tracing.Config `yaml:"tracing"`
 }
@@ -177,7 +176,6 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 	c.RuntimeConfig.RegisterFlags(f)
 	c.MemberlistKV.RegisterFlags(f)
 	c.QueryScheduler.RegisterFlags(f)
-	c.Overrides.RegisterFlags(f)
 	c.Tracing.RegisterFlags(f)
 }
 

pkg/cortex/modules.go

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

pkg/overrides/overrides.go

@@ -2,154 +2,34 @@ package overrides
 
 import (
 	"context"
-	"errors"
-	"flag"
 	"fmt"
-	"time"
 
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/thanos-io/objstore"
 
 	"github.com/cortexproject/cortex/pkg/storage/bucket"
+	"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
 	"github.com/cortexproject/cortex/pkg/util/services"
 )
 
 const (
-	// Error messages
-	ErrNoStorageBackendSpecified     = "overrides module requires a storage backend to be specified"
-	ErrFilesystemBackendNotSupported = "filesystem backend is not supported for overrides module; use S3, GCS, Azure, or Swift instead"
-	ErrInvalidBucketConfiguration    = "invalid bucket configuration"
 	ErrInvalidOverridesConfiguration = "invalid overrides configuration"
 	ErrFailedToCreateBucketClient    = "failed to create bucket client for overrides"
 )
 
-// Config holds configuration for the overrides module
-type Config struct {
-	// Enable the overrides API module
-
-	// Path to the runtime configuration file that can be updated via the overrides API
-	// CLI flag: -overrides.runtime-config-file
-	RuntimeConfigFile string `yaml:"runtime_config_file"`
-
-	// Storage configuration for the runtime config file
-	// All bucket backends (S3, GCS, Azure, Swift) are supported, but not filesystem
-	bucket.Config `yaml:",inline"`
-}
-
-// RegisterFlags registers the overrides module flags
-func (c *Config) RegisterFlags(f *flag.FlagSet) {
-
-	f.StringVar(&c.RuntimeConfigFile, "overrides.runtime-config-file", "runtime.yaml", "Path to the runtime configuration file that can be updated via the overrides API")
-
-	c.RegisterFlagsWithPrefix("overrides.", f)
-}
-
-// Validate validates the configuration and returns an error if validation fails
-func (c *Config) Validate() error {
-
-	if c.RuntimeConfigFile == "" {
-		c.RuntimeConfigFile = "runtime.yaml"
-	}
-
-	if c.Backend == "" {
-		c.Backend = bucket.S3
-	}
-
-	if c.Backend == bucket.Filesystem {
-		return errors.New(ErrFilesystemBackendNotSupported)
-	}
-
-	if c.Backend == bucket.S3 {
-		if c.S3.SignatureVersion == "" {
-			c.S3.SignatureVersion = "v4"
-		}
-		if c.S3.BucketLookupType == "" {
-			c.S3.BucketLookupType = "auto"
-		}
-		if !c.S3.SendContentMd5 {
-			c.S3.SendContentMd5 = true
-		}
-		if c.S3.HTTP.IdleConnTimeout == 0 {
-			c.S3.HTTP.IdleConnTimeout = 90 * time.Second
-		}
-		if c.S3.HTTP.ResponseHeaderTimeout == 0 {
-			c.S3.HTTP.ResponseHeaderTimeout = 2 * time.Minute
-		}
-		if c.S3.HTTP.TLSHandshakeTimeout == 0 {
-			c.S3.HTTP.TLSHandshakeTimeout = 10 * time.Second
-		}
-		if c.S3.HTTP.ExpectContinueTimeout == 0 {
-			c.S3.HTTP.ExpectContinueTimeout = 1 * time.Second
-		}
-		if c.S3.HTTP.MaxIdleConns == 0 {
-			c.S3.HTTP.MaxIdleConns = 100
-		}
-		if c.S3.HTTP.MaxIdleConnsPerHost == 0 {
-			c.S3.HTTP.MaxIdleConnsPerHost = 100
-		}
-	}
-
-	if c.Backend == bucket.Azure {
-		if c.Azure.MaxRetries == 0 {
-			c.Azure.MaxRetries = 20
-		}
-		if c.Azure.IdleConnTimeout == 0 {
-			c.Azure.IdleConnTimeout = 90 * time.Second
-		}
-		if c.Azure.ResponseHeaderTimeout == 0 {
-			c.Azure.ResponseHeaderTimeout = 2 * time.Minute
-		}
-		if c.Azure.TLSHandshakeTimeout == 0 {
-			c.Azure.TLSHandshakeTimeout = 10 * time.Second
-		}
-		if c.Azure.ExpectContinueTimeout == 0 {
-			c.Azure.ExpectContinueTimeout = 1 * time.Second
-		}
-		if c.Azure.MaxIdleConns == 0 {
-			c.Azure.MaxIdleConns = 100
-		}
-		if c.Azure.MaxIdleConnsPerHost == 0 {
-			c.Azure.MaxIdleConnsPerHost = 100
-		}
-	}
-
-	if c.Backend == bucket.Swift {
-		if c.Swift.AuthVersion == 0 {
-			c.Swift.AuthVersion = 0
-		}
-		if c.Swift.MaxRetries == 0 {
-			c.Swift.MaxRetries = 3
-		}
-		if c.Swift.ConnectTimeout == 0 {
-			c.Swift.ConnectTimeout = 10 * time.Second
-		}
-		if c.Swift.RequestTimeout == 0 {
-			c.Swift.RequestTimeout = 5 * time.Second
-		}
-	}
-
-	if err := c.Config.Validate(); err != nil {
-		return fmt.Errorf("%s: %w", ErrInvalidBucketConfiguration, err)
-	}
-
-	return nil
-}
-
-// API represents the overrides API module
 type API struct {
 	services.Service
-	cfg               Config
+	cfg               runtimeconfig.Config
 	logger            log.Logger
 	registerer        prometheus.Registerer
 	bucketClient      objstore.Bucket
 	runtimeConfigPath string
 }
 
-// New creates a new overrides API instance
-func New(cfg Config, logger log.Logger, registerer prometheus.Registerer) (*API, error) {
-	if err := cfg.Validate(); err != nil {
+func New(cfg runtimeconfig.Config, logger log.Logger, registerer prometheus.Registerer) (*API, error) {
+	if err := cfg.StorageConfig.Validate(); err != nil {
 		return nil, fmt.Errorf("%s: %w", ErrInvalidOverridesConfiguration, err)
 	}
 
@@ -163,18 +43,18 @@ func New(cfg Config, logger log.Logger, registerer prometheus.Registerer) (*API,
 }
 
 func (a *API) starting(ctx context.Context) error {
-	level.Info(a.logger).Log("msg", "overrides API starting", "runtime_config_file", a.cfg.RuntimeConfigFile, "backend", a.cfg.Backend)
+	level.Info(a.logger).Log("msg", "overrides API starting", "runtime_config_file", a.cfg.LoadPath, "backend", a.cfg.StorageConfig.Backend)
 
-	bucketClient, err := bucket.NewClient(ctx, a.cfg.Config, nil, "overrides", a.logger, a.registerer)
+	bucketClient, err := bucket.NewClient(ctx, a.cfg.StorageConfig, nil, "overrides", a.logger, a.registerer)
 	if err != nil {
 		level.Error(a.logger).Log("msg", ErrFailedToCreateBucketClient, "err", err)
 		return fmt.Errorf("%s: %w", ErrFailedToCreateBucketClient, err)
 	}
 	a.bucketClient = bucketClient
 
-	a.runtimeConfigPath = a.cfg.RuntimeConfigFile
+	a.runtimeConfigPath = a.cfg.LoadPath
 
-	level.Info(a.logger).Log("msg", "overrides API started successfully", "backend", a.cfg.Backend)
+	level.Info(a.logger).Log("msg", "overrides API started successfully", "backend", a.cfg.StorageConfig.Backend)
 	return nil
 }