From 75b90922171d32aa1483aebcd74078b31dda982a Mon Sep 17 00:00:00 2001 From: Nick Tindall Date: Mon, 5 May 2025 18:47:23 +1000 Subject: [PATCH] Default S3 endpoint scheme to HTTPS when not specified (#127489) (cherry picked from commit 8dc6bf8893a800b59bb4d2549411ce771843a5cf) --- .../repositories/s3/S3Service.java | 16 +++++++++++++- .../repositories/s3/S3ServiceTests.java | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) 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 e4b774e414b9a..07dfb332cbf8c 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 @@ -254,7 +254,21 @@ protected S3ClientBuilder buildClientBuilder(S3ClientSettings clientSettings, Sd } if (Strings.hasLength(clientSettings.endpoint)) { - s3clientBuilder.endpointOverride(URI.create(clientSettings.endpoint)); + String endpoint = clientSettings.endpoint; + if ((endpoint.startsWith("http://") || endpoint.startsWith("https://")) == false) { + // The SDK does not know how to interpret endpoints without a scheme prefix and will error. Therefore, when the scheme is + // absent, we'll supply HTTPS as a default to avoid errors. + // See https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/client-configuration.html#client-config-other-diffs + endpoint = "https://" + endpoint; + LOGGER.warn( + """ + found S3 client with endpoint [{}] that is missing a scheme, guessing it should use 'https://'; \ + to suppress this warning, add a scheme prefix to the [{}] setting on this node""", + clientSettings.endpoint, + S3ClientSettings.ENDPOINT_SETTING.getConcreteSettingForNamespace("CLIENT_NAME").getKey() + ); + } + s3clientBuilder.endpointOverride(URI.create(endpoint)); } return s3clientBuilder; 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 2701476afa71b..d0d3c8df7c9f5 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 @@ -8,7 +8,9 @@ */ 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 software.amazon.awssdk.services.s3.endpoints.S3EndpointParams; import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointProvider; @@ -23,8 +25,10 @@ import org.elasticsearch.watcher.ResourceWatcherService; import java.io.IOException; +import java.net.URI; import java.util.concurrent.atomic.AtomicBoolean; +import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; public class S3ServiceTests extends ESTestCase { @@ -184,4 +188,22 @@ public void testGetClientRegionFallbackToUsEast1() { ); } } + + public void testEndpointOverrideSchemeDefaultsToHttpsWhenNotSpecified() { + final S3Service s3Service = new S3Service( + mock(Environment.class), + Settings.EMPTY, + mock(ResourceWatcherService.class), + () -> Region.of("es-test-region") + ); + final String endpointWithoutScheme = randomIdentifier() + ".ignore"; + S3Client s3Client = s3Service.buildClient( + S3ClientSettings.getClientSettings( + Settings.builder().put("s3.client.test-client.endpoint", endpointWithoutScheme).build(), + "test-client" + ), + mock(SdkHttpClient.class) + ); + assertThat(s3Client.serviceClientConfiguration().endpointOverride().get(), equalTo(URI.create("https://" + endpointWithoutScheme))); + } }