Skip to content

Commit 8ecdc93

Browse files
authored
Fix object storage path handling (#27024)
Object storage path rules: * No single `/` or `.`, use empty string for root path * Need to use trailing `/` for a list prefix to distinguish a "dir/"
1 parent 7d56459 commit 8ecdc93

File tree

2 files changed

+48
-14
lines changed

2 files changed

+48
-14
lines changed

modules/storage/minio.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -136,9 +136,18 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
136136
}
137137

138138
func (m *MinioStorage) buildMinioPath(p string) string {
139-
p = util.PathJoinRelX(m.basePath, p)
139+
p = strings.TrimPrefix(util.PathJoinRelX(m.basePath, p), "/") // object store doesn't use slash for root path
140140
if p == "." {
141-
p = "" // minio doesn't use dot as relative path
141+
p = "" // object store doesn't use dot as relative path
142+
}
143+
return p
144+
}
145+
146+
func (m *MinioStorage) buildMinioDirPrefix(p string) string {
147+
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
148+
p = m.buildMinioPath(p) + "/"
149+
if p == "/" {
150+
p = "" // object store doesn't use slash for root path
142151
}
143152
return p
144153
}
@@ -237,20 +246,11 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
237246
// IterateObjects iterates across the objects in the miniostorage
238247
func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
239248
opts := minio.GetObjectOptions{}
240-
lobjectCtx, cancel := context.WithCancel(m.ctx)
241-
defer cancel()
242-
243-
basePath := m.basePath
244-
if dirName != "" {
245-
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
246-
basePath = m.buildMinioPath(dirName) + "/"
247-
}
248-
249-
for mObjInfo := range m.client.ListObjects(lobjectCtx, m.bucket, minio.ListObjectsOptions{
250-
Prefix: basePath,
249+
for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{
250+
Prefix: m.buildMinioDirPrefix(dirName),
251251
Recursive: true,
252252
}) {
253-
object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts)
253+
object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, opts)
254254
if err != nil {
255255
return convertMinioErr(err)
256256
}

modules/storage/minio_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,40 @@ func TestMinioStorageIterator(t *testing.T) {
3131
})
3232
}
3333

34+
func TestMinioStoragePath(t *testing.T) {
35+
m := &MinioStorage{basePath: ""}
36+
assert.Equal(t, "", m.buildMinioPath("/"))
37+
assert.Equal(t, "", m.buildMinioPath("."))
38+
assert.Equal(t, "a", m.buildMinioPath("/a"))
39+
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/"))
40+
assert.Equal(t, "", m.buildMinioDirPrefix(""))
41+
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/"))
42+
43+
m = &MinioStorage{basePath: "/"}
44+
assert.Equal(t, "", m.buildMinioPath("/"))
45+
assert.Equal(t, "", m.buildMinioPath("."))
46+
assert.Equal(t, "a", m.buildMinioPath("/a"))
47+
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/"))
48+
assert.Equal(t, "", m.buildMinioDirPrefix(""))
49+
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/"))
50+
51+
m = &MinioStorage{basePath: "/base"}
52+
assert.Equal(t, "base", m.buildMinioPath("/"))
53+
assert.Equal(t, "base", m.buildMinioPath("."))
54+
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
55+
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
56+
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
57+
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))
58+
59+
m = &MinioStorage{basePath: "/base/"}
60+
assert.Equal(t, "base", m.buildMinioPath("/"))
61+
assert.Equal(t, "base", m.buildMinioPath("."))
62+
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
63+
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
64+
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
65+
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))
66+
}
67+
3468
func TestS3StorageBadRequest(t *testing.T) {
3569
if os.Getenv("CI") == "" {
3670
t.Skip("S3Storage not present outside of CI")

0 commit comments

Comments
 (0)