Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide an Option to Use Path-Style-Access with S3 Repo #41966

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions docs/plugins/repository-s3.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,20 @@ settings belong in the `elasticsearch.yml` file.
Whether retries should be throttled (i.e. should back off). Must be `true`
or `false`. Defaults to `true`.

`path_style_access`::

Whether to force the use of the path style access pattern. If `true`, the
path style access pattern will be used. If `false`, the access pattern will
be automatically determined by the AWS Java SDK (See
https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3Builder.html#setPathStyleAccessEnabled-java.lang.Boolean-[AWS
documentation] for details). Defaults to `false`.

[[repository-s3-path-style-deprecation]]
NOTE: In versions `7.0`, `7.1`, `7.2` and `7.3` all bucket operations used the
https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/[now-deprecated]
path style access pattern. If your deployment requires the path style access
pattern then you should set this setting to `true` when upgrading.

[float]
[[repository-s3-compatible-services]]
===== S3-compatible services
Expand Down Expand Up @@ -381,10 +395,6 @@ bucket, in this example, named "foo".
The bucket needs to exist to register a repository for snapshots. If you did not
create the bucket then the repository registration will fail.

Note: Starting in version 7.0, all bucket operations are using the path style
access pattern. In previous versions the decision to use virtual hosted style or
path style access was made by the AWS Java SDK.

[[repository-s3-aws-vpc]]
[float]
==== AWS VPC Bandwidth Settings
Expand Down
15 changes: 15 additions & 0 deletions docs/reference/migration/migrate_8_0/snapshots.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,18 @@ This change will affect both newly created repositories and existing repositorie
explicitly specified.

For more information on the compress option, see <<modules-snapshots>>

[float]
==== The S3 repository plugin uses the DNS style access pattern by default

Starting in version 7.4 the `repository-s3` plugin does not use the
now-deprecated path-style access pattern by default. In versions 7.0, 7.1, 7.2
and 7.3 the `repository-s3` plugin always used the path-style access pattern.
This is a breaking change for deployments that only support path-style access
but which are recognized as supporting DNS-style access by the AWS SDK. If your
deployment only supports path-style access and is affected by this change then
you must configure the S3 client setting `path_style_access` to `true`. This
breaking change was made necessary by
https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/[AWS's
announcement] that the path-style access pattern is deprecated and will be
unsupported on buckets created after September 30th 2020.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ final class S3ClientSettings {
static final Setting.AffixSetting<Boolean> USE_THROTTLE_RETRIES_SETTING = Setting.affixKeySetting(PREFIX, "use_throttle_retries",
key -> Setting.boolSetting(key, ClientConfiguration.DEFAULT_THROTTLE_RETRIES, Property.NodeScope));

/** Whether the s3 client should use path style access. */
static final Setting.AffixSetting<Boolean> USE_PATH_STYLE_ACCESS = Setting.affixKeySetting(PREFIX, "path_style_access",
key -> Setting.boolSetting(key, false, Property.NodeScope));

/** Credentials to authenticate with s3. */
final S3BasicCredentials credentials;

Expand Down Expand Up @@ -127,9 +131,13 @@ final class S3ClientSettings {
/** Whether the s3 client should use an exponential backoff retry policy. */
final boolean throttleRetries;

/** Whether the s3 client should use path style access. */
final boolean pathStyleAccess;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AWS SDK uses this as a ternary setting (true, false, null), with null being the default, which enables the path-style setting based on the configured endpoint. I think we should do treat this setting exactly the same way, allowing path-style setting both being explicitly enabled and disabled.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, so we break behavior after all by changing the default to null? See my comment above, this may not be necessary (at least in 7.x)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear what the behaviour of setPathStyleAccessEnabled(Boolean.FALSE) is from the SDK docs. I think we're doing the right thing here, either enabling it or leaving it unset.


private S3ClientSettings(S3BasicCredentials credentials, String endpoint, Protocol protocol,
String proxyHost, int proxyPort, String proxyUsername, String proxyPassword,
int readTimeoutMillis, int maxRetries, boolean throttleRetries) {
int readTimeoutMillis, int maxRetries, boolean throttleRetries,
boolean pathStyleAccess) {
this.credentials = credentials;
this.endpoint = endpoint;
this.protocol = protocol;
Expand All @@ -140,6 +148,7 @@ private S3ClientSettings(S3BasicCredentials credentials, String endpoint, Protoc
this.readTimeoutMillis = readTimeoutMillis;
this.maxRetries = maxRetries;
this.throttleRetries = throttleRetries;
this.pathStyleAccess = pathStyleAccess;
}

/**
Expand All @@ -162,6 +171,7 @@ S3ClientSettings refine(RepositoryMetaData metadata) {
getRepoSettingOrDefault(READ_TIMEOUT_SETTING, normalizedSettings, TimeValue.timeValueMillis(readTimeoutMillis)).millis());
final int newMaxRetries = getRepoSettingOrDefault(MAX_RETRIES_SETTING, normalizedSettings, maxRetries);
final boolean newThrottleRetries = getRepoSettingOrDefault(USE_THROTTLE_RETRIES_SETTING, normalizedSettings, throttleRetries);
final boolean usePathStyleAccess = getRepoSettingOrDefault(USE_PATH_STYLE_ACCESS, normalizedSettings, pathStyleAccess);
final S3BasicCredentials newCredentials;
if (checkDeprecatedCredentials(repoSettings)) {
newCredentials = loadDeprecatedCredentials(repoSettings);
Expand All @@ -183,7 +193,8 @@ S3ClientSettings refine(RepositoryMetaData metadata) {
proxyPassword,
newReadTimeoutMillis,
newMaxRetries,
newThrottleRetries
newThrottleRetries,
usePathStyleAccess
);
}

Expand Down Expand Up @@ -270,7 +281,8 @@ static S3ClientSettings getClientSettings(final Settings settings, final String
proxyPassword.toString(),
Math.toIntExact(getConfigValue(settings, clientName, READ_TIMEOUT_SETTING).millis()),
getConfigValue(settings, clientName, MAX_RETRIES_SETTING),
getConfigValue(settings, clientName, USE_THROTTLE_RETRIES_SETTING)
getConfigValue(settings, clientName, USE_THROTTLE_RETRIES_SETTING),
getConfigValue(settings, clientName, USE_PATH_STYLE_ACCESS)
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public List<Setting<?>> getSettings() {
S3ClientSettings.READ_TIMEOUT_SETTING,
S3ClientSettings.MAX_RETRIES_SETTING,
S3ClientSettings.USE_THROTTLE_RETRIES_SETTING,
S3ClientSettings.USE_PATH_STYLE_ACCESS,
S3Repository.ACCESS_KEY_SETTING,
S3Repository.SECRET_KEY_SETTING);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,10 @@ AmazonS3 buildClient(final S3ClientSettings clientSettings) {
//
// We do this because directly constructing the client is deprecated (was already deprecated in 1.1.223 too)
// so this change removes that usage of a deprecated API.
builder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(endpoint, null))
.enablePathStyleAccess();

builder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(endpoint, null));
if (clientSettings.pathStyleAccess) {
builder.enablePathStyleAccess();
}
return builder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,11 @@ public void testRefineWithRepoSettings() {
assertThat(credentials.getSessionToken(), is("session_token"));
}
}

public void testPathStyleAccessCanBeSet() {
final Map<String, S3ClientSettings> settings = S3ClientSettings.load(
Settings.builder().put("s3.client.other.path_style_access", true).build());
assertThat(settings.get("default").pathStyleAccess, is(false));
assertThat(settings.get("other").pathStyleAccess, is(true));
}
}