From ffac6ed1f8952718a02fb621f6aabd308b6aa096 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 1 Sep 2025 09:36:32 +0100 Subject: [PATCH] Delay S3 repo warning if default region absent (#133848) Today we emit a log message at startup if we fail to look up the default region for use by the S3 repository. However, this is only a potential issue if the default region is needed. If the user doesn't use `repository-s3` at all, or specifies the `region` for each S3 repository, then the absence of the default region is not an issue. With this commit we delay logging anything until we encounter a situation where we actually need the default region for something. --- docs/changelog/133848.yaml | 5 + .../s3/RepositoryCredentialsTests.java | 3 +- .../s3/S3DefaultRegionHolder.java | 61 +++++++ .../repositories/s3/S3RepositoryPlugin.java | 7 +- .../repositories/s3/S3Service.java | 10 +- .../s3/S3BlobContainerRetriesTests.java | 5 +- .../s3/S3ClientSettingsTests.java | 10 +- .../s3/S3DefaultRegionHolderTests.java | 85 +++++++++ .../repositories/s3/S3RepositoryTests.java | 3 +- .../repositories/s3/S3ServiceTests.java | 171 ++++++++++++------ 10 files changed, 290 insertions(+), 70 deletions(-) create mode 100644 docs/changelog/133848.yaml create mode 100644 modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolder.java create mode 100644 modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolderTests.java diff --git a/docs/changelog/133848.yaml b/docs/changelog/133848.yaml new file mode 100644 index 0000000000000..107513d21555b --- /dev/null +++ b/docs/changelog/133848.yaml @@ -0,0 +1,5 @@ +pr: 133848 +summary: Delay S3 repo warning if default region absent +area: Snapshot/Restore +type: bug +issues: [] diff --git a/modules/repository-s3/qa/insecure-credentials/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java b/modules/repository-s3/qa/insecure-credentials/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java index 420794ed75d75..f879477ab064b 100644 --- a/modules/repository-s3/qa/insecure-credentials/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java +++ b/modules/repository-s3/qa/insecure-credentials/src/test/java/org/elasticsearch/repositories/s3/RepositoryCredentialsTests.java @@ -12,6 +12,7 @@ import software.amazon.awssdk.auth.credentials.AwsCredentials; import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3Client; import org.apache.logging.log4j.LogManager; @@ -306,7 +307,7 @@ public static final class ProxyS3Service extends S3Service { ProjectResolver projectResolver, ResourceWatcherService resourceWatcherService ) { - super(environment, clusterService, projectResolver, resourceWatcherService, () -> null); + super(environment, clusterService, projectResolver, resourceWatcherService, () -> Region.of(randomIdentifier())); } @Override diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolder.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolder.java new file mode 100644 index 0000000000000..9a76616c3afb6 --- /dev/null +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolder.java @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.repositories.s3; + +import software.amazon.awssdk.regions.Region; + +import org.elasticsearch.common.util.concurrent.RunOnce; +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; + +import java.util.function.Supplier; + +/** + * Holds onto the {@link Region} provided by the given supplier (think: the AWS SDK's default provider chain) in case it's needed for a S3 + * repository. If the supplier fails with an exception, the first call to {@link #getDefaultRegion} will log a warning message recording + * the exception. + */ +class S3DefaultRegionHolder { + + private static final Logger logger = LogManager.getLogger(S3DefaultRegionHolder.class); + + // no synchronization required, assignments happen in start() which happens-before all reads + private Region defaultRegion; + private Runnable defaultRegionFailureLogger = () -> {}; + + private final Runnable initializer; + + /** + * @param delegateRegionSupplier Supplies a non-null {@link Region} or throws a {@link RuntimeException}. + *

+ * Retained until its first-and-only invocation when {@link #start()} is called, and then released. + */ + S3DefaultRegionHolder(Supplier delegateRegionSupplier) { + initializer = new RunOnce(() -> { + try { + defaultRegion = delegateRegionSupplier.get(); + assert defaultRegion != null; + } catch (Exception e) { + defaultRegion = null; + defaultRegionFailureLogger = new RunOnce(() -> logger.warn("failed to obtain region from default provider chain", e)); + } + }); + } + + void start() { + initializer.run(); + } + + Region getDefaultRegion() { + assert defaultRegion != null || defaultRegionFailureLogger instanceof RunOnce : "not initialized"; + defaultRegionFailureLogger.run(); + return defaultRegion; + } +} diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java index ab836040efa9a..59f12d2b9a716 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java @@ -98,12 +98,7 @@ S3Service s3Service( } private static Region getDefaultRegion() { - try { - return DefaultAwsRegionProviderChain.builder().build().getRegion(); - } catch (Exception e) { - logger.info("failed to obtain region from default provider chain", e); - return null; - } + return DefaultAwsRegionProviderChain.builder().build().getRegion(); } @Override diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java index 51e1681129b82..fc18f4bc97a84 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3Service.java @@ -48,7 +48,6 @@ import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.RunOnce; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.Releasable; import org.elasticsearch.core.Releasables; @@ -94,8 +93,7 @@ class S3Service extends AbstractLifecycleComponent { Setting.Property.NodeScope ); - private final Runnable defaultRegionSetter; - private volatile Region defaultRegion; + private final S3DefaultRegionHolder defaultRegionHolder; /** * Use a signer that does not require to pre-read (and checksum) the body of PutObject and UploadPart requests since we can rely on @@ -129,7 +127,7 @@ class S3Service extends AbstractLifecycleComponent { compareAndExchangeTimeToLive = REPOSITORY_S3_CAS_TTL_SETTING.get(nodeSettings); compareAndExchangeAntiContentionDelay = REPOSITORY_S3_CAS_ANTI_CONTENTION_DELAY_SETTING.get(nodeSettings); isStateless = DiscoveryNode.isStateless(nodeSettings); - defaultRegionSetter = new RunOnce(() -> defaultRegion = defaultRegionSupplier.get()); + defaultRegionHolder = new S3DefaultRegionHolder(defaultRegionSupplier); s3ClientsManager = new S3ClientsManager( nodeSettings, this::buildClientReference, @@ -266,7 +264,7 @@ Region getClientRegion(S3ClientSettings clientSettings) { } else { endpointDescription = "no configured endpoint"; } - final var defaultRegion = this.defaultRegion; + final var defaultRegion = defaultRegionHolder.getDefaultRegion(); if (defaultRegion != null) { LOGGER.debug(""" found S3 client with no configured region and {}, using region [{}] from SDK""", endpointDescription, defaultRegion); @@ -415,7 +413,7 @@ public void onBlobStoreClose(@Nullable ProjectId projectId) { @Override protected void doStart() { - defaultRegionSetter.run(); + defaultRegionHolder.start(); } @Override diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java index 8cc19fd4c870d..4da5cb43e7f44 100644 --- a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobContainerRetriesTests.java @@ -101,6 +101,7 @@ import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_CONNECTIONS_SETTING; import static org.elasticsearch.repositories.s3.S3ClientSettings.MAX_RETRIES_SETTING; import static org.elasticsearch.repositories.s3.S3ClientSettings.READ_TIMEOUT_SETTING; +import static org.elasticsearch.repositories.s3.S3ClientSettingsTests.DEFAULT_REGION_UNAVAILABLE; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.contains; @@ -134,7 +135,7 @@ public void setUp() throws Exception { ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()), TestProjectResolvers.DEFAULT_PROJECT_ONLY, Mockito.mock(ResourceWatcherService.class), - () -> null + DEFAULT_REGION_UNAVAILABLE ) { private InetAddress[] resolveHost(String host) throws UnknownHostException { assertEquals("127.0.0.1", host); @@ -1323,7 +1324,7 @@ public void testRetryOn403InStateless() { ), TestProjectResolvers.DEFAULT_PROJECT_ONLY, Mockito.mock(ResourceWatcherService.class), - () -> null + DEFAULT_REGION_UNAVAILABLE ); service.start(); recordingMeterRegistry = new RecordingMeterRegistry(); diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java index 26b9aab5569e1..927ef00b1106a 100644 --- a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ClientSettingsTests.java @@ -13,6 +13,7 @@ import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; import software.amazon.awssdk.regions.Region; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cluster.project.TestProjectResolvers; import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; @@ -24,6 +25,7 @@ import org.mockito.Mockito; import java.util.Map; +import java.util.function.Supplier; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.emptyString; @@ -190,9 +192,11 @@ public void testRegionCanBeSet() { ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()), TestProjectResolvers.DEFAULT_PROJECT_ONLY, Mockito.mock(ResourceWatcherService.class), - () -> null + DEFAULT_REGION_UNAVAILABLE ) ) { + s3Service.start(); + var otherSettings = settings.get("other"); Region otherRegion = s3Service.getClientRegion(otherSettings); assertEquals(randomRegion, otherRegion.toString()); @@ -213,4 +217,8 @@ public void testMaxConnectionsCanBeSet() { // the default appears in the docs so let's make sure it doesn't change: assertEquals(50, S3ClientSettings.Defaults.MAX_CONNECTIONS); } + + public static final Supplier DEFAULT_REGION_UNAVAILABLE = () -> { + throw new ElasticsearchException("default region unavailable in this test"); + }; } diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolderTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolderTests.java new file mode 100644 index 0000000000000..3471ce0205b06 --- /dev/null +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3DefaultRegionHolderTests.java @@ -0,0 +1,85 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.repositories.s3; + +import software.amazon.awssdk.regions.Region; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.MockLog; + +import java.util.concurrent.atomic.AtomicBoolean; + +public class S3DefaultRegionHolderTests extends ESTestCase { + public void testSuccess() { + try (var mockLog = MockLog.capture(S3DefaultRegionHolder.class)) { + mockLog.addExpectation(new NoLogEventsExpectation()); + + final var region = Region.of(randomIdentifier()); + final var regionSupplied = new AtomicBoolean(); + final var regionHolder = new S3DefaultRegionHolder(() -> { + assertTrue(regionSupplied.compareAndSet(false, true)); // only called once + return region; + }); + + regionHolder.start(); + assertTrue(regionSupplied.get()); + assertSame(region, regionHolder.getDefaultRegion()); + assertSame(region, regionHolder.getDefaultRegion()); + + mockLog.assertAllExpectationsMatched(); + } + } + + public void testFailure() { + try (var mockLog = MockLog.capture(S3DefaultRegionHolder.class)) { + final var warningSeenExpectation = new MockLog.EventuallySeenEventExpectation( + "warning", + S3DefaultRegionHolder.class.getCanonicalName(), + Level.WARN, + "failed to obtain region from default provider chain" + ); + mockLog.addExpectation(warningSeenExpectation); + + final var regionSupplied = new AtomicBoolean(); + final var regionHolder = new S3DefaultRegionHolder(() -> { + assertTrue(regionSupplied.compareAndSet(false, true)); // only called once + throw new ElasticsearchException("simulated"); + }); + + regionHolder.start(); + assertTrue(regionSupplied.get()); + + warningSeenExpectation.setExpectSeen(); // not seen yet, but will be seen now + regionHolder.getDefaultRegion(); + + mockLog.addExpectation(new NoLogEventsExpectation()); // log message not duplicated + regionHolder.getDefaultRegion(); + + mockLog.assertAllExpectationsMatched(); + } + } + + private static class NoLogEventsExpectation implements MockLog.LoggingExpectation { + private boolean seenLogEvent; + + @Override + public void match(LogEvent event) { + seenLogEvent = true; + } + + @Override + public void assertMatched() { + assertFalse(seenLogEvent); + } + } +} diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java index d41631a79739f..7beb5ec58af20 100644 --- a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3RepositoryTests.java @@ -10,6 +10,7 @@ package org.elasticsearch.repositories.s3; import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3Client; import org.elasticsearch.cluster.metadata.ProjectId; @@ -68,7 +69,7 @@ private static class DummyS3Service extends S3Service { ProjectResolver projectResolver, ResourceWatcherService resourceWatcherService ) { - super(environment, clusterService, projectResolver, resourceWatcherService, () -> null); + super(environment, clusterService, projectResolver, resourceWatcherService, () -> Region.of(randomIdentifier())); } @Override diff --git a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java index 6c663c1594102..e7e0e206aa24b 100644 --- a/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java +++ b/modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3ServiceTests.java @@ -19,6 +19,8 @@ import software.amazon.awssdk.services.s3.model.S3Exception; import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.cluster.metadata.ProjectId; import org.elasticsearch.cluster.metadata.RepositoryMetadata; import org.elasticsearch.cluster.project.TestProjectResolvers; @@ -33,7 +35,6 @@ import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.watcher.ResourceWatcherService; -import java.io.IOException; import java.net.URI; import java.util.concurrent.atomic.AtomicBoolean; @@ -42,7 +43,7 @@ public class S3ServiceTests extends ESTestCase { - public void testCachedClientsAreReleased() throws IOException { + public void testCachedClientsAreReleased() { final S3Service s3Service = new S3Service( mock(Environment.class), ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()), @@ -107,30 +108,45 @@ public void testGetClientRegionFromSetting() { mock(ResourceWatcherService.class), () -> { assertTrue(regionRequested.compareAndSet(false, true)); - return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()), null); + if (randomBoolean()) { + throw new ElasticsearchException("simulated"); + } else { + return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier())); + } } - ) + ); + var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class) ) { + mockLog.addExpectation( + new MockLog.UnseenEventExpectation( + "no default region warning", + S3DefaultRegionHolder.class.getCanonicalName(), + Level.WARN, + "*" + ) + ); + s3Service.start(); assertTrue(regionRequested.get()); final var clientName = randomBoolean() ? "default" : randomIdentifier(); final var region = randomBoolean() ? randomFrom(Region.regions()) : Region.of(randomIdentifier()); - MockLog.assertThatLogger( - () -> assertSame( - region, - s3Service.getClientRegion( - S3ClientSettings.getClientSettings( - Settings.builder().put("s3.client." + clientName + ".region", region.id()).build(), - clientName - ) + + mockLog.addExpectation(new MockLog.UnseenEventExpectation("no warning", S3Service.class.getCanonicalName(), Level.WARN, "*")); + mockLog.addExpectation(new MockLog.UnseenEventExpectation("no debug", S3Service.class.getCanonicalName(), Level.DEBUG, "*")); + + assertSame( + region, + s3Service.getClientRegion( + S3ClientSettings.getClientSettings( + Settings.builder().put("s3.client." + clientName + ".region", region.id()).build(), + clientName ) - ), - S3Service.class, - new MockLog.UnseenEventExpectation("no warning", S3Service.class.getCanonicalName(), Level.WARN, "*"), - new MockLog.UnseenEventExpectation("no debug", S3Service.class.getCanonicalName(), Level.DEBUG, "*") + ) ); + + mockLog.assertAllExpectationsMatched(); } } @@ -145,10 +161,24 @@ public void testGetClientRegionFromEndpointSettingGuess() { mock(ResourceWatcherService.class), () -> { assertTrue(regionRequested.compareAndSet(false, true)); - return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier()), null); + if (randomBoolean()) { + throw new ElasticsearchException("simulated"); + } else { + return randomFrom(randomFrom(Region.regions()), Region.of(randomIdentifier())); + } } - ) + ); + var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class) ) { + mockLog.addExpectation( + new MockLog.UnseenEventExpectation( + "no default region warning", + S3DefaultRegionHolder.class.getCanonicalName(), + Level.WARN, + "*" + ) + ); + s3Service.start(); assertTrue(regionRequested.get()); @@ -163,18 +193,7 @@ public void testGetClientRegionFromEndpointSettingGuess() { ).url(); final var endpoint = randomFrom(endpointUrl.toString(), endpointUrl.getHost()); - MockLog.assertThatLogger( - () -> assertEquals( - endpoint, - guessedRegion, - s3Service.getClientRegion( - S3ClientSettings.getClientSettings( - Settings.builder().put("s3.client." + clientName + ".endpoint", endpoint).build(), - clientName - ) - ) - ), - S3Service.class, + mockLog.addExpectation( new MockLog.SeenEventExpectation( endpoint + " -> " + guessedRegion, S3Service.class.getCanonicalName(), @@ -188,6 +207,18 @@ public void testGetClientRegionFromEndpointSettingGuess() { ) ) ); + assertEquals( + endpoint, + guessedRegion, + s3Service.getClientRegion( + S3ClientSettings.getClientSettings( + Settings.builder().put("s3.client." + clientName + ".endpoint", endpoint).build(), + clientName + ) + ) + ); + + mockLog.assertAllExpectationsMatched(); } } @@ -205,16 +236,24 @@ public void testGetClientRegionFromDefault() { assertTrue(regionRequested.compareAndSet(false, true)); return defaultRegion; } - ) + ); + var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class) ) { + mockLog.addExpectation( + new MockLog.UnseenEventExpectation( + "no default region warning", + S3DefaultRegionHolder.class.getCanonicalName(), + Level.WARN, + "*" + ) + ); + s3Service.start(); assertTrue(regionRequested.get()); final var clientName = randomBoolean() ? "default" : randomIdentifier(); - MockLog.assertThatLogger( - () -> assertSame(defaultRegion, s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))), - S3Service.class, + mockLog.addExpectation( new MockLog.SeenEventExpectation( "warning", S3Service.class.getCanonicalName(), @@ -224,12 +263,17 @@ public void testGetClientRegionFromDefault() { + "] from SDK" ) ); + + assertSame(defaultRegion, s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))); + + mockLog.assertAllExpectationsMatched(); } } @TestLogging(reason = "testing WARN log output", value = "org.elasticsearch.repositories.s3.S3Service:WARN") public void testGetClientRegionFallbackToUsEast1() { final var regionRequested = new AtomicBoolean(); + final var exceptionMessage = randomIdentifier(); try ( var s3Service = new S3Service( mock(Environment.class), @@ -238,23 +282,39 @@ public void testGetClientRegionFallbackToUsEast1() { mock(ResourceWatcherService.class), () -> { assertTrue(regionRequested.compareAndSet(false, true)); - return null; + throw new ElasticsearchException(exceptionMessage); } - ) + ); + var mockLog = MockLog.capture(S3Service.class, S3DefaultRegionHolder.class) ) { s3Service.start(); assertTrue(regionRequested.get()); final var clientName = randomBoolean() ? "default" : randomIdentifier(); - MockLog.assertThatLogger( - () -> assertNull(s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))), - S3Service.class, - new MockLog.SeenEventExpectation("warning", S3Service.class.getCanonicalName(), Level.WARN, """ - found S3 client with no configured region and no configured endpoint, \ - falling back to [us-east-1] and enabling cross-region access; \ - to suppress this warning, configure the [s3.client.CLIENT_NAME.region] setting on this node""") + mockLog.addExpectation( + new MockLog.SeenEventExpectation( + "default provider chain failure", + S3DefaultRegionHolder.class.getCanonicalName(), + Level.WARN, + "failed to obtain region from default provider chain" + ) { + @Override + public void match(LogEvent event) { + if (event.getThrown() instanceof ElasticsearchException e && exceptionMessage.equals(e.getMessage())) { + super.match(event); + } + } + } ); + mockLog.addExpectation(new MockLog.SeenEventExpectation("warning", S3Service.class.getCanonicalName(), Level.WARN, """ + found S3 client with no configured region and no configured endpoint, \ + falling back to [us-east-1] and enabling cross-region access; \ + to suppress this warning, configure the [s3.client.CLIENT_NAME.region] setting on this node""")); + + assertNull(s3Service.getClientRegion(S3ClientSettings.getClientSettings(Settings.EMPTY, clientName))); + + mockLog.assertAllExpectationsMatched(); } } @@ -302,15 +362,20 @@ public void testEndpointOverrideSchemeUsesHttpIfHttpProtocolSpecified() { } private URI getEndpointUri(Settings.Builder settings, String clientName) { - return new S3Service( - mock(Environment.class), - ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()), - TestProjectResolvers.DEFAULT_PROJECT_ONLY, - mock(ResourceWatcherService.class), - () -> Region.of(randomIdentifier()) - ).buildClient(S3ClientSettings.getClientSettings(settings.build(), clientName), mock(SdkHttpClient.class)) - .serviceClientConfiguration() - .endpointOverride() - .get(); + try ( + var s3Service = new S3Service( + mock(Environment.class), + ClusterServiceUtils.createClusterService(new DeterministicTaskQueue().getThreadPool()), + TestProjectResolvers.DEFAULT_PROJECT_ONLY, + mock(ResourceWatcherService.class), + () -> Region.of(randomIdentifier()) + ) + ) { + s3Service.start(); + return s3Service.buildClient(S3ClientSettings.getClientSettings(settings.build(), clientName), mock(SdkHttpClient.class)) + .serviceClientConfiguration() + .endpointOverride() + .get(); + } } }