Skip to content

Fix object storage path handling #27024

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

Merged
merged 3 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 14 additions & 14 deletions modules/storage/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,18 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
}

func (m *MinioStorage) buildMinioPath(p string) string {
p = util.PathJoinRelX(m.basePath, p)
p = strings.TrimPrefix(util.PathJoinRelX(m.basePath, p), "/") // object store doesn't use slash for root path
if p == "." {
p = "" // minio doesn't use dot as relative path
p = "" // object store doesn't use dot as relative path
}
return p
}

func (m *MinioStorage) buildMinioDirPrefix(p string) string {
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
p = m.buildMinioPath(p) + "/"
if p == "/" {
p = "" // object store doesn't use slash for root path
}
return p
}
Expand Down Expand Up @@ -237,20 +246,11 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
// IterateObjects iterates across the objects in the miniostorage
func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
opts := minio.GetObjectOptions{}
lobjectCtx, cancel := context.WithCancel(m.ctx)
defer cancel()

basePath := m.basePath
if dirName != "" {
// ending slash is required for avoiding matching like "foo/" and "foobar/" with prefix "foo"
basePath = m.buildMinioPath(dirName) + "/"
}

for mObjInfo := range m.client.ListObjects(lobjectCtx, m.bucket, minio.ListObjectsOptions{
Prefix: basePath,
for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{
Prefix: m.buildMinioDirPrefix(dirName),
Recursive: true,
}) {
object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts)
object, err := m.client.GetObject(m.ctx, m.bucket, mObjInfo.Key, opts)
if err != nil {
return convertMinioErr(err)
}
Expand Down
34 changes: 34 additions & 0 deletions modules/storage/minio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,40 @@ func TestMinioStorageIterator(t *testing.T) {
})
}

func TestMinioStoragePath(t *testing.T) {
m := &MinioStorage{basePath: ""}
assert.Equal(t, "", m.buildMinioPath("/"))
assert.Equal(t, "", m.buildMinioPath("."))
assert.Equal(t, "a", m.buildMinioPath("/a"))
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "", m.buildMinioDirPrefix(""))
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/"))

m = &MinioStorage{basePath: "/"}
assert.Equal(t, "", m.buildMinioPath("/"))
assert.Equal(t, "", m.buildMinioPath("."))
assert.Equal(t, "a", m.buildMinioPath("/a"))
assert.Equal(t, "a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "", m.buildMinioDirPrefix(""))
assert.Equal(t, "a/", m.buildMinioDirPrefix("/a/"))

m = &MinioStorage{basePath: "/base"}
assert.Equal(t, "base", m.buildMinioPath("/"))
assert.Equal(t, "base", m.buildMinioPath("."))
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))

m = &MinioStorage{basePath: "/base/"}
assert.Equal(t, "base", m.buildMinioPath("/"))
assert.Equal(t, "base", m.buildMinioPath("."))
assert.Equal(t, "base/a", m.buildMinioPath("/a"))
assert.Equal(t, "base/a/b", m.buildMinioPath("/a/b/"))
assert.Equal(t, "base/", m.buildMinioDirPrefix(""))
assert.Equal(t, "base/a/", m.buildMinioDirPrefix("/a/"))
}

func TestS3StorageBadRequest(t *testing.T) {
if os.Getenv("CI") == "" {
t.Skip("S3Storage not present outside of CI")
Expand Down