diff --git a/modules/storage/minio.go b/modules/storage/minio.go index bf51a1642a..8d4f9d6627 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -76,8 +76,13 @@ var getBucketVersioning = func(ctx context.Context, minioClient *minio.Client, b return err } +var initializationTimeout = 30 * time.Second + // NewMinioStorage returns a minio storage func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, error) { + initCtx, cancel := context.WithTimeout(ctx, initializationTimeout) + defer cancel() + config := cfg.MinioConfig if config.ChecksumAlgorithm != "" && config.ChecksumAlgorithm != "default" && config.ChecksumAlgorithm != "md5" { return nil, fmt.Errorf("invalid minio checksum algorithm: %s", config.ChecksumAlgorithm) @@ -112,7 +117,7 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, // 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) + err = getBucketVersioning(initCtx, minioClient, config.Bucket) if err != nil { errResp, ok := err.(minio.ErrorResponse) if !ok { @@ -125,13 +130,13 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, } // Check to see if we already own this bucket - exists, err := minioClient.BucketExists(ctx, config.Bucket) + exists, err := minioClient.BucketExists(initCtx, config.Bucket) if err != nil { return nil, convertMinioErr(err) } if !exists { - if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{ + if err := minioClient.MakeBucket(initCtx, config.Bucket, minio.MakeBucketOptions{ Region: config.Location, }); err != nil { return nil, convertMinioErr(err) diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index ec1b2fc77a..18fe91edfb 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -9,8 +9,10 @@ import ( "net/http/httptest" "os" "testing" + "time" "forgejo.org/modules/setting" + "forgejo.org/modules/test" "github.com/minio/minio-go/v7" "github.com/stretchr/testify/assert" @@ -217,3 +219,41 @@ func TestMinioCredentials(t *testing.T) { }) }) } + +func TestNewMinioStorageInitializationTimeout(t *testing.T) { + defer test.MockVariableValue(&getBucketVersioning, func(ctx context.Context, minioClient *minio.Client, bucket string) error { + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(1 * time.Millisecond): + return minio.ErrorResponse{ + StatusCode: http.StatusBadRequest, + Code: "TestError", + Message: "Mocked error for testing", + } + } + })() + + settings := &setting.Storage{ + MinioConfig: setting.MinioStorageConfig{ + Endpoint: "localhost", + AccessKeyID: "123456", + SecretAccessKey: "12345678", + Bucket: "bucket", + Location: "us-east-1", + }, + } + + // Verify that we reach `getBucketVersioning` and return the error from our mock. + storage, err := NewMinioStorage(t.Context(), settings) + require.ErrorContains(t, err, "Mocked error for testing") + assert.Nil(t, storage) + + defer test.MockVariableValue(&initializationTimeout, 1*time.Nanosecond)() + + // Now that the timeout is super low, verify that we get a context deadline exceeded error from our mock. + storage, err = NewMinioStorage(t.Context(), settings) + require.Error(t, err) + require.ErrorIs(t, err, context.DeadlineExceeded, "err must be a context deadline exceeded error, but was %v", err) + assert.Nil(t, storage) +}