Skip to content

Commit db751af

Browse files
committed
fix how the url is built
1 parent c280531 commit db751af

File tree

3 files changed

+79
-3
lines changed

3 files changed

+79
-3
lines changed

pkg/storage/chunk/client/aws/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/aws/aws-sdk-go-v2/aws"
1717
"github.com/aws/aws-sdk-go-v2/credentials"
1818
"github.com/aws/aws-sdk-go-v2/service/dynamodb"
19-
"github.com/pkg/errors"
2019
)
2120

2221
// DynamoConfigFromURL returns AWS config from given URL. It expects escaped
@@ -83,5 +82,7 @@ func CredentialsFromURL(awsURL *url.URL) (key, secret, session string, err error
8382
return username, password, "", nil
8483
}
8584
}
86-
return "", "", "", errors.New("Unable to build AWS credentials from URL")
85+
// Return empty credentials instead of error to allow AWS SDK to use default credential chain
86+
// (environment variables, IAM roles, etc.)
87+
return "", "", "", nil
8788
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package aws
2+
3+
import (
4+
"net/url"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestCredentialsFromURL(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
urlStr string
14+
expectedKey string
15+
expectedSecret string
16+
expectedSession string
17+
expectError bool
18+
}{
19+
{
20+
name: "URL with username and password",
21+
urlStr: "s3://mykey:mysecret@us-east-1",
22+
expectedKey: "mykey",
23+
expectedSecret: "mysecret",
24+
expectedSession: "",
25+
expectError: false,
26+
},
27+
{
28+
name: "URL with only username",
29+
urlStr: "s3://mykey@us-east-1",
30+
expectedKey: "mykey",
31+
expectedSecret: "",
32+
expectedSession: "",
33+
expectError: false,
34+
},
35+
{
36+
name: "URL without credentials (should not error)",
37+
urlStr: "s3://us-east-1",
38+
expectedKey: "",
39+
expectedSecret: "",
40+
expectedSession: "",
41+
expectError: false,
42+
},
43+
{
44+
name: "URL with endpoint without credentials",
45+
urlStr: "s3://s3.amazonaws.com",
46+
expectedKey: "",
47+
expectedSecret: "",
48+
expectedSession: "",
49+
expectError: false,
50+
},
51+
}
52+
53+
for _, tt := range tests {
54+
t.Run(tt.name, func(t *testing.T) {
55+
u, err := url.Parse(tt.urlStr)
56+
require.NoError(t, err)
57+
58+
key, secret, session, err := CredentialsFromURL(u)
59+
60+
if tt.expectError {
61+
require.Error(t, err)
62+
} else {
63+
require.NoError(t, err)
64+
require.Equal(t, tt.expectedKey, key)
65+
require.Equal(t, tt.expectedSecret, secret)
66+
require.Equal(t, tt.expectedSession, session)
67+
}
68+
})
69+
}
70+
}
71+

pkg/storage/chunk/client/aws/s3_storage_client.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,11 @@ func buildS3Client(cfg S3Config, hedgingCfg hedging.Config, hedging bool) (*s3.C
219219
if err != nil {
220220
return nil, err
221221
}
222-
s3Options.Credentials = credentials.NewStaticCredentialsProvider(key, secret, session)
222+
// Only set credentials if they were provided in the URL
223+
// Otherwise, let AWS SDK use the default credential chain
224+
if key != "" || secret != "" {
225+
s3Options.Credentials = credentials.NewStaticCredentialsProvider(key, secret, session)
226+
}
223227
} else {
224228
s3Options.Region = "dummy"
225229
}

0 commit comments

Comments
 (0)