Merge pull request #16080 from prometheus/owilliams/escapeconfig-00-cleanup

utf8: Remove support for legacy global validation setting

cmd/prometheus/main.go

@@ -140,8 +140,9 @@ var (
 )
 
 func init() {
-	// This can be removed when the default validation scheme in common is updated.
+	// This can be removed when the legacy global mode is fully deprecated.
 	model.NameValidationScheme = model.UTF8Validation
+
 	prometheus.MustRegister(versioncollector.NewCollector(strings.ReplaceAll(appName, "-", "_")))
 
 	var err error

cmd/prometheus/main_test.go

@@ -44,7 +44,7 @@ import (
 )
 
 func init() {
-	// This can be removed when the default validation scheme in common is updated.
+	// This can be removed when the legacy global mode is fully deprecated.
 	model.NameValidationScheme = model.UTF8Validation
 }
 

cmd/promtool/main.go

@@ -63,7 +63,7 @@ import (
 )
 
 func init() {
-	// This can be removed when the default validation scheme in common is updated.
+	// This can be removed when the legacy global mode is fully deprecated.
 	model.NameValidationScheme = model.UTF8Validation
 }
 

cmd/promtool/main_test.go

@@ -40,7 +40,7 @@ import (
 )
 
 func init() {
-	// This can be removed when the default validation scheme in common is updated.
+	// This can be removed when the legacy global mode is fully deprecated.
 	model.NameValidationScheme = model.UTF8Validation
 }
 

config/config.go

@@ -837,12 +837,11 @@ func (c *ScrapeConfig) Validate(globalConfig GlobalConfig) error {
 		}
 	}
 
-	switch globalConfig.MetricNameValidationScheme {
-	case LegacyValidationConfig:
-	case "", UTF8ValidationConfig:
 	if model.NameValidationScheme != model.UTF8Validation {
-			panic("utf8 name validation requested but model.NameValidationScheme is not set to UTF8")
+		return errors.New("model.NameValidationScheme must be set to UTF8")
 	}
+	switch globalConfig.MetricNameValidationScheme {
+	case "", LegacyValidationConfig, UTF8ValidationConfig:
 	default:
 		return fmt.Errorf("unknown name validation method specified, must be either 'legacy' or 'utf8', got %s", globalConfig.MetricNameValidationScheme)
 	}

config/config_test.go

@@ -15,7 +15,6 @@ package config
 
 import (
 	"crypto/tls"
-	"encoding/json"
 	"fmt"
 	"net/url"
 	"os"
@@ -62,11 +61,6 @@ import (
 	"github.com/prometheus/prometheus/util/testutil"
 )
 
-func init() {
-	// This can be removed when the default validation scheme in common is updated.
-	model.NameValidationScheme = model.UTF8Validation
-}
-
 func mustParseURL(u string) *config.URL {
 	parsed, err := url.Parse(u)
 	if err != nil {
@@ -1716,11 +1710,7 @@ var expectedErrors = []struct {
 	},
 	{
 		filename: "labelname.bad.yml",
-		errMsg:   `"not$allowed" is not a valid label name`,
+		errMsg:   `"\xff" is not a valid label name`,
-	},
-	{
-		filename: "labelname2.bad.yml",
-		errMsg:   `"not:allowed" is not a valid label name`,
 	},
 	{
 		filename: "labelvalue.bad.yml",
@@ -1792,16 +1782,12 @@ var expectedErrors = []struct {
 	},
 	{
 		filename: "labelmap.bad.yml",
-		errMsg:   "\"l-$1\" is invalid 'replacement' for labelmap action",
+		errMsg:   "!!binary value contains invalid base64 data",
 	},
 	{
 		filename: "lowercase.bad.yml",
 		errMsg:   "relabel configuration for lowercase action requires 'target_label' value",
 	},
-	{
-		filename: "lowercase2.bad.yml",
-		errMsg:   "\"42lab\" is invalid 'target_label' for lowercase action",
-	},
 	{
 		filename: "lowercase3.bad.yml",
 		errMsg:   "'replacement' can not be set for lowercase action",
@@ -1810,10 +1796,6 @@ var expectedErrors = []struct {
 		filename: "uppercase.bad.yml",
 		errMsg:   "relabel configuration for uppercase action requires 'target_label' value",
 	},
-	{
-		filename: "uppercase2.bad.yml",
-		errMsg:   "\"42lab\" is invalid 'target_label' for uppercase action",
-	},
 	{
 		filename: "uppercase3.bad.yml",
 		errMsg:   "'replacement' can not be set for uppercase action",
@@ -2181,10 +2163,6 @@ var expectedErrors = []struct {
 }
 
 func TestBadConfigs(t *testing.T) {
-	model.NameValidationScheme = model.LegacyValidation
-	defer func() {
-		model.NameValidationScheme = model.UTF8Validation
-	}()
 	for _, ee := range expectedErrors {
 		_, err := LoadFile("testdata/"+ee.filename, false, promslog.NewNopLogger())
 		require.ErrorContains(t, err, ee.errMsg,
@@ -2192,23 +2170,7 @@ func TestBadConfigs(t *testing.T) {
 	}
 }
 
-func TestBadStaticConfigsJSON(t *testing.T) {
-	model.NameValidationScheme = model.LegacyValidation
-	defer func() {
-		model.NameValidationScheme = model.UTF8Validation
-	}()
-	content, err := os.ReadFile("testdata/static_config.bad.json")
-	require.NoError(t, err)
-	var tg targetgroup.Group
-	err = json.Unmarshal(content, &tg)
-	require.Error(t, err)
-}
-
 func TestBadStaticConfigsYML(t *testing.T) {
-	model.NameValidationScheme = model.LegacyValidation
-	defer func() {
-		model.NameValidationScheme = model.UTF8Validation
-	}()
 	content, err := os.ReadFile("testdata/static_config.bad.yml")
 	require.NoError(t, err)
 	var tg targetgroup.Group
@@ -2453,11 +2415,6 @@ func TestScrapeConfigDisableCompression(t *testing.T) {
 }
 
 func TestScrapeConfigNameValidationSettings(t *testing.T) {
-	model.NameValidationScheme = model.UTF8Validation
-	defer func() {
-		model.NameValidationScheme = model.LegacyValidation
-	}()
-
 	tests := []struct {
 		name         string
 		inputFile    string

config/testdata/labelmap.bad.yml

@@ -2,4 +2,4 @@ scrape_configs:
   - job_name: prometheus
     relabel_configs:
       - action: labelmap
-        replacement: l-$1
+        replacement: !!binary "/w==$1"

config/testdata/labelname.bad.yml

@@ -1,3 +1,3 @@
 global:
   external_labels:
-    not$allowed: value
+    !!binary "/w==": value

config/testdata/labelname2.bad.yml

@@ -1,3 +0,0 @@
-global:
-  external_labels:
-    'not:allowed': value

config/testdata/lowercase2.bad.yml

@@ -1,6 +0,0 @@
-scrape_configs:
-  - job_name: prometheus
-    relabel_configs:
-      - action: lowercase
-        source_labels: [__name__]
-        target_label: 42lab

config/testdata/static_config.bad.json

@@ -1,7 +0,0 @@
-{
-  "targets": ["1.2.3.4:9100"],
-  "labels": {
-    "some_valid_label": "foo",
-    "oops:this-label-is-invalid": "bar"
-  }
-}

config/testdata/static_config.bad.yml

@@ -1,4 +1,4 @@
 targets: ['1.2.3.4:9001', '1.2.3.5:9090']
 labels:
   valid_label: foo
-  not:valid_label: bar
+  !!binary "/w==": bar

config/testdata/uppercase2.bad.yml

@@ -1,6 +0,0 @@
-scrape_configs:
-  - job_name: prometheus
-    relabel_configs:
-      - action: uppercase
-        source_labels: [__name__]
-        target_label: 42lab

model/labels/labels_common.go

@@ -104,14 +104,14 @@ func (ls Labels) IsValid(validationScheme model.ValidationScheme) bool {
 		if l.Name == model.MetricNameLabel {
 			// If the default validation scheme has been overridden with legacy mode,
 			// we need to call the special legacy validation checker.
-			if validationScheme == model.LegacyValidation && model.NameValidationScheme == model.UTF8Validation && !model.IsValidLegacyMetricName(string(model.LabelValue(l.Value))) {
+			if validationScheme == model.LegacyValidation && !model.IsValidLegacyMetricName(string(model.LabelValue(l.Value))) {
 				return strconv.ErrSyntax
 			}
 			if !model.IsValidMetricName(model.LabelValue(l.Value)) {
 				return strconv.ErrSyntax
 			}
 		}
-		if validationScheme == model.LegacyValidation && model.NameValidationScheme == model.UTF8Validation {
+		if validationScheme == model.LegacyValidation {
 			if !model.LabelName(l.Name).IsValidLegacy() || !model.LabelValue(l.Value).IsValid() {
 				return strconv.ErrSyntax
 			}

model/labels/labels_test.go

@@ -285,7 +285,6 @@ func TestLabels_IsValid(t *testing.T) {
 func TestLabels_ValidationModes(t *testing.T) {
 	for _, test := range []struct {
 		input    Labels
-		globalMode model.ValidationScheme
 		callMode model.ValidationScheme
 		expected bool
 	}{
@@ -295,7 +294,6 @@ func TestLabels_ValidationModes(t *testing.T) {
 				"hostname", "localhost",
 				"job", "check",
 			),
-			globalMode: model.UTF8Validation,
 			callMode: model.UTF8Validation,
 			expected: true,
 		},
@@ -305,39 +303,15 @@ func TestLabels_ValidationModes(t *testing.T) {
 				"\xc5 bad utf8", "localhost",
 				"job", "check",
 			),
-			globalMode: model.UTF8Validation,
 			callMode: model.UTF8Validation,
 			expected: false,
 		},
-		{
-			// Setting the common model to legacy validation and then trying to check for UTF-8 on a
-			// per-call basis is not supported.
-			input: FromStrings(
-				"__name__", "test.utf8.metric",
-				"hostname", "localhost",
-				"job", "check",
-			),
-			globalMode: model.LegacyValidation,
-			callMode:   model.UTF8Validation,
-			expected:   false,
-		},
-		{
-			input: FromStrings(
-				"__name__", "test",
-				"hostname", "localhost",
-				"job", "check",
-			),
-			globalMode: model.LegacyValidation,
-			callMode:   model.LegacyValidation,
-			expected:   true,
-		},
 		{
 			input: FromStrings(
 				"__name__", "test.utf8.metric",
 				"hostname", "localhost",
 				"job", "check",
 			),
-			globalMode: model.UTF8Validation,
 			callMode: model.LegacyValidation,
 			expected: false,
 		},
@@ -347,12 +321,10 @@ func TestLabels_ValidationModes(t *testing.T) {
 				"host.name", "localhost",
 				"job", "check",
 			),
-			globalMode: model.UTF8Validation,
 			callMode: model.LegacyValidation,
 			expected: false,
 		},
 	} {
-		model.NameValidationScheme = test.globalMode
 		require.Equal(t, test.expected, test.input.IsValid(test.callMode))
 	}
 }

model/relabel/relabel.go

@@ -135,11 +135,6 @@ func (c *Config) Validate() error {
 		// Design escaping mechanism to allow that, once valid use case appears.
 		return model.LabelName(value).IsValid()
 	}
-	if model.NameValidationScheme == model.LegacyValidation {
-		isValidLabelNameWithRegexVarFn = func(value string) bool {
-			return relabelTargetLegacy.MatchString(value)
-		}
-	}
 	if c.Action == Replace && varInRegexTemplate(c.TargetLabel) && !isValidLabelNameWithRegexVarFn(c.TargetLabel) {
 		return fmt.Errorf("%q is invalid 'target_label' for %s action", c.TargetLabel, c.Action)
 	}

model/textparse/openmetricsparse_test.go

@@ -469,12 +469,6 @@ foobar{quantile="0.99"} 150.1`
 }
 
 func TestUTF8OpenMetricsParse(t *testing.T) {
-	oldValidationScheme := model.NameValidationScheme
-	model.NameValidationScheme = model.UTF8Validation
-	defer func() {
-		model.NameValidationScheme = oldValidationScheme
-	}()
-
 	input := `# HELP "go.gc_duration_seconds" A summary of the GC invocation durations.
 # TYPE "go.gc_duration_seconds" summary
 # UNIT "go.gc_duration_seconds" seconds

model/textparse/promparse_test.go

@@ -205,12 +205,6 @@ testmetric{le="10"} 1`
 }
 
 func TestUTF8PromParse(t *testing.T) {
-	oldValidationScheme := model.NameValidationScheme
-	model.NameValidationScheme = model.UTF8Validation
-	defer func() {
-		model.NameValidationScheme = oldValidationScheme
-	}()
-
 	input := `# HELP "go.gc_duration_seconds" A summary of the GC invocation durations.
 # 	TYPE "go.gc_duration_seconds" summary
 {"go.gc_duration_seconds",quantile="0"} 4.9351e-05

promql/parser/parse_test.go

@@ -3970,7 +3970,6 @@ func TestParseExpressions(t *testing.T) {
 		EnableExperimentalFunctions = false
 	})
 
-	model.NameValidationScheme = model.UTF8Validation
 	for _, test := range testExpr {
 		t.Run(readable(test.input), func(t *testing.T) {
 			expr, err := ParseExpr(test.input)

promql/parser/printer_test.go

@@ -16,7 +16,6 @@ package parser
 import (
 	"testing"
 
-	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 
 	"github.com/prometheus/prometheus/model/labels"
@@ -170,8 +169,6 @@ func TestExprString(t *testing.T) {
 		},
 	}
 
-	model.NameValidationScheme = model.UTF8Validation
-
 	for _, test := range inputs {
 		expr, err := ParseExpr(test.in)
 		require.NoError(t, err)

promql/promqltest/test.go

@@ -57,10 +57,6 @@ const (
 	DefaultMaxSamplesPerQuery = 10000
 )
 
-func init() {
-	model.NameValidationScheme = model.UTF8Validation
-}
-
 type TBRun interface {
 	testing.TB
 	Run(string, func(*testing.T)) bool

scrape/manager_test.go

@@ -56,11 +56,6 @@ import (
 	"github.com/prometheus/prometheus/util/testutil"
 )
 
-func init() {
-	// This can be removed when the default validation scheme in common is updated.
-	model.NameValidationScheme = model.UTF8Validation
-}
-
 func TestPopulateLabels(t *testing.T) {
 	cases := []struct {
 		in      model.LabelSet

scrape/scrape_test.go

@@ -1325,14 +1325,13 @@ func TestScrapeLoopSeriesAdded(t *testing.T) {
 }
 
 func TestScrapeLoopFailWithInvalidLabelsAfterRelabel(t *testing.T) {
-	model.NameValidationScheme = model.LegacyValidation
 	s := teststorage.New(t)
 	defer s.Close()
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
 	target := &Target{
-		labels: labels.FromStrings("pod_label_invalid_012", "test"),
+		labels: labels.FromStrings("pod_label_invalid_012\xff", "test"),
 	}
 	relabelConfig := []*relabel.Config{{
 		Action:      relabel.LabelMap,
@@ -1357,10 +1356,6 @@ func TestScrapeLoopFailWithInvalidLabelsAfterRelabel(t *testing.T) {
 func TestScrapeLoopFailLegacyUnderUTF8(t *testing.T) {
 	// Test that scrapes fail when default validation is utf8 but scrape config is
 	// legacy.
-	model.NameValidationScheme = model.UTF8Validation
-	defer func() {
-		model.NameValidationScheme = model.LegacyValidation
-	}()
 	s := teststorage.New(t)
 	defer s.Close()
 	ctx, cancel := context.WithCancel(context.Background())
@@ -3711,8 +3706,6 @@ func TestScrapeReportLimit(t *testing.T) {
 func TestScrapeUTF8(t *testing.T) {
 	s := teststorage.New(t)
 	defer s.Close()
-	model.NameValidationScheme = model.UTF8Validation
-	t.Cleanup(func() { model.NameValidationScheme = model.LegacyValidation })
 
 	cfg := &config.ScrapeConfig{
 		JobName:                    "test",

storage/remote/codec_test.go

@@ -165,11 +165,6 @@ func TestWriteV2RequestFixture(t *testing.T) {
 }
 
 func TestValidateLabelsAndMetricName(t *testing.T) {
-	oldScheme := model.NameValidationScheme
-	model.NameValidationScheme = model.LegacyValidation
-	defer func() {
-		model.NameValidationScheme = oldScheme
-	}()
 	tests := []struct {
 		input       []prompb.Label
 		expectedErr string
@@ -194,18 +189,10 @@ func TestValidateLabelsAndMetricName(t *testing.T) {
 		{
 			input: []prompb.Label{
 				{Name: "__name__", Value: "name"},
-				{Name: "@labelName", Value: "labelValue"},
+				{Name: "@labelName\xff", Value: "labelValue"},
 			},
-			expectedErr: "invalid label name: @labelName",
+			expectedErr: "invalid label name: @labelName\xff",
-			description: "label name with @",
+			description: "label name with \xff",
-		},
-		{
-			input: []prompb.Label{
-				{Name: "__name__", Value: "name"},
-				{Name: "123labelName", Value: "labelValue"},
-			},
-			expectedErr: "invalid label name: 123labelName",
-			description: "label name starts with numbers",
 		},
 		{
 			input: []prompb.Label{
@@ -225,10 +212,10 @@ func TestValidateLabelsAndMetricName(t *testing.T) {
 		},
 		{
 			input: []prompb.Label{
-				{Name: "__name__", Value: "@invalid_name"},
+				{Name: "__name__", Value: "invalid_name\xff"},
 			},
-			expectedErr: "invalid metric name: @invalid_name",
+			expectedErr: "invalid metric name: invalid_name\xff",
-			description: "metric name starts with @",
+			description: "metric name has invalid utf8",
 		},
 		{
 			input: []prompb.Label{

storage/remote/write_handler.go

@@ -251,7 +251,7 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err
 
 		// TODO(bwplotka): Even as per 1.0 spec, this should be a 400 error, while other samples are
 		// potentially written. Perhaps unify with fixed writeV2 implementation a bit.
-		if !ls.Has(labels.MetricName) || !ls.IsValid(model.NameValidationScheme) {
+		if !ls.Has(labels.MetricName) || !ls.IsValid(model.UTF8Validation) {
 			h.logger.Warn("Invalid metric names or labels", "got", ls.String())
 			samplesWithInvalidLabels++
 			continue
@@ -392,7 +392,7 @@ func (h *writeHandler) appendV2(app storage.Appender, req *writev2.Request, rs *
 		// Validate series labels early.
 		// NOTE(bwplotka): While spec allows UTF-8, Prometheus Receiver may impose
 		// specific limits and follow https://prometheus.io/docs/specs/remote_write_spec_2_0/#invalid-samples case.
-		if !ls.Has(labels.MetricName) || !ls.IsValid(model.NameValidationScheme) {
+		if !ls.Has(labels.MetricName) || !ls.IsValid(model.UTF8Validation) {
 			badRequestErrs = append(badRequestErrs, fmt.Errorf("invalid metric name or labels, got %v", ls.String()))
 			samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms)
 			continue

web/api/v1/api_test.go

@@ -1117,7 +1117,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 		metadata              []targetMetadata
 		exemplars             []exemplar.QueryResult
 		zeroFunc              func(interface{})
-		nameValidationScheme  model.ValidationScheme
 	}
 
 	rulesZeroFunc := func(i interface{}) {
@@ -3247,13 +3246,12 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 					"boo",
 				},
 			},
-			// Bad name parameter for legacy validation.
+			// Bad name parameter
 			{
 				endpoint: api.labelValues,
 				params: map[string]string{
-					"name": "host.name",
+					"name": "host.name\xff",
 				},
-				nameValidationScheme: model.LegacyValidation,
 				errType: errorBadData,
 			},
 			// Valid utf8 name parameter for utf8 validation.
@@ -3262,7 +3260,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 				params: map[string]string{
 					"name": "host.name",
 				},
-				nameValidationScheme: model.UTF8Validation,
 				response: []string{
 					"localhost",
 				},
@@ -3273,7 +3270,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 				params: map[string]string{
 					"name": "U__junk_0a__7b__7d__2c__3d_:_20__20_chars",
 				},
-				nameValidationScheme: model.UTF8Validation,
 				response: []string{
 					"bar",
 				},
@@ -3706,8 +3702,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 						ctx = route.WithParam(ctx, p, v)
 					}
 
-					model.NameValidationScheme = test.nameValidationScheme
-
 					req, err := request(method, test.query)
 					require.NoError(t, err)