User-configurable overrides: add optional check to block API requests…

… that conflict with runtime overrides

cmd/tempo/app/modules.go

@@ -207,7 +207,7 @@ func (t *App) initOverridesAPI() (services.Service, error) {
 		return services.NewIdleService(nil, nil), nil
 	}
 
-	userConfigOverridesAPI, err := userconfigurableoverridesapi.New(&cfg.Client, t.Overrides, NewOverridesValidator(&t.cfg))
+	userConfigOverridesAPI, err := userconfigurableoverridesapi.New(&cfg.API, &cfg.Client, t.Overrides, NewOverridesValidator(&t.cfg))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create user-configurable overrides API: %w", err)
 	}

go.mod

@@ -54,7 +54,7 @@ require (
 	github.com/prometheus/client_model v0.4.0
 	github.com/prometheus/common v0.44.0
 	github.com/prometheus/prometheus v0.47.1
-	github.com/prometheus/statsd_exporter v0.22.7 // indirect
+	github.com/prometheus/statsd_exporter v0.22.7
 	github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e
 	github.com/sirupsen/logrus v1.9.3 // indirect
 	github.com/sony/gobreaker v0.4.1
@@ -101,6 +101,7 @@ require (
 	github.com/Azure/go-autorest/autorest v0.11.29
 	github.com/Azure/go-autorest/autorest/adal v0.9.23
 	github.com/Azure/go-autorest/autorest/azure/auth v0.5.12
+	github.com/brianvoe/gofakeit/v6 v6.25.0
 	github.com/evanphx/json-patch v4.12.0+incompatible
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da
 	github.com/googleapis/gax-go/v2 v2.12.0

go.sum

@@ -187,6 +187,8 @@ github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+Ce
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
 github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
+github.com/brianvoe/gofakeit/v6 v6.25.0 h1:ZpFjktOpLZUeF8q223o0rUuXtA+m5qW5srjvVi+JkXk=
+github.com/brianvoe/gofakeit/v6 v6.25.0/go.mod h1:Xj58BMSnFqcn/fAQeSK+/PLtC5kSb7FJIq4JyGa8vEs=
 github.com/cenkalti/backoff/v4 v4.2.1 h1:y4OZtCnogmCPw98Zjyt5a6+QwPLGkiQsYW5oUqylYbM=
 github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=

modules/overrides/config_test.go

@@ -1,14 +1,18 @@
 package overrides
 
 import (
+	"bytes"
 	"encoding/json"
 	"flag"
 	"reflect"
 	"testing"
 
+	"github.com/brianvoe/gofakeit/v6"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v2"
+
+	"github.com/grafana/tempo/modules/overrides/userconfigurable/client"
 )
 
 // Copied from Cortex
@@ -250,3 +254,33 @@ func rvCountFields(rv reflect.Value) int {
 	}
 	return n
 }
+
+func TestOverrides_AssertUserConfigurableOverridesAreASubsetOfRuntimeOverrides(t *testing.T) {
+	userConfigurableOverrides := client.Limits{}
+
+	err := gofakeit.Struct(&userConfigurableOverrides)
+	assert.NoError(t, err)
+
+	// TODO clear out collection_interval because unmarshalling a time.Duration into overrides.Overrides
+	//  fails. The JSON decoder is not able to parse creations correctly, so e.g. a string like "30s" is
+	//  not considered valid.
+	//  To fix this we should migrate the various time.Duration to a similar type like client.Duration and
+	//  verify they operate the same when marshalling/unmshalling yaml.
+	userConfigurableOverrides.MetricsGenerator.CollectionInterval = nil
+
+	// encode to json
+	var buf bytes.Buffer
+	encoder := json.NewEncoder(&buf)
+	err = encoder.Encode(&userConfigurableOverrides)
+	assert.NoError(t, err)
+
+	// and decode back to overrides.Overrides
+	d := json.NewDecoder(&buf)
+
+	// all fields should be known
+	d.DisallowUnknownFields()
+
+	var runtimeOverrides Overrides
+	err = d.Decode(&runtimeOverrides)
+	assert.NoError(t, err)
+}

modules/overrides/interface.go

@@ -18,9 +18,23 @@ type Service interface {
 	Interface
 }
 
+type Level string
+
+const (
+	LevelRuntime          Level = "runtime"
+	LevelUserConfigurable Level = "user-configurable"
+)
+
 type Interface interface {
 	prometheus.Collector
 
+	// GetLevel returns Interface constrained to the given level. You can use this to get overrides
+	// from a lower level that are masked.
+	GetLevel(level Level) Interface
+
+	// temp
+	GetOverrides(userID string) *Overrides
+
 	// Config
 	IngestionRateStrategy() string
 	MaxLocalTracesPerUser(userID string) int

modules/overrides/runtime_config_overrides.go

@@ -241,6 +241,14 @@ func (o *runtimeConfigOverridesManager) WriteStatusRuntimeConfig(w io.Writer, r
 	return nil
 }
 
+func (o *runtimeConfigOverridesManager) GetLevel(_ Level) Interface {
+	return o
+}
+
+func (o *runtimeConfigOverridesManager) GetOverrides(userID string) *Overrides {
+	return o.getOverridesForUser(userID)
+}
+
 // IngestionRateStrategy returns whether the ingestion rate limit should be individually applied
 // to each distributor instance (local) or evenly shared across the cluster (global).
 func (o *runtimeConfigOverridesManager) IngestionRateStrategy() string {

modules/overrides/user_configurable_overrides.go

@@ -39,7 +39,16 @@ type UserConfigurableOverridesConfig struct {
 	// PollInterval controls how often the overrides will be refreshed by polling the backend
 	PollInterval time.Duration `yaml:"poll_interval"`
 
-	Client userconfigurableoverrides.Config `yaml:"client"`
+	Client userconfigurableoverrides.Config   `yaml:"client"`
+	API    UserConfigurableOverridesAPIConfig `yaml:"api"`
+}
+
+type UserConfigurableOverridesAPIConfig struct {
+	// CheckForConflictingRuntimeOverrides will refuse requests that create new user-configurable
+	// overrides for a tenant that has conflicting runtime overrides. If the user already has
+	// user-configurable overrides requests will still be allowed.
+	// This check can be ignored by the caller by setting the query parameter skip-conflicting-overrides-check=true
+	CheckForConflictingRuntimeOverrides bool `yaml:"check_for_conflicting_runtime_overrides"`
 }
 
 func (cfg *UserConfigurableOverridesConfig) RegisterFlagsAndApplyDefaults(f *flag.FlagSet) {
@@ -195,6 +204,13 @@ func (o *userConfigurableOverridesManager) setTenantLimit(userID string, limits
 	}
 }
 
+func (o *userConfigurableOverridesManager) GetLevel(level Level) Interface {
+	if level == LevelUserConfigurable {
+		return o
+	}
+	return o.Interface.GetLevel(level)
+}
+
 func (o *userConfigurableOverridesManager) Forwarders(userID string) []string {
 	if forwarders, ok := o.getTenantLimits(userID).GetForwarders(); ok {
 		return forwarders

modules/overrides/user_configurable_overrides_test.go

@@ -95,18 +95,18 @@ func TestUserConfigOverridesManager_allFields(t *testing.T) {
 	// Inject user-configurable overrides
 	mgr.tenantLimits[tenant1] = &userconfigurableoverrides.Limits{
 		Forwarders: &[]string{"my-forwarder"},
-		MetricsGenerator: &userconfigurableoverrides.LimitsMetricsGenerator{
+		MetricsGenerator: userconfigurableoverrides.LimitsMetricsGenerator{
 			Processors:         map[string]struct{}{"service-graphs": {}},
 			DisableCollection:  boolPtr(true),
 			CollectionInterval: &userconfigurableoverrides.Duration{Duration: 60 * time.Second},
-			Processor: &userconfigurableoverrides.LimitsMetricsGeneratorProcessor{
-				ServiceGraphs: &userconfigurableoverrides.LimitsMetricsGeneratorProcessorServiceGraphs{
+			Processor: userconfigurableoverrides.LimitsMetricsGeneratorProcessor{
+				ServiceGraphs: userconfigurableoverrides.LimitsMetricsGeneratorProcessorServiceGraphs{
 					Dimensions:               &[]string{"sg-dimension"},
 					EnableClientServerPrefix: boolPtr(true),
 					PeerAttributes:           &[]string{"attribute"},
 					HistogramBuckets:         &[]float64{1, 2, 3, 4, 5},
 				},
-				SpanMetrics: &userconfigurableoverrides.LimitsMetricsGeneratorProcessorSpanMetrics{
+				SpanMetrics: userconfigurableoverrides.LimitsMetricsGeneratorProcessorSpanMetrics{
 					Dimensions:       &[]string{"sm-dimension"},
 					EnableTargetInfo: boolPtr(true),
 					FilterPolicies: &[]filterconfig.FilterPolicy{
@@ -372,7 +372,7 @@ func TestUserConfigOverridesManager_MergeRuntimeConfig(t *testing.T) {
 	// Set Forwarders in UserConfigOverrides limits
 	mgr.tenantLimits[tenantID] = &userconfigurableoverrides.Limits{
 		Forwarders: &[]string{"my-other-forwarder"},
-		MetricsGenerator: &userconfigurableoverrides.LimitsMetricsGenerator{
+		MetricsGenerator: userconfigurableoverrides.LimitsMetricsGenerator{
 			Processors: map[string]struct{}{"local-blocks": {}},
 		},
 	}

modules/overrides/userconfigurable/api/api.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"reflect"
 
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/go-kit/log"
@@ -21,6 +22,10 @@ import (
 	"github.com/grafana/tempo/tempodb/backend"
 )
 
+var (
+	errConflictingRuntimeOverrides = errors.New("tenant has conflicting overrides set in runtime config, contact your system administrator to perform changes through the API")
+)
+
 type Validator interface {
 	Validate(limits *client.Limits) error
 }
@@ -30,20 +35,22 @@ type Validator interface {
 type UserConfigOverridesAPI struct {
 	services.Service
 
+	cfg       *overrides.UserConfigurableOverridesAPIConfig
 	client    client.Client
 	overrides overrides.Interface
 	validator Validator
 
 	logger log.Logger
 }
 
-func New(config *client.Config, overrides overrides.Interface, validator Validator) (*UserConfigOverridesAPI, error) {
-	client, err := client.New(config)
+func New(cfg *overrides.UserConfigurableOverridesAPIConfig, clientCfg *client.Config, overrides overrides.Interface, validator Validator) (*UserConfigOverridesAPI, error) {
+	client, err := client.New(clientCfg)
 	if err != nil {
 		return nil, err
 	}
 
 	api := &UserConfigOverridesAPI{
+		cfg:       cfg,
 		client:    client,
 		overrides: overrides,
 		validator: validator,
@@ -75,7 +82,7 @@ func (a *UserConfigOverridesAPI) get(ctx context.Context, userID string) (*clien
 }
 
 // set the Limits. Can return backend.ErrVersionDoesNotMatch, validationError
-func (a *UserConfigOverridesAPI) set(ctx context.Context, userID string, limits *client.Limits, version backend.Version) (backend.Version, error) {
+func (a *UserConfigOverridesAPI) set(ctx context.Context, userID string, limits *client.Limits, version backend.Version, skipConflictingOverridesCheck bool) (backend.Version, error) {
 	span, ctx := opentracing.StartSpanFromContext(ctx, "UserConfigOverridesAPI.set", opentracing.Tags{
 		"userID":  userID,
 		"version": version,
@@ -89,6 +96,13 @@ func (a *UserConfigOverridesAPI) set(ctx context.Context, userID string, limits
 		return "", newValidationError(err)
 	}
 
+	if a.cfg.CheckForConflictingRuntimeOverrides && !skipConflictingOverridesCheck {
+		err = a.assertNoConflictingRuntimeOverrides(ctx, userID)
+		if err != nil {
+			return "", err
+		}
+	}
+
 	level.Info(a.logger).Log("traceID", traceID, "msg", "storing user-configurable overrides", "userID", userID, "limits", logLimits(limits), "version", version)
 
 	newVersion, err := a.client.Set(ctx, userID, limits, version)
@@ -97,7 +111,7 @@ func (a *UserConfigOverridesAPI) set(ctx context.Context, userID string, limits
 	return newVersion, err
 }
 
-func (a *UserConfigOverridesAPI) update(ctx context.Context, userID string, patch []byte) (*client.Limits, backend.Version, error) {
+func (a *UserConfigOverridesAPI) update(ctx context.Context, userID string, patch []byte, skipConflictingOverridesCheck bool) (*client.Limits, backend.Version, error) {
 	span, ctx := opentracing.StartSpanFromContext(ctx, "UserConfigOverridesAPI.update", opentracing.Tags{
 		"userID": userID,
 	})
@@ -133,7 +147,7 @@ func (a *UserConfigOverridesAPI) update(ctx context.Context, userID string, patc
 		return nil, "", newValidationError(err)
 	}
 
-	version, err := a.set(ctx, userID, patchedLimits, currVersion)
+	version, err := a.set(ctx, userID, patchedLimits, currVersion, skipConflictingOverridesCheck)
 	if errors.Is(err, backend.ErrVersionDoesNotMatch) {
 		return nil, "", errors.New("overrides have been modified during request processing, try again")
 	}
@@ -169,6 +183,42 @@ func (a *UserConfigOverridesAPI) parseLimits(body io.Reader) (*client.Limits, er
 	return &limits, err
 }
 
+func (a *UserConfigOverridesAPI) assertNoConflictingRuntimeOverrides(ctx context.Context, userID string) error {
+	limits, _, err := a.client.Get(ctx, userID)
+	if err != nil && !errors.Is(err, backend.ErrDoesNotExist) {
+		return err
+	}
+	// we already store limits for this user, don't check further
+	if limits != nil {
+		return nil
+	}
+
+	runtimeOverridesManager := a.overrides.GetLevel(overrides.LevelRuntime)
+	runtimeOverrides := runtimeOverridesManager.GetOverrides(userID)
+
+	// convert overrides.Overrides to client.Limits through marshalling and unmarshalling it from json
+	// this is the easiest way to convert the optional fields
+	marshalledOverrides, err := jsoniter.Marshal(runtimeOverrides)
+	if err != nil {
+		return err
+	}
+	var runtimeLimits client.Limits
+	err = jsoniter.Unmarshal(marshalledOverrides, &runtimeLimits)
+	if err != nil {
+		return err
+	}
+
+	// clear out processors since we merge this field
+	runtimeLimits.MetricsGenerator.Processors = nil
+
+	emptyLimits := client.Limits{}
+	if reflect.DeepEqual(runtimeLimits, emptyLimits) {
+		return nil
+	}
+
+	return errConflictingRuntimeOverrides
+}
+
 // validationError is returned when the request can not be accepted because of a client error
 type validationError struct {
 	err error