fileservice: in minio and qcloud sdk, do not load credentials from en…

…v vars if provided in arguments (#20564) do not load credentials from env vars if provided in arguments. Approved by: @fengttt
matrixorigin · Dec 10, 2024 · f1acf1d · f1acf1d
1 parent 14a354c
commit f1acf1d
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 56 deletions.
diff --git a/pkg/fileservice/minio_sdk.go b/pkg/fileservice/minio_sdk.go
@@ -21,6 +21,7 @@ import (
 	"iter"
 	"net/http"
 	"net/url"
+	"os"
 	gotrace "runtime/trace"
 	"strings"
 	"sync/atomic"
@@ -56,49 +57,69 @@ func NewMinioSDK(
 
 	options := new(minio.Options)
 
-	// credentials
-	var credentialProviders []credentials.Provider
+	// credential arguments
+	keyID := args.KeyID
+	keySecret := args.KeySecret
+	sessionToken := args.SessionToken
 	if args.shouldLoadDefaultCredentials() {
-		credentialProviders = append(credentialProviders,
-			// aws env
-			new(credentials.EnvAWS),
-			// minio env
-			new(credentials.EnvMinio),
+		keyID = firstNonZero(
+			args.KeyID,
+			os.Getenv("AWS_ACCESS_KEY_ID"),
+			os.Getenv("AWS_ACCESS_KEY"),
+			os.Getenv("MINIO_ROOT_USER"),
+			os.Getenv("MINIO_ACCESS_KEY"),
+		)
+		keySecret = firstNonZero(
+			args.KeySecret,
+			os.Getenv("AWS_SECRET_ACCESS_KEY"),
+			os.Getenv("AWS_SECRET_KEY"),
+			os.Getenv("MINIO_ROOT_PASSWORD"),
+			os.Getenv("MINIO_SECRET_KEY"),
+		)
+		sessionToken = firstNonZero(
+			args.SessionToken,
+			os.Getenv("AWS_SESSION_TOKEN"),
 		)
 	}
-	if args.KeyID != "" && args.KeySecret != "" {
+
+	// credentials providers
+	var credentialProviders []credentials.Provider
+
+	if keyID != "" && keySecret != "" {
 		// static
 		credentialProviders = append(credentialProviders, &credentials.Static{
 			Value: credentials.Value{
-				AccessKeyID:     args.KeyID,
-				SecretAccessKey: args.KeySecret,
-				SessionToken:    args.SessionToken,
+				AccessKeyID:     keyID,
+				SecretAccessKey: keySecret,
+				SessionToken:    sessionToken,
 				SignerType:      credentials.SignatureV2,
 			},
 		})
 		credentialProviders = append(credentialProviders, &credentials.Static{
 			Value: credentials.Value{
-				AccessKeyID:     args.KeyID,
-				SecretAccessKey: args.KeySecret,
-				SessionToken:    args.SessionToken,
+				AccessKeyID:     keyID,
+				SecretAccessKey: keySecret,
+				SessionToken:    sessionToken,
 				SignerType:      credentials.SignatureV4,
 			},
 		})
 		credentialProviders = append(credentialProviders, &credentials.Static{
 			Value: credentials.Value{
-				AccessKeyID:     args.KeyID,
-				SecretAccessKey: args.KeySecret,
-				SessionToken:    args.SessionToken,
+				AccessKeyID:     keyID,
+				SecretAccessKey: keySecret,
+				SessionToken:    sessionToken,
 				SignerType:      credentials.SignatureDefault,
 			},
 		})
+
 	}
+
 	if args.RoleARN != "" {
 		// assume role
 		credentialProviders = append(credentialProviders, &credentials.STSAssumeRole{
 			Options: credentials.STSAssumeRoleOptions{
-				AccessKey:       args.KeyID,
-				SecretKey:       args.KeySecret,
+				AccessKey:       keyID,
+				SecretKey:       keySecret,
 				RoleARN:         args.RoleARN,
 				RoleSessionName: args.ExternalID,
 			},
@@ -107,23 +128,23 @@ func NewMinioSDK(
 
 	// special treatments for 天翼云
 	if strings.Contains(args.Endpoint, "ctyunapi.cn") {
-		if args.KeyID == "" {
+		if keyID == "" {
 			// try to fetch one
 			creds := credentials.NewChainCredentials(credentialProviders)
 			value, err := creds.Get()
 			if err != nil {
 				return nil, err
 			}
-			args.KeyID = value.AccessKeyID
-			args.KeySecret = value.SecretAccessKey
-			args.SessionToken = value.SessionToken
+			keyID = value.AccessKeyID
+			keySecret = value.SecretAccessKey
+			sessionToken = value.SessionToken
 		}
 		credentialProviders = []credentials.Provider{
 			&credentials.Static{
 				Value: credentials.Value{
-					AccessKeyID:     args.KeyID,
-					SecretAccessKey: args.KeySecret,
-					SessionToken:    args.SessionToken,
+					AccessKeyID:     keyID,
+					SecretAccessKey: keySecret,
+					SessionToken:    sessionToken,
 					SignerType:      credentials.SignatureV2,
 				},
 			},

diff --git a/pkg/fileservice/minio_sdk_test.go b/pkg/fileservice/minio_sdk_test.go
@@ -139,3 +139,28 @@ func startMinio(dir string) (*exec.Cmd, error) {
 
 	return cmd, nil
 }
+
+func TestMinioSDKRoleARN(t *testing.T) {
+	_, err := NewMinioSDK(
+		context.Background(),
+		ObjectStorageArguments{
+			Endpoint:           "http://localhost",
+			RoleARN:            "abc",
+			NoBucketValidation: true,
+		},
+		nil,
+	)
+	assert.Nil(t, err)
+}
+
+func TestMinioSDKTianYiYun(t *testing.T) {
+	_, err := NewMinioSDK(
+		context.Background(),
+		ObjectStorageArguments{
+			Endpoint:           "http://ctyunapi.cn",
+			NoBucketValidation: true,
+		},
+		nil,
+	)
+	assert.Nil(t, err)
+}
diff --git a/pkg/fileservice/object_storage_arguments.go b/pkg/fileservice/object_storage_arguments.go
@@ -18,7 +18,6 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/url"
-	"os"
 	"regexp"
 	"strconv"
 	"strings"
@@ -181,20 +180,6 @@ func (o *ObjectStorageArguments) validate() error {
 		o.RoleSessionName = "mo-service"
 	}
 
-	// 腾讯云使用 AWS 环境变量配置 key id/secret
-	if strings.Contains(o.Endpoint, "myqcloud.com") {
-		if o.KeyID == "" {
-			if value := os.Getenv("AWS_ACCESS_KEY_ID"); value != "" {
-				o.KeyID = value
-			}
-		}
-		if o.KeySecret == "" {
-			if value := os.Getenv("AWS_SECRET_ACCESS_KEY"); value != "" {
-				o.KeySecret = value
-			}
-		}
-	}
-
 	return nil
 }
 

diff --git a/pkg/fileservice/object_storage_arguments_test.go b/pkg/fileservice/object_storage_arguments_test.go
@@ -167,14 +167,3 @@ func TestAWSRegion(t *testing.T) {
 	args.validate()
 	assert.Equal(t, "us-east-1", args.Region)
 }
-
-func TestQCloudKeyIDSecretFromAwsEnv(t *testing.T) {
-	args := ObjectStorageArguments{
-		Endpoint: "http://cos.foobar.myqcloud.com",
-	}
-	t.Setenv("AWS_ACCESS_KEY_ID", "foo")
-	t.Setenv("AWS_SECRET_ACCESS_KEY", "bar")
-	args.validate()
-	assert.Equal(t, "foo", args.KeyID)
-	assert.Equal(t, "bar", args.KeySecret)
-}
diff --git a/pkg/fileservice/qcloud_sdk.go b/pkg/fileservice/qcloud_sdk.go
@@ -23,6 +23,7 @@ import (
 	"iter"
 	"net/http"
 	"net/url"
+	"os"
 	gotrace "runtime/trace"
 	"strconv"
 	"time"
@@ -64,12 +65,36 @@ func NewQCloudSDK(
 		return nil, err
 	}
 
+	// credential arguments
+	keyID := args.KeyID
+	keySecret := args.KeySecret
+	sessionToken := args.SessionToken
+	if args.shouldLoadDefaultCredentials() {
+		keyID = firstNonZero(
+			args.KeyID,
+			os.Getenv("AWS_ACCESS_KEY_ID"),
+			os.Getenv("AWS_ACCESS_KEY"),
+			os.Getenv("TENCENTCLOUD_SECRETID"),
+		)
+		keySecret = firstNonZero(
+			args.KeySecret,
+			os.Getenv("AWS_SECRET_ACCESS_KEY"),
+			os.Getenv("AWS_SECRET_KEY"),
+			os.Getenv("TENCENTCLOUD_SECRETKEY"),
+		)
+		sessionToken = firstNonZero(
+			args.SessionToken,
+			os.Getenv("AWS_SESSION_TOKEN"),
+			os.Getenv("TENCENTCLOUD_SESSIONTOKEN"),
+		)
+	}
+
 	// http client
 	httpClient := newHTTPClient(args)
 	httpClient.Transport = &cos.AuthorizationTransport{
-		SecretID:     args.KeyID,
-		SecretKey:    args.KeySecret,
-		SessionToken: args.SessionToken,
+		SecretID:     keyID,
+		SecretKey:    keySecret,
+		SessionToken: sessionToken,
 		Transport:    httpClient.Transport,
 	}
 

diff --git a/pkg/fileservice/utils.go b/pkg/fileservice/utils.go
@@ -52,3 +52,13 @@ func SortedList(seq iter.Seq2[*DirEntry, error]) (ret []DirEntry, err error) {
 	})
 	return
 }
+
+func firstNonZero[T comparable](args ...T) T {
+	var zero T
+	for _, arg := range args {
+		if arg != zero {
+			return arg
+		}
+	}
+	return zero
+}
diff --git a/pkg/fileservice/utils_test.go b/pkg/fileservice/utils_test.go
@@ -14,7 +14,11 @@
 
 package fileservice
 
-import "testing"
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
 
 func TestZeroToNil(t *testing.T) {
 	if testing.AllocsPerRun(10, func() {
@@ -28,3 +32,8 @@ func TestZeroToNil(t *testing.T) {
 		t.Fatal()
 	}
 }
+
+func TestFirstNonZero(t *testing.T) {
+	assert.Equal(t, 42, firstNonZero(0, 42))
+	assert.Equal(t, 0, firstNonZero[int]())
+}