Skip to content

Commit ced063b

Browse files
laramielcopybara-github
authored andcommitted
s3: Fallback to anonymous credentials by default
This makes reading public s3 datasets work by default, whereas before "anonymous" credentials were required. PiperOrigin-RevId: 745343842 Change-Id: I20f86f1b4ab989f85d2e1afca040c935f46dbb83
1 parent 510b435 commit ced063b

File tree

6 files changed

+68
-23
lines changed

6 files changed

+68
-23
lines changed

tensorstore/internal/aws/credentials/common.cc

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ AwsCredentialsProvider MakeCache(AwsCredentialsProvider provider) {
5454
AwsCredentialsProvider MakeDefault(std::string_view profile_name_override) {
5555
aws_allocator* allocator = GetAwsAllocator();
5656

57-
struct aws_credentials_provider_chain_default_options options;
57+
aws_credentials_provider_chain_default_options options;
5858
AWS_ZERO_STRUCT(options);
5959
options.bootstrap = GetAwsClientBootstrap();
6060
options.tls_ctx = GetAwsTlsCtx();
@@ -67,6 +67,26 @@ AwsCredentialsProvider MakeDefault(std::string_view profile_name_override) {
6767
aws_credentials_provider_new_chain_default(allocator, &options));
6868
}
6969

70+
AwsCredentialsProvider MakeDefaultWithAnonymous(
71+
std::string_view profile_name_override) {
72+
aws_allocator* allocator = GetAwsAllocator();
73+
74+
auto default_provider = MakeDefault(profile_name_override);
75+
auto anonymous_provider = MakeAnonymous();
76+
77+
aws_credentials_provider* providers[2];
78+
AWS_ZERO_ARRAY(providers);
79+
providers[0] = default_provider.get();
80+
providers[1] = anonymous_provider.get();
81+
82+
aws_credentials_provider_chain_options options;
83+
AWS_ZERO_STRUCT(options);
84+
options.provider_count = 2;
85+
options.providers = providers;
86+
87+
return AsProvider(aws_credentials_provider_new_chain(allocator, &options));
88+
}
89+
7090
AwsCredentialsProvider MakeAnonymous() {
7191
aws_allocator* allocator = GetAwsAllocator();
7292

tensorstore/internal/aws/credentials/common.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ namespace internal_aws {
2525
/// Returns a credentials provider that uses the AWS default credentials chain.
2626
AwsCredentialsProvider MakeDefault(std::string_view profile_name_override);
2727

28+
/// Returns a credentials provider that uses the AWS default credentials chain
29+
/// with anonymous credentials as a fallback.
30+
AwsCredentialsProvider MakeDefaultWithAnonymous(
31+
std::string_view profile_name_override);
32+
2833
/// Returns anonymous credentials.
2934
AwsCredentialsProvider MakeAnonymous();
3035

tensorstore/internal/aws/credentials/default_credential_provider_test.cc

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ using ::tensorstore::internal_aws::AwsCredentialsProvider;
4040
using ::tensorstore::internal_aws::DisableAwsHttpMocking;
4141
using ::tensorstore::internal_aws::EnableAwsHttpMocking;
4242
using ::tensorstore::internal_aws::GetAwsCredentials;
43+
using ::tensorstore::internal_aws::MakeAnonymous;
4344
using ::tensorstore::internal_aws::MakeDefault;
45+
using ::tensorstore::internal_aws::MakeDefaultWithAnonymous;
4446

4547
static constexpr char kAccessKeyId[] = "ASIA1234567890";
4648
static constexpr char kSecretKey[] = "1234567890abcdef";
@@ -100,13 +102,27 @@ class DefaultCredentialProviderTest : public ::testing::Test {
100102
absl::Time expiry;
101103
};
102104

103-
TEST_F(DefaultCredentialProviderTest, Basic) {
105+
TEST_F(DefaultCredentialProviderTest, Anonymous) {
106+
AwsCredentialsProvider provider = MakeAnonymous();
107+
auto credentials_future = GetAwsCredentials(provider.get());
108+
ASSERT_THAT(credentials_future, IsOk());
109+
EXPECT_TRUE(credentials_future.result()->IsAnonymous());
110+
}
111+
112+
TEST_F(DefaultCredentialProviderTest, DefaultWithoutFallback) {
104113
AwsCredentialsProvider provider = MakeDefault({});
105114
auto credentials_future = GetAwsCredentials(provider.get());
106115
ASSERT_THAT(credentials_future,
107116
MatchesStatus(absl::StatusCode::kInternal, ".*aws-c-.*"));
108117
}
109118

119+
TEST_F(DefaultCredentialProviderTest, DefaultWithFallback) {
120+
AwsCredentialsProvider provider = MakeDefaultWithAnonymous({});
121+
auto credentials_future = GetAwsCredentials(provider.get());
122+
ASSERT_THAT(credentials_future, IsOk());
123+
EXPECT_TRUE(credentials_future.result()->IsAnonymous());
124+
}
125+
110126
TEST_F(DefaultCredentialProviderTest, FromEnvironment) {
111127
SetEnv("AWS_ACCESS_KEY_ID", kAccessKeyId);
112128
SetEnv("AWS_SECRET_ACCESS_KEY", kSecretKey);

tensorstore/kvstore/s3/aws_credentials_spec.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ using Spec = ::tensorstore::internal_kvstore_s3::AwsCredentialsSpec;
4949
using ::tensorstore::internal_aws::AwsCredentialsProvider;
5050
using ::tensorstore::internal_aws::MakeAnonymous;
5151
using ::tensorstore::internal_aws::MakeCache;
52-
using ::tensorstore::internal_aws::MakeDefault;
52+
using ::tensorstore::internal_aws::MakeDefaultWithAnonymous;
5353
using ::tensorstore::internal_aws::MakeEcsRole;
5454
using ::tensorstore::internal_aws::MakeEnvironment;
5555
using ::tensorstore::internal_aws::MakeImds;
@@ -152,7 +152,9 @@ Result<AwsCredentialsProvider> MakeAwsCredentialsProvider(const Spec& spec) {
152152
struct MakeCredentialsVisitor {
153153
using R = AwsCredentialsProvider;
154154

155-
R operator()(const Spec::Default& v) { return MakeDefault(v.profile); }
155+
R operator()(const Spec::Default& v) {
156+
return MakeDefaultWithAnonymous(v.profile);
157+
}
156158
R operator()(const Spec::Anonymous&) { return MakeAnonymous(); }
157159
R operator()(const Spec::Environment&) { return MakeEnvironment(); }
158160
R operator()(const Spec::Imds&) { return MakeImds(); }

tensorstore/kvstore/s3/s3_endpoint.cc

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ struct ResolveHost {
114114
default_aws_region,
115115
});
116116
}
117+
// TODO: Get the message from the response body, if any.
117118
promise.SetResult(absl::FailedPreconditionError(tensorstore::StrCat(
118119
"Failed to resolve aws_region for bucket ", QuoteString(bucket))));
119120
}
@@ -201,21 +202,20 @@ Future<S3EndpointRegion> ResolveEndpointRegion(
201202
assert(IsValidBucketName(bucket));
202203

203204
// TODO: Handle retries, and sign the request.
204-
205205
if (endpoint.empty()) {
206206
// Use a HEAD request on bucket.s3.amazonaws.com to acquire the aws_region
207207
// does not work when there is a . in the bucket name; for now require the
208208
// aws_region to be set.
209209
if (!absl::StrContains(bucket, ".")) {
210-
std::string url = absl::StrFormat("https://%s.s3.amazonaws.com", bucket);
210+
auto request =
211+
HttpRequestBuilder(
212+
"HEAD", absl::StrFormat("https://%s.s3.amazonaws.com", bucket))
213+
.AddHostHeader(host_header)
214+
.BuildRequest();
211215
return PromiseFuturePair<S3EndpointRegion>::Link(
212216
ResolveHost<S3VirtualHostFormatter>{
213217
std::move(bucket), {}, S3VirtualHostFormatter{}},
214-
transport->IssueRequest(
215-
HttpRequestBuilder("HEAD", std::move(url))
216-
.AddHostHeader(host_header)
217-
.BuildRequest(),
218-
{}))
218+
transport->IssueRequest(std::move(request), {}))
219219
.future;
220220
}
221221

@@ -224,30 +224,30 @@ Future<S3EndpointRegion> ResolveEndpointRegion(
224224
// zone. The response will be a 301 request with an 'x-amz-bucket-region'
225225
// header. We might be able to just do a signed HEAD request against an
226226
// possibly non-existent file... But try this later.
227-
std::string url =
228-
absl::StrFormat("https://s3.us-east-1.amazonaws.com/%s", bucket);
227+
auto request =
228+
HttpRequestBuilder(
229+
"HEAD",
230+
absl::StrFormat("https://s3.us-east-1.amazonaws.com/%s", bucket))
231+
.AddHostHeader(host_header)
232+
.BuildRequest();
229233
return PromiseFuturePair<S3EndpointRegion>::Link(
230234
ResolveHost<S3PathFormatter>{
231235
std::move(bucket), {}, S3PathFormatter{}},
232-
transport->IssueRequest(
233-
HttpRequestBuilder("HEAD", std ::move(url))
234-
.AddHostHeader(host_header)
235-
.BuildRequest(),
236-
{}))
236+
transport->IssueRequest(std::move(request), {}))
237237
.future;
238238
}
239239

240240
// Issue a HEAD request against the endpoint+bucket, which should work for
241241
// mock S3 backends like localstack or minio.
242-
std::string url = absl::StrFormat("%s/%s", endpoint, bucket);
242+
auto request =
243+
HttpRequestBuilder("HEAD", absl::StrFormat("%s/%s", endpoint, bucket))
244+
.AddHostHeader(host_header)
245+
.BuildRequest();
243246
return PromiseFuturePair<S3EndpointRegion>::Link(
244247
ResolveHost<S3CustomFormatter>{
245248
std::move(bucket), "us-east-1",
246249
S3CustomFormatter{std::string(endpoint)}},
247-
transport->IssueRequest(HttpRequestBuilder("HEAD", std::move(url))
248-
.AddHostHeader(host_header)
249-
.BuildRequest(),
250-
{}))
250+
transport->IssueRequest(std::move(request), {}))
251251
.future;
252252
}
253253

tensorstore/kvstore/s3/schema.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,8 @@ definitions:
210210
description: |-
211211
Source credentials using the `default AWS credentials chain
212212
<https://github.com/aws/aws-sdk-cpp/blob/main/docs/Credentials_Providers.md>`__.
213+
If the default credentials chain cannot resolve credentials then anonymous credentials
214+
will be used.
213215
allOf:
214216
- $ref: Context.aws_credentials
215217
- type: object

0 commit comments

Comments
 (0)