Skip to content

Commit f7909e8

Browse files
dagnirdagnir
authored
Fixes to endpoint resolution (#3451)
- Don't include query params when resolving the SDK::Endpoint BuiltIn - Correctly remove the endpoint override path from the request path before recombining the endpoints after endpoint resolution. Otherwise, the override's path will be repeated in the final endpoint. - Fix some test expactations based around endpoint overrides in S3. Previously we did not do virtual addressing for normal bucket unless the endpoint startswith 's3'. This is no longer the case as the rules will use virtual addressing unless path style is enabled. - Add additional tests for AwsProviderUtils - Uncomment commented out endpoint param resolution in client builder. This was previously commented out for to make client endpoint tests pass but they will just be ignored in the generated tests instead.
1 parent 79cb513 commit f7909e8

File tree

12 files changed

+331
-198
lines changed

12 files changed

+331
-198
lines changed

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointProviderSpec.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.util.Map;
2626
import javax.lang.model.element.Modifier;
2727
import software.amazon.awssdk.annotations.SdkInternalApi;
28-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
28+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
2929
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
3030
import software.amazon.awssdk.codegen.model.intermediate.Metadata;
3131
import software.amazon.awssdk.codegen.model.rules.endpoints.BuiltInParameter;
@@ -139,7 +139,7 @@ private MethodSpec resolveEndpointMethod() {
139139
b.addStatement("$T res = new $T().evaluate($N, toIdentifierValueMap($N))",
140140
Value.class, DefaultRuleEngine.class, RULE_SET_FIELD_NAME, paramsName);
141141

142-
b.addStatement("return $T.valueAsEndpointOrThrow($N)", AwsProviderUtils.class, "res");
142+
b.addStatement("return $T.valueAsEndpointOrThrow($N)", AwsEndpointProviderUtils.class, "res");
143143

144144
return b.build();
145145
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/EndpointResolverInterceptorSpec.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import javax.lang.model.element.Modifier;
2727
import software.amazon.awssdk.annotations.SdkInternalApi;
2828
import software.amazon.awssdk.awscore.AwsExecutionAttribute;
29-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
29+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
3030
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
3131
import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
3232
import software.amazon.awssdk.codegen.model.rules.endpoints.ParameterModel;
@@ -96,7 +96,7 @@ private MethodSpec modifyRequestMethod() {
9696

9797
// We skip resolution if the source of the endpoint is the endpoint discovery call
9898
b.beginControlFlow("if ($1T.endpointIsDiscovered(executionAttributes))",
99-
AwsProviderUtils.class)
99+
AwsEndpointProviderUtils.class)
100100
.addStatement("return context.request()")
101101
.endControlFlow().build();
102102

@@ -165,7 +165,7 @@ private MethodSpec ruleParams() {
165165
throw new RuntimeException("Don't know how to set built-in " + m.getBuiltInEnum());
166166
}
167167

168-
b.addStatement("builder.$N($T.$N(executionAttributes))", setterName, AwsProviderUtils.class, builtInFn);
168+
b.addStatement("builder.$N($T.$N(executionAttributes))", setterName, AwsEndpointProviderUtils.class, builtInFn);
169169
});
170170

171171
b.addStatement("return builder.build()");

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/rules/RequestEndpointInterceptorSpec.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@
2020
import com.squareup.javapoet.TypeSpec;
2121
import javax.lang.model.element.Modifier;
2222
import software.amazon.awssdk.annotations.SdkInternalApi;
23-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
23+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
2424
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
2525
import software.amazon.awssdk.codegen.poet.ClassSpec;
2626
import software.amazon.awssdk.codegen.poet.PoetUtils;
2727
import software.amazon.awssdk.core.interceptor.Context;
2828
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
2929
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
30+
import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
3031
import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
3132
import software.amazon.awssdk.core.rules.model.Endpoint;
3233
import software.amazon.awssdk.http.SdkHttpRequest;
@@ -67,14 +68,18 @@ private MethodSpec modifyHttpRequestMethod() {
6768

6869
// We skip setting the endpoint here if the source of the endpoint is the endpoint discovery call
6970
b.beginControlFlow("if ($1T.endpointIsDiscovered(executionAttributes))",
70-
AwsProviderUtils.class)
71+
AwsEndpointProviderUtils.class)
7172
.addStatement("return context.httpRequest()")
7273
.endControlFlow().build();
7374

7475
b.addStatement("$1T endpoint = ($1T) executionAttributes.getAttribute($2T.RESOLVED_ENDPOINT)",
7576
Endpoint.class,
7677
SdkInternalExecutionAttribute.class);
77-
b.addStatement("return $T.setUri(context.httpRequest(), endpoint.url())", AwsProviderUtils.class);
78+
b.addStatement("return $T.setUri(context.httpRequest(),"
79+
+ "executionAttributes.getAttribute($T.CLIENT_ENDPOINT),"
80+
+ "endpoint.url())",
81+
AwsEndpointProviderUtils.class,
82+
SdkExecutionAttribute.class);
7883
return b.build();
7984
}
8085

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-provider-class.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import java.util.Map;
66
import software.amazon.awssdk.annotations.Generated;
77
import software.amazon.awssdk.annotations.SdkInternalApi;
8-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
8+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
99
import software.amazon.awssdk.core.rules.Condition;
1010
import software.amazon.awssdk.core.rules.DefaultRuleEngine;
1111
import software.amazon.awssdk.core.rules.EndpointResult;
@@ -32,7 +32,7 @@ public final class DefaultQueryEndpointProvider implements QueryEndpointProvider
3232
@Override
3333
public Endpoint resolveEndpoint(QueryEndpointParams endpointParams) {
3434
Value res = new DefaultRuleEngine().evaluate(ENDPOINT_RULE_SET, toIdentifierValueMap(endpointParams));
35-
return AwsProviderUtils.valueAsEndpointOrThrow(res);
35+
return AwsEndpointProviderUtils.valueAsEndpointOrThrow(res);
3636
}
3737

3838
private static Map<Identifier, Value> toIdentifierValueMap(QueryEndpointParams params) {

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/endpoint-resolve-interceptor.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import software.amazon.awssdk.annotations.Generated;
55
import software.amazon.awssdk.annotations.SdkInternalApi;
66
import software.amazon.awssdk.awscore.AwsExecutionAttribute;
7-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
7+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
88
import software.amazon.awssdk.core.SdkRequest;
99
import software.amazon.awssdk.core.interceptor.Context;
1010
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
@@ -22,7 +22,7 @@
2222
public final class QueryResolveEndpointInterceptor implements ExecutionInterceptor {
2323
@Override
2424
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
25-
if (AwsProviderUtils.endpointIsDiscovered(executionAttributes)) {
25+
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
2626
return context.request();
2727
}
2828
QueryEndpointProvider provider = (QueryEndpointProvider) executionAttributes
@@ -37,9 +37,9 @@ private static QueryEndpointParams ruleParams(Context.ModifyRequest context, Exe
3737
setStaticContextParams(builder, executionAttributes.getAttribute(AwsExecutionAttribute.OPERATION_NAME));
3838
setContextParams(builder, executionAttributes.getAttribute(AwsExecutionAttribute.OPERATION_NAME), context.request());
3939
setClientContextParams(builder, executionAttributes);
40-
builder.region(AwsProviderUtils.regionBuiltIn(executionAttributes));
41-
builder.useDualStackEndpoint(AwsProviderUtils.dualStackEnabledBuiltIn(executionAttributes));
42-
builder.useFipsEndpoint(AwsProviderUtils.fipsEnabledBuiltIn(executionAttributes));
40+
builder.region(AwsEndpointProviderUtils.regionBuiltIn(executionAttributes));
41+
builder.useDualStackEndpoint(AwsEndpointProviderUtils.dualStackEnabledBuiltIn(executionAttributes));
42+
builder.useFipsEndpoint(AwsEndpointProviderUtils.fipsEnabledBuiltIn(executionAttributes));
4343
return builder.build();
4444
}
4545

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/rules/request-set-endpoint-interceptor.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22

33
import software.amazon.awssdk.annotations.Generated;
44
import software.amazon.awssdk.annotations.SdkInternalApi;
5-
import software.amazon.awssdk.awscore.rules.AwsProviderUtils;
5+
import software.amazon.awssdk.awscore.rules.AwsEndpointProviderUtils;
66
import software.amazon.awssdk.core.interceptor.Context;
77
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
88
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
9+
import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
910
import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute;
1011
import software.amazon.awssdk.core.rules.model.Endpoint;
1112
import software.amazon.awssdk.http.SdkHttpRequest;
@@ -15,10 +16,11 @@
1516
public final class QueryRequestSetEndpointInterceptor implements ExecutionInterceptor {
1617
@Override
1718
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
18-
if (AwsProviderUtils.endpointIsDiscovered(executionAttributes)) {
19+
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
1920
return context.httpRequest();
2021
}
2122
Endpoint endpoint = (Endpoint) executionAttributes.getAttribute(SdkInternalExecutionAttribute.RESOLVED_ENDPOINT);
22-
return AwsProviderUtils.setUri(context.httpRequest(), endpoint.url());
23+
return AwsEndpointProviderUtils.setUri(context.httpRequest(),
24+
executionAttributes.getAttribute(SdkExecutionAttribute.CLIENT_ENDPOINT), endpoint.url());
2325
}
2426
}

‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,10 @@ private URI endpointFromConfig(SdkClientConfiguration config) {
242242
.withRegion(config.option(AwsClientOption.AWS_REGION))
243243
.withProfileFile(() -> config.option(SdkClientOption.PROFILE_FILE))
244244
.withProfileName(config.option(SdkClientOption.PROFILE_NAME))
245-
// .putAdvancedOption(ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT,
246-
// config.option(ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT))
247-
// .withDualstackEnabled(config.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED))
248-
// .withFipsEnabled(config.option(AwsClientOption.FIPS_ENDPOINT_ENABLED))
245+
.putAdvancedOption(ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT,
246+
config.option(ServiceMetadataAdvancedOption.DEFAULT_S3_US_EAST_1_REGIONAL_ENDPOINT))
247+
.withDualstackEnabled(config.option(AwsClientOption.DUALSTACK_ENDPOINT_ENABLED))
248+
.withFipsEnabled(config.option(AwsClientOption.FIPS_ENDPOINT_ENABLED))
249249
.getServiceEndpoint();
250250
}
251251

‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/rules/AwsProviderUtils.java renamed to ‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/rules/AwsEndpointProviderUtils.java

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
package software.amazon.awssdk.awscore.rules;
1717

18+
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
19+
1820
import java.net.URI;
1921
import java.util.ArrayList;
2022
import java.util.List;
@@ -32,12 +34,13 @@
3234
import software.amazon.awssdk.regions.Region;
3335
import software.amazon.awssdk.utils.Logger;
3436
import software.amazon.awssdk.utils.StringUtils;
37+
import software.amazon.awssdk.utils.http.SdkHttpUtils;
3538

3639
@SdkInternalApi
37-
public final class AwsProviderUtils {
38-
private static final Logger LOG = Logger.loggerFor(AwsProviderUtils.class);
40+
public final class AwsEndpointProviderUtils {
41+
private static final Logger LOG = Logger.loggerFor(AwsEndpointProviderUtils.class);
3942

40-
private AwsProviderUtils() {
43+
private AwsEndpointProviderUtils() {
4144
}
4245

4346
public static Region regionBuiltIn(ExecutionAttributes executionAttributes) {
@@ -52,9 +55,17 @@ public static Boolean fipsEnabledBuiltIn(ExecutionAttributes executionAttributes
5255
return executionAttributes.getAttribute(AwsExecutionAttribute.FIPS_ENDPOINT_ENABLED);
5356
}
5457

58+
/**
59+
* Returns the endpoint set on the client. Note that this strips off the query part of the URI because the endpoint rules
60+
* library, e.g. {@code ParseURL} will return an exception if the URI it parses has query parameters.
61+
*/
5562
public static String endpointBuiltIn(ExecutionAttributes executionAttributes) {
5663
if (endpointIsOverridden(executionAttributes)) {
57-
return executionAttributes.getAttribute(SdkExecutionAttribute.CLIENT_ENDPOINT).toString();
64+
return invokeSafely(() -> {
65+
URI endpointOverride = executionAttributes.getAttribute(SdkExecutionAttribute.CLIENT_ENDPOINT);
66+
return new URI(endpointOverride.getScheme(), null, endpointOverride.getHost(), endpointOverride.getPort(),
67+
endpointOverride.getPath(), null, null).toString();
68+
});
5869
}
5970
return null;
6071
}
@@ -102,23 +113,44 @@ public static Endpoint valueAsEndpointOrThrow(Value value) {
102113
}
103114
}
104115

105-
public static SdkHttpRequest setUri(SdkHttpRequest request, URI newUri) {
106-
String newPath = newUri.getRawPath();
107-
String existingPath = request.getUri().getRawPath();
116+
/**
117+
* This sets the request URI to the resolved URI returned by the endpoint provider. There some things to be careful about
118+
* to make this work properly:
119+
* <p>
120+
* If the client endpoint is an endpoint override, it may contain a path. In addition, the request marshaller itself may add
121+
* components to the path if it's modeled for the operation. Unfortunately, {@link SdkHttpRequest#encodedPath()} returns
122+
* the combined path from both the endpoint and the request. There is no way to know, just from the HTTP request object,
123+
* where the override path ends (if it's even there) and where the request path starts. Additionally, the rule itself may
124+
* also append other parts to the endpoint override path.
125+
* <p>
126+
* To solve this issue, we pass in the endpoint set on the path, which allows us to the strip the path from the endpoint
127+
* override from the request path, and then correctly combine the paths.
128+
* <p>
129+
* For example, let's suppose the endpoint override on the client is {@code https://example.com/a}. Then we call an
130+
* operation {@code Foo()}, that marshalls {@code /c} to the path. The resulting request path is {@code /a/c}. However, we
131+
* also pass the endpoint to provider as a parameter, and the resolver returns {@code https://example.com/a/b}. This method
132+
* takes care of combining the paths correctly so that the resulting path is {@code https://example.com/a/b/c}.
133+
*/
134+
public static SdkHttpRequest setUri(SdkHttpRequest request, URI clientEndpoint, URI resolvedUri) {
135+
// [client endpoint path]
136+
String clientEndpointPath = clientEndpoint.getRawPath();
108137

109-
if (StringUtils.isNotBlank(existingPath)) {
110-
if (newPath.endsWith("/") || existingPath.startsWith("/")) {
111-
newPath += existingPath;
112-
} else {
113-
newPath = newPath + "/" + existingPath;
114-
}
115-
}
138+
// [client endpoint path]/[request path]
139+
String requestPath = request.getUri().getRawPath();
140+
141+
// [client endpoint path]/[additional path added by resolver]
142+
String resolvedUriPath = resolvedUri.getRawPath();
143+
144+
// our goal is to construct [client endpoint path]/[additional path added by resolver]/[request path], so we just need
145+
// to strip the client endpoint path from the marshalled request path to isolate just the part added by the marshaller
146+
String requestPathWithClientPathRemoved = StringUtils.replaceOnce(requestPath, clientEndpointPath, "");
147+
String finalPath = SdkHttpUtils.appendUri(resolvedUriPath, requestPathWithClientPathRemoved);
116148

117149
return request.toBuilder()
118-
.protocol(newUri.getScheme())
119-
.host(newUri.getHost())
120-
.port(newUri.getPort())
121-
.encodedPath(newPath)
150+
.protocol(resolvedUri.getScheme())
151+
.host(resolvedUri.getHost())
152+
.port(resolvedUri.getPort())
153+
.encodedPath(finalPath)
122154
.build();
123155
}
124156

‎services/s3/src/main/resources/software/amazon/awssdk/services/s3/execution.interceptors

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,3 @@ software.amazon.awssdk.services.s3.internal.handlers.EnableTrailingChecksumInter
1111
software.amazon.awssdk.services.s3.internal.handlers.ExceptionTranslationInterceptor
1212
software.amazon.awssdk.services.s3.internal.handlers.GetObjectInterceptor
1313
software.amazon.awssdk.services.s3.internal.handlers.CopySourceInterceptor
14-
software.amazon.awssdk.services.s3.internal.handlers.RemoveBucketFromPathInterceptor

0 commit comments

Comments
 (0)