From 7efb62fa4c0a56f07a3f7c790f15dbcb6f64ecb2 Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Fri, 10 Jul 2026 19:38:27 +0200 Subject: [PATCH 1/2] feat: support Tempo Query V2 and search tag values endpoint in trace RBAC Signed-off-by: Andreas Gerstmayr --- api/traces/v1/trace_rbac.go | 39 ++++- api/traces/v1/trace_rbac_test.go | 287 ++++++++++++++++++++++++++++++- 2 files changed, 317 insertions(+), 9 deletions(-) diff --git a/api/traces/v1/trace_rbac.go b/api/traces/v1/trace_rbac.go index 16fd2669b..205f4db6b 100644 --- a/api/traces/v1/trace_rbac.go +++ b/api/traces/v1/trace_rbac.go @@ -24,10 +24,15 @@ const ( serviceAttributeKey = "service.name" ) -var allowedTempoAPIs = []*regexp.Regexp{ - regexp.MustCompile(`^/api/traces/\w+$`), - regexp.MustCompile(`^/api/search$`), -} +var ( + routeQueryV1 = regexp.MustCompile(`^/api/traces/\w+$`) + routeQueryV2 = regexp.MustCompile(`^/api/v2/traces/\w+$`) + routeSearch = regexp.MustCompile(`^/api/search$`) + routeSearchTagValues = regexp.MustCompile(`^/api/v2/search/tag/(resource\.service\.name|resource\.k8s\.namespace\.name)/values$`) + + allowedTempoAPIs = []*regexp.Regexp{routeQueryV1, routeQueryV2, routeSearch, routeSearchTagValues} + filteredAPIs = []*regexp.Regexp{routeQueryV1, routeQueryV2, routeSearch} +) func matchesAnyRegex(s string, patterns []*regexp.Regexp) bool { for _, re := range patterns { @@ -77,7 +82,7 @@ func WithTraceQLNamespaceSelectAndForbidOtherAPIs(enabled bool) func(http.Handle func responseRBACModifier(log log.Logger) func(response *http.Response) error { return func(response *http.Response) error { - if strings.HasPrefix(response.Request.URL.Path, "/api/traces/") || strings.HasPrefix(response.Request.URL.Path, "/api/search") { + if matchesAnyRegex(response.Request.URL.Path, filteredAPIs) { allowedNamespaces := map[string]bool{} namespaces := apilogsv1.AllowedNamespaces(response.Request.Context()) for _, ns := range namespaces { @@ -92,7 +97,8 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { } responseBuffer := &bytes.Buffer{} - if strings.HasPrefix(response.Request.URL.Path, "/api/traces/") { + switch { + case routeQueryV1.MatchString(response.Request.URL.Path): trace := &tempopb.Trace{} err = tempopb.UnmarshalFromJSONV1(b, trace) if err != nil { @@ -105,9 +111,26 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { return err } responseBuffer = bytes.NewBuffer(traceResponseBody) - } else { + + case routeQueryV2.MatchString(response.Request.URL.Path): + traceByIDResponse := &tempopb.TraceByIDResponse{} + unmarshaller := jsonpb.Unmarshaler{} + err = unmarshaller.Unmarshal(bytes.NewReader(b), traceByIDResponse) + if err != nil { + return err + } + traceByIDResponse.Trace = traceRBAC(allowedNamespaces, traceByIDResponse.Trace) + + marshaller := jsonpb.Marshaler{} + err = marshaller.Marshal(responseBuffer, traceByIDResponse) + if err != nil { + return err + } + + case routeSearch.MatchString(response.Request.URL.Path): searchResponse := &tempopb.SearchResponse{} - err = jsonpb.UnmarshalString(string(b), searchResponse) + unmarshaller := jsonpb.Unmarshaler{} + err = unmarshaller.Unmarshal(bytes.NewReader(b), searchResponse) if err != nil { return err } diff --git a/api/traces/v1/trace_rbac_test.go b/api/traces/v1/trace_rbac_test.go index a207f42af..eceb2e9fc 100644 --- a/api/traces/v1/trace_rbac_test.go +++ b/api/traces/v1/trace_rbac_test.go @@ -1,15 +1,25 @@ package v1 import ( + "context" + "fmt" + "io" "net/http" "net/http/httptest" + "net/url" + "strings" "testing" + "github.com/go-kit/log" "github.com/grafana/tempo/pkg/tempopb" commonv1 "github.com/grafana/tempo/pkg/tempopb/common/v1" resourcev1 "github.com/grafana/tempo/pkg/tempopb/resource/v1" tracev1 "github.com/grafana/tempo/pkg/tempopb/trace/v1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + apilogsv1 "github.com/observatorium/api/api/logs/v1" + "github.com/observatorium/api/authorization" ) func TestForbidOtherAPIs(t *testing.T) { @@ -28,10 +38,14 @@ func TestForbidOtherAPIs(t *testing.T) { {"search tags blocked", "/tempo/api/search/tags", http.StatusForbidden}, {"search tag values blocked", "/tempo/api/search/tag/name/values", http.StatusForbidden}, {"v2 search tags blocked", "/tempo/api/v2/search/tags", http.StatusForbidden}, + {"v2 search tag values for service.name", "/tempo/api/v2/search/tag/resource.service.name/values", http.StatusOK}, + {"v2 search tag values for k8s.namespace.name", "/tempo/api/v2/search/tag/resource.k8s.namespace.name/values", http.StatusOK}, + {"v2 search tag values for other resource attributes blocked", "/tempo/api/v2/search/tag/resource.other/values", http.StatusForbidden}, + {"v2 search tag values for span attribtues blocked", "/tempo/api/v2/search/tag/span.http.method/values", http.StatusForbidden}, {"metrics blocked", "/tempo/api/metrics/query_range", http.StatusForbidden}, {"echo blocked", "/tempo/api/echo", http.StatusForbidden}, {"overrides blocked", "/tempo/api/overrides", http.StatusForbidden}, - {"v2 traces blocked", "/tempo/api/v2/traces/abc123", http.StatusForbidden}, + {"v2 trace by ID", "/tempo/api/v2/traces/abc123", http.StatusOK}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -528,3 +542,274 @@ func TestRBACSearchResult(t *testing.T) { }) } } + +func contextWithAllowedNamespaces(t *testing.T, namespaces []string) context.Context { + t.Helper() + data := fmt.Sprintf(`{"matchers":[{"name":"namespace","value":"%s","type":1}]}`, url.QueryEscape(strings.Join(namespaces, "|"))) + + var captured context.Context + handler := apilogsv1.WithEnforceAuthorizationLabels()(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured = r.Context() + })) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req = req.WithContext(authorization.WithData(req.Context(), data)) + handler.ServeHTTP(httptest.NewRecorder(), req) + require.NotNil(t, captured) + return captured +} + +func makeResponse(ctx context.Context, statusCode int, path string, body string, header http.Header) *http.Response { + if header == nil { + header = http.Header{} + } + return &http.Response{ + StatusCode: statusCode, + Header: header, + Body: io.NopCloser(strings.NewReader(body)), + Request: (&http.Request{URL: &url.URL{Path: path}}).WithContext(ctx), + } +} + +func TestResponseRBACModifier(t *testing.T) { + modifier := responseRBACModifier(log.NewNopLogger()) + ctx := contextWithAllowedNamespaces(t, []string{"allowed-ns"}) + + t.Run("v1 trace endpoint", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/traces/abc123", `{ + "batches": [ + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span1", "value": {"stringValue": "val"}}]} + ]} + ] + }, + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}}, + {"key": "service.name", "value": {"stringValue": "blocked-svc"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span2", "value": {"stringValue": "val"}}]} + ]} + ] + } + ] +}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.JSONEq(t, `{ + "batches": [ + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span1", "value": {"stringValue": "val"}}]} + ]} + ] + }, + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}}, + {"key": "service.name", "value": {"stringValue": "blocked-svc"}} + ] + }, + "scopeSpans": [ + {"scope": {"attributes": []}, "spans": [ + {"attributes": [], "events": []} + ]} + ] + } + ] +}`, string(body)) + }) + + t.Run("v2 trace endpoint", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/v2/traces/abc123", `{ + "trace": { + "resourceSpans": [ + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span1", "value": {"stringValue": "val"}}]} + ]} + ] + }, + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}}, + {"key": "service.name", "value": {"stringValue": "blocked-svc"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span2", "value": {"stringValue": "val"}}]} + ]} + ] + } + ] + } +}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.JSONEq(t, `{ + "trace": { + "resourceSpans": [ + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [ + {"attributes": [{"key": "span1", "value": {"stringValue": "val"}}]} + ]} + ] + }, + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}}, + {"key": "service.name", "value": {"stringValue": "blocked-svc"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [{}]} + ] + } + ] + } +}`, string(body)) + }) + + t.Run("search endpoint", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/search", `{ + "traces": [ + { + "spanSets": [ + { + "spans": [ + {"attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}}, + {"key": "extra", "value": {"stringValue": "val"}} + ]}, + {"attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}}, + {"key": "extra", "value": {"stringValue": "val"}} + ]} + ] + } + ] + } + ] +}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.JSONEq(t, `{ + "traces": [ + { + "spanSets": [ + { + "spans": [ + {"attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}}, + {"key": "extra", "value": {"stringValue": "val"}} + ]}, + {"attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "blocked-ns"}} + ]} + ] + } + ] + } + ] +}`, string(body)) + }) + + t.Run("search tag values endpoint is not modified", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/v2/search/tag/resource.service.name/values", `{ + "tagValues": [ + {"type": "string", "value": "frontend"}, + {"type": "string", "value": "backend"} + ] +}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.JSONEq(t, `{ + "tagValues": [ + {"type": "string", "value": "frontend"}, + {"type": "string", "value": "backend"} + ] +}`, string(body)) + }) + + t.Run("non-matching path is not modified", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/echo", "unmodified body", nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.Equal(t, "unmodified body", string(body)) + }) + + t.Run("non-200 response is not modified", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusNotFound, "/api/traces/abc123", "error body", nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.Equal(t, "error body", string(body)) + }) + + t.Run("Content-Length and Content-Encoding headers are updated", func(t *testing.T) { + resp := makeResponse(ctx, http.StatusOK, "/api/traces/abc123", `{ + "batches": [ + { + "resource": { + "attributes": [ + {"key": "k8s.namespace.name", "value": {"stringValue": "allowed-ns"}} + ] + }, + "scopeSpans": [ + {"scope": {}, "spans": [{}]} + ] + } + ] +}`, http.Header{"Content-Encoding": []string{"gzip"}}) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.Equal(t, []string{fmt.Sprint(len(body))}, resp.Header["Content-Length"]) + assert.Empty(t, resp.Header["Content-Encoding"]) + }) +} From b1e85f6c5ae95187239baedf50f4d700050d9dc6 Mon Sep 17 00:00:00 2001 From: Andreas Gerstmayr Date: Thu, 23 Jul 2026 12:47:59 +0200 Subject: [PATCH 2/2] add warning in default case Signed-off-by: Andreas Gerstmayr --- api/traces/v1/trace_rbac.go | 17 ++++++++++++----- api/traces/v1/trace_rbac_test.go | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/api/traces/v1/trace_rbac.go b/api/traces/v1/trace_rbac.go index 205f4db6b..00174cfe1 100644 --- a/api/traces/v1/trace_rbac.go +++ b/api/traces/v1/trace_rbac.go @@ -82,9 +82,11 @@ func WithTraceQLNamespaceSelectAndForbidOtherAPIs(enabled bool) func(http.Handle func responseRBACModifier(log log.Logger) func(response *http.Response) error { return func(response *http.Response) error { - if matchesAnyRegex(response.Request.URL.Path, filteredAPIs) { + request := response.Request + + if matchesAnyRegex(request.URL.Path, filteredAPIs) { allowedNamespaces := map[string]bool{} - namespaces := apilogsv1.AllowedNamespaces(response.Request.Context()) + namespaces := apilogsv1.AllowedNamespaces(request.Context()) for _, ns := range namespaces { allowedNamespaces[ns] = true } @@ -98,7 +100,7 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { responseBuffer := &bytes.Buffer{} switch { - case routeQueryV1.MatchString(response.Request.URL.Path): + case routeQueryV1.MatchString(request.URL.Path): trace := &tempopb.Trace{} err = tempopb.UnmarshalFromJSONV1(b, trace) if err != nil { @@ -112,7 +114,7 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { } responseBuffer = bytes.NewBuffer(traceResponseBody) - case routeQueryV2.MatchString(response.Request.URL.Path): + case routeQueryV2.MatchString(request.URL.Path): traceByIDResponse := &tempopb.TraceByIDResponse{} unmarshaller := jsonpb.Unmarshaler{} err = unmarshaller.Unmarshal(bytes.NewReader(b), traceByIDResponse) @@ -127,7 +129,7 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { return err } - case routeSearch.MatchString(response.Request.URL.Path): + case routeSearch.MatchString(request.URL.Path): searchResponse := &tempopb.SearchResponse{} unmarshaller := jsonpb.Unmarshaler{} err = unmarshaller.Unmarshal(bytes.NewReader(b), searchResponse) @@ -141,6 +143,11 @@ func responseRBACModifier(log log.Logger) func(response *http.Response) error { if err != nil { return err } + + default: + level.Warn(log).Log("msg", "unhandled filtered API path", "path", request.URL.Path) + responseBuffer = bytes.NewBufferString("forbidden") + response.StatusCode = http.StatusForbidden } response.Body = io.NopCloser(responseBuffer) response.Header["Content-Length"] = []string{fmt.Sprint(responseBuffer.Len())} diff --git a/api/traces/v1/trace_rbac_test.go b/api/traces/v1/trace_rbac_test.go index eceb2e9fc..b65d1b9b4 100644 --- a/api/traces/v1/trace_rbac_test.go +++ b/api/traces/v1/trace_rbac_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "regexp" "strings" "testing" @@ -790,6 +791,22 @@ func TestResponseRBACModifier(t *testing.T) { assert.Equal(t, "error body", string(body)) }) + t.Run("unhandled filtered API path returns forbidden", func(t *testing.T) { + // Temporarily add a catch-all to filteredAPIs so a path can pass the + // filteredAPIs gate but not match any switch case. + origFiltered := filteredAPIs + filteredAPIs = append(filteredAPIs, regexp.MustCompile(`^/api/other$`)) + t.Cleanup(func() { filteredAPIs = origFiltered }) + + resp := makeResponse(ctx, http.StatusOK, "/api/other", `{"some":"data"}`, nil) + + require.NoError(t, modifier(resp)) + + body, _ := io.ReadAll(resp.Body) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Equal(t, "forbidden", string(body)) + }) + t.Run("Content-Length and Content-Encoding headers are updated", func(t *testing.T) { resp := makeResponse(ctx, http.StatusOK, "/api/traces/abc123", `{ "batches": [