diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 3246993bb1..714530ab50 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -71,6 +71,11 @@ func convertMinioErr(err error) error { return err } +var getBucketVersioning = func(ctx context.Context, minioClient *minio.Client, bucket string) error { + _, err := minioClient.GetBucketVersioning(ctx, bucket) + return err +} + // NewMinioStorage returns a minio storage func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, error) { config := cfg.MinioConfig @@ -90,6 +95,23 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, return nil, convertMinioErr(err) } + // The GetBucketVersioning is only used for checking whether the Object Storage parameters are generally good. It doesn't need to succeed. + // The assumption is that if the API returns the HTTP code 400, then the parameters could be incorrect. + // Otherwise even if the request itself fails (403, 404, etc), the code should still continue because the parameters seem "good" enough. + // Keep in mind that GetBucketVersioning requires "owner" to really succeed, so it can't be used to check the existence. + // Not using "BucketExists (HeadBucket)" because it doesn't include detailed failure reasons. + err = getBucketVersioning(ctx, minioClient, config.Bucket) + if err != nil { + errResp, ok := err.(minio.ErrorResponse) + if !ok { + return nil, err + } + if errResp.StatusCode == http.StatusBadRequest { + log.Error("S3 storage connection failure at %s:%s with base path %s and region: %s", config.Endpoint, config.Bucket, config.Location, errResp.Message) + return nil, err + } + } + // Check to see if we already own this bucket exists, err := minioClient.BucketExists(ctx, config.Bucket) if err != nil { diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index af392b7e22..56dfd9100a 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -4,10 +4,15 @@ package storage import ( + "context" + "net/http" "os" "testing" "code.gitea.io/gitea/modules/setting" + + "github.com/minio/minio-go/v7" + "github.com/stretchr/testify/assert" ) func TestMinioStorageIterator(t *testing.T) { @@ -25,3 +30,31 @@ func TestMinioStorageIterator(t *testing.T) { }, }) } + +func TestS3StorageBadRequest(t *testing.T) { + if os.Getenv("CI") == "" { + t.Skip("S3Storage not present outside of CI") + return + } + cfg := &setting.Storage{ + MinioConfig: setting.MinioStorageConfig{ + Endpoint: "minio:9000", + AccessKeyID: "123456", + SecretAccessKey: "12345678", + Bucket: "bucket", + Location: "us-east-1", + }, + } + message := "ERROR" + old := getBucketVersioning + defer func() { getBucketVersioning = old }() + getBucketVersioning = func(ctx context.Context, minioClient *minio.Client, bucket string) error { + return minio.ErrorResponse{ + StatusCode: http.StatusBadRequest, + Code: "FixtureError", + Message: message, + } + } + _, err := NewStorage(setting.MinioStorageType, cfg) + assert.ErrorContains(t, err, message) +}