diff --git a/CHANGELOG.md b/CHANGELOG.md index b79f7913477..cbd932b0a68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ * [BUGFIX] Querier: Fix panic when marshaling QueryResultRequest. #6601 * [BUGFIX] Ingester: Avoid resharding for query when restart readonly ingesters. #6642 * [BUGFIX] Query Frontend: Fix query frontend per `user` metrics clean up. #6698 +* [BUGFIX] Add `__markers__` tenant ID validation. #6761 ## 1.19.0 2025-02-27 diff --git a/docs/guides/limitations.md b/docs/guides/limitations.md index 357f4e526ad..d8059b0b7c1 100644 --- a/docs/guides/limitations.md +++ b/docs/guides/limitations.md @@ -32,6 +32,13 @@ The following character sets are generally **safe for use in the tenant ID**: All other characters are not safe to use. In particular, slashes `/` and whitespaces (` `) are **not supported**. +### Invalid tenant IDs +The following tenant IDs are considered invalid in Cortex. + +- Current directory (`.`) +- Parent directory (`..`) +- Markers directory (`__markers__`) + ### Length The tenant ID length should not exceed 150 bytes/characters. diff --git a/pkg/storage/tsdb/tenant_deletion_mark.go b/pkg/storage/tsdb/tenant_deletion_mark.go index 33ba7b22ceb..5e0eda2d34e 100644 --- a/pkg/storage/tsdb/tenant_deletion_mark.go +++ b/pkg/storage/tsdb/tenant_deletion_mark.go @@ -11,7 +11,7 @@ import ( "github.com/pkg/errors" "github.com/thanos-io/objstore" - "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/tenant" util_log "github.com/cortexproject/cortex/pkg/util/log" ) @@ -77,7 +77,7 @@ func GetLocalDeletionMarkPath(userID string) string { } func GetGlobalDeletionMarkPath(userID string) string { - return path.Join(util.GlobalMarkersDir, userID, TenantDeletionMarkFile) + return path.Join(tenant.GlobalMarkersDir, userID, TenantDeletionMarkFile) } func exists(ctx context.Context, bkt objstore.BucketReader, markerFile string) (bool, error) { diff --git a/pkg/storage/tsdb/users_scanner.go b/pkg/storage/tsdb/users_scanner.go index 0fe24395bf5..0e4f7acbe0b 100644 --- a/pkg/storage/tsdb/users_scanner.go +++ b/pkg/storage/tsdb/users_scanner.go @@ -8,13 +8,13 @@ import ( "github.com/go-kit/log/level" "github.com/thanos-io/objstore" - "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/tenant" ) // AllUsers returns true to each call and should be used whenever the UsersScanner should not filter out // any user due to sharding. func AllUsers(user string) (bool, error) { - if user == util.GlobalMarkersDir { + if user == tenant.GlobalMarkersDir { return false, nil } return true, nil @@ -52,7 +52,7 @@ func (s *UsersScanner) ScanUsers(ctx context.Context) (users, markedForDeletion } // Scan users from the __markers__ directory. - err = s.bucketClient.Iter(ctx, util.GlobalMarkersDir, func(entry string) error { + err = s.bucketClient.Iter(ctx, tenant.GlobalMarkersDir, func(entry string) error { // entry will be of the form __markers__// parts := strings.Split(entry, objstore.DirDelim) userID := parts[1] diff --git a/pkg/tenant/resolver_test.go b/pkg/tenant/resolver_test.go index 6d52cc7369d..e29bde51b31 100644 --- a/pkg/tenant/resolver_test.go +++ b/pkg/tenant/resolver_test.go @@ -76,6 +76,12 @@ var commonResolverTestCases = []resolverTestCase{ errTenantID: errTenantIDUnsafe, errTenantIDs: errTenantIDUnsafe, }, + { + name: "__markers__", + headerValue: strptr("__markers__"), + errTenantID: errTenantIDMarkers, + errTenantIDs: errTenantIDMarkers, + }, { name: "white space", headerValue: strptr(" "), diff --git a/pkg/tenant/tenant.go b/pkg/tenant/tenant.go index 57281239f62..07e8156d54c 100644 --- a/pkg/tenant/tenant.go +++ b/pkg/tenant/tenant.go @@ -10,9 +10,12 @@ import ( "github.com/weaveworks/common/user" ) +const GlobalMarkersDir = "__markers__" + var ( errTenantIDTooLong = errors.New("tenant ID is too long: max 150 characters") errTenantIDUnsafe = errors.New("tenant ID is '.' or '..'") + errTenantIDMarkers = errors.New("tenant ID '__markers__' is not allowed") ) type errTenantIDUnsupportedCharacter struct { @@ -66,6 +69,12 @@ func ValidTenantID(s string) error { return errTenantIDTooLong } + // check tenantID is "__markers__" + if s == GlobalMarkersDir { + return errTenantIDMarkers + } + + // check tenantID is "." or ".." if containsUnsafePathSegments(s) { return errTenantIDUnsafe } diff --git a/pkg/tenant/tenant_test.go b/pkg/tenant/tenant_test.go index bc3a60b54fd..665f699c493 100644 --- a/pkg/tenant/tenant_test.go +++ b/pkg/tenant/tenant_test.go @@ -37,6 +37,10 @@ func TestValidTenantIDs(t *testing.T) { name: "..", err: strptr("tenant ID is '.' or '..'"), }, + { + name: "__markers__", + err: strptr("tenant ID '__markers__' is not allowed"), + }, } { t.Run(tc.name, func(t *testing.T) { err := ValidTenantID(tc.name) diff --git a/pkg/util/allowed_tenants.go b/pkg/util/allowed_tenants.go index d5b7af773e5..ff406b381b8 100644 --- a/pkg/util/allowed_tenants.go +++ b/pkg/util/allowed_tenants.go @@ -1,6 +1,6 @@ package util -const GlobalMarkersDir = "__markers__" +import "github.com/cortexproject/cortex/pkg/tenant" // AllowedTenants that can answer whether tenant is allowed or not based on configuration. // Default value (nil) allows all tenants. @@ -36,7 +36,7 @@ func NewAllowedTenants(enabled []string, disabled []string) *AllowedTenants { } func (a *AllowedTenants) IsAllowed(tenantID string) bool { - if tenantID == GlobalMarkersDir { + if tenantID == tenant.GlobalMarkersDir { // __markers__ is reserved for global markers and no tenant should be allowed to have that name. return false }