Ignore objects with names that end with / when listing rule groups (#3999)

* Ignore objects with names that end with /

Signed-off-by: Peter Štibraný <[email protected]>

* CHANGELOG.md

Signed-off-by: Peter Štibraný <[email protected]>

* Update CHANGELOG.md

Signed-off-by: Marco Pracucci <[email protected]>

* Validate user, namespace and group.

Signed-off-by: Peter Štibraný <[email protected]>

Co-authored-by: Marco Pracucci <[email protected]>

Author: pstibrany

CHANGELOG.md

@@ -22,6 +22,7 @@
 * [BUGFIX] Distributor: reverted changes done to rate limiting in #3825. #3948
 * [BUGFIX] Ingester: Fix race condition when opening and closing tsdb concurrently. #3959
 * [BUGFIX] Querier: streamline tracing spans. #3924
+* [BUGFIX] Ruler Storage: ignore objects with empty namespace or group in the name. #3999
 * [BUGFIX] Distributor: fix issue causing distributors to not extend the replication set because of failing instances when zone-aware replication is enabled. #3977
 
 ## 1.8.0 in progress

pkg/ruler/rulestore/bucketclient/bucket_client.go

@@ -29,6 +29,9 @@ const (
 
 var (
 	errInvalidRuleGroupKey = errors.New("invalid rule group object key")
+	errEmptyUser           = errors.New("empty user")
+	errEmptyNamespace      = errors.New("empty namespace")
+	errEmptyGroupName      = errors.New("empty group name")
 )
 
 // BucketRuleStore is used to support the RuleStore interface against an object storage backend. It is implemented
@@ -104,7 +107,7 @@ func (b *BucketRuleStore) ListAllRuleGroups(ctx context.Context) (map[string]rul
 	err := b.bucket.Iter(ctx, "", func(key string) error {
 		userID, namespace, group, err := parseRuleGroupObjectKeyWithUser(key)
 		if err != nil {
-			level.Warn(b.logger).Log("msg", "invalid rule group object key found while listing rule groups", "key", key)
+			level.Warn(b.logger).Log("msg", "invalid rule group object key found while listing rule groups", "key", key, "err", err)
 
 			// Do not fail just because of a spurious item in the bucket.
 			return nil
@@ -141,7 +144,7 @@ func (b *BucketRuleStore) ListRuleGroupsForUserAndNamespace(ctx context.Context,
 	err := userBucket.Iter(ctx, prefix, func(key string) error {
 		namespace, group, err := parseRuleGroupObjectKey(key)
 		if err != nil {
-			level.Warn(b.logger).Log("msg", "invalid rule group object key found while listing rule groups", "user", userID, "key", key)
+			level.Warn(b.logger).Log("msg", "invalid rule group object key found while listing rule groups", "user", userID, "key", key, "err", err)
 
 			// Do not fail just because of a spurious item in the bucket.
 			return nil
@@ -280,25 +283,36 @@ func parseRuleGroupObjectKeyWithUser(key string) (user, namespace, group string,
 	}
 
 	user = parts[0]
+	if user == "" {
+		return "", "", "", errEmptyUser
+	}
 	namespace, group, err = parseRuleGroupObjectKey(parts[1])
 	return
 }
 
 // parseRuleGroupObjectKey parses a bucket object key in the format "<namespace>/<rules group>".
-func parseRuleGroupObjectKey(key string) (namespace, group string, err error) {
+func parseRuleGroupObjectKey(key string) (namespace, group string, _ error) {
 	parts := strings.Split(key, objstore.DirDelim)
 	if len(parts) != 2 {
 		return "", "", errInvalidRuleGroupKey
 	}
 
 	decodedNamespace, err := base64.URLEncoding.DecodeString(parts[0])
 	if err != nil {
-		return
+		return "", "", err
+	}
+
+	if len(decodedNamespace) == 0 {
+		return "", "", errEmptyNamespace
 	}
 
 	decodedGroup, err := base64.URLEncoding.DecodeString(parts[1])
 	if err != nil {
-		return
+		return "", "", err
+	}
+
+	if len(decodedGroup) == 0 {
+		return "", "", errEmptyGroupName
 	}
 
 	return string(decodedNamespace), string(decodedGroup), nil

pkg/ruler/rulestore/bucketclient/bucket_client_test.go

@@ -291,10 +291,18 @@ func TestParseRuleGroupObjectKey(t *testing.T) {
 			key:         "way/too/long",
 			expectedErr: errInvalidRuleGroupKey,
 		},
+		"empty namespace": {
+			key:         fmt.Sprintf("/%s", encodedGroup),
+			expectedErr: errEmptyNamespace,
+		},
 		"invalid namespace encoding": {
 			key:         fmt.Sprintf("invalid/%s", encodedGroup),
 			expectedErr: errors.New("illegal base64 data at input byte 4"),
 		},
+		"empty group": {
+			key:         fmt.Sprintf("%s/", encodedNamespace),
+			expectedErr: errEmptyGroupName,
+		},
 		"invalid group encoding": {
 			key:         fmt.Sprintf("%s/invalid", encodedNamespace),
 			expectedErr: errors.New("illegal base64 data at input byte 4"),
@@ -343,10 +351,22 @@ func TestParseRuleGroupObjectKeyWithUser(t *testing.T) {
 			key:         "way/too/much/long",
 			expectedErr: errInvalidRuleGroupKey,
 		},
+		"empty user": {
+			key:         fmt.Sprintf("/%s/%s", encodedNamespace, encodedGroup),
+			expectedErr: errEmptyUser,
+		},
+		"empty namespace": {
+			key:         fmt.Sprintf("user-1//%s", encodedGroup),
+			expectedErr: errEmptyNamespace,
+		},
 		"invalid namespace encoding": {
 			key:         fmt.Sprintf("user-1/invalid/%s", encodedGroup),
 			expectedErr: errors.New("illegal base64 data at input byte 4"),
 		},
+		"empty group name": {
+			key:         fmt.Sprintf("user-1/%s/", encodedNamespace),
+			expectedErr: errEmptyGroupName,
+		},
 		"invalid group encoding": {
 			key:         fmt.Sprintf("user-1/%s/invalid", encodedNamespace),
 			expectedErr: errors.New("illegal base64 data at input byte 4"),
@@ -374,3 +394,37 @@ func TestParseRuleGroupObjectKeyWithUser(t *testing.T) {
 		})
 	}
 }
+
+func TestListAllRuleGroupsWithNoNamespaceOrGroup(t *testing.T) {
+	obj := mockBucket{
+		names: []string{
+			"rules/",
+			"rules/user1/",
+			"rules/user2/bnM=/",         // namespace "ns", ends with '/'
+			"rules/user3/bnM=/Z3JvdXAx", // namespace "ns", group "group1"
+		},
+	}
+
+	s := NewBucketRuleStore(obj, nil, log.NewNopLogger())
+	out, err := s.ListAllRuleGroups(context.Background())
+	require.NoError(t, err)
+
+	require.Equal(t, 1, len(out))                    // one user
+	require.Equal(t, 1, len(out["user3"]))           // one group
+	require.Equal(t, "group1", out["user3"][0].Name) // one group
+}
+
+type mockBucket struct {
+	objstore.Bucket
+
+	names []string
+}
+
+func (mb mockBucket) Iter(_ context.Context, dir string, f func(string) error, options ...objstore.IterOption) error {
+	for _, n := range mb.names {
+		if err := f(n); err != nil {
+			return err
+		}
+	}
+	return nil
+}