Skip to content

Commit 7c3f82e

Browse files
committed
Enable cross-region access sometimes
1 parent e0df223 commit 7c3f82e

File tree

3 files changed

+46
-29
lines changed

3 files changed

+46
-29
lines changed

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,14 @@ protected S3ClientBuilder buildClientBuilder(S3ClientSettings clientSettings, Sd
244244
s3clientBuilder.forcePathStyle(true);
245245
}
246246

247-
s3clientBuilder.region(getClientRegion(clientSettings));
247+
final var clientRegion = getClientRegion(clientSettings);
248+
if (clientRegion == null) {
249+
// If no region or endpoint is specified then (for BwC with SDKv1) default to us-east-1 and enable cross-region access:
250+
s3clientBuilder.region(Region.US_EAST_1);
251+
s3clientBuilder.crossRegionAccessEnabled(true);
252+
} else {
253+
s3clientBuilder.region(clientRegion);
254+
}
248255

249256
if (Strings.hasLength(clientSettings.endpoint)) {
250257
s3clientBuilder.endpointOverride(URI.create(clientSettings.endpoint));
@@ -253,11 +260,14 @@ protected S3ClientBuilder buildClientBuilder(S3ClientSettings clientSettings, Sd
253260
return s3clientBuilder;
254261
}
255262

263+
@Nullable // if the region is wholly unknown (falls back to us-east-1 and enables cross-region access)
256264
Region getClientRegion(S3ClientSettings clientSettings) {
257265
if (Strings.hasLength(clientSettings.region)) {
258266
return Region.of(clientSettings.region);
259267
}
260-
if (Strings.hasLength(clientSettings.endpoint)) {
268+
final String endpointDescription;
269+
final var hasEndpoint = Strings.hasLength(clientSettings.endpoint);
270+
if (hasEndpoint) {
261271
final var guessedRegion = RegionFromEndpointGuesser.guessRegion(clientSettings.endpoint);
262272
if (guessedRegion != null) {
263273
LOGGER.warn(
@@ -269,30 +279,39 @@ Region getClientRegion(S3ClientSettings clientSettings) {
269279
S3ClientSettings.REGION.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
270280
);
271281
return Region.of(guessedRegion);
272-
} else {
273-
LOGGER.info(
274-
"""
275-
found S3 client with endpoint [{}] which does not map to a known AWS region; \
276-
to suppress this message, configure the [{}] setting on this node""",
277-
clientSettings.endpoint,
278-
S3ClientSettings.REGION.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
279-
);
280282
}
283+
endpointDescription = "configured endpoint [" + clientSettings.endpoint + "]";
284+
} else {
285+
endpointDescription = "no configured endpoint";
281286
}
282287
final var defaultRegion = this.defaultRegion;
283288
if (defaultRegion != null) {
284289
LOGGER.debug("""
285-
found S3 client with no configured region, using region [{}] from SDK""", defaultRegion);
290+
found S3 client with no configured region and {}, using region [{}] from SDK""", endpointDescription, defaultRegion);
286291
return defaultRegion;
287292
}
288-
LOGGER.warn(
289-
"""
290-
found S3 client with no configured region, falling back to [{}]; \
291-
to suppress this warning, configure the [{}] setting on this node""",
292-
Region.US_EAST_1,
293-
S3ClientSettings.REGION.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
294-
);
295-
return Region.US_EAST_1;
293+
294+
if (hasEndpoint) {
295+
LOGGER.warn(
296+
"""
297+
found S3 client with no configured region and {}, falling back to [{}]; \
298+
to suppress this warning, configure the [{}] setting on this node""",
299+
endpointDescription,
300+
Region.US_EAST_1,
301+
S3ClientSettings.REGION.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
302+
);
303+
return Region.US_EAST_1;
304+
} else {
305+
LOGGER.warn(
306+
"""
307+
found S3 client with no configured region and {}, falling back to [{}] and enabling cross-region access; \
308+
to suppress this warning, configure the [{}] setting on this node""",
309+
endpointDescription,
310+
Region.US_EAST_1,
311+
S3ClientSettings.REGION.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
312+
);
313+
return null;
314+
}
296315
}
297316

298317
@Nullable // in production, but exposed for tests to override

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,8 @@ public void testRegionCanBeSet() {
192192
Region otherRegion = s3Service.getClientRegion(otherSettings);
193193
assertEquals(randomRegion, otherRegion.toString());
194194

195-
var defaultSettings = settings.get("default");
196-
Region defaultRegion = s3Service.getClientRegion(defaultSettings);
197-
assertNotEquals(randomRegion, defaultRegion.toString());
198-
assertEquals("us-east-1", defaultRegion.toString()); // us-east-1 is the default absent any other settings.
195+
// by default, we simply do not know the region (which S3Service maps to us-east-1 with cross-region access enabled)
196+
assertNull(s3Service.getClientRegion(settings.get("default")));
199197
}
200198
}
201199

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,9 @@ public void testGetClientRegionFromDefault() {
187187
"warning",
188188
S3Service.class.getCanonicalName(),
189189
Level.DEBUG,
190-
"found S3 client with no configured region, using region [" + defaultRegion.id() + "] from SDK"
190+
"found S3 client with no configured region and no configured endpoint, using region ["
191+
+ defaultRegion.id()
192+
+ "] from SDK"
191193
)
192194
);
193195
}
@@ -206,13 +208,11 @@ public void testGetClientRegionFallbackToUsEast1() {
206208
final var clientName = randomBoolean() ? "default" : randomIdentifier();
207209

208210
MockLog.assertThatLogger(
209-
() -> assertSame(
210-
Region.US_EAST_1,
211-
s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))
212-
),
211+
() -> assertNull(s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))),
213212
S3Service.class,
214213
new MockLog.SeenEventExpectation("warning", S3Service.class.getCanonicalName(), Level.WARN, """
215-
found S3 client with no configured region, falling back to [us-east-1]; \
214+
found S3 client with no configured region and no configured endpoint, \
215+
falling back to [us-east-1] and enabling cross-region access; \
216216
to suppress this warning, configure the [s3.client.CLIENT_NAME.region] setting on this node""")
217217
);
218218
}

0 commit comments

Comments
 (0)