diff options
author | wxiaoguang <wxiaoguang@gmail.com> | 2023-03-28 23:10:24 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-28 11:10:24 -0400 |
commit | 5727056ea109eb04ee535b981349cdfb44df9fae (patch) | |
tree | af2b96f7b7871dd3d07169642b6234369c032632 /modules | |
parent | 6a0ef71984b9246fc1bb378caaa614f3dedaa9e9 (diff) | |
download | gitea-5727056ea109eb04ee535b981349cdfb44df9fae.tar.gz gitea-5727056ea109eb04ee535b981349cdfb44df9fae.zip |
Make minio package support legacy MD5 checksum (#23768)
A feedback from discord:
https://discord.com/channels/322538954119184384/561007778139734027/1090185427115319386
Some storages like:
* https://developers.cloudflare.com/r2/api/s3/api/
* https://www.backblaze.com/b2/docs/s3_compatible_api.html
They do not support "x-amz-checksum-algorithm" header
But minio recently uses that header with CRC32C by default. So we have
to tell minio to use legacy MD5 checksum.
I guess this needs to be backported because IIRC we 1.19 and 1.20 are
using similar minio package.
The minio package code for SendContentMD5 looks like this:
<details>
<img width="755" alt="image"
src="https://user-images.githubusercontent.com/2114189/228186768-4f2f6f67-62b9-4aee-9251-5af714ad9674.png">
</details>
Diffstat (limited to 'modules')
-rw-r--r-- | modules/lfs/content_store.go | 40 | ||||
-rw-r--r-- | modules/setting/storage.go | 1 | ||||
-rw-r--r-- | modules/storage/minio.go | 21 |
3 files changed, 48 insertions, 14 deletions
diff --git a/modules/lfs/content_store.go b/modules/lfs/content_store.go index a4ae21bfd6..53fac4ab85 100644 --- a/modules/lfs/content_store.go +++ b/modules/lfs/content_store.go @@ -60,15 +60,22 @@ func (s *ContentStore) Put(pointer Pointer, r io.Reader) error { return err } - // This shouldn't happen but it is sensible to test - if written != pointer.Size { - if err := s.Delete(p); err != nil { - log.Error("Cleaning the LFS OID[%s] failed: %v", pointer.Oid, err) + // check again whether there is any error during the Save operation + // because some errors might be ignored by the Reader's caller + if wrappedRd.lastError != nil && !errors.Is(wrappedRd.lastError, io.EOF) { + err = wrappedRd.lastError + } else if written != pointer.Size { + err = ErrSizeMismatch + } + + // if the upload failed, try to delete the file + if err != nil { + if errDel := s.Delete(p); errDel != nil { + log.Error("Cleaning the LFS OID[%s] failed: %v", pointer.Oid, errDel) } - return ErrSizeMismatch } - return nil + return err } // Exists returns true if the object exists in the content store. @@ -109,6 +116,17 @@ type hashingReader struct { expectedSize int64 hash hash.Hash expectedHash string + lastError error +} + +// recordError records the last error during the Save operation +// Some callers of the Reader doesn't respect the returned "err" +// For example, MinIO's Put will ignore errors if the written size could equal to expected size +// So we must remember the error by ourselves, +// and later check again whether ErrSizeMismatch or ErrHashMismatch occurs during the Save operation +func (r *hashingReader) recordError(err error) error { + r.lastError = err + return err } func (r *hashingReader) Read(b []byte) (int, error) { @@ -118,22 +136,22 @@ func (r *hashingReader) Read(b []byte) (int, error) { r.currentSize += int64(n) wn, werr := r.hash.Write(b[:n]) if wn != n || werr != nil { - return n, werr + return n, r.recordError(werr) } } - if err != nil && err == io.EOF { + if errors.Is(err, io.EOF) || r.currentSize >= r.expectedSize { if r.currentSize != r.expectedSize { - return n, ErrSizeMismatch + return n, r.recordError(ErrSizeMismatch) } shaStr := hex.EncodeToString(r.hash.Sum(nil)) if shaStr != r.expectedHash { - return n, ErrHashMismatch + return n, r.recordError(ErrHashMismatch) } } - return n, err + return n, r.recordError(err) } func newHashingReader(expectedSize int64, expectedHash string, reader io.Reader) *hashingReader { diff --git a/modules/setting/storage.go b/modules/setting/storage.go index 4d401614e4..50d4c8439e 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -42,6 +42,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section sec.Key("MINIO_LOCATION").MustString("us-east-1") sec.Key("MINIO_USE_SSL").MustBool(false) sec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false) + sec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default") if targetSec == nil { targetSec, _ = rootCfg.NewSection(name) diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 5c67dbf26a..250f17827f 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -6,6 +6,7 @@ package storage import ( "context" "crypto/tls" + "fmt" "io" "net/http" "net/url" @@ -53,10 +54,12 @@ type MinioStorageConfig struct { BasePath string `ini:"MINIO_BASE_PATH"` UseSSL bool `ini:"MINIO_USE_SSL"` InsecureSkipVerify bool `ini:"MINIO_INSECURE_SKIP_VERIFY"` + ChecksumAlgorithm string `ini:"MINIO_CHECKSUM_ALGORITHM"` } // MinioStorage returns a minio bucket storage type MinioStorage struct { + cfg *MinioStorageConfig ctx context.Context client *minio.Client bucket string @@ -91,6 +94,10 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error } config := configInterface.(MinioStorageConfig) + if config.ChecksumAlgorithm != "" && config.ChecksumAlgorithm != "default" && config.ChecksumAlgorithm != "md5" { + return nil, fmt.Errorf("invalid minio checksum algorithm: %s", config.ChecksumAlgorithm) + } + log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath) minioClient, err := minio.New(config.Endpoint, &minio.Options{ @@ -113,6 +120,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error } return &MinioStorage{ + cfg: &config, ctx: ctx, client: minioClient, bucket: config.Bucket, @@ -124,7 +132,7 @@ func (m *MinioStorage) buildMinioPath(p string) string { return util.PathJoinRelX(m.basePath, p) } -// Open open a file +// Open opens a file func (m *MinioStorage) Open(path string) (Object, error) { opts := minio.GetObjectOptions{} object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts) @@ -134,7 +142,7 @@ func (m *MinioStorage) Open(path string) (Object, error) { return &minioObject{object}, nil } -// Save save a file to minio +// Save saves a file to minio func (m *MinioStorage) Save(path string, r io.Reader, size int64) (int64, error) { uploadInfo, err := m.client.PutObject( m.ctx, @@ -142,7 +150,14 @@ func (m *MinioStorage) Save(path string, r io.Reader, size int64) (int64, error) m.buildMinioPath(path), r, size, - minio.PutObjectOptions{ContentType: "application/octet-stream"}, + minio.PutObjectOptions{ + ContentType: "application/octet-stream", + // some storages like: + // * https://developers.cloudflare.com/r2/api/s3/api/ + // * https://www.backblaze.com/b2/docs/s3_compatible_api.html + // do not support "x-amz-checksum-algorithm" header, so use legacy MD5 checksum + SendContentMd5: m.cfg.ChecksumAlgorithm == "md5", + }, ) if err != nil { return 0, convertMinioErr(err) |