-
Couldn't load subscription status.
- Fork 701
feat: Implement utf8 label name client capability #4442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
72a3b80
feat: Allow utf-8 characters in label names
jake-kramer cbb8eb2
Address feedback
jake-kramer f651740
Prettier format
jake-kramer 6e8c895
Handle multiple values set for `Accept` header
jake-kramer a2b48a1
Address comments
jake-kramer d28e172
Merge branch 'main' into utf-label-names
jake-kramer 8b0a87e
Filter (instead of sanitize) on read
jake-kramer 12c20e7
Finish v2 read path filtering
jake-kramer 52d3ed3
v1 read path filtering
jake-kramer cb7c9b2
Add tests
jake-kramer 705b47e
fmt/lint
jake-kramer 84bc1b2
Fix comment on legacy label name valid chars
jake-kramer b9d1ec2
Fix race condition in test
jake-kramer b2888fa
Merge branch 'main' into utf-label-names
jake-kramer 9b0d790
Use `LabelNames` instead of backdoor query
jake-kramer f41512f
Update querier label name filtering test
jake-kramer 55810eb
fmt
jake-kramer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| package featureflags | ||
|
|
||
| import ( | ||
| "context" | ||
| "mime" | ||
| "net/http" | ||
| "strings" | ||
|
|
||
| "connectrpc.com/connect" | ||
| "github.com/go-kit/log/level" | ||
| "github.com/grafana/dskit/middleware" | ||
| "github.com/grafana/pyroscope/pkg/util" | ||
| "google.golang.org/grpc" | ||
| "google.golang.org/grpc/metadata" | ||
| ) | ||
|
|
||
| const ( | ||
| // Capability names - update parseClientCapabilities below when new capabilities added | ||
| allowUtf8LabelNamesCapabilityName string = "allow-utf8-labelnames" | ||
| ) | ||
|
|
||
| // Define a custom context key type to avoid collisions | ||
| type contextKey struct{} | ||
|
|
||
| type ClientCapabilities struct { | ||
| AllowUtf8LabelNames bool | ||
| } | ||
|
|
||
| func WithClientCapabilities(ctx context.Context, clientCapabilities ClientCapabilities) context.Context { | ||
| return context.WithValue(ctx, contextKey{}, clientCapabilities) | ||
| } | ||
|
|
||
| func GetClientCapabilities(ctx context.Context) (ClientCapabilities, bool) { | ||
| value, ok := ctx.Value(contextKey{}).(ClientCapabilities) | ||
| return value, ok | ||
| } | ||
|
|
||
| func ClientCapabilitiesGRPCMiddleware() grpc.UnaryServerInterceptor { | ||
| return func( | ||
| ctx context.Context, | ||
| req interface{}, | ||
| info *grpc.UnaryServerInfo, | ||
| handler grpc.UnaryHandler, | ||
| ) (interface{}, error) { | ||
| // Extract metadata from context | ||
| md, ok := metadata.FromIncomingContext(ctx) | ||
| if !ok { | ||
| return handler(ctx, req) | ||
| } | ||
|
|
||
| // Convert metadata to http.Header for reuse of existing parsing logic | ||
| httpHeader := make(http.Header) | ||
| for key, values := range md { | ||
| // gRPC metadata keys are lowercase, HTTP headers are case-insensitive | ||
| httpHeader[http.CanonicalHeaderKey(key)] = values | ||
| } | ||
|
|
||
| // Reuse existing HTTP header parsing | ||
| clientCapabilities, err := parseClientCapabilities(httpHeader) | ||
| if err != nil { | ||
| return nil, connect.NewError(connect.CodeInvalidArgument, err) | ||
| } | ||
|
|
||
| enhancedCtx := WithClientCapabilities(ctx, clientCapabilities) | ||
| return handler(enhancedCtx, req) | ||
| } | ||
| } | ||
|
|
||
| // ClientCapabilitiesHttpMiddleware creates middleware that extracts and parses the | ||
| // `Accept` header for capabilities the client supports | ||
| func ClientCapabilitiesHttpMiddleware() middleware.Interface { | ||
| return middleware.Func(func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| clientCapabilities, err := parseClientCapabilities(r.Header) | ||
| if err != nil { | ||
| http.Error(w, "Invalid header format: "+err.Error(), http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| ctx := WithClientCapabilities(r.Context(), clientCapabilities) | ||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| func parseClientCapabilities(header http.Header) (ClientCapabilities, error) { | ||
| acceptHeaderValues := header.Values("Accept") | ||
|
|
||
| var capabilities ClientCapabilities | ||
|
|
||
| for _, acceptHeaderValue := range acceptHeaderValues { | ||
| if acceptHeaderValue != "" { | ||
| accepts := strings.Split(acceptHeaderValue, ",") | ||
|
|
||
| for _, accept := range accepts { | ||
| if _, params, err := mime.ParseMediaType(accept); err != nil { | ||
| return capabilities, err | ||
| } else { | ||
| for k, v := range params { | ||
| switch k { | ||
| case allowUtf8LabelNamesCapabilityName: | ||
| if v == "true" { | ||
| capabilities.AllowUtf8LabelNames = true | ||
| } | ||
| default: | ||
| level.Debug(util.Logger).Log( | ||
| "msg", "unknown capability parsed from Accept header", | ||
| "acceptHeaderKey", k, | ||
| "acceptHeaderValue", v) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return capabilities, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,239 @@ | ||
| package featureflags | ||
|
|
||
| import ( | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func Test_parseClientCapabilities(t *testing.T) { | ||
| tests := []struct { | ||
| Name string | ||
| Header http.Header | ||
| Want ClientCapabilities | ||
| WantError bool | ||
| ErrorMessage string | ||
| }{ | ||
| { | ||
| Name: "empty header returns default capabilities", | ||
| Header: http.Header{}, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "no Accept header returns default capabilities", | ||
| Header: http.Header{ | ||
| "Content-Type": []string{"application/json"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "empty Accept header value returns default capabilities", | ||
| Header: http.Header{ | ||
| "Accept": []string{""}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "simple Accept header without capabilities", | ||
| Header: http.Header{ | ||
| "Accept": []string{"application/json"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "Accept header with utf8 label names capability true", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; allow-utf8-labelnames=true"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "Accept header with utf8 label names capability false", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; allow-utf8-labelnames=false"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "Accept header with utf8 label names capability invalid value", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; allow-utf8-labelnames=invalid"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "Accept header with unknown capability", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; unknown-capability=true"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| { | ||
| Name: "Accept header with multiple capabilities", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; allow-utf8-labelnames=true; unknown-capability=false"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "multiple Accept header values", | ||
| Header: http.Header{ | ||
| "Accept": []string{"application/json", "*/*; allow-utf8-labelnames=true"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "multiple Accept header values with different capabilities", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json; allow-utf8-labelnames=false", | ||
| "*/*; allow-utf8-labelnames=true", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "Accept header with quality values", | ||
| Header: http.Header{ | ||
| "Accept": []string{"text/html; q=0.9; allow-utf8-labelnames=true"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "complex Accept header", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8;allow-utf8-labelnames=true", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "multiple Accept header entries", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json", | ||
| "text/plain; allow-utf8-labelnames=true", | ||
| "*/*; q=0.1", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "invalid mime type in Accept header", | ||
| Header: http.Header{ | ||
| "Accept": []string{"invalid/mime/type/format"}, | ||
| }, | ||
| Want: ClientCapabilities{}, | ||
| WantError: true, | ||
| ErrorMessage: "mime: unexpected content after media subtype", | ||
| }, | ||
| { | ||
| Name: "Accept header with invalid syntax", | ||
| Header: http.Header{ | ||
| "Accept": []string{"text/html; invalid-parameter-syntax"}, | ||
| }, | ||
| Want: ClientCapabilities{}, | ||
| WantError: true, | ||
| ErrorMessage: "mime: invalid media parameter", | ||
| }, | ||
| { | ||
| Name: "mixed valid and invalid Accept header values", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json", | ||
| "invalid/mime/type/format", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{}, | ||
| WantError: true, | ||
| ErrorMessage: "mime: unexpected content after media subtype", | ||
| }, | ||
| { | ||
| // Parameter names are case-insensitive in mime.ParseMediaType | ||
| Name: "case sensitivity test for capability name", | ||
| Header: http.Header{ | ||
| "Accept": []string{"*/*; Allow-Utf8-Labelnames=true"}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "whitespace handling in Accept header", | ||
| Header: http.Header{ | ||
| "Accept": []string{" application/json ; allow-utf8-labelnames=true "}, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.Name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| got, err := parseClientCapabilities(tt.Header) | ||
|
|
||
| if tt.WantError { | ||
| require.Error(t, err) | ||
| if tt.ErrorMessage != "" { | ||
| require.Contains(t, err.Error(), tt.ErrorMessage) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| require.NoError(t, err) | ||
| require.Equal(t, tt.Want, got) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func Test_parseClientCapabilities_MultipleCapabilities(t *testing.T) { | ||
| // This test specifically checks that when the same capability appears | ||
| // multiple times with different values, the last "true" value wins | ||
| tests := []struct { | ||
| Name string | ||
| Header http.Header | ||
| Want ClientCapabilities | ||
| }{ | ||
| { | ||
| Name: "capability appears multiple times - last true wins", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json; allow-utf8-labelnames=false", | ||
| "text/plain; allow-utf8-labelnames=true", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "capability appears multiple times - last false loses to earlier true", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json; allow-utf8-labelnames=true", | ||
| "text/plain; allow-utf8-labelnames=false", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: true}, | ||
| }, | ||
| { | ||
| Name: "capability appears multiple times - all false", | ||
| Header: http.Header{ | ||
| "Accept": []string{ | ||
| "application/json; allow-utf8-labelnames=false", | ||
| "text/plain; allow-utf8-labelnames=false", | ||
| }, | ||
| }, | ||
| Want: ClientCapabilities{AllowUtf8LabelNames: false}, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.Name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| got, err := parseClientCapabilities(tt.Header) | ||
| require.NoError(t, err) | ||
| require.Equal(t, tt.Want, got) | ||
| }) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.