hanzo/traces
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

frontend, distributor: Add tenant ID validation (#5786)

Browse Source 
* frontend, distributor: Add tenant ID validation

Use the same validation than in Mimir for consistency.
In addition, reject empty IDs ("").

* Validate tenantID everywhere except in frontend handlers

We can't do it in frontend handlers because we need to wait for the multitenant middleware,
but we rely on the validation middleware.

Validating everywhere follows the same pattern as Mimir and ensures we don't miss any edge case.

* Check status codes

* Add test for user overrides API

* add comment
This commit is contained in:
Carles Garcia
2025-11-11 14:51:26 +01:00
committed by GitHub
parent 1af2825f1b
commit 4c29c88e51
20 changed files with 342 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -1,6 +1,7 @@
## main / unreleased
* [CHANGE] Remove remaining aws-sdk-go references and migrate tests to MinIO [#5856](https://github.com/grafana/tempo/pull/5856) (@anglerfishlyy)
* [CHANGE] **BREAKING CHANGE** Validate tenant ID in frontend and distributor [#5786](https://github.com/grafana/tempo/pull/5786) (@carles-grafana)
* [CHANGE] Remove busybox from Tempo image to make it more minimal and prevent future vulnerabilities [#5717](https://github.com/grafana/tempo/pull/5717) (@carles-grafana)
* [CHANGE] Allow RetryInfo to be disabled in per tenant overrides [#5741](https://github.com/grafana/tempo/pull/5741) (@electron0zero)
* [FEATURE] Add `tempo_metrics_generator_registry_active_series_demand_estimate` that estimates metrics-generator active series demand even when the active series limit is reached [#5710](https://github.com/grafana/tempo/pull/5710) (@carles-grafana)
integration/e2e/api/tenant_validation_test.go
+171
View File
@@ -0,0 +1,171 @@
package api
import (
"os"
"strings"
"testing"
"time"
"github.com/grafana/dskit/tenant"
"github.com/grafana/e2e"
"github.com/grafana/tempo/integration/util"
"github.com/grafana/tempo/pkg/httpclient"
tempoUtil "github.com/grafana/tempo/pkg/util"
"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
"github.com/grafana/tempo/cmd/tempo/app"
"github.com/grafana/tempo/integration/e2e/backend"
)
func TestInvalidTenants(t *testing.T) {
t.Parallel()
s, err := e2e.NewScenario(generateNetworkName())
require.NoError(t, err)
defer s.Close()
cfg := app.Config{}
buff, err := os.ReadFile(configMultiTenant)
require.NoError(t, err)
err = yaml.UnmarshalStrict(buff, &cfg)
require.NoError(t, err)
_, err = backend.New(s, cfg)
require.NoError(t, err)
require.NoError(t, util.CopyFileToSharedDir(s, configMultiTenant, "config.yaml"))
tempo := util.NewTempoAllInOne()
prometheus := util.NewPrometheus()
require.NoError(t, s.StartAndWaitReady(tempo, prometheus))
baseURL := "http://" + tempo.Endpoint(tempoPort)
longTenant := strings.Repeat("a", tenant.MaxTenantIDLength+1)
testCases := []struct {
name string
orgID string
multitenant bool
errContains string
}{
{
name: "invalid unsupported character",
orgID: "tenant#123",
errContains: "unsupported character",
},
{
name: "invalid slash character",
orgID: "tenant/123",
errContains: "unsupported character",
},
{
name: "invalid unsafe path segment",
orgID: "..",
errContains: "tenant ID is '.' or '..'",
},
{
name: "invalid too long tenant id",
orgID: longTenant,
errContains: "tenant ID is too long",
},
{
name: "invalid empty tenant",
orgID: "",
errContains: "no org id",
},
{
name: "invalid leading separator",
orgID: "|tenantA",
multitenant: true,
errContains: "no org id",
},
{
name: "invalid trailing separator",
orgID: "tenantA|",
multitenant: true,
errContains: "no org id",
},
{
name: "invalid double separator",
orgID: "tenantA||tenantB",
multitenant: true,
errContains: "no org id",
},
{
name: "invalid multi tenant unsupported character",
orgID: "tenantA|tenant#B",
multitenant: true,
errContains: "tenant#B",
},
{
name: "invalid multi tenant unsafe segment",
orgID: "tenantA|..",
multitenant: true,
errContains: "tenant ID is '.' or '..'",
},
{
name: "invalid multi tenant too long segment",
orgID: "tenantA|" + longTenant,
multitenant: true,
errContains: "tenant ID is too long",
},
}
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
client := httpclient.New(baseURL, tc.orgID)
assertHTTP400 := func(t *testing.T, err error) {
require.Error(t, err)
// no org ID in single-tenant returns 401 from Auth middleware
// others return 400
require.Contains(t, err.Error(), "response: 40")
require.Contains(t, err.Error(), tc.errContains)
}
_, err := client.QueryTrace("0047ac2c027451d89e3f3ba6d2a6b7b")
assertHTTP400(t, err)
_, err = client.QueryTraceV2("0047ac2c027451d89e3f3ba6d2a6b7b")
assertHTTP400(t, err)
_, err = client.SearchTraceQL("{}")
assertHTTP400(t, err)
_, err = client.SearchTags()
assertHTTP400(t, err)
_, err = client.SearchTagsV2()
assertHTTP400(t, err)
_, err = client.SearchTagValues("span.name")
assertHTTP400(t, err)
_, err = client.SearchTagValuesV2("span.name", "{}")
assertHTTP400(t, err)
if !tc.multitenant {
_, err := client.MetricsSummary("{}", "name", 0, 0)
assertHTTP400(t, err)
_, err = client.MetricsQueryRange("{} | count_over_time()", 0, 0, "", 0)
assertHTTP400(t, err)
}
})
if !tc.multitenant {
t.Run(tc.name+"_write", func(t *testing.T) {
exporter, err := util.NewJaegerToOTLPExporterWithAuth(tempo.Endpoint(4317), tc.orgID, "", false)
require.NoError(t, err)
require.NotNil(t, exporter)
writeTraceInfo := tempoUtil.NewTraceInfo(time.Now(), tc.orgID)
writeErr := writeTraceInfo.EmitAllBatches(exporter)
require.Error(t, writeErr)
require.Contains(t, writeErr.Error(), "code = InvalidArgument")
require.Contains(t, writeErr.Error(), tc.errContains)
})
}
}
}
modules/distributor/distributor.go
+3 -3
View File
@@ -396,12 +396,12 @@ func (d *Distributor) checkForRateLimits(tracesSize, spanCount int, userID strin
}
func (d *Distributor) extractBasicInfo(ctx context.Context, traces ptrace.Traces) (userID string, spanCount, tracesSize int, err error) {
user, e := user.ExtractOrgID(ctx)
orgID, e := validation.ExtractValidTenantID(ctx)
if e != nil {
return "", 0, 0, e
return "", 0, 0, status.Error(codes.InvalidArgument, e.Error())
}
return user, traces.SpanCount(), (&ptrace.ProtoMarshaler{}).TracesSize(traces), nil
return orgID, traces.SpanCount(), (&ptrace.ProtoMarshaler{}).TracesSize(traces), nil
}
// PushTraces pushes a batch of traces
modules/frontend/frontend.go
+9
View File
@@ -138,6 +138,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
urlDenyListWare := pipeline.NewURLDenyListWare(cfg.URLDenyList)
queryValidatorWare := pipeline.NewQueryValidatorWare(cfg.MaxQueryExpressionSizeBytes)
headerStripWare := pipeline.NewStripHeadersWare(cfg.AllowedHeaders)
tenantValidatorWare := pipeline.NewTenantValidatorMiddleware()
tracePipeline := pipeline.Build(
[]pipeline.AsyncMiddleware[combiner.PipelineResponse]{
@@ -145,6 +146,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
urlDenyListWare,
pipeline.NewWeightRequestWare(pipeline.TraceByID, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncTraceIDSharder(&cfg.TraceByID, logger),
},
[]pipeline.Middleware{traceIDStatusCodeWare, retryWare},
@@ -158,6 +160,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
queryValidatorWare,
pipeline.NewWeightRequestWare(pipeline.TraceQLSearch, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncSearchSharder(reader, o, cfg.Search.Sharder, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
@@ -170,6 +173,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
urlDenyListWare,
pipeline.NewWeightRequestWare(pipeline.Default, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncTagSharder(reader, o, cfg.Search.Sharder, parseTagsRequest, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
@@ -182,6 +186,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
urlDenyListWare,
pipeline.NewWeightRequestWare(pipeline.Default, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncTagSharder(reader, o, cfg.Search.Sharder, parseTagValuesRequest, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
@@ -194,6 +199,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
urlDenyListWare,
pipeline.NewWeightRequestWare(pipeline.Default, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncTagSharder(reader, o, cfg.Search.Sharder, parseTagValuesRequestV2, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
@@ -207,6 +213,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
queryValidatorWare,
pipeline.NewWeightRequestWare(pipeline.Default, cfg.Weights),
multiTenantUnsupportedMiddleware(cfg, logger),
tenantValidatorWare,
},
[]pipeline.Middleware{statusCodeWare, retryWare},
next)
@@ -220,6 +227,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
queryValidatorWare,
pipeline.NewWeightRequestWare(pipeline.TraceQLMetrics, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncQueryRangeSharder(reader, o, cfg.Metrics.Sharder, false, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
@@ -233,6 +241,7 @@ func New(cfg Config, next pipeline.RoundTripper, o overrides.Interface, reader t
queryValidatorWare,
pipeline.NewWeightRequestWare(pipeline.TraceQLMetrics, cfg.Weights),
multiTenantMiddleware(cfg, logger),
tenantValidatorWare,
newAsyncQueryRangeSharder(reader, o, cfg.Metrics.Sharder, true, logger),
},
[]pipeline.Middleware{cacheWare, statusCodeWare, retryWare},
modules/frontend/metrics_query_range_sharder.go
+2 -2
View File
@@ -11,7 +11,6 @@ import (
"github.com/go-kit/log" //nolint:all deprecated
"github.com/go-kit/log/level"
"github.com/gogo/protobuf/jsonpb"
"github.com/grafana/dskit/user"
"github.com/segmentio/fasthash/fnv1a"
"go.opentelemetry.io/otel/attribute"
@@ -22,6 +21,7 @@ import (
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb"
"github.com/grafana/tempo/tempodb/backend"
)
@@ -88,7 +88,7 @@ func (s queryRangeSharder) RoundTrip(pipelineRequest pipeline.Request) (pipeline
return pipeline.NewAsyncSharderChan(ctx, s.cfg.ConcurrentRequests, ch, nil, s.next), nil
}
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return pipeline.NewBadRequest(err), nil
}
modules/frontend/pipeline/async_tenant_validator_middleware.go
+27
View File
@@ -0,0 +1,27 @@
package pipeline
import (
"github.com/grafana/tempo/modules/frontend/combiner"
"github.com/grafana/tempo/pkg/validation"
)
type tenantValidatorRoundTripper struct {
next AsyncRoundTripper[combiner.PipelineResponse]
}
// NewTenantValidatorMiddleware returns a middleware that validates the tenant ID in requests.
// It assumes there's only one tenant ID per request, otherwise it returns an error.
// For this reason it must run after the multiTenantMiddleware.
func NewTenantValidatorMiddleware() AsyncMiddleware[combiner.PipelineResponse] {
return AsyncMiddlewareFunc[combiner.PipelineResponse](func(next AsyncRoundTripper[combiner.PipelineResponse]) AsyncRoundTripper[combiner.PipelineResponse] {
return &tenantValidatorRoundTripper{next: next}
})
}
func (t *tenantValidatorRoundTripper) RoundTrip(req Request) (Responses[combiner.PipelineResponse], error) {
_, err := validation.ExtractValidTenantID(req.Context())
if err != nil {
return NewBadRequest(err), nil
}
return t.next.RoundTrip(req)
}
modules/frontend/search_sharder.go
+2 -2
View File
@@ -8,7 +8,6 @@ import (
"time"
"github.com/go-kit/log" //nolint:all deprecated
"github.com/grafana/dskit/user"
"github.com/segmentio/fasthash/fnv1a"
"github.com/grafana/tempo/modules/frontend/combiner"
@@ -17,6 +16,7 @@ import (
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb"
"github.com/grafana/tempo/tempodb/backend"
)
@@ -86,7 +86,7 @@ func (s asyncSearchSharder) RoundTrip(pipelineRequest pipeline.Request) (pipelin
}
requestCtx := r.Context()
tenantID, err := user.ExtractOrgID(requestCtx)
tenantID, err := validation.ExtractValidTenantID(requestCtx)
if err != nil {
return pipeline.NewBadRequest(err), nil
}
modules/frontend/tag_sharder.go
+2 -2
View File
@@ -7,13 +7,13 @@ import (
"time"
"github.com/go-kit/log"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/modules/frontend/combiner"
"github.com/grafana/tempo/modules/frontend/pipeline"
"github.com/grafana/tempo/modules/overrides"
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb"
"github.com/grafana/tempo/tempodb/backend"
"github.com/segmentio/fasthash/fnv1a"
@@ -208,7 +208,7 @@ func (s searchTagSharder) RoundTrip(pipelineRequest pipeline.Request) (pipeline.
r := pipelineRequest.HTTPRequest()
ctx := pipelineRequest.Context()
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return pipeline.NewBadRequest(err), nil
}
modules/frontend/traceid_sharder.go
+2 -2
View File
@@ -6,12 +6,12 @@ import (
"time"
"github.com/go-kit/log" //nolint:all //deprecated
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/modules/frontend/combiner"
"github.com/grafana/tempo/modules/frontend/pipeline"
"github.com/grafana/tempo/modules/querier"
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/blockboundary"
"github.com/grafana/tempo/pkg/validation"
)
const (
@@ -71,7 +71,7 @@ func (s asyncTraceSharder) RoundTrip(pipelineRequest pipeline.Request) (pipeline
// buildShardedRequests returns a slice of requests sharded on the precalculated
// block boundaries
func (s *asyncTraceSharder) buildShardedRequests(parent pipeline.Request) ([]pipeline.Request, error) {
userID, err := user.ExtractOrgID(parent.Context())
userID, err := validation.ExtractValidTenantID(parent.Context())
if err != nil {
return nil, err
}
modules/generator/generator.go
+4 -4
View File
@@ -14,7 +14,6 @@ import (
"github.com/grafana/dskit/kv"
"github.com/grafana/dskit/ring"
"github.com/grafana/dskit/services"
"github.com/grafana/dskit/user"
"github.com/prometheus/client_golang/prometheus"
"github.com/twmb/franz-go/pkg/kadm"
"github.com/twmb/franz-go/pkg/kgo"
@@ -28,6 +27,7 @@ import (
objStorage "github.com/grafana/tempo/modules/storage"
"github.com/grafana/tempo/pkg/ingest"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/validation"
tempodb_wal "github.com/grafana/tempo/tempodb/wal"
)
@@ -268,7 +268,7 @@ func (g *Generator) PushSpans(ctx context.Context, req *tempopb.PushSpansRequest
ctx, span := tracer.Start(ctx, "generator.PushSpans")
defer span.End()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -416,7 +416,7 @@ func (g *Generator) OnRingInstanceHeartbeat(*ring.BasicLifecycler, *ring.Desc, *
}
func (g *Generator) GetMetrics(ctx context.Context, req *tempopb.SpanMetricsRequest) (*tempopb.SpanMetricsResponse, error) {
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -431,7 +431,7 @@ func (g *Generator) GetMetrics(ctx context.Context, req *tempopb.SpanMetricsRequ
}
func (g *Generator) QueryRange(ctx context.Context, req *tempopb.QueryRangeRequest) (*tempopb.QueryRangeResponse, error) {
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
modules/ingester/ingester.go
+2 -3
View File
@@ -14,7 +14,6 @@ import (
"github.com/grafana/dskit/kv"
"github.com/grafana/dskit/ring"
"github.com/grafana/dskit/services"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/pkg/ingest"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
@@ -310,7 +309,7 @@ func (i *Ingester) PushBytesV2(ctx context.Context, req *tempopb.PushBytesReques
return nil, status.Errorf(codes.InvalidArgument, "mismatched traces/ids length: %d, %d", len(req.Traces), len(req.Ids))
}
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -341,7 +340,7 @@ func (i *Ingester) FindTraceByID(ctx context.Context, req *tempopb.TraceByIDRequ
ctx, span := tracer.Start(ctx, "Ingester.FindTraceByID")
defer span.End()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
modules/ingester/ingester_search.go
+6 -6
View File
@@ -6,9 +6,9 @@ import (
"runtime/debug"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/util/log"
"github.com/grafana/tempo/pkg/validation"
)
func (i *Ingester) SearchRecent(ctx context.Context, req *tempopb.SearchRequest) (res *tempopb.SearchResponse, err error) {
@@ -19,7 +19,7 @@ func (i *Ingester) SearchRecent(ctx context.Context, req *tempopb.SearchRequest)
}
}()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -44,7 +44,7 @@ func (i *Ingester) SearchTags(ctx context.Context, req *tempopb.SearchTagsReques
}
}()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -69,7 +69,7 @@ func (i *Ingester) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequ
}
}()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -94,7 +94,7 @@ func (i *Ingester) SearchTagValues(ctx context.Context, req *tempopb.SearchTagVa
}
}()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -119,7 +119,7 @@ func (i *Ingester) SearchTagValuesV2(ctx context.Context, req *tempopb.SearchTag
}
}()
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
modules/ingester/instance_search.go
+4 -4
View File
@@ -16,7 +16,6 @@ import (
"go.uber.org/atomic"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/boundedwaitgroup"
"github.com/grafana/tempo/pkg/collector"
@@ -24,6 +23,7 @@ import (
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/util"
"github.com/grafana/tempo/pkg/util/log"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/backend"
"github.com/grafana/tempo/tempodb/encoding/common"
)
@@ -211,7 +211,7 @@ func (i *instance) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequ
ctx, span := tracer.Start(ctx, "instance.SearchTagsV2")
defer span.End()
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -317,7 +317,7 @@ func (i *instance) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequ
}
func (i *instance) SearchTagValues(ctx context.Context, tagName string, limit uint32, staleValueThreshold uint32) (*tempopb.SearchTagValuesResponse, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -384,7 +384,7 @@ func (i *instance) SearchTagValues(ctx context.Context, tagName string, limit ui
}
func (i *instance) SearchTagValuesV2(ctx context.Context, req *tempopb.SearchTagValuesRequest) (*tempopb.SearchTagValuesV2Response, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
modules/livestore/instance_search.go
+4 -4
View File
@@ -20,7 +20,6 @@ import (
"go.uber.org/atomic"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/modules/ingester"
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/boundedwaitgroup"
@@ -30,6 +29,7 @@ import (
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/util"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/backend"
"github.com/grafana/tempo/tempodb/encoding/common"
)
@@ -282,7 +282,7 @@ func (i *instance) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequ
ctx, span := tracer.Start(ctx, "instance.SearchTagsV2")
defer span.End()
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -368,7 +368,7 @@ func (i *instance) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequ
}
func (i *instance) SearchTagValues(ctx context.Context, req *tempopb.SearchTagValuesRequest) (*tempopb.SearchTagValuesResponse, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
@@ -425,7 +425,7 @@ func (i *instance) SearchTagValues(ctx context.Context, req *tempopb.SearchTagVa
}
func (i *instance) SearchTagValuesV2(ctx context.Context, req *tempopb.SearchTagValuesRequest) (*tempopb.SearchTagValuesV2Response, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, err
}
modules/livestore/live_store.go
+2 -2
View File
@@ -13,12 +13,12 @@ import (
"github.com/grafana/dskit/kv"
"github.com/grafana/dskit/ring"
"github.com/grafana/dskit/services"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/modules/overrides"
"github.com/grafana/tempo/pkg/flushqueues"
"github.com/grafana/tempo/pkg/ingest"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/util/shutdownmarker"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/wal"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
@@ -591,7 +591,7 @@ func (s *LiveStore) QueryRange(ctx context.Context, req *tempopb.QueryRangeReque
func withInstance[T any](ctx context.Context, s *LiveStore, fn func(*instance) (*T, error)) (*T, error) {
var defaultValue T
instanceID, err := user.ExtractOrgID(ctx)
instanceID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return &defaultValue, err
}
modules/overrides/userconfigurable/api/api_test.go
+65
View File
@@ -134,6 +134,71 @@ func Test_UserConfigOverridesAPI_overridesHandlers(t *testing.T) {
}
}
func TestUserConfigOverridesAPI_invalidOrgID(t *testing.T) {
invalidTenant := "invalid/org"
cfg := client.Config{
Backend: backend.Local,
Local: &local.Config{Path: t.TempDir()},
}
o, err := overrides.NewOverrides(overrides.Config{}, nil, prometheus.DefaultRegisterer)
assert.NoError(t, err)
overridesAPI, err := New(&overrides.UserConfigurableOverridesAPIConfig{}, &cfg, o, &mockValidator{})
require.NoError(t, err)
expectedError := "tenant ID 'invalid/org' contains unsupported character '/'"
tests := []struct {
name string
handler func(http.ResponseWriter, *http.Request)
request func() *http.Request
}{
{
name: "GET",
handler: overridesAPI.GetHandler,
request: func() *http.Request {
return prepareRequest(invalidTenant, http.MethodGet, nil)
},
},
{
name: "POST",
handler: overridesAPI.PostHandler,
request: func() *http.Request {
return prepareRequest(invalidTenant, http.MethodPost, []byte("{}"))
},
},
{
name: "PATCH",
handler: overridesAPI.PatchHandler,
request: func() *http.Request {
return prepareRequest(invalidTenant, http.MethodPatch, []byte("{}"))
},
},
{
name: "DELETE",
handler: overridesAPI.DeleteHandler,
request: func() *http.Request {
return prepareRequest(invalidTenant, http.MethodDelete, nil)
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
w := httptest.NewRecorder()
req := tc.request()
tc.handler(w, req)
res := w.Result()
assert.Equal(t, http.StatusBadRequest, res.StatusCode)
assert.Equal(t, expectedError+"\n", w.Body.String())
})
}
}
func Test_UserConfigOverridesAPI_patchOverridesHandlers(t *testing.T) {
tenant := "my-tenant"
modules/overrides/userconfigurable/api/http.go
+5 -5
View File
@@ -9,13 +9,13 @@ import (
"strconv"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/user"
jsoniter "github.com/json-iterator/go"
"go.opentelemetry.io/otel/codes"
"github.com/grafana/tempo/modules/overrides/userconfigurable/client"
"github.com/grafana/tempo/pkg/api"
"github.com/grafana/tempo/pkg/util/tracing"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/backend"
)
@@ -39,7 +39,7 @@ func (a *UserConfigOverridesAPI) GetHandler(w http.ResponseWriter, r *http.Reque
ctx, f := a.logRequest(r.Context(), "UserConfigOverridesAPI.GetHandler", r)
defer f(&err)
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
@@ -72,7 +72,7 @@ func (a *UserConfigOverridesAPI) PostHandler(w http.ResponseWriter, r *http.Requ
ctx, f := a.logRequest(r.Context(), "UserConfigOverridesAPI.PostHandler", r)
defer f(&err)
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
@@ -112,7 +112,7 @@ func (a *UserConfigOverridesAPI) PatchHandler(w http.ResponseWriter, r *http.Req
ctx, f := a.logRequest(r.Context(), "UserConfigOverridesAPI.PatchHandler", r)
defer f(&err)
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
@@ -150,7 +150,7 @@ func (a *UserConfigOverridesAPI) DeleteHandler(w http.ResponseWriter, r *http.Re
ctx, f := a.logRequest(r.Context(), "UserConfigOverridesAPI.DeleteHandler", r)
defer f(&err)
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
modules/querier/querier.go
+10 -11
View File
@@ -14,7 +14,6 @@ import (
"github.com/grafana/dskit/ring"
ring_client "github.com/grafana/dskit/ring/client"
"github.com/grafana/dskit/services"
"github.com/grafana/dskit/user"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"go.opentelemetry.io/otel"
@@ -257,7 +256,7 @@ func (q *Querier) FindTraceByID(ctx context.Context, req *tempopb.TraceByIDReque
return nil, errors.New("invalid trace id")
}
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.FindTraceByID: %w", err)
}
@@ -529,7 +528,7 @@ func (q *Querier) forGivenGenerators(ctx context.Context, f forEachGeneratorFn)
}
func (q *Querier) SearchRecent(ctx context.Context, req *tempopb.SearchRequest) (*tempopb.SearchResponse, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.SearchRecent: %w", err)
}
@@ -581,7 +580,7 @@ func (q *Querier) SearchTagValuesBlocksV2(ctx context.Context, req *tempopb.Sear
}
func (q *Querier) SearchTags(ctx context.Context, req *tempopb.SearchTagsRequest) (*tempopb.SearchTagsResponse, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.SearchTags: %w", err)
}
@@ -623,7 +622,7 @@ outer:
}
func (q *Querier) SearchTagsV2(ctx context.Context, req *tempopb.SearchTagsRequest) (*tempopb.SearchTagsV2Response, error) {
orgID, err := user.ExtractOrgID(ctx)
orgID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.SearchTags: %w", err)
}
@@ -676,7 +675,7 @@ outer:
}
func (q *Querier) SearchTagValues(ctx context.Context, req *tempopb.SearchTagValuesRequest) (*tempopb.SearchTagValuesResponse, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.SearchTagValues: %w", err)
}
@@ -724,7 +723,7 @@ outer:
}
func (q *Querier) SearchTagValuesV2(ctx context.Context, req *tempopb.SearchTagValuesRequest) (*tempopb.SearchTagValuesV2Response, error) {
userID, err := user.ExtractOrgID(ctx)
userID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.SearchTagValues: %w", err)
}
@@ -857,7 +856,7 @@ func valuesToV2Response(distinctValues *collector.DistinctValue[tempopb.TagValue
// SearchBlock searches the specified subset of the block for the passed tags.
func (q *Querier) SearchBlock(ctx context.Context, req *tempopb.SearchBlockRequest) (*tempopb.SearchResponse, error) {
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.BackendSearch: %w", err)
}
@@ -913,7 +912,7 @@ func (q *Querier) internalTagsSearchBlockV2(ctx context.Context, req *tempopb.Se
return &tempopb.SearchTagsV2Response{}, nil
}
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.BackendSearch: %w", err)
}
@@ -994,7 +993,7 @@ func (q *Querier) internalTagsSearchBlockV2(ctx context.Context, req *tempopb.Se
}
func (q *Querier) internalTagValuesSearchBlock(ctx context.Context, req *tempopb.SearchTagValuesBlockRequest) (*tempopb.SearchTagValuesResponse, error) {
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return &tempopb.SearchTagValuesResponse{}, fmt.Errorf("error extracting org id in Querier.BackendSearch: %w", err)
}
@@ -1040,7 +1039,7 @@ func (q *Querier) internalTagValuesSearchBlock(ctx context.Context, req *tempopb
}
func (q *Querier) internalTagValuesSearchBlockV2(ctx context.Context, req *tempopb.SearchTagValuesBlockRequest) (*tempopb.SearchTagValuesV2Response, error) {
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return &tempopb.SearchTagValuesV2Response{}, fmt.Errorf("error extracting org id in Querier.BackendSearch: %w", err)
}
modules/querier/querier_query_range.go
+2 -2
View File
@@ -6,10 +6,10 @@ import (
"time"
"github.com/go-kit/log/level"
"github.com/grafana/dskit/user"
"github.com/grafana/tempo/pkg/tempopb"
"github.com/grafana/tempo/pkg/traceql"
"github.com/grafana/tempo/pkg/util/log"
"github.com/grafana/tempo/pkg/validation"
"github.com/grafana/tempo/tempodb/backend"
"github.com/grafana/tempo/tempodb/encoding/common"
)
@@ -49,7 +49,7 @@ func (q *Querier) queryRangeRecent(ctx context.Context, req *tempopb.QueryRangeR
}
func (q *Querier) queryBlock(ctx context.Context, req *tempopb.QueryRangeRequest) (*tempopb.QueryRangeResponse, error) {
tenantID, err := user.ExtractOrgID(ctx)
tenantID, err := validation.ExtractValidTenantID(ctx)
if err != nil {
return nil, fmt.Errorf("error extracting org id in Querier.queryBlock: %w", err)
}
pkg/validation/tenantid.go
+19
View File
@@ -0,0 +1,19 @@
package validation
import (
"context"
"github.com/grafana/dskit/tenant"
"github.com/grafana/dskit/user"
)
func ExtractValidTenantID(ctx context.Context) (string, error) {
tenantID, err := tenant.TenantID(ctx)
if err != nil {
return "", err
}
if tenantID == "" {
return "", user.ErrNoOrgID
}
return tenantID, nil
}