-
Notifications
You must be signed in to change notification settings - Fork 1k
Remove ServiceMetadata usage from DefaultS3Presigner, S3Utilities,and DefaultPollyPresigner #7110
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
base: feature/master/remove-service-metadata-usage
Are you sure you want to change the base?
Changes from all commits
a5b2868
2a7e840
a30b55f
6080eb8
05b96f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| import static software.amazon.awssdk.utils.CollectionUtils.mergeLists; | ||
| import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; | ||
|
|
||
| import java.net.URI; | ||
| import java.time.Clock; | ||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
|
|
@@ -45,6 +46,7 @@ | |
| import software.amazon.awssdk.awscore.internal.defaultsmode.DefaultsModeConfiguration; | ||
| import software.amazon.awssdk.awscore.presigner.PresignRequest; | ||
| import software.amazon.awssdk.awscore.presigner.PresignedRequest; | ||
| import software.amazon.awssdk.core.ClientEndpointProvider; | ||
| import software.amazon.awssdk.core.ClientType; | ||
| import software.amazon.awssdk.core.RequestOverrideConfiguration; | ||
| import software.amazon.awssdk.core.SdkBytes; | ||
|
|
@@ -53,6 +55,7 @@ | |
| import software.amazon.awssdk.core.client.builder.SdkDefaultClientBuilder; | ||
| import software.amazon.awssdk.core.client.config.SdkClientConfiguration; | ||
| import software.amazon.awssdk.core.client.config.SdkClientOption; | ||
| import software.amazon.awssdk.core.exception.SdkClientException; | ||
| import software.amazon.awssdk.core.http.ExecutionContext; | ||
| import software.amazon.awssdk.core.interceptor.ClasspathInterceptorChainFactory; | ||
| import software.amazon.awssdk.core.interceptor.ExecutionAttributes; | ||
|
|
@@ -251,28 +254,32 @@ private List<ExecutionInterceptor> initializeInterceptors() { | |
| * Copied from {@link AwsDefaultClientBuilder}. | ||
| */ | ||
| private SdkClientConfiguration createClientConfiguration() { | ||
| AwsClientEndpointProvider endpointProvider = | ||
| AwsClientEndpointProvider.builder() | ||
| .clientEndpointOverride(endpointOverride()) | ||
| .serviceEndpointOverrideEnvironmentVariable("AWS_ENDPOINT_URL_S3") | ||
| .serviceEndpointOverrideSystemProperty("aws.endpointUrlS3") | ||
| .serviceProfileProperty("s3") | ||
| .serviceEndpointPrefix(SERVICE_NAME) | ||
| .defaultProtocol("https") | ||
| .region(region()) | ||
| .profileFile(profileFileSupplier()) | ||
| .profileName(profileName()) | ||
| .dualstackEnabled(serviceConfiguration.dualstackEnabled()) | ||
| .fipsEnabled(fipsEnabled()) | ||
| .build(); | ||
|
|
||
| // Make sure the endpoint resolver can actually resolve an endpoint, so that we fail now instead of | ||
| // when a request is made. | ||
| endpointProvider.clientEndpoint(); | ||
| Optional<URI> overrideEndpoint = AwsClientEndpointProvider.builder() | ||
| .clientEndpointOverride(endpointOverride()) | ||
| .serviceEndpointOverrideEnvironmentVariable("AWS_ENDPOINT_URL_S3") | ||
| .serviceEndpointOverrideSystemProperty("aws.endpointUrlS3") | ||
| .serviceProfileProperty("s3") | ||
| .profileFile(profileFileSupplier()) | ||
| .profileName(profileName()) | ||
| .resolveFromOverrides(); | ||
|
|
||
| ClientEndpointProvider endpointProvider; | ||
| if (overrideEndpoint.isPresent()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason we are not using S3EndpointProvider here? |
||
| endpointProvider = ClientEndpointProvider.create(overrideEndpoint.get(), true); | ||
| } else { | ||
| // Validate region at construction time to fail fast for invalid regions (e.g., US_EAST_1 with underscores). | ||
| URI testEndpoint = URI.create("https://s3." + region().id() + ".amazonaws.com"); | ||
| if (testEndpoint.getHost() == null) { | ||
| throw SdkClientException.create("Configured region (" + region() + ") resulted in an invalid URI: " | ||
| + testEndpoint + ". This is usually caused by an invalid region " | ||
| + "configuration."); | ||
| } | ||
| // Need an endpoint to marshall but this will be overwritten in modifyHttpRequest | ||
| endpointProvider = ClientEndpointProvider.create(URI.create("https://localhost"), false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
NVM, it's used as a placeholder and will be replaced anyway. Can we follow the same pattern in EC2 and add some comments https://github.com/aws/aws-sdk-java-v2/blob/master/services/ec2/src/main/java/software/amazon/awssdk/services/ec2/transform/internal/GeneratePreSignUrlInterceptor.java#L67 |
||
| } | ||
|
|
||
| return SdkClientConfiguration.builder() | ||
| .option(SdkClientOption.CLIENT_ENDPOINT_PROVIDER, | ||
| endpointProvider) | ||
| .option(SdkClientOption.CLIENT_ENDPOINT_PROVIDER, endpointProvider) | ||
| .build(); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we are not using localhost like we do in other presigners?