Skip to content

Commit f0fb312

Browse files
br: support s3 profile config (#62199)
ref #46609
1 parent c2c36cd commit f0fb312

4 files changed

Lines changed: 530 additions & 11 deletions

File tree

br/pkg/storage/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ go_test(
9999
"locking_test.go",
100100
"memstore_test.go",
101101
"parse_test.go",
102+
"s3_flags_test.go",
102103
"s3_test.go",
103104
"storage_test.go",
104105
"writer_test.go",
@@ -120,6 +121,7 @@ go_test(
120121
"@com_github_pingcap_errors//:errors",
121122
"@com_github_pingcap_failpoint//:failpoint",
122123
"@com_github_pingcap_kvproto//pkg/brpb",
124+
"@com_github_spf13_pflag//:pflag",
123125
"@com_github_stretchr_testify//require",
124126
"@org_golang_x_sync//errgroup",
125127
"@org_uber_go_mock//gomock",

br/pkg/storage/parse_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,131 @@ func TestIsLocal(t *testing.T) {
310310
}
311311
}
312312

313+
func TestS3ProfileOption(t *testing.T) {
314+
// Test parsing profile from URL query parameter
315+
testProfile := "my-test-profile"
316+
s, err := ParseBackend(
317+
fmt.Sprintf("s3://bucket/prefix/?profile=%s", url.QueryEscape(testProfile)),
318+
nil,
319+
)
320+
require.NoError(t, err)
321+
s3 := s.GetS3()
322+
require.NotNil(t, s3)
323+
require.Equal(t, "bucket", s3.Bucket)
324+
require.Equal(t, "prefix", s3.Prefix)
325+
require.Equal(t, testProfile, s3.Profile)
326+
327+
// Test with BackendOptions
328+
s3opt := &BackendOptions{
329+
S3: S3BackendOptions{
330+
Profile: "profile-from-options",
331+
},
332+
}
333+
s, err = ParseBackend("s3://bucket2/prefix/", s3opt)
334+
require.NoError(t, err)
335+
s3 = s.GetS3()
336+
require.NotNil(t, s3)
337+
require.Equal(t, "bucket2", s3.Bucket)
338+
require.Equal(t, "prefix", s3.Prefix)
339+
require.Equal(t, "profile-from-options", s3.Profile)
340+
341+
// Test profile with other S3 options
342+
s, err = ParseBackend(
343+
"s3://bucket3/prefix/?profile=dev-profile&region=us-west-2&endpoint=https://s3.example.com",
344+
nil,
345+
)
346+
require.NoError(t, err)
347+
s3 = s.GetS3()
348+
require.NotNil(t, s3)
349+
require.Equal(t, "bucket3", s3.Bucket)
350+
require.Equal(t, "prefix", s3.Prefix)
351+
require.Equal(t, "dev-profile", s3.Profile)
352+
require.Equal(t, "us-west-2", s3.Region)
353+
require.Equal(t, "https://s3.example.com", s3.Endpoint)
354+
355+
// Test empty profile (should remain empty)
356+
s, err = ParseBackend("s3://bucket4/prefix/", nil)
357+
require.NoError(t, err)
358+
s3 = s.GetS3()
359+
require.NotNil(t, s3)
360+
require.Equal(t, "bucket4", s3.Bucket)
361+
require.Equal(t, "prefix", s3.Prefix)
362+
require.Equal(t, "", s3.Profile) // Should be empty when not specified
363+
}
364+
365+
func TestS3ProfileCredentialsValidation(t *testing.T) {
366+
// Test that profile makes credentials optional in URL parsing
367+
368+
// Case 1: Profile without credentials - should be allowed
369+
s, err := ParseBackend("s3://bucket/prefix/?profile=production&region=us-west-2", nil)
370+
require.NoError(t, err, "Should not require credentials when using profile")
371+
s3 := s.GetS3()
372+
require.NotNil(t, s3)
373+
require.Equal(t, "production", s3.Profile)
374+
require.Equal(t, "us-west-2", s3.Region)
375+
require.Equal(t, "", s3.AccessKey)
376+
require.Equal(t, "", s3.SecretAccessKey)
377+
378+
// Case 2: Profile with partial credentials - should be allowed
379+
s, err = ParseBackend("s3://bucket/prefix/?profile=dev&access-key=override-key", nil)
380+
require.NoError(t, err, "Should allow partial credentials with profile")
381+
s3 = s.GetS3()
382+
require.NotNil(t, s3)
383+
require.Equal(t, "dev", s3.Profile)
384+
require.Equal(t, "override-key", s3.AccessKey)
385+
require.Equal(t, "", s3.SecretAccessKey) // No secret key, but should be OK with profile
386+
387+
// Case 3: Profile with explicit credentials override
388+
s, err = ParseBackend("s3://bucket/prefix/?profile=staging&access-key=explicit-access&secret-access-key=explicit-secret", nil)
389+
require.NoError(t, err)
390+
s3 = s.GetS3()
391+
require.NotNil(t, s3)
392+
require.Equal(t, "staging", s3.Profile)
393+
require.Equal(t, "explicit-access", s3.AccessKey)
394+
require.Equal(t, "explicit-secret", s3.SecretAccessKey)
395+
}
396+
397+
func TestS3NoProfileCredentialsValidation(t *testing.T) {
398+
// Test that without profile, credential validation still applies
399+
400+
// Case 1: No profile, partial credentials - should fail
401+
s3opt := &BackendOptions{
402+
S3: S3BackendOptions{
403+
AccessKey: "only-access-key",
404+
// Missing SecretAccessKey
405+
},
406+
}
407+
_, err := ParseBackend("s3://bucket/prefix/", s3opt)
408+
require.Error(t, err, "Should fail when access key provided without secret key")
409+
require.Contains(t, err.Error(), "secret_access_key not found")
410+
411+
// Case 2: No profile, both credentials - should be valid
412+
s3opt2 := &BackendOptions{
413+
S3: S3BackendOptions{
414+
AccessKey: "test-access",
415+
SecretAccessKey: "test-secret",
416+
},
417+
}
418+
s, err := ParseBackend("s3://bucket/prefix/", s3opt2)
419+
require.NoError(t, err)
420+
s3Backend2 := s.GetS3()
421+
require.Equal(t, "test-access", s3Backend2.AccessKey)
422+
require.Equal(t, "test-secret", s3Backend2.SecretAccessKey)
423+
424+
// Case 3: No profile, no credentials - should be valid (IAM role, etc.)
425+
s3opt3 := &BackendOptions{
426+
S3: S3BackendOptions{
427+
Region: "us-east-1",
428+
// No credentials
429+
},
430+
}
431+
s, err = ParseBackend("s3://bucket/prefix/", s3opt3)
432+
require.NoError(t, err, "Should allow no credentials when no profile (for IAM roles, etc.)")
433+
s3Backend3 := s.GetS3()
434+
require.Equal(t, "", s3Backend3.AccessKey)
435+
require.Equal(t, "", s3Backend3.SecretAccessKey)
436+
}
437+
313438
func TestParseBackend(t *testing.T) {
314439
{
315440
backendOptions := &BackendOptions{

br/pkg/storage/s3.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const (
5656
s3ProviderOption = "s3.provider"
5757
s3RoleARNOption = "s3.role-arn"
5858
s3ExternalIDOption = "s3." + S3ExternalID
59+
s3ProfileOption = "s3.profile"
5960
notFound = "NotFound"
6061
// number of retries to make of operations.
6162
maxRetries = 7
@@ -180,6 +181,7 @@ type S3BackendOptions struct {
180181
UseAccelerateEndpoint bool `json:"use-accelerate-endpoint" toml:"use-accelerate-endpoint"`
181182
RoleARN string `json:"role-arn" toml:"role-arn"`
182183
ExternalID string `json:"external-id" toml:"external-id"`
184+
Profile string `json:"profile" toml:"profile"`
183185
ObjectLockEnabled bool `json:"object-lock-enabled" toml:"object-lock-enabled"`
184186
}
185187

@@ -203,11 +205,14 @@ func (options *S3BackendOptions) Apply(s3 *backuppb.S3) error {
203205
options.UseAccelerateEndpoint {
204206
options.ForcePathStyle = false
205207
}
206-
if options.AccessKey == "" && options.SecretAccessKey != "" {
207-
return errors.Annotate(berrors.ErrStorageInvalidConfig, "access_key not found")
208-
}
209-
if options.AccessKey != "" && options.SecretAccessKey == "" {
210-
return errors.Annotate(berrors.ErrStorageInvalidConfig, "secret_access_key not found")
208+
// When not using a profile, if either key is provided, both must be provided
209+
if options.Profile == "" {
210+
if options.AccessKey == "" && options.SecretAccessKey != "" {
211+
return errors.Annotate(berrors.ErrStorageInvalidConfig, "access_key not found")
212+
}
213+
if options.AccessKey != "" && options.SecretAccessKey == "" {
214+
return errors.Annotate(berrors.ErrStorageInvalidConfig, "secret_access_key not found")
215+
}
211216
}
212217

213218
s3.Endpoint = strings.TrimSuffix(options.Endpoint, "/")
@@ -224,6 +229,8 @@ func (options *S3BackendOptions) Apply(s3 *backuppb.S3) error {
224229
s3.RoleArn = options.RoleARN
225230
s3.ExternalId = options.ExternalID
226231
s3.Provider = options.Provider
232+
s3.Profile = options.Profile
233+
227234
return nil
228235
}
229236

@@ -241,6 +248,8 @@ func defineS3Flags(flags *pflag.FlagSet) {
241248
flags.String(s3ProviderOption, "", "(experimental) Set the S3 provider, e.g. aws, alibaba, ceph")
242249
flags.String(s3RoleARNOption, "", "(experimental) Set the ARN of the IAM role to assume when accessing AWS S3")
243250
flags.String(s3ExternalIDOption, "", "(experimental) Set the external ID when assuming the role to access AWS S3")
251+
flags.String(s3ProfileOption, "", "(experimental) Set the AWS profile to use for AWS S3 authentication. "+
252+
"Command line options take precedence over profile settings")
244253
}
245254

246255
// parseFromFlags parse S3BackendOptions from command line flags.
@@ -284,6 +293,11 @@ func (options *S3BackendOptions) parseFromFlags(flags *pflag.FlagSet) error {
284293
if err != nil {
285294
return errors.Trace(err)
286295
}
296+
options.Profile, err = flags.GetString(s3ProfileOption)
297+
if err != nil {
298+
return errors.Trace(err)
299+
}
300+
287301
return nil
288302
}
289303

@@ -362,17 +376,26 @@ func NewS3Storage(ctx context.Context, backend *backuppb.S3, opts *ExternalStora
362376
if opts.HTTPClient != nil {
363377
awsConfig.WithHTTPClient(opts.HTTPClient)
364378
}
365-
cred, err := autoNewCred(&qs)
366-
if err != nil {
367-
return nil, errors.Trace(err)
368-
}
369-
if cred != nil {
370-
awsConfig.WithCredentials(cred)
379+
// When using a profile, let AWS SDK handle credentials through the profile
380+
// Don't call autoNewCred as it interferes with profile-based authentication
381+
if qs.Profile == "" {
382+
cred, err := autoNewCred(&qs)
383+
if err != nil {
384+
return nil, errors.Trace(err)
385+
}
386+
if cred != nil {
387+
awsConfig.WithCredentials(cred)
388+
}
371389
}
372390
// awsConfig.WithLogLevel(aws.LogDebugWithSigning)
373391
awsSessionOpts := session.Options{
374392
Config: *awsConfig,
375393
}
394+
if qs.Profile != "" {
395+
awsSessionOpts.Profile = qs.Profile
396+
// Use default credential chain when profile is specified
397+
awsSessionOpts.SharedConfigState = session.SharedConfigEnable
398+
}
376399
ses, err := session.NewSessionWithOptions(awsSessionOpts)
377400
if err != nil {
378401
return nil, errors.Trace(err)

0 commit comments

Comments
 (0)