Repository: matrixorigin/matrixone
Pull request #14808 (2024-03-06 05:10:54)

fileservice: add ObjectStorageArguments.NoBucketValidation (#14510)




## What type of PR is this?

- [ ] API-change
- [ ] BUG
- [x] Improvement
- [ ] Documentation
- [ ] Feature
- [ ] Test and CI
- [ ] Code Refactoring

## Which issue(s) this PR fixes:

issue #14113

## What this PR does / why we need it:
add more control to s3


Patch:
diff --git a/pkg/fileservice/aliyun_sdk.go b/pkg/fileservice/aliyun_sdk.go
index ecb140f59..26e9a605c 100644
--- a/pkg/fileservice/aliyun_sdk.go
+++ b/pkg/fileservice/aliyun_sdk.go
@@ -87,26 +87,24 @@ func NewAliyunSDK(
 		zap.Any("arguments", args),
 	)
 
-	res, err := client.ListBuckets()
-	if err != nil {
-		return nil, err
+	if !args.NoBucketValidation {
+		// validate bucket
+		_, err := client.GetBucketInfo(args.Bucket)
+		if err != nil {
+			return nil, err
+		}
 	}
-	for _, info := range res.Buckets {
-		if info.Name == args.Bucket {
-			bucket, err := client.Bucket(args.Bucket)
-			if err != nil {
-				return nil, err
-			}
 
-			return &AliyunSDK{
-				name:            args.Name,
-				bucket:          bucket,
-				perfCounterSets: perfCounterSets,
-			}, nil
-		}
+	bucket, err := client.Bucket(args.Bucket)
+	if err != nil {
+		return nil, err
 	}
 
-	return nil, moerr.NewInvalidArgNoCtx("Bucket", args.Bucket)
+	return &AliyunSDK{
+		name:            args.Name,
+		bucket:          bucket,
+		perfCounterSets: perfCounterSets,
+	}, nil
 }
 
 var _ ObjectStorage = new(AliyunSDK)
diff --git a/pkg/fileservice/aws_sdk_v1.go b/pkg/fileservice/aws_sdk_v1.go
index 44445b00c..d1677a858 100644
--- a/pkg/fileservice/aws_sdk_v1.go
+++ b/pkg/fileservice/aws_sdk_v1.go
@@ -141,12 +141,14 @@ func NewAwsSDKv1(
 		zap.Any("arguments", args),
 	)
 
-	// head bucket to validate
-	_, err = client.HeadBucket(&s3.HeadBucketInput{
-		Bucket: ptrTo(args.Bucket),
-	})
-	if err != nil {
-		return nil, moerr.NewInternalErrorNoCtx("bad s3 config: %v", err)
+	if !args.NoBucketValidation {
+		// head bucket to validate
+		_, err = client.HeadBucket(&s3.HeadBucketInput{
+			Bucket: ptrTo(args.Bucket),
+		})
+		if err != nil {
+			return nil, moerr.NewInternalErrorNoCtx("bad s3 config: %v", err)
+		}
 	}
 
 	return &AwsSDKv1{
diff --git a/pkg/fileservice/aws_sdk_v2.go b/pkg/fileservice/aws_sdk_v2.go
index 7adbace74..f1ed07139 100644
--- a/pkg/fileservice/aws_sdk_v2.go
+++ b/pkg/fileservice/aws_sdk_v2.go
@@ -231,12 +231,14 @@ func NewAwsSDKv2(
 		zap.Any("arguments", args),
 	)
 
-	// head bucket to validate
-	_, err = client.HeadBucket(ctx, &s3.HeadBucketInput{
-		Bucket: ptrTo(args.Bucket),
-	})
-	if err != nil {
-		return nil, moerr.NewInternalErrorNoCtx("bad s3 config: %v", err)
+	if !args.NoBucketValidation {
+		// head bucket to validate
+		_, err = client.HeadBucket(ctx, &s3.HeadBucketInput{
+			Bucket: ptrTo(args.Bucket),
+		})
+		if err != nil {
+			return nil, moerr.NewInternalErrorNoCtx("bad s3 config: %v", err)
+		}
 	}
 
 	return &AwsSDKv2{
diff --git a/pkg/fileservice/get.go b/pkg/fileservice/get.go
index 250fbb7e9..51cf3d384 100644
--- a/pkg/fileservice/get.go
+++ b/pkg/fileservice/get.go
@@ -102,13 +102,14 @@ func GetForETL(ctx context.Context, fs FileService, path string) (res ETLFileSer
 			res, err = NewS3FS(
 				ctx,
 				ObjectStorageArguments{
-					Endpoint:  endpoint,
-					Region:    region,
-					Bucket:    bucket,
-					KeyID:     accessKey,
-					KeySecret: accessSecret,
-					KeyPrefix: keyPrefix,
-					Name:      name,
+					NoBucketValidation: true,
+					Endpoint:           endpoint,
+					Region:             region,
+					Bucket:             bucket,
+					KeyID:              accessKey,
+					KeySecret:          accessSecret,
+					KeyPrefix:          keyPrefix,
+					Name:               name,
 				},
 				DisabledCacheConfig,
 				nil,
@@ -136,11 +137,12 @@ func GetForETL(ctx context.Context, fs FileService, path string) (res ETLFileSer
 			res, err = NewS3FS(
 				ctx,
 				ObjectStorageArguments{
-					Endpoint:  endpoint,
-					Region:    region,
-					Bucket:    bucket,
-					KeyPrefix: keyPrefix,
-					Name:      name,
+					NoBucketValidation: true,
+					Endpoint:           endpoint,
+					Region:             region,
+					Bucket:             bucket,
+					KeyPrefix:          keyPrefix,
+					Name:               name,
 				},
 				DisabledCacheConfig,
 				nil,
@@ -153,6 +155,7 @@ func GetForETL(ctx context.Context, fs FileService, path string) (res ETLFileSer
 			if err := args.SetFromString(fsPath.ServiceArguments); err != nil {
 				return nil, "", err
 			}
+			args.NoBucketValidation = true
 			res, err = NewS3FS(
 				ctx,
 				args,
@@ -185,14 +188,15 @@ func GetForETL(ctx context.Context, fs FileService, path string) (res ETLFileSer
 			res, err = NewS3FS(
 				ctx,
 				ObjectStorageArguments{
-					Endpoint:  endpoint,
-					Region:    region,
-					Bucket:    bucket,
-					KeyID:     accessKey,
-					KeySecret: accessSecret,
-					KeyPrefix: keyPrefix,
-					Name:      name,
-					IsMinio:   true,
+					NoBucketValidation: true,
+					Endpoint:           endpoint,
+					Region:             region,
+					Bucket:             bucket,
+					KeyID:              accessKey,
+					KeySecret:          accessSecret,
+					KeyPrefix:          keyPrefix,
+					Name:               name,
+					IsMinio:            true,
 				},
 				DisabledCacheConfig,
 				nil,
@@ -248,6 +252,7 @@ func GetForBackup(ctx context.Context, spec string) (res FileService, err error)
 			if err := args.SetFromString(fsPath.ServiceArguments); err != nil {
 				return nil, err
 			}
+			args.NoBucketValidation = true
 			res, err = NewS3FS(
 				ctx,
 				args,
diff --git a/pkg/fileservice/minio_sdk.go b/pkg/fileservice/minio_sdk.go
index 182ff04b4..f85da25a0 100644
--- a/pkg/fileservice/minio_sdk.go
+++ b/pkg/fileservice/minio_sdk.go
@@ -187,16 +187,18 @@ func NewMinioSDK(
 		zap.Any("arguments", args),
 	)
 
-	// validate
-	ok, err := client.BucketExists(ctx, args.Bucket)
-	if err != nil {
-		return nil, err
-	}
-	if !ok {
-		return nil, moerr.NewInternalErrorNoCtx(
-			"bad s3 config, no such bucket or no permissions: %v",
-			args.Bucket,
-		)
+	if !args.NoBucketValidation {
+		// validate
+		ok, err := client.BucketExists(ctx, args.Bucket)
+		if err != nil {
+			return nil, err
+		}
+		if !ok {
+			return nil, moerr.NewInternalErrorNoCtx(
+				"bad s3 config, no such bucket or no permissions: %v",
+				args.Bucket,
+			)
+		}
 	}
 
 	return &MinioSDK{
diff --git a/pkg/fileservice/object_storage_arguments.go b/pkg/fileservice/object_storage_arguments.go
index 121fe530c..ccafe0de9 100644
--- a/pkg/fileservice/object_storage_arguments.go
+++ b/pkg/fileservice/object_storage_arguments.go
@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 
 	"github.com/matrixorigin/matrixone/pkg/common/moerr"
@@ -29,6 +30,7 @@ type ObjectStorageArguments struct {
 	KeyPrefix            string `toml:"key-prefix"`
 	SharedConfigProfile  string `toml:"shared-config-profile"`
 	NoDefaultCredentials bool   `toml:"no-default-credentials"`
+	NoBucketValidation   bool   `toml:"no-bucket-validation"`
 
 	// s3
 	Bucket    string   `toml:"bucket"`
@@ -66,40 +68,52 @@ func (o *ObjectStorageArguments) SetFromString(arguments []string) error {
 
 		switch strings.ToLower(key) {
 
-		case "bearer-token":
-			o.BearerToken = value
+		case "name":
+			o.Name = value
+		case "prefix", "key-prefix":
+			o.KeyPrefix = value
+		case "shared-config-profile":
+			o.SharedConfigProfile = value
+		case "no-bucket-validation":
+			b, err := strconv.ParseBool(value)
+			if err == nil {
+				o.NoBucketValidation = b
+			}
+		case "no-default-credentials":
+			b, err := strconv.ParseBool(value)
+			if err == nil {
+				o.NoDefaultCredentials = b
+			}
+
 		case "bucket":
 			o.Bucket = value
 		case "endpoint":
 			o.Endpoint = value
-		case "external-id":
-			o.ExternalID = value
 		case "is-minio", "minio":
 			o.IsMinio = value != "false" && value != "0"
-		case "key", "key-id":
-			o.KeyID = value
-		case "name":
-			o.Name = value
-		case "prefix", "key-prefix":
-			o.KeyPrefix = value
-		case "ram-role":
-			o.RAMRole = value
 		case "region":
 			o.Region = value
+		case "cert-files":
+			o.CertFiles = strings.Split(value, ",")
+
 		case "role-arn":
 			o.RoleARN = value
-		case "role-session-name":
-			o.RoleSessionName = value
+		case "bearer-token":
+			o.BearerToken = value
+		case "external-id":
+			o.ExternalID = value
+		case "key", "key-id":
+			o.KeyID = value
 		case "secret", "key-secret", "secret-id":
 			o.KeySecret = value
+		case "ram-role":
+			o.RAMRole = value
+		case "role-session-name":
+			o.RoleSessionName = value
 		case "security-token":
 			o.SecurityToken = value
-		case "shared-config-profile":
-			o.SharedConfigProfile = value
 		case "token", "session-token":
 			o.SessionToken = value
-		case "cert-files":
-			o.CertFiles = strings.Split(value, ",")
 
 		default:
 			return moerr.NewInvalidInputNoCtx("invalid S3 argument: %s", pair)


Test patch:
diff --git a/pkg/fileservice/s3_fs_test.go b/pkg/fileservice/s3_fs_test.go
index 86a0d2f2f..19d2cac77 100644
--- a/pkg/fileservice/s3_fs_test.go
+++ b/pkg/fileservice/s3_fs_test.go
@@ -1019,6 +1019,25 @@ func TestNewS3FSFromSpec(t *testing.T) {
 			_ = fs
 
 		})
+
+		t.Run(kase.Name+" bad bucket", func(t *testing.T) {
+
+			args := kase.ObjectStorageArguments
+			args.Bucket = args.Bucket + "foobarbaz"
+			ctx := context.Background()
+			_, err := NewS3FS(
+				ctx,
+				args,
+				DisabledCacheConfig,
+				nil,
+				true,
+				false,
+			)
+			if err == nil {
+				t.Fatal("should fail")
+			}
+
+		})
 	}
 
 }