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/"
This commit is contained in:
wxiaoguang 2023-09-13 09:18:52 +08:00 committed by GitHub
parent 7d56459c6c
commit 8ecdc93f8b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 48 additions and 14 deletions

View file

@ -136,9 +136,18 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
} }
func (m *MinioStorage) buildMinioPath(p string) string { 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 == "." { 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 return p
} }
@ -237,20 +246,11 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
// IterateObjects iterates across the objects in the miniostorage // IterateObjects iterates across the objects in the miniostorage
func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error { func (m *MinioStorage) IterateObjects(dirName string, fn func(path string, obj Object) error) error {
opts := minio.GetObjectOptions{} opts := minio.GetObjectOptions{}
lobjectCtx, cancel := context.WithCancel(m.ctx) for mObjInfo := range m.client.ListObjects(m.ctx, m.bucket, minio.ListObjectsOptions{
defer cancel() Prefix: m.buildMinioDirPrefix(dirName),
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,
Recursive: true, 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 { if err != nil {
return convertMinioErr(err) return convertMinioErr(err)
} }

View file

@ -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) { func TestS3StorageBadRequest(t *testing.T) {
if os.Getenv("CI") == "" { if os.Getenv("CI") == "" {
t.Skip("S3Storage not present outside of CI") t.Skip("S3Storage not present outside of CI")