Add support for utf8 names on /v1/label/:name/values endpoint

Previously, the api was evaluating this regex to determine if the label
name was valid or not:

14bac55a99/model/labels.go (L94)

However, I believe that the `IsValid()` function is what ought to be
used in the brave new utf8 era.

**Before**

```
$ curl localhost:9090/api/v1/label/host.name/values
{"status":"error","errorType":"bad_data","error":"invalid label name: \"host.name\""}
```

**After**

```
$ curl localhost:9090/api/v1/label/host.name/values
{"status":"success","data":["localhost"]}
```

It's very likely that I'm missing something here or you were already
planning to do this at some point but I just encountered this issue and
figured I'd give it a go.
This commit is contained in:
newtonne 2024-09-19 19:34:28 +01:00
parent e480cf21eb
commit 54930e0698
4 changed files with 423 additions and 378 deletions

View file

@ -667,10 +667,16 @@ label_set_list : label_set_list COMMA label_set_item
label_set_item : IDENTIFIER EQL STRING
{ $$ = labels.Label{Name: $1.Val, Value: yylex.(*parser).unquoteString($3.Val) } }
| string_identifier EQL STRING
{ $$ = labels.Label{Name: $1.Val, Value: yylex.(*parser).unquoteString($3.Val) } }
| IDENTIFIER EQL error
{ yylex.(*parser).unexpected("label set", "string"); $$ = labels.Label{}}
| string_identifier EQL error
{ yylex.(*parser).unexpected("label set", "string"); $$ = labels.Label{}}
| IDENTIFIER error
{ yylex.(*parser).unexpected("label set", "\"=\""); $$ = labels.Label{}}
| string_identifier error
{ yylex.(*parser).unexpected("label set", "\"=\""); $$ = labels.Label{}}
| error
{ yylex.(*parser).unexpected("label set", "identifier or \"}\""); $$ = labels.Label{} }
;

File diff suppressed because it is too large Load diff

View file

@ -733,7 +733,8 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) {
ctx := r.Context()
name := route.Param(ctx, "name")
if !model.LabelNameRE.MatchString(name) {
label := model.LabelName(name)
if !label.IsValid() {
return apiFuncResult{nil, &apiError{errorBadData, fmt.Errorf("invalid label name: %q", name)}, nil, nil}
}

View file

@ -380,6 +380,7 @@ func TestEndpoints(t *testing.T) {
test_metric4{foo="bar", dup="1"} 1+0x100
test_metric4{foo="boo", dup="1"} 1+0x100
test_metric4{foo="boo"} 1+0x100
test_metric5{"host.name"="localhost"} 1+0x100
`)
t.Cleanup(func() { storage.Close() })
@ -1110,6 +1111,7 @@ 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{}) {
@ -2779,6 +2781,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"test_metric2",
"test_metric3",
"test_metric4",
"test_metric5",
},
},
{
@ -2791,14 +2794,26 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"boo",
},
},
// Bad name parameter.
// Bad name parameter for legacy validation.
{
endpoint: api.labelValues,
params: map[string]string{
"name": "not!!!allowed",
"name": "host.name",
},
nameValidationScheme: model.LegacyValidation,
errType: errorBadData,
},
// Valid utf8 name parameter for utf8 validation.
{
endpoint: api.labelValues,
params: map[string]string{
"name": "host.name",
},
nameValidationScheme: model.UTF8Validation,
response: []string{
"localhost",
},
},
// Start and end before LabelValues starts.
{
endpoint: api.labelValues,
@ -3033,15 +3048,15 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"name": "__name__",
},
query: url.Values{
"limit": []string{"4"},
"limit": []string{"5"},
},
responseLen: 4, // API does not specify which particular values will come back.
responseLen: 5, // API does not specify which particular values will come back.
warningsCount: 0, // No warnings if limit isn't exceeded.
},
// Label names.
{
endpoint: api.labelNames,
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Start and end before Label names starts.
{
@ -3059,7 +3074,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"start": []string{"1"},
"end": []string{"100"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Start before Label names, end within Label names.
{
@ -3068,7 +3083,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"start": []string{"-1"},
"end": []string{"10"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Start before Label names starts, end after Label names ends.
@ -3078,7 +3093,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"start": []string{"-1"},
"end": []string{"100000"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Start with bad data for Label names, end within Label names.
{
@ -3096,7 +3111,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
"start": []string{"1"},
"end": []string{"1000000006"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Start and end after Label names ends.
{
@ -3113,7 +3128,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
query: url.Values{
"start": []string{"4"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Only provide End within Label names, don't provide a start time.
{
@ -3121,7 +3136,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
query: url.Values{
"end": []string{"20"},
},
response: []string{"__name__", "dup", "foo"},
response: []string{"__name__", "dup", "foo", "host.name"},
},
// Label names with bad matchers.
{
@ -3189,9 +3204,9 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
{
endpoint: api.labelNames,
query: url.Values{
"limit": []string{"3"},
"limit": []string{"4"},
},
responseLen: 3, // API does not specify which particular values will come back.
responseLen: 4, // API does not specify which particular values will come back.
warningsCount: 0, // No warnings if limit isn't exceeded.
},
}...)
@ -3227,6 +3242,8 @@ 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)