Skip to content

[Default Configuration Part 6]: apply default s3 us-east-1 regional setting #2825

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 8, 2021
Merged

[Default Configuration Part 6]: apply default s3 us-east-1 regional setting #2825

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
040b7fd
Add s3 regional setting
zoewangg Nov 4, 2021
8681855
Rename option name
zoewangg Nov 5, 2021
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
14 changes: 12 additions & 2 deletions .../software/amazon/awssdk/codegen/lite/defaultsmode/DefaultsModeConfigurationGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ public class DefaultsModeConfigurationGenerator implements PoetClass {
private static final Map<String, OptionMetadata> HTTP_CONFIGURATION_MAPPING = new HashMap<>();
private static final String CONNECT_TIMEOUT_IN_MILLIS = "connectTimeoutInMillis";
private static final String TLS_NEGOTIATION_TIMEOUT_IN_MILLIS = "tlsNegotiationTimeoutInMillis";
private static final String S3_US_EAST_1_REGIONAL_ENDPOINTS = "s3UsEast1RegionalEndpoints";

private final String basePackage;
private final String defaultsModeBase;
Expand All @@ -68,6 +69,12 @@ public class DefaultsModeConfigurationGenerator implements PoetClass {
"TLS_NEGOTIATION_TIMEOUT")));
CONFIGURATION_MAPPING.put("retryMode", new OptionMetadata(ClassName.get("software.amazon.awssdk.core.retry", "RetryMode"
), ClassName.get("software.amazon.awssdk.core.client.config", "SdkClientOption", "DEFAULT_RETRY_MODE")));

CONFIGURATION_MAPPING.put(S3_US_EAST_1_REGIONAL_ENDPOINTS,
new OptionMetadata(ClassName.get(String.class),
ClassName.get("software.amazon.awssdk.regions",
"ServiceMetadataAdvancedOption",
"S3_US_EAST_1_REGIONAL_ENDPOINT")));
}

public DefaultsModeConfigurationGenerator(String basePackage, String defaultsModeBase, DefaultConfiguration configuration) {
Expand Down Expand Up @@ -121,8 +128,8 @@ private void addStaticEnumMapBlock(TypeSpec.Builder builder) {

private void addEnumMapField(TypeSpec.Builder builder, String name) {
ParameterizedTypeName map = ParameterizedTypeName.get(ClassName.get(Map.class),
defaultsModeClassName(),
ClassName.get(AttributeMap.class));
defaultsModeClassName(),
ClassName.get(AttributeMap.class));
FieldSpec field = FieldSpec.builder(map, name, PRIVATE, STATIC, FINAL)
.initializer("new $T<>(DefaultsMode.class)", EnumMap.class).build();
builder.addField(field);
Expand Down Expand Up @@ -179,6 +186,9 @@ private void attributeMapBuilder(String option, String value, CodeBlock.Builder
attributeBuilder.add(".put($T, $T.$N)", optionMetadata.attribute, optionMetadata.type,
value.toUpperCase(Locale.US));
break;
case S3_US_EAST_1_REGIONAL_ENDPOINTS:
attributeBuilder.add(".put($T, $S)", optionMetadata.attribute, value);
break;
default:
throw new IllegalStateException("Unsupported option " + option);
}
Expand Down
13 changes: 9 additions & 4 deletions ...sources/software/amazon/awssdk/codegen/lite/defaultsmode/defaults-mode-configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import software.amazon.awssdk.core.client.config.SdkClientOption;
import software.amazon.awssdk.core.retry.RetryMode;
import software.amazon.awssdk.http.SdkHttpConfigurationOption;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.utils.AttributeMap;

/**
Expand All @@ -17,28 +18,32 @@
@Generated("software.amazon.awssdk:codegen")
public final class DefaultsModeConfiguration {
private static final AttributeMap STANDARD_DEFAULTS = AttributeMap.builder()
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD).build();
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD)
.put(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT, "regional").build();

private static final AttributeMap STANDARD_HTTP_DEFAULTS = AttributeMap.builder()
.put(SdkHttpConfigurationOption.CONNECTION_TIMEOUT, Duration.ofMillis(2000))
.put(SdkHttpConfigurationOption.TLS_NEGOTIATION_TIMEOUT, Duration.ofMillis(2000)).build();

private static final AttributeMap MOBILE_DEFAULTS = AttributeMap.builder()
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.ADAPTIVE).build();
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.ADAPTIVE)
.put(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT, "regional").build();

private static final AttributeMap MOBILE_HTTP_DEFAULTS = AttributeMap.builder()
.put(SdkHttpConfigurationOption.CONNECTION_TIMEOUT, Duration.ofMillis(10000))
.put(SdkHttpConfigurationOption.TLS_NEGOTIATION_TIMEOUT, Duration.ofMillis(11000)).build();

private static final AttributeMap CROSS_REGION_DEFAULTS = AttributeMap.builder()
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD).build();
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD)
.put(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT, "regional").build();

private static final AttributeMap CROSS_REGION_HTTP_DEFAULTS = AttributeMap.builder()
.put(SdkHttpConfigurationOption.CONNECTION_TIMEOUT, Duration.ofMillis(2800))
.put(SdkHttpConfigurationOption.TLS_NEGOTIATION_TIMEOUT, Duration.ofMillis(2800)).build();

private static final AttributeMap IN_REGION_DEFAULTS = AttributeMap.builder()
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD).build();
.put(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.STANDARD)
.put(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT, "regional").build();

private static final AttributeMap IN_REGION_HTTP_DEFAULTS = AttributeMap.builder()
.put(SdkHttpConfigurationOption.CONNECTION_TIMEOUT, Duration.ofMillis(1000))
Expand Down
12 changes: 9 additions & 3 deletions .../src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.ServiceMetadata;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain;
import software.amazon.awssdk.utils.AttributeMap;
import software.amazon.awssdk.utils.CollectionUtils;
Expand Down Expand Up @@ -188,12 +189,15 @@ protected final SdkClientConfiguration finalizeChildConfiguration(SdkClientConfi

private SdkClientConfiguration mergeSmartDefaults(SdkClientConfiguration configuration) {
DefaultsMode defaultsMode = resolveDefaultsMode(configuration);
RetryMode retryMode = DefaultsModeConfiguration.defaultConfig(defaultsMode).get(SdkClientOption.DEFAULT_RETRY_MODE);

AttributeMap defaultConfig = DefaultsModeConfiguration.defaultConfig(defaultsMode);
return configuration.toBuilder()
.option(DEFAULTS_MODE, defaultsMode)
.build()
.merge(c -> c.option(SdkClientOption.DEFAULT_RETRY_MODE, retryMode));
.merge(c -> c.option(SdkClientOption.DEFAULT_RETRY_MODE,
defaultConfig.get(SdkClientOption.DEFAULT_RETRY_MODE))
.option(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT,
defaultConfig.get(
ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT)));
}

/**
Expand Down Expand Up @@ -232,6 +236,8 @@ private URI endpointFromConfig(SdkClientConfiguration config) {
.withRegion(config.option(AwsClientOption.AWS_REGION))
.withProfileFile(config.option(SdkClientOption.PROFILE_FILE))
.withProfileName(config.option(SdkClientOption.PROFILE_NAME))
.putAdvancedOption(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT,
config.option(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT))
.getServiceEndpoint();
}

Expand Down
12 changes: 11 additions & 1 deletion .../src/main/java/software/amazon/awssdk/awscore/endpoint/DefaultServiceEndpointBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@

import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import software.amazon.awssdk.annotations.NotThreadSafe;
import software.amazon.awssdk.annotations.SdkProtectedApi;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.ServiceMetadata;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.utils.Validate;

/**
Expand All @@ -40,6 +43,7 @@ public final class DefaultServiceEndpointBuilder {
private Region region;
private ProfileFile profileFile;
private String profileName;
private final Map<ServiceMetadataAdvancedOption<?>, Object> advancedOptions = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does AttributeMap.Builder work for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, but the public API ServiceMetadataConfiguration would need to take AttributeMap or AttributeMap.Builder instead, which is not consistent with ClientOverrideConfiguration#advancedOptions

aws-sdk-java-v2/core/regions/src/main/java/software/amazon/awssdk/regions/ServiceMetadataConfiguration.java

Line 116 in 040b7fd

public Builder advancedOptions(Map<ServiceMetadataAdvancedOption<?>, ?> advancedOptions) {

aws-sdk-java-v2/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java

Line 350 in 040b7fd

Builder advancedOptions(Map<SdkAdvancedClientOption<?>, ?> advancedOptions);


public DefaultServiceEndpointBuilder(String serviceName, String protocol) {
this.serviceName = Validate.paramNotNull(serviceName, "serviceName");
Expand All @@ -64,10 +68,16 @@ public DefaultServiceEndpointBuilder withProfileName(String profileName) {
return this;
}

public <T> DefaultServiceEndpointBuilder putAdvancedOption(ServiceMetadataAdvancedOption<T> option, T value) {
advancedOptions.put(option, value);
return this;
}

public URI getServiceEndpoint() {
ServiceMetadata serviceMetadata = ServiceMetadata.of(serviceName)
.reconfigure(c -> c.profileFile(() -> profileFile)
.profileName(profileName));
.profileName(profileName)
.advancedOptions(advancedOptions));
URI endpoint = addProtocolToServiceEndpoint(serviceMetadata.endpointFor(region));

if (endpoint.getHost() == null) {
Expand Down
3 changes: 3 additions & 0 deletions ...ws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultsModeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static software.amazon.awssdk.awscore.client.config.AwsClientOption.DEFAULTS_MODE;
import static software.amazon.awssdk.core.client.config.SdkClientOption.DEFAULT_RETRY_MODE;
import static software.amazon.awssdk.core.client.config.SdkClientOption.RETRY_POLICY;
import static software.amazon.awssdk.regions.ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT;

import java.time.Duration;
import org.junit.Test;
Expand Down Expand Up @@ -75,6 +76,7 @@ public void defaultClient_shouldUseLegacyModeWithExistingDefaults() {

assertThat(client.clientConfiguration.option(DEFAULTS_MODE)).isEqualTo(DefaultsMode.LEGACY);
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(RetryMode.defaultRetryMode());
assertThat(client.clientConfiguration.option(S3_US_EAST_1_REGIONAL_ENDPOINT)).isNull();
}

@Test
Expand All @@ -96,6 +98,7 @@ public void nonLegacyDefaultsMode_shouldApplySdkDefaultsAndHttpDefaults() {
AttributeMap attributes = DefaultsModeConfiguration.defaultConfig(targetMode);

assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(attributes.get(DEFAULT_RETRY_MODE));
assertThat(client.clientConfiguration.option(S3_US_EAST_1_REGIONAL_ENDPOINT)).isEqualTo("regional");
}

@Test
Expand Down
9 changes: 9 additions & 0 deletions ...ava/software/amazon/awssdk/awscore/client/endpoint/DefaultServiceEndpointBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.junit.Test;
import software.amazon.awssdk.awscore.endpoint.DefaultServiceEndpointBuilder;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;

public class DefaultServiceEndpointBuilderTest {

Expand All @@ -43,4 +44,12 @@ public void getServiceEndpoint_S3NonStandardRegion_HttpProtocol() throws Excepti
.withRegion(Region.EU_CENTRAL_1);
assertEquals("http://s3.eu-central-1.amazonaws.com", endpointBuilder.getServiceEndpoint().toString());
}

@Test
public void getServiceEndpoint_regionalOption_shouldUseRegionalEndpoint() throws Exception {
DefaultServiceEndpointBuilder endpointBuilder = new DefaultServiceEndpointBuilder("s3", "http")
.withRegion(Region.US_EAST_1).putAdvancedOption(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT,
"regional");
assertEquals("http://s3.us-east-1.amazonaws.com", endpointBuilder.getServiceEndpoint().toString());
}
}
42 changes: 42 additions & 0 deletions core/regions/src/main/java/software/amazon/awssdk/regions/ServiceMetadataAdvancedOption.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.awssdk.regions;

import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.core.client.config.ClientOption;


/**
* A collection of advanced options that can be configured on a {@link ServiceMetadata} via
* {@link ServiceMetadataConfiguration.Builder#putAdvancedOption(ServiceMetadataAdvancedOption, Object)}.
*
* @param <T> The type of value associated with the option.
*/
@SdkPublicApi
public class ServiceMetadataAdvancedOption<T> extends ClientOption<T> {

/**
* The S3 regional endpoint setting for the {@code us-east-1} region. Setting the value to {@code regional} causes
* the SDK to use the {@code s3.us-east-1.amazonaws.com} endpoint when using the {@link Region#US_EAST_1} region instead of
* the global {@code s3.amazonaws.com}. Using the regional endpoint is disabled by default.
*/
public static final ServiceMetadataAdvancedOption<String> S3_US_EAST_1_REGIONAL_ENDPOINT =
new ServiceMetadataAdvancedOption<>(String.class);

protected ServiceMetadataAdvancedOption(Class<T> valueClass) {
super(valueClass);
}
}
33 changes: 33 additions & 0 deletions core/regions/src/main/java/software/amazon/awssdk/regions/ServiceMetadataConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@

package software.amazon.awssdk.regions;

import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import software.amazon.awssdk.annotations.SdkPublicApi;
import software.amazon.awssdk.profiles.ProfileFile;
import software.amazon.awssdk.profiles.ProfileFileSystemSetting;
import software.amazon.awssdk.utils.AttributeMap;

/**
* Configuration for a {@link ServiceMetadata}. This allows modifying the values used by default when a metadata instance is
Expand All @@ -30,10 +33,12 @@
public final class ServiceMetadataConfiguration {
private final Supplier<ProfileFile> profileFile;
private final String profileName;
private final AttributeMap advancedOptions;

private ServiceMetadataConfiguration(Builder builder) {
this.profileFile = builder.profileFile;
this.profileName = builder.profileName;
this.advancedOptions = builder.advancedOptions.build();
}

/**
Expand All @@ -57,9 +62,19 @@ public String profileName() {
return profileName;
}

/**
* Load the optional requested advanced option that was configured on the service metadata builder.
*
* @see ServiceMetadataConfiguration.Builder#putAdvancedOption(ServiceMetadataAdvancedOption, Object)
*/
public <T> Optional<T> advancedOption(ServiceMetadataAdvancedOption<T> option) {
return Optional.ofNullable(advancedOptions.get(option));
}

public static final class Builder {
private Supplier<ProfileFile> profileFile;
private String profileName;
private AttributeMap.Builder advancedOptions = AttributeMap.builder();

private Builder() {
}
Expand All @@ -85,6 +100,24 @@ public Builder profileName(String profileName) {
return this;
}

/**
* Configure the map of advanced override options. This will override all values currently configured. The values in the
* map must match the key type of the map, or a runtime exception will be raised.
*/
public <T> Builder putAdvancedOption(ServiceMetadataAdvancedOption<T> option, T value) {
this.advancedOptions.put(option, value);
return this;
}

/**
* Configure an advanced override option.
* @see ServiceMetadataAdvancedOption
*/
public Builder advancedOptions(Map<ServiceMetadataAdvancedOption<?>, ?> advancedOptions) {
this.advancedOptions.putAll(advancedOptions);
return this;
}

/**
* Build the {@link ServiceMetadata} instance with the updated configuration.
*/
Expand Down
9 changes: 6 additions & 3 deletions ...c/main/java/software/amazon/awssdk/regions/servicemetadata/EnhancedS3ServiceMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import software.amazon.awssdk.profiles.ProfileProperty;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.ServiceMetadata;
import software.amazon.awssdk.regions.ServiceMetadataAdvancedOption;
import software.amazon.awssdk.regions.ServiceMetadataConfiguration;
import software.amazon.awssdk.regions.ServicePartitionMetadata;
import software.amazon.awssdk.utils.Lazy;
Expand Down Expand Up @@ -53,7 +54,7 @@ private EnhancedS3ServiceMetadata(ServiceMetadataConfiguration config) {
Supplier<String> profileName = config.profileName() != null ? () -> config.profileName()
: ProfileFileSystemSetting.AWS_PROFILE::getStringValueOrThrow;

this.useUsEast1RegionalEndpoint = new Lazy<>(() -> useUsEast1RegionalEndpoint(profileFile, profileName));
this.useUsEast1RegionalEndpoint = new Lazy<>(() -> useUsEast1RegionalEndpoint(profileFile, profileName, config));
this.s3ServiceMetadata = new S3ServiceMetadata().reconfigure(config);
}

Expand All @@ -80,7 +81,8 @@ public List<ServicePartitionMetadata> servicePartitions() {
return s3ServiceMetadata.servicePartitions();
}

private boolean useUsEast1RegionalEndpoint(Supplier<ProfileFile> profileFile, Supplier<String> profileName) {
private boolean useUsEast1RegionalEndpoint(Supplier<ProfileFile> profileFile, Supplier<String> profileName,
ServiceMetadataConfiguration config) {
String env = envVarSetting();

if (env != null) {
Expand All @@ -93,7 +95,8 @@ private boolean useUsEast1RegionalEndpoint(Supplier<ProfileFile> profileFile, Su
return REGIONAL_SETTING.equalsIgnoreCase(profile);
}

return false;
return config.advancedOption(ServiceMetadataAdvancedOption.S3_US_EAST_1_REGIONAL_ENDPOINT)
.filter(REGIONAL_SETTING::equalsIgnoreCase).isPresent();
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually directly-configured options are higher priority than environment variables and system properties. I know in this case it's actually a default instead of a directly configured option (the customer can't configure this directly, right?), but should we name it accordingly to clarify it's the default, not the actual setting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good point. Customers can't configure this if they are using the SDK client. I'll rename it to reflect that it's the default.

}

private static String envVarSetting() {
Expand Down
Loading