Skip to content

Commit b73d1ac

Browse files
authored
Make minio package support legacy MD5 checksum (#23768) (#23770)
Backport #23768 (no source code conflict, only some unrelated docs/test-ini conflicts) 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.
1 parent 428d26d commit b73d1ac

File tree

7 files changed

+72
-22
lines changed

7 files changed

+72
-22
lines changed

cmd/migrate_storage.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,21 @@ var CmdMigrateStorage = cli.Command{
7272
cli.StringFlag{
7373
Name: "minio-base-path",
7474
Value: "",
75-
Usage: "Minio storage basepath on the bucket",
75+
Usage: "Minio storage base path on the bucket",
7676
},
7777
cli.BoolFlag{
7878
Name: "minio-use-ssl",
7979
Usage: "Enable SSL for minio",
8080
},
81+
cli.BoolFlag{
82+
Name: "minio-insecure-skip-verify",
83+
Usage: "Skip SSL verification",
84+
},
85+
cli.StringFlag{
86+
Name: "minio-checksum-algorithm",
87+
Value: "",
88+
Usage: "Minio checksum algorithm (default/md5)",
89+
},
8190
},
8291
}
8392

@@ -168,13 +177,15 @@ func runMigrateStorage(ctx *cli.Context) error {
168177
dstStorage, err = storage.NewMinioStorage(
169178
stdCtx,
170179
storage.MinioStorageConfig{
171-
Endpoint: ctx.String("minio-endpoint"),
172-
AccessKeyID: ctx.String("minio-access-key-id"),
173-
SecretAccessKey: ctx.String("minio-secret-access-key"),
174-
Bucket: ctx.String("minio-bucket"),
175-
Location: ctx.String("minio-location"),
176-
BasePath: ctx.String("minio-base-path"),
177-
UseSSL: ctx.Bool("minio-use-ssl"),
180+
Endpoint: ctx.String("minio-endpoint"),
181+
AccessKeyID: ctx.String("minio-access-key-id"),
182+
SecretAccessKey: ctx.String("minio-secret-access-key"),
183+
Bucket: ctx.String("minio-bucket"),
184+
Location: ctx.String("minio-location"),
185+
BasePath: ctx.String("minio-base-path"),
186+
UseSSL: ctx.Bool("minio-use-ssl"),
187+
InsecureSkipVerify: ctx.Bool("minio-insecure-skip-verify"),
188+
ChecksumAlgorithm: ctx.String("minio-checksum-algorithm"),
178189
})
179190
default:
180191
return fmt.Errorf("unsupported storage type: %s", ctx.String("storage"))

custom/conf/app.example.ini

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,6 +1874,9 @@ ROUTER = console
18741874
;;
18751875
;; Minio skip SSL verification available when STORAGE_TYPE is `minio`
18761876
;MINIO_INSECURE_SKIP_VERIFY = false
1877+
;;
1878+
;; Minio checksum algorithm: default (for MinIO or AWS S3) or md5 (for Cloudflare or Backblaze)
1879+
;MINIO_CHECKSUM_ALGORITHM = default
18771880

18781881
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
18791882
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

docs/content/doc/administration/config-cheat-sheet.en-us.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,7 @@ Default templates for project boards:
855855
- `MINIO_BASE_PATH`: **attachments/**: Minio base path on the bucket only available when STORAGE_TYPE is `minio`
856856
- `MINIO_USE_SSL`: **false**: Minio enabled ssl only available when STORAGE_TYPE is `minio`
857857
- `MINIO_INSECURE_SKIP_VERIFY`: **false**: Minio skip SSL verification available when STORAGE_TYPE is `minio`
858+
- `MINIO_CHECKSUM_ALGORITHM`: **default**: Minio checksum algorithm: `default` (for MinIO or AWS S3) or `md5` (for Cloudflare or Backblaze)
858859

859860
## Log (`log`)
860861

modules/lfs/content_store.go

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,22 @@ func (s *ContentStore) Put(pointer Pointer, r io.Reader) error {
5959
return err
6060
}
6161

62-
// This shouldn't happen but it is sensible to test
63-
if written != pointer.Size {
64-
if err := s.Delete(p); err != nil {
65-
log.Error("Cleaning the LFS OID[%s] failed: %v", pointer.Oid, err)
62+
// check again whether there is any error during the Save operation
63+
// because some errors might be ignored by the Reader's caller
64+
if wrappedRd.lastError != nil && !errors.Is(wrappedRd.lastError, io.EOF) {
65+
err = wrappedRd.lastError
66+
} else if written != pointer.Size {
67+
err = ErrSizeMismatch
68+
}
69+
70+
// if the upload failed, try to delete the file
71+
if err != nil {
72+
if errDel := s.Delete(p); errDel != nil {
73+
log.Error("Cleaning the LFS OID[%s] failed: %v", pointer.Oid, errDel)
6674
}
67-
return ErrSizeMismatch
6875
}
6976

70-
return nil
77+
return err
7178
}
7279

7380
// Exists returns true if the object exists in the content store.
@@ -108,6 +115,17 @@ type hashingReader struct {
108115
expectedSize int64
109116
hash hash.Hash
110117
expectedHash string
118+
lastError error
119+
}
120+
121+
// recordError records the last error during the Save operation
122+
// Some callers of the Reader doesn't respect the returned "err"
123+
// For example, MinIO's Put will ignore errors if the written size could equal to expected size
124+
// So we must remember the error by ourselves,
125+
// and later check again whether ErrSizeMismatch or ErrHashMismatch occurs during the Save operation
126+
func (r *hashingReader) recordError(err error) error {
127+
r.lastError = err
128+
return err
111129
}
112130

113131
func (r *hashingReader) Read(b []byte) (int, error) {
@@ -117,22 +135,22 @@ func (r *hashingReader) Read(b []byte) (int, error) {
117135
r.currentSize += int64(n)
118136
wn, werr := r.hash.Write(b[:n])
119137
if wn != n || werr != nil {
120-
return n, werr
138+
return n, r.recordError(werr)
121139
}
122140
}
123141

124-
if err != nil && err == io.EOF {
142+
if errors.Is(err, io.EOF) || r.currentSize >= r.expectedSize {
125143
if r.currentSize != r.expectedSize {
126-
return n, ErrSizeMismatch
144+
return n, r.recordError(ErrSizeMismatch)
127145
}
128146

129147
shaStr := hex.EncodeToString(r.hash.Sum(nil))
130148
if shaStr != r.expectedHash {
131-
return n, ErrHashMismatch
149+
return n, r.recordError(ErrHashMismatch)
132150
}
133151
}
134152

135-
return n, err
153+
return n, r.recordError(err)
136154
}
137155

138156
func newHashingReader(expectedSize int64, expectedHash string, reader io.Reader) *hashingReader {

modules/setting/storage.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func getStorage(rootCfg ConfigProvider, name, typ string, targetSec *ini.Section
4242
sec.Key("MINIO_LOCATION").MustString("us-east-1")
4343
sec.Key("MINIO_USE_SSL").MustBool(false)
4444
sec.Key("MINIO_INSECURE_SKIP_VERIFY").MustBool(false)
45+
sec.Key("MINIO_CHECKSUM_ALGORITHM").MustString("default")
4546

4647
if targetSec == nil {
4748
targetSec, _ = rootCfg.NewSection(name)

modules/storage/minio.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package storage
66
import (
77
"context"
88
"crypto/tls"
9+
"fmt"
910
"io"
1011
"net/http"
1112
"net/url"
@@ -52,10 +53,12 @@ type MinioStorageConfig struct {
5253
BasePath string `ini:"MINIO_BASE_PATH"`
5354
UseSSL bool `ini:"MINIO_USE_SSL"`
5455
InsecureSkipVerify bool `ini:"MINIO_INSECURE_SKIP_VERIFY"`
56+
ChecksumAlgorithm string `ini:"MINIO_CHECKSUM_ALGORITHM"`
5557
}
5658

5759
// MinioStorage returns a minio bucket storage
5860
type MinioStorage struct {
61+
cfg *MinioStorageConfig
5962
ctx context.Context
6063
client *minio.Client
6164
bucket string
@@ -90,6 +93,10 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
9093
}
9194
config := configInterface.(MinioStorageConfig)
9295

96+
if config.ChecksumAlgorithm != "" && config.ChecksumAlgorithm != "default" && config.ChecksumAlgorithm != "md5" {
97+
return nil, fmt.Errorf("invalid minio checksum algorithm: %s", config.ChecksumAlgorithm)
98+
}
99+
93100
log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath)
94101

95102
minioClient, err := minio.New(config.Endpoint, &minio.Options{
@@ -112,6 +119,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
112119
}
113120

114121
return &MinioStorage{
122+
cfg: &config,
115123
ctx: ctx,
116124
client: minioClient,
117125
bucket: config.Bucket,
@@ -123,7 +131,7 @@ func (m *MinioStorage) buildMinioPath(p string) string {
123131
return strings.TrimPrefix(path.Join(m.basePath, path.Clean("/" + strings.ReplaceAll(p, "\\", "/"))[1:]), "/")
124132
}
125133

126-
// Open open a file
134+
// Open opens a file
127135
func (m *MinioStorage) Open(path string) (Object, error) {
128136
opts := minio.GetObjectOptions{}
129137
object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts)
@@ -133,15 +141,22 @@ func (m *MinioStorage) Open(path string) (Object, error) {
133141
return &minioObject{object}, nil
134142
}
135143

136-
// Save save a file to minio
144+
// Save saves a file to minio
137145
func (m *MinioStorage) Save(path string, r io.Reader, size int64) (int64, error) {
138146
uploadInfo, err := m.client.PutObject(
139147
m.ctx,
140148
m.bucket,
141149
m.buildMinioPath(path),
142150
r,
143151
size,
144-
minio.PutObjectOptions{ContentType: "application/octet-stream"},
152+
minio.PutObjectOptions{
153+
ContentType: "application/octet-stream",
154+
// some storages like:
155+
// * https://developers.cloudflare.com/r2/api/s3/api/
156+
// * https://www.backblaze.com/b2/docs/s3_compatible_api.html
157+
// do not support "x-amz-checksum-algorithm" header, so use legacy MD5 checksum
158+
SendContentMd5: m.cfg.ChecksumAlgorithm == "md5",
159+
},
145160
)
146161
if err != nil {
147162
return 0, convertMinioErr(err)

tests/mysql.ini.tmpl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ MINIO_SECRET_ACCESS_KEY = 12345678
7676
MINIO_BUCKET = gitea
7777
MINIO_LOCATION = us-east-1
7878
MINIO_USE_SSL = false
79+
MINIO_CHECKSUM_ALGORITHM = md5
7980

8081
[mailer]
8182
ENABLED = true

0 commit comments

Comments
 (0)