diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index be3b38f6e6..a1439abaf7 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -68,6 +68,10 @@ Where default_value is the value to use if the environment variable is undefined # CLI flag: -http.prefix [http_prefix: | default = "/api/prom"] +# Set to "legacy" to enforce strict legacy-compatible name rules. +# CLI flag: -name.validation-scheme +[name_validation_scheme: | default = "legacy"] + resource_monitor: # Comma-separated list of resources to monitor. Supported values are cpu and # heap, which tracks metrics from github.com/prometheus/procfs and diff --git a/pkg/cortex/configinit/init.go b/pkg/cortex/configinit/init.go deleted file mode 100644 index bfb180797b..0000000000 --- a/pkg/cortex/configinit/init.go +++ /dev/null @@ -1,8 +0,0 @@ -package configinit - -import "github.com/prometheus/common/model" - -func init() { - // nolint:staticcheck - model.NameValidationScheme = model.LegacyValidation -} diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index f50fcf26e1..9e0073a9ba 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -14,6 +14,9 @@ import ( "github.com/go-kit/log/level" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" + prom_config "github.com/prometheus/prometheus/config" + "github.com/prometheus/prometheus/promql" prom_storage "github.com/prometheus/prometheus/storage" "github.com/weaveworks/common/server" "github.com/weaveworks/common/signals" @@ -30,7 +33,6 @@ import ( "github.com/cortexproject/cortex/pkg/configs" configAPI "github.com/cortexproject/cortex/pkg/configs/api" "github.com/cortexproject/cortex/pkg/configs/db" - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" "github.com/cortexproject/cortex/pkg/cortex/storage" "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/distributor" @@ -90,11 +92,12 @@ var ( // Config is the root config for Cortex. type Config struct { - Target flagext.StringSliceCSV `yaml:"target"` - AuthEnabled bool `yaml:"auth_enabled"` - PrintConfig bool `yaml:"-"` - HTTPPrefix string `yaml:"http_prefix"` - ResourceMonitor configs.ResourceMonitor `yaml:"resource_monitor"` + Target flagext.StringSliceCSV `yaml:"target"` + AuthEnabled bool `yaml:"auth_enabled"` + PrintConfig bool `yaml:"-"` + HTTPPrefix string `yaml:"http_prefix"` + ResourceMonitor configs.ResourceMonitor `yaml:"resource_monitor"` + NameValidationScheme string `yaml:"name_validation_scheme"` ExternalQueryable prom_storage.Queryable `yaml:"-"` ExternalPusher ruler.Pusher `yaml:"-"` @@ -146,6 +149,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.") f.BoolVar(&c.PrintConfig, "print.config", false, "Print the config and exit.") f.StringVar(&c.HTTPPrefix, "http.prefix", "/api/prom", "HTTP path prefix for Cortex API.") + f.StringVar(&c.NameValidationScheme, "name.validation-scheme", "legacy", "Validation scheme for metric and label names. Set to utf8 to allow UTF-8 characters.") c.API.RegisterFlags(f) c.registerServerFlagsWithChangedDefaultValues(f) @@ -181,6 +185,11 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { // Validate the cortex config and returns an error if the validation // doesn't pass func (c *Config) Validate(log log.Logger) error { + switch c.NameValidationScheme { + case "", prom_config.LegacyValidationConfig, prom_config.UTF8ValidationConfig: + default: + return fmt.Errorf("invalid name validation scheme: %s", c.NameValidationScheme) + } if err := c.validateYAMLEmptyNodes(); err != nil { return err } @@ -349,7 +358,12 @@ func New(cfg Config) (*Cortex, error) { } os.Exit(0) } - + //nolint:staticcheck // SA1019: using deprecated NameValidationScheme intentionally as a temporary compatibility workaround for UTF-8 migration issues + if cfg.NameValidationScheme == prom_config.UTF8ValidationConfig { + model.NameValidationScheme = model.UTF8Validation + } else { + model.NameValidationScheme = model.LegacyValidation + } // Swap out the default resolver to support multiple tenant IDs separated by a '|' if cfg.TenantFederation.Enabled { util_log.WarnExperimentalUse("tenant-federation") diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index 6cac224319..8cc466932f 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + prom_config "github.com/prometheus/prometheus/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/weaveworks/common/server" @@ -217,6 +218,42 @@ func TestConfigValidation(t *testing.T) { }, expectedError: nil, }, + { + name: "should not fail validation for empty name validation scheme (use legacy by default)", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = "" + return configuration + }, + expectedError: nil, + }, + { + name: "should not fail validation for legacy name validation scheme", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = prom_config.LegacyValidationConfig + return configuration + }, + expectedError: nil, + }, + { + name: "should not fail validation for utf-8 name validation scheme", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = prom_config.UTF8ValidationConfig + return configuration + }, + expectedError: nil, + }, + { + name: "should fail validation for invalid name validation scheme", + getTestConfig: func() *Config { + configuration := newDefaultConfig() + configuration.NameValidationScheme = "invalid" + return configuration + }, + expectedError: fmt.Errorf("invalid name validation scheme"), + }, } { t.Run(tc.name, func(t *testing.T) { err := tc.getTestConfig().Validate(nil) diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index c9f931199a..9f5e684eba 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -35,7 +35,6 @@ import ( "google.golang.org/grpc/status" promchunk "github.com/cortexproject/cortex/pkg/chunk/encoding" - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ha" "github.com/cortexproject/cortex/pkg/ingester" diff --git a/pkg/util/validation/limits.go b/pkg/util/validation/limits.go index fd077ebd18..c5d35390a2 100644 --- a/pkg/util/validation/limits.go +++ b/pkg/util/validation/limits.go @@ -151,7 +151,7 @@ type Limits struct { MetricRelabelConfigs []*relabel.Config `yaml:"metric_relabel_configs,omitempty" json:"metric_relabel_configs,omitempty" doc:"nocli|description=List of metric relabel configurations. Note that in most situations, it is more effective to use metrics relabeling directly in the Prometheus server, e.g. remote_write.write_relabel_configs."` MaxNativeHistogramBuckets int `yaml:"max_native_histogram_buckets" json:"max_native_histogram_buckets"` PromoteResourceAttributes []string `yaml:"promote_resource_attributes" json:"promote_resource_attributes"` - + UseUTF8Validation bool `yaml:"use_utf8_validation" json:"use_utf8_validation"` // Ingester enforced limits. // Series MaxLocalSeriesPerUser int `yaml:"max_series_per_user" json:"max_series_per_user"` diff --git a/pkg/util/validation/validate.go b/pkg/util/validation/validate.go index 7f6b09c231..2312e92932 100644 --- a/pkg/util/validation/validate.go +++ b/pkg/util/validation/validate.go @@ -250,6 +250,12 @@ func ValidateExemplar(validateMetrics *ValidateMetrics, userID string, ls []cort // ValidateLabels returns an err if the labels are invalid. // The returned error may retain the provided series labels. func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, skipLabelNameValidation bool) ValidationError { + //nolint:staticcheck // SA1019: using deprecated NameValidationScheme intentionally as a temporary compatibility workaround for UTF-8 migration issues + if limits.UseUTF8Validation { + model.NameValidationScheme = model.UTF8Validation + } else { + model.NameValidationScheme = model.LegacyValidation + } if limits.EnforceMetricName { unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls) if err != nil { diff --git a/pkg/util/validation/validate_test.go b/pkg/util/validation/validate_test.go index 11a1b2fc24..36764fa801 100644 --- a/pkg/util/validation/validate_test.go +++ b/pkg/util/validation/validate_test.go @@ -5,6 +5,8 @@ import ( "strings" "testing" + "github.com/cortexproject/cortex/pkg/cortexpb" + util_log "github.com/cortexproject/cortex/pkg/util/log" "github.com/go-kit/log" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" @@ -15,10 +17,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/weaveworks/common/httpgrpc" - - _ "github.com/cortexproject/cortex/pkg/cortex/configinit" - "github.com/cortexproject/cortex/pkg/cortexpb" - util_log "github.com/cortexproject/cortex/pkg/util/log" ) func TestValidateLabels(t *testing.T) { @@ -153,6 +151,137 @@ func TestValidateLabels(t *testing.T) { `), "cortex_discarded_samples_total")) } +// same test +func TestValidateLabels_Legacy(t *testing.T) { + cfg := new(Limits) + userID := "testUser" + + reg := prometheus.NewRegistry() + validateMetrics := NewValidateMetrics(reg) + + cfg.MaxLabelValueLength = 25 + cfg.MaxLabelNameLength = 25 + cfg.MaxLabelNamesPerSeries = 2 + cfg.MaxLabelsSizeBytes = 90 + cfg.EnforceMetricName = true + cfg.UseUTF8Validation = false // Legacy validation + cfg.LimitsPerLabelSet = []LimitsPerLabelSet{ + { + Limits: LimitsPerLabelSetEntry{MaxSeries: 0}, + LabelSet: labels.FromMap(map[string]string{ + model.MetricNameLabel: "foo", + }), + Hash: 0, + }, + { + Limits: LimitsPerLabelSetEntry{MaxSeries: 0}, + LabelSet: labels.EmptyLabels(), + Hash: 1, + }, + } + + for i, c := range []struct { + metric model.Metric + skipLabelNameValidation bool + expectedErr error + }{ + {map[model.LabelName]model.LabelValue{}, false, newNoMetricNameError()}, + {map[model.LabelName]model.LabelValue{model.MetricNameLabel: " "}, false, newInvalidMetricNameError(" ")}, + {map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid", "foo ": "bar"}, false, + newInvalidLabelError([]cortexpb.LabelAdapter{ + {Name: model.MetricNameLabel, Value: "valid"}, + {Name: "foo ", Value: "bar"}, + }, "foo ")}, + {map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid"}, false, nil}, + { + map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelName", "this_is_a_really_really_long_name_that_should_cause_an_error": "test_value_please_ignore"}, + false, + newLabelNameTooLongError([]cortexpb.LabelAdapter{ + {Name: model.MetricNameLabel, Value: "badLabelName"}, + {Name: "this_is_a_really_really_long_name_that_should_cause_an_error", Value: "test_value_please_ignore"}, + }, "this_is_a_really_really_long_name_that_should_cause_an_error", cfg.MaxLabelNameLength), + }, + { + map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelValue", "much_shorter_name": "test_value_please_ignore_no_really_nothing_to_see_here"}, + false, + newLabelValueTooLongError([]cortexpb.LabelAdapter{ + {Name: model.MetricNameLabel, Value: "badLabelValue"}, + {Name: "much_shorter_name", Value: "test_value_please_ignore_no_really_nothing_to_see_here"}, + }, "much_shorter_name", "test_value_please_ignore_no_really_nothing_to_see_here", cfg.MaxLabelValueLength), + }, + { + map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop"}, + false, + newTooManyLabelsError([]cortexpb.LabelAdapter{ + {Name: model.MetricNameLabel, Value: "foo"}, + {Name: "bar", Value: "baz"}, + {Name: "blip", Value: "blop"}, + }, 2), + }, + { + map[model.LabelName]model.LabelValue{model.MetricNameLabel: "exactly_twenty_five_chars", "exactly_twenty_five_chars": "exactly_twenty_five_chars"}, + false, + labelSizeBytesExceededError([]cortexpb.LabelAdapter{ + {Name: model.MetricNameLabel, Value: "exactly_twenty_five_chars"}, + {Name: "exactly_twenty_five_chars", Value: "exactly_twenty_five_chars"}, + }, 91, cfg.MaxLabelsSizeBytes), + }, + {map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "invalid%label&name": "bar"}, true, nil}, + } { + convertedLabels := cortexpb.FromMetricsToLabelAdapters(c.metric) + + t.Logf("Case %d:\n Input Metric: %+v\n SkipLabelNameValidation: %v", i, c.metric, c.skipLabelNameValidation) + + err := ValidateLabels(validateMetrics, cfg, userID, convertedLabels, c.skipLabelNameValidation) + + if err != nil { + t.Logf(" Got Error: %v", err) + } else { + t.Log(" Got Error: ") + } + + if c.expectedErr != nil { + t.Logf(" Expected Error: %v", c.expectedErr) + } else { + t.Log(" Expected Error: ") + } + + assert.Equal(t, c.expectedErr, err, "wrong error (case %d)", i) + } + + // Metrics validation + validateMetrics.DiscardedSamples.WithLabelValues("random reason", "different user").Inc() + + require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(` + # HELP cortex_discarded_samples_total The total number of samples that were discarded. + # TYPE cortex_discarded_samples_total counter + cortex_discarded_samples_total{reason="label_invalid",user="testUser"} 1 + cortex_discarded_samples_total{reason="label_name_too_long",user="testUser"} 1 + cortex_discarded_samples_total{reason="label_value_too_long",user="testUser"} 1 + cortex_discarded_samples_total{reason="max_label_names_per_series",user="testUser"} 1 + cortex_discarded_samples_total{reason="metric_name_invalid",user="testUser"} 1 + cortex_discarded_samples_total{reason="missing_metric_name",user="testUser"} 1 + cortex_discarded_samples_total{reason="labels_size_bytes_exceeded",user="testUser"} 1 + cortex_discarded_samples_total{reason="random reason",user="different user"} 1 + `), "cortex_discarded_samples_total")) + + require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(` + # HELP cortex_label_size_bytes The combined size in bytes of all labels and label values for a time series. + # TYPE cortex_label_size_bytes histogram + cortex_label_size_bytes_bucket{user="testUser",le="+Inf"} 3 + cortex_label_size_bytes_sum{user="testUser"} 148 + cortex_label_size_bytes_count{user="testUser"} 3 + `), "cortex_label_size_bytes")) + + DeletePerUserValidationMetrics(validateMetrics, userID, util_log.Logger) + + require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(` + # HELP cortex_discarded_samples_total The total number of samples that were discarded. + # TYPE cortex_discarded_samples_total counter + cortex_discarded_samples_total{reason="random reason",user="different user"} 1 + `), "cortex_discarded_samples_total")) +} + func TestValidateExemplars(t *testing.T) { userID := "testUser" reg := prometheus.NewRegistry()