mirrors
/
elasticsearch
mirror of https://mirror.osredm.com/root/elasticsearch.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reinstate use of S3 `protocol` client setting (#127744) 

The `s3.client.CLIENT_NAME.protocol` setting became unused in #126843 as
it is inapplicable in the v2 SDK. However, the v2 SDK requires the
`s3.client.CLIENT_NAME.endpoint` setting to be a URL that includes a
scheme, so in #127489 we prepend a `https://` to the endpoint if needed.
This commit generalizes this slightly so that we prepend `http://` if
the endpoint has no scheme and the `.protocol` setting is set to `http`.
This commit is contained in:
David Turner 2025-05-07 10:02:22 +01:00 committed by GitHub
parent 9e3ae5b224
commit d934a0c540
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 162 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
docs/changelog/126843.yaml
View File

@ -42,8 +42,7 @@ breaking:
`com.amazonaws.sdk.ec2MetadataServiceEndpointOverride` system property.
* AWS SDK v2 does not permit specifying a choice between HTTP and HTTPS so
the `s3.client.${CLIENT_NAME}.protocol` setting is deprecated and no longer
has any effect.
the `s3.client.${CLIENT_NAME}.protocol` setting is deprecated.
* AWS SDK v2 does not permit control over throttling for retries, so the
the `s3.client.${CLIENT_NAME}.use_throttle_retries` setting is deprecated
@ -81,9 +80,9 @@ breaking:
* If applicable, discontinue use of the
`com.amazonaws.sdk.ec2MetadataServiceEndpointOverride` system property.
* If applicable, specify that you wish to use the insecure HTTP protocol to
access the S3 API by setting `s3.client.${CLIENT_NAME}.endpoint` to a URL
which starts with `http://`.
* If applicable, specify the protocol to use to access the S3 API by
setting `s3.client.${CLIENT_NAME}.endpoint` to a URL which starts with
`http://` or `https://`.
* If applicable, discontinue use of the `log-delivery-write` canned ACL.

85
modules/repository-s3/src/javaRestTest/java/org/elasticsearch/repositories/s3/RepositoryS3ExplicitProtocolRestIT.java Normal file
View File

@ -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 fixture.aws.DynamicRegionSupplier;
import fixture.s3.S3HttpFixture;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.fixtures.testcontainers.TestContainersThreadFilter;
import org.junit.ClassRule;
import org.junit.rules.RuleChain;
import org.junit.rules.TestRule;
import java.util.function.Supplier;
import static fixture.aws.AwsCredentialsUtils.fixedAccessKey;
import static org.hamcrest.Matchers.startsWith;
@ThreadLeakFilters(filters = { TestContainersThreadFilter.class })
@ThreadLeakScope(ThreadLeakScope.Scope.NONE) // https://github.com/elastic/elasticsearch/issues/102482
public class RepositoryS3ExplicitProtocolRestIT extends AbstractRepositoryS3RestTestCase {
private static final String PREFIX = getIdentifierPrefix("RepositoryS3ExplicitProtocolRestIT");
private static final String BUCKET = PREFIX + "bucket";
private static final String BASE_PATH = PREFIX + "base_path";
private static final String ACCESS_KEY = PREFIX + "access-key";
private static final String SECRET_KEY = PREFIX + "secret-key";
private static final String CLIENT = "explicit_protocol_client";
private static final Supplier<String> regionSupplier = new DynamicRegionSupplier();
private static final S3HttpFixture s3Fixture = new S3HttpFixture(
true,
BUCKET,
BASE_PATH,
fixedAccessKey(ACCESS_KEY, regionSupplier, "s3")
);
private static String getEndpoint() {
final var s3FixtureAddress = s3Fixture.getAddress();
assertThat(s3FixtureAddress, startsWith("http://"));
return s3FixtureAddress.substring("http://".length());
}
public static ElasticsearchCluster cluster = ElasticsearchCluster.local()
.module("repository-s3")
.systemProperty("aws.region", regionSupplier)
.keystore("s3.client." + CLIENT + ".access_key", ACCESS_KEY)
.keystore("s3.client." + CLIENT + ".secret_key", SECRET_KEY)
.setting("s3.client." + CLIENT + ".endpoint", RepositoryS3ExplicitProtocolRestIT::getEndpoint)
.setting("s3.client." + CLIENT + ".protocol", () -> "http")
.build();
@ClassRule
public static TestRule ruleChain = RuleChain.outerRule(s3Fixture).around(cluster);
@Override
protected String getTestRestCluster() {
return cluster.getHttpAddresses();
}
@Override
protected String getBucketName() {
return BUCKET;
}
@Override
protected String getBasePath() {
return BASE_PATH;
}
@Override
protected String getClientName() {
return CLIENT;
}
}

18
modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3ClientSettings.java
View File

@ -77,9 +77,10 @@ final class S3ClientSettings {
key -> new Setting<>(key, "", s -> s.toLowerCase(Locale.ROOT), Property.NodeScope)
);
/** Formerly the protocol to use to connect to s3, now unused. V2 AWS SDK can infer the protocol from {@link #endpoint}. */
/** The protocol to use to connect to s3, now only used if {@link #endpoint} is not a proper URI that starts with {@code http://} or
* {@code https://}. */
@UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION) // no longer used, should be removed in v10
static final Setting.AffixSetting<HttpScheme> UNUSED_PROTOCOL_SETTING = Setting.affixKeySetting(
static final Setting.AffixSetting<HttpScheme> PROTOCOL_SETTING = Setting.affixKeySetting(
PREFIX,
"protocol",
key -> new Setting<>(key, "https", s -> HttpScheme.valueOf(s.toUpperCase(Locale.ROOT)), Property.NodeScope, Property.Deprecated)
@ -181,6 +182,9 @@ final class S3ClientSettings {
/** Credentials to authenticate with s3. */
final AwsCredentials credentials;
/** The scheme (HTTP or HTTPS) for talking to the endpoint, for use only if the endpoint doesn't contain an explicit scheme */
final HttpScheme protocol;
/** The s3 endpoint the client should talk to, or empty string to use the default. */
final String endpoint;
@ -221,6 +225,7 @@ final class S3ClientSettings {
private S3ClientSettings(
AwsCredentials credentials,
HttpScheme protocol,
String endpoint,
String proxyHost,
int proxyPort,
@ -235,6 +240,7 @@ final class S3ClientSettings {
String region
) {
this.credentials = credentials;
this.protocol = protocol;
this.endpoint = endpoint;
this.proxyHost = proxyHost;
this.proxyPort = proxyPort;
@ -261,6 +267,7 @@ final class S3ClientSettings {
.put(repositorySettings)
.normalizePrefix(PREFIX + PLACEHOLDER_CLIENT + '.')
.build();
final HttpScheme newProtocol = getRepoSettingOrDefault(PROTOCOL_SETTING, normalizedSettings, protocol);
final String newEndpoint = getRepoSettingOrDefault(ENDPOINT_SETTING, normalizedSettings, endpoint);
final String newProxyHost = getRepoSettingOrDefault(PROXY_HOST_SETTING, normalizedSettings, proxyHost);
@ -284,7 +291,8 @@ final class S3ClientSettings {
newCredentials = credentials;
}
final String newRegion = getRepoSettingOrDefault(REGION, normalizedSettings, region);
if (Objects.equals(endpoint, newEndpoint)
if (Objects.equals(protocol, newProtocol)
&& Objects.equals(endpoint, newEndpoint)
&& Objects.equals(proxyHost, newProxyHost)
&& proxyPort == newProxyPort
&& proxyScheme == newProxyScheme
@ -299,6 +307,7 @@ final class S3ClientSettings {
}
return new S3ClientSettings(
newCredentials,
newProtocol,
newEndpoint,
newProxyHost,
newProxyPort,
@ -405,6 +414,7 @@ final class S3ClientSettings {
) {
return new S3ClientSettings(
S3ClientSettings.loadCredentials(settings, clientName),
getConfigValue(settings, clientName, PROTOCOL_SETTING),
getConfigValue(settings, clientName, ENDPOINT_SETTING),
getConfigValue(settings, clientName, PROXY_HOST_SETTING),
getConfigValue(settings, clientName, PROXY_PORT_SETTING),
@ -435,6 +445,7 @@ final class S3ClientSettings {
&& maxConnections == that.maxConnections
&& maxRetries == that.maxRetries
&& Objects.equals(credentials, that.credentials)
&& Objects.equals(protocol, that.protocol)
&& Objects.equals(endpoint, that.endpoint)
&& Objects.equals(proxyHost, that.proxyHost)
&& proxyScheme == that.proxyScheme
@ -448,6 +459,7 @@ final class S3ClientSettings {
public int hashCode() {
return Objects.hash(
credentials,
protocol,
endpoint,
proxyHost,
proxyPort,

2
modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3RepositoryPlugin.java
View File

@ -131,7 +131,7 @@ public class S3RepositoryPlugin extends Plugin implements RepositoryPlugin, Relo
S3ClientSettings.SECRET_KEY_SETTING,
S3ClientSettings.SESSION_TOKEN_SETTING,
S3ClientSettings.ENDPOINT_SETTING,
S3ClientSettings.UNUSED_PROTOCOL_SETTING,
S3ClientSettings.PROTOCOL_SETTING,
S3ClientSettings.PROXY_HOST_SETTING,
S3ClientSettings.PROXY_PORT_SETTING,
S3ClientSettings.PROXY_SCHEME_SETTING,

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

@ -257,14 +257,18 @@ class S3Service extends AbstractLifecycleComponent {
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.
// absent, we'll look at the deprecated .protocol setting
// See https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/client-configuration.html#client-config-other-diffs
endpoint = "https://" + endpoint;
endpoint = switch (clientSettings.protocol) {
case HTTP -> "http://" + endpoint;
case HTTPS -> "https://" + endpoint;
};
LOGGER.warn(
"""
found S3 client with endpoint [{}] that is missing a scheme, guessing it should use 'https://'; \
found S3 client with endpoint [{}] that is missing a scheme, guessing it should be [{}]; \
to suppress this warning, add a scheme prefix to the [{}] setting on this node""",
clientSettings.endpoint,
endpoint,
S3ClientSettings.ENDPOINT_SETTING.getConcreteSettingForNamespace("CLIENT_NAME").getKey()
);
}

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

@ -34,6 +34,7 @@ public class S3ClientSettingsTests extends ESTestCase {
final S3ClientSettings defaultSettings = settings.get("default");
assertThat(defaultSettings.credentials, nullValue());
assertThat(defaultSettings.protocol, is(HttpScheme.HTTPS));
assertThat(defaultSettings.endpoint, is(emptyString()));
assertThat(defaultSettings.proxyHost, is(emptyString()));
assertThat(defaultSettings.proxyPort, is(80));

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

@ -14,7 +14,6 @@ import software.amazon.awssdk.core.retry.RetryPolicyContext;
import software.amazon.awssdk.core.retry.conditions.RetryCondition;
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;
import software.amazon.awssdk.services.s3.model.S3Exception;
@ -223,20 +222,57 @@ public class S3ServiceTests extends ESTestCase {
}
public void testEndpointOverrideSchemeDefaultsToHttpsWhenNotSpecified() {
final S3Service s3Service = new S3Service(
final var endpointWithoutScheme = randomIdentifier() + ".ignore";
final var clientName = randomIdentifier();
assertThat(
getEndpointUri(Settings.builder().put("s3.client." + clientName + ".endpoint", endpointWithoutScheme), clientName),
equalTo(URI.create("https://" + endpointWithoutScheme))
);
}
public void testEndpointOverrideSchemeUsesHttpsIfHttpsProtocolSpecified() {
final var endpointWithoutScheme = randomIdentifier() + ".ignore";
final var clientName = randomIdentifier();
assertThat(
getEndpointUri(
Settings.builder()
.put("s3.client." + clientName + ".endpoint", endpointWithoutScheme)
.put("s3.client." + clientName + ".protocol", "https"),
clientName
),
equalTo(URI.create("https://" + endpointWithoutScheme))
);
assertWarnings(Strings.format("""
[s3.client.%s.protocol] setting was deprecated in Elasticsearch and will be removed in a future release. \
See the breaking changes documentation for the next major version.""", clientName));
}
public void testEndpointOverrideSchemeUsesHttpIfHttpProtocolSpecified() {
final var endpointWithoutScheme = randomIdentifier() + ".ignore";
final var clientName = randomIdentifier();
assertThat(
getEndpointUri(
Settings.builder()
.put("s3.client." + clientName + ".endpoint", endpointWithoutScheme)
.put("s3.client." + clientName + ".protocol", "http"),
clientName
),
equalTo(URI.create("http://" + endpointWithoutScheme))
);
assertWarnings(Strings.format("""
[s3.client.%s.protocol] setting was deprecated in Elasticsearch and will be removed in a future release. \
See the breaking changes documentation for the next major version.""", clientName));
}
private static URI getEndpointUri(Settings.Builder settings, String clientName) {
return 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)));
() -> Region.of(randomIdentifier())
).buildClient(S3ClientSettings.getClientSettings(settings.build(), clientName), mock(SdkHttpClient.class))
.serviceClientConfiguration()
.endpointOverride()
.get();
}
}