From 34a99ee614657d05fb7d582aacd8a37355586057 Mon Sep 17 00:00:00 2001 From: 3pointer Date: Thu, 2 Mar 2023 18:14:32 +0800 Subject: [PATCH] br: support skip automatically get bucket region with other s3 compatible provider --- br/pkg/storage/s3.go | 39 ++++++++++++--------- br/pkg/storage/s3_test.go | 71 +++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 4 +-- 4 files changed, 98 insertions(+), 18 deletions(-) diff --git a/br/pkg/storage/s3.go b/br/pkg/storage/s3.go index 7abbd9d0e814b..37d3d6ebf8ae2 100644 --- a/br/pkg/storage/s3.go +++ b/br/pkg/storage/s3.go @@ -189,6 +189,7 @@ func (options *S3BackendOptions) Apply(s3 *backuppb.S3) error { s3.ForcePathStyle = options.ForcePathStyle s3.RoleArn = options.RoleARN s3.ExternalId = options.ExternalID + s3.Provider = options.Provider return nil } @@ -203,7 +204,7 @@ func defineS3Flags(flags *pflag.FlagSet) { flags.String(s3SseKmsKeyIDOption, "", "KMS CMK key id to use with S3 server-side encryption."+ "Leave empty to use S3 owned key.") flags.String(s3ACLOption, "", "(experimental) Set the S3 canned ACLs, e.g. authenticated-read") - flags.String(s3ProviderOption, "", "(experimental) Set the S3 provider, e.g. aws, alibaba, ceph") + flags.String(s3ProviderOption, "aws", "(experimental) Set the S3 provider, e.g. aws, alibaba, ceph") flags.String(s3RoleARNOption, "", "(experimental) Set the ARN of the IAM role to assume when accessing AWS S3") flags.String(s3ExternalIDOption, "", "(experimental) Set the external ID when assuming the role to access AWS S3") } @@ -358,22 +359,30 @@ func NewS3Storage(ctx context.Context, backend *backuppb.S3, opts *ExternalStora ) } c := s3.New(ses, s3CliConfigs...) - confCred := ses.Config.Credentials - setCredOpt := func(req *request.Request) { - // s3manager.GetBucketRegionWithClient will set credential anonymous, which works with s3. - // we need reassign credential to be compatible with minio authentication. - if confCred != nil { - req.Config.Credentials = confCred + + var region string + if qs.Provider == "aws" { + confCred := ses.Config.Credentials + setCredOpt := func(req *request.Request) { + // s3manager.GetBucketRegionWithClient will set credential anonymous, which works with s3. + // we need reassign credential to be compatible with minio authentication. + if confCred != nil { + req.Config.Credentials = confCred + } + // s3manager.GetBucketRegionWithClient use path style addressing default. + // we need set S3ForcePathStyle by our config if we set endpoint. + if qs.Endpoint != "" { + req.Config.S3ForcePathStyle = ses.Config.S3ForcePathStyle + } } - // s3manager.GetBucketRegionWithClient use path style addressing default. - // we need set S3ForcePathStyle by our config if we set endpoint. - if qs.Endpoint != "" { - req.Config.S3ForcePathStyle = ses.Config.S3ForcePathStyle + region, err = s3manager.GetBucketRegionWithClient(ctx, c, qs.Bucket, setCredOpt) + if err != nil { + return nil, errors.Annotatef(err, "failed to get region of bucket %s", qs.Bucket) } - } - region, err := s3manager.GetBucketRegionWithClient(ctx, c, qs.Bucket, setCredOpt) - if err != nil { - return nil, errors.Annotatef(err, "failed to get region of bucket %s", qs.Bucket) + } else { + // for other s3 compatible provider like ovh storage didn't return the region correctlly + // so we cannot automatically get the bucket region. just fallback to manually region setting. + region = qs.Region } if qs.Region != region { diff --git a/br/pkg/storage/s3_test.go b/br/pkg/storage/s3_test.go index 865c1e726e708..e5e84e37c9bd1 100644 --- a/br/pkg/storage/s3_test.go +++ b/br/pkg/storage/s3_test.go @@ -1217,3 +1217,74 @@ func TestObjectLock(t *testing.T) { ) require.Equal(t, true, s.storage.IsObjectLockEnabled()) } + +func TestS3StorageBucketRegion(t *testing.T) { + type testcase struct { + name string + region string + s3 *backuppb.S3 + } + + require.NoError(t, os.Setenv("AWS_ACCESS_KEY_ID", "ab")) + require.NoError(t, os.Setenv("AWS_SECRET_ACCESS_KEY", "cd")) + require.NoError(t, os.Setenv("AWS_SESSION_TOKEN", "ef")) + + cases := []testcase{ + { + "empty region from aws", + "us-east-1", + &backuppb.S3{ + Region: "", + Bucket: "bucket", + Prefix: "prefix", + Provider: "aws", + }, + }, + { + "region from different provider", + "sdg", + &backuppb.S3{ + Region: "sdg", + Bucket: "bucket", + Prefix: "prefix", + Provider: "ovh", + }, + }, + { + "empty region from different provider", + "", + &backuppb.S3{ + Region: "", + Bucket: "bucket", + Prefix: "prefix", + Provider: "ovh", + }, + }, + { + "region from aws", + "us-west-2", + &backuppb.S3{ + Region: "us-west-2", + Bucket: "bucket", + Prefix: "prefix", + Provider: "aws", + }, + }, + } + for _, ca := range cases { + func(name string, region string, s3 *backuppb.S3) { + s := createGetBucketRegionServer(region, 200, true) + defer s.Close() + + t.Log(name) + es, err := New(context.Background(), + &backuppb.StorageBackend{Backend: &backuppb.StorageBackend_S3{S3: s3}}, + &ExternalStorageOptions{}) + require.NoError(t, err) + ss, ok := es.(*S3Storage) + require.True(t, ok) + require.Equal(t, region, ss.GetOptions().Region) + + }(ca.name, ca.region, ca.s3) + } +} diff --git a/go.mod b/go.mod index 1d1a9484cfa4c..80a74bbe8e3fc 100644 --- a/go.mod +++ b/go.mod @@ -71,7 +71,7 @@ require ( github.com/pingcap/errors v0.11.5-0.20221009092201-b66cddb77c32 github.com/pingcap/failpoint v0.0.0-20220801062533-2eaa32854a6c github.com/pingcap/fn v0.0.0-20200306044125-d5540d389059 - github.com/pingcap/kvproto v0.0.0-20230216153817-c6df78cc9dea + github.com/pingcap/kvproto v0.0.0-20230302085208-bebf415a2244 github.com/pingcap/log v1.1.1-0.20221116035753-734d527bc87c github.com/pingcap/sysutil v0.0.0-20220114020952-ea68d2dbf5b4 github.com/pingcap/tidb/parser v0.0.0-20211011031125-9b13dc409c5e diff --git a/go.sum b/go.sum index 0054abec46f15..8c31f13013288 100644 --- a/go.sum +++ b/go.sum @@ -772,8 +772,8 @@ github.com/pingcap/fn v0.0.0-20200306044125-d5540d389059/go.mod h1:fMRU1BA1y+r89 github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989 h1:surzm05a8C9dN8dIUmo4Be2+pMRb6f55i+UIYrluu2E= github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20191211054548-3c6b38ea5107/go.mod h1:WWLmULLO7l8IOcQG+t+ItJ3fEcrL5FxF0Wu+HrMy26w= -github.com/pingcap/kvproto v0.0.0-20230216153817-c6df78cc9dea h1:Qt8xe4CWgA/pPfYLHwCl8Mz0g7Mbnbhx4l0gVf9eH1w= -github.com/pingcap/kvproto v0.0.0-20230216153817-c6df78cc9dea/go.mod h1:+on3Lfk/fb1lXkud3XvskJumhSIEEgN2TTbMObUlrxE= +github.com/pingcap/kvproto v0.0.0-20230302085208-bebf415a2244 h1:t7udP4U+NdF0mDNzes3BWfxnxqV5nfB32vEAcADEo/E= +github.com/pingcap/kvproto v0.0.0-20230302085208-bebf415a2244/go.mod h1:KUrW1FGoznGMMTssYBu0czfAhn6vQcIrHyZoSC6T990= github.com/pingcap/log v0.0.0-20191012051959-b742a5d432e9/go.mod h1:4rbK1p9ILyIfb6hU7OG2CiWSqMXnp3JMbiaVJ6mvoY8= github.com/pingcap/log v0.0.0-20210625125904-98ed8e2eb1c7/go.mod h1:8AanEdAHATuRurdGxZXBz0At+9avep+ub7U1AGYLIMM= github.com/pingcap/log v1.1.0/go.mod h1:DWQW5jICDR7UJh4HtxXSM20Churx4CQL0fwL/SoOSA4=