diff --git a/CHANGELOG.md b/CHANGELOG.md index 27e9104e89e..4ffe5c76787 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/ruler/rulestore/bucketclient/bucket_client.go b/pkg/ruler/rulestore/bucketclient/bucket_client.go index 9f996307a13..411f883a223 100644 --- a/pkg/ruler/rulestore/bucketclient/bucket_client.go +++ b/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,12 +283,15 @@ 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 "/". -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 @@ -293,12 +299,20 @@ func parseRuleGroupObjectKey(key string) (namespace, group string, err error) { 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 diff --git a/pkg/ruler/rulestore/bucketclient/bucket_client_test.go b/pkg/ruler/rulestore/bucketclient/bucket_client_test.go index 9c639c8ee4a..decad4b7232 100644 --- a/pkg/ruler/rulestore/bucketclient/bucket_client_test.go +++ b/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 +}