Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

br: skip automatically get bucket region with other s3 compatible provider. #41889

Merged
merged 10 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions DEPS.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3373,8 +3373,8 @@ def go_deps():
name = "com_github_pingcap_kvproto",
build_file_proto_mode = "disable_global",
importpath = "github.com/pingcap/kvproto",
sum = "h1:bgLRG7gPJCq6aduA65ZV7xWQBThTcuarBB9VdfAzV4g=",
version = "v0.0.0-20230228041042-1e9aca94bab6",
sum = "h1:t7udP4U+NdF0mDNzes3BWfxnxqV5nfB32vEAcADEo/E=",
version = "v0.0.0-20230302085208-bebf415a2244",
)
go_repository(
name = "com_github_pingcap_log",
Expand Down Expand Up @@ -3984,8 +3984,8 @@ def go_deps():
name = "com_github_stretchr_testify",
build_file_proto_mode = "disable_global",
importpath = "github.com/stretchr/testify",
sum = "h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=",
version = "v1.8.2",
sum = "h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=",
version = "v1.8.1",
)
go_repository(
name = "com_github_subosito_gotenv",
Expand Down Expand Up @@ -4101,8 +4101,8 @@ def go_deps():
name = "com_github_tikv_client_go_v2",
build_file_proto_mode = "disable_global",
importpath = "github.com/tikv/client-go/v2",
sum = "h1:3BQR4RmBxu391t3z/q9h7BjDTS3cuRn8dfgYgMWk57s=",
version = "v2.0.6-0.20230302054057-3f7860f10959",
sum = "h1:hZ0aZfGAaS3rM0hYiMq6j0xHpVFgoYw6KeqrniGb7Zo=",
version = "v2.0.6-0.20230227032358-40a82457ebaa",
)
go_repository(
name = "com_github_tikv_pd",
Expand All @@ -4116,8 +4116,8 @@ def go_deps():
name = "com_github_tikv_pd_client",
build_file_proto_mode = "disable_global",
importpath = "github.com/tikv/pd/client",
sum = "h1:1fomIvN2iiKT5uZbe2E6uNHZnRzmS6O47D/PJ9BAuPw=",
version = "v0.0.0-20230301094509-c82b237672a0",
sum = "h1:hauBQBHSyrUxAI0zvkTiBKd472c+Iy+aY0Jd+b9VOJ8=",
version = "v0.0.0-20230209034200-6d23a31c24be",
)
go_repository(
name = "com_github_timakin_bodyclose",
Expand Down
39 changes: 24 additions & 15 deletions br/pkg/storage/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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")
}
Expand Down Expand Up @@ -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 {
Expand Down
73 changes: 73 additions & 0 deletions br/pkg/storage/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1217,3 +1217,76 @@ func TestObjectLock(t *testing.T) {
)
require.Equal(t, true, s.storage.IsObjectLockEnabled())
}

func TestS3StorageBucketRegion(t *testing.T) {
type testcase struct {
name string
expectRegion 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()
s3.ForcePathStyle = true
s3.Endpoint = s.URL

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.expectRegion, ca.s3)
}
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-20230228041042-1e9aca94bab6
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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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-20230228041042-1e9aca94bab6 h1:bgLRG7gPJCq6aduA65ZV7xWQBThTcuarBB9VdfAzV4g=
github.com/pingcap/kvproto v0.0.0-20230228041042-1e9aca94bab6/go.mod h1:KUrW1FGoznGMMTssYBu0czfAhn6vQcIrHyZoSC6T990=
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=
Expand Down