From b0d216ed9ceaf77d8693a879f5f2395bd9b13e1c Mon Sep 17 00:00:00 2001 From: SungJin1212 Date: Fri, 23 May 2025 12:03:15 +0900 Subject: [PATCH] Add __markers__ tenantID validation Signed-off-by: SungJin1212 --- CHANGELOG.md | 1 + docs/guides/limitations.md | 7 +++++++ pkg/storage/tsdb/tenant_deletion_mark.go | 4 ++-- pkg/storage/tsdb/users_scanner.go | 6 +++--- pkg/tenant/resolver_test.go | 6 ++++++ pkg/tenant/tenant.go | 9 +++++++++ pkg/tenant/tenant_test.go | 4 ++++ pkg/util/allowed_tenants.go | 4 ++-- 8 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b79f791347..cbd932b0a6 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 357f4e526a..d8059b0b7c 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 33ba7b22ce..5e0eda2d34 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 0fe24395bf..0e4f7acbe0 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 6d52cc7369..e29bde51b3 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 57281239f6..07e8156d54 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 bc3a60b54f..665f699c49 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 d5b7af773e..ff406b381b 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 }