Merge pull request '[GITEA] S3: log human readable error on connection failure' (#1365) from earl-warren/forgejo:wip-s3-400 into forgejo-dependency

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1365

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,19 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage,
 		return nil, convertMinioErr(err)
 	}
 
+	// Check if the connection works
+	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 {

modules/storage/minio_test.go

@@ -4,10 +4,17 @@
 package storage
 
 import (
+	"context"
+	"net/http"
 	"os"
 	"testing"
+	"time"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+
+	"github.com/minio/minio-go/v7"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestMinioStorageIterator(t *testing.T) {
@@ -25,3 +32,35 @@ func TestMinioStorageIterator(t *testing.T) {
 		},
 	})
 }
+
+func TestS3StorageBadRequest(t *testing.T) {
+	if os.Getenv("CI") == "" {
+		t.Skip("S3Storage not present outside of CI")
+		return
+	}
+	lc, cleanup := test.NewLogChecker("bad-request")
+	lc.StopMark("S3 storage connection failure")
+	defer cleanup()
+	cfg := &setting.Storage{
+		MinioConfig: setting.MinioStorageConfig{
+			Endpoint:        "minio:9000",
+			AccessKeyID:     "123456",
+			SecretAccessKey: "12345678",
+			Bucket:          "bucket",
+			Location:        "us-east-1",
+		},
+	}
+	message := "ERROR"
+	defer test.MockVariableValue(&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)
+
+	_, stopped := lc.Check(100 * time.Millisecond)
+	assert.False(t, stopped)
+}