Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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

## 1.8.0 in progress

Expand Down
24 changes: 19 additions & 5 deletions pkg/ruler/rulestore/bucketclient/bucket_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions pkg/ruler/rulestore/bucketclient/bucket_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
}