Skip to content

Commit f8401be

Browse files
nicktindallywangd
authored andcommitted
Default S3 endpoint scheme to HTTPS when not specified (elastic#127489)
1 parent ff7ffb6 commit f8401be

File tree

2 files changed

+37
-1
lines changed

2 files changed

+37
-1
lines changed

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,21 @@ protected S3ClientBuilder buildClientBuilder(S3ClientSettings clientSettings, Sd
304304
}
305305

306306
if (Strings.hasLength(clientSettings.endpoint)) {
307-
s3clientBuilder.endpointOverride(URI.create(clientSettings.endpoint));
307+
String endpoint = clientSettings.endpoint;
308+
if ((endpoint.startsWith("http://") || endpoint.startsWith("https://")) == false) {
309+
// The SDK does not know how to interpret endpoints without a scheme prefix and will error. Therefore, when the scheme is
310+
// absent, we'll supply HTTPS as a default to avoid errors.
311+
// See https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/client-configuration.html#client-config-other-diffs
312+
endpoint = "https://" + endpoint;
313+
LOGGER.warn(
314+
"""
315+
found S3 client with endpoint [{}] that is missing a scheme, guessing it should use 'https://'; \
316+
to suppress this warning, add a scheme prefix to the [{}] setting on this node""",
317+
clientSettings.endpoint,
318+
S3ClientSettings.ENDPOINT_SETTING.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
319+
);
320+
}
321+
s3clientBuilder.endpointOverride(URI.create(endpoint));
308322
}
309323

310324
return s3clientBuilder;

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
import software.amazon.awssdk.awscore.exception.AwsServiceException;
1313
import software.amazon.awssdk.core.retry.RetryPolicyContext;
1414
import software.amazon.awssdk.core.retry.conditions.RetryCondition;
15+
import software.amazon.awssdk.http.SdkHttpClient;
1516
import software.amazon.awssdk.regions.Region;
17+
import software.amazon.awssdk.services.s3.S3Client;
1618
import software.amazon.awssdk.services.s3.endpoints.S3EndpointParams;
1719
import software.amazon.awssdk.services.s3.endpoints.internal.DefaultS3EndpointProvider;
1820
import software.amazon.awssdk.services.s3.model.S3Exception;
@@ -32,8 +34,10 @@
3234
import org.elasticsearch.watcher.ResourceWatcherService;
3335

3436
import java.io.IOException;
37+
import java.net.URI;
3538
import java.util.concurrent.atomic.AtomicBoolean;
3639

40+
import static org.hamcrest.Matchers.equalTo;
3741
import static org.mockito.Mockito.mock;
3842

3943
public class S3ServiceTests extends ESTestCase {
@@ -272,4 +276,22 @@ public void testGetClientRegionFallbackToUsEast1() {
272276
);
273277
}
274278
}
279+
280+
public void testEndpointOverrideSchemeDefaultsToHttpsWhenNotSpecified() {
281+
final S3Service s3Service = new S3Service(
282+
mock(Environment.class),
283+
Settings.EMPTY,
284+
mock(ResourceWatcherService.class),
285+
() -> Region.of("es-test-region")
286+
);
287+
final String endpointWithoutScheme = randomIdentifier() + ".ignore";
288+
S3Client s3Client = s3Service.buildClient(
289+
S3ClientSettings.getClientSettings(
290+
Settings.builder().put("s3.client.test-client.endpoint", endpointWithoutScheme).build(),
291+
"test-client"
292+
),
293+
mock(SdkHttpClient.class)
294+
);
295+
assertThat(s3Client.serviceClientConfiguration().endpointOverride().get(), equalTo(URI.create("https://" + endpointWithoutScheme)));
296+
}
275297
}

0 commit comments

Comments
 (0)