Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ private MethodSpec modifyRequestMethod(String endpointAuthSchemeStrategyFieldNam
String providerVar = "provider";

b.addStatement("$T result = context.request()", SdkRequest.class);
// We skip resolution if the source of the endpoint is the endpoint discovery call
b.beginControlFlow("if ($1T.endpointIsDiscovered(executionAttributes))",
// We skip resolution if endpoint resolution should be skipped (e.g., endpoint discovery or pre-signed URLs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this comment reads a bit funny

b.beginControlFlow("if ($1T.skipEndpointResolution(executionAttributes))",
endpointRulesSpecUtils.rulesRuntimeClassName("AwsEndpointProviderUtils"));
b.addStatement("return result");
b.endControlFlow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ private MethodSpec modifyHttpRequestMethod() {
.addParameter(ExecutionAttributes.class, "executionAttributes");


// We skip setting the endpoint here if the source of the endpoint is the endpoint discovery call
b.beginControlFlow("if ($1T.endpointIsDiscovered(executionAttributes))",
// We skip setting the endpoint here if endpoint resolution should be skipped
b.beginControlFlow("if ($1T.skipEndpointResolution(executionAttributes))",
endpointRulesSpecUtils.rulesRuntimeClassName("AwsEndpointProviderUtils"))
.addStatement("return context.httpRequest()")
.endControlFlow().build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,24 @@ public final class AwsEndpointProviderUtils {
/**
* True if the the {@link SdkInternalExecutionAttribute#IS_DISCOVERED_ENDPOINT} attribute is present and its value
* is {@code true}, {@code false} otherwise.
*
* @deprecated Use {@link #skipEndpointResolution(ExecutionAttributes)} instead.
*/
@Deprecated
public static boolean endpointIsDiscovered(ExecutionAttributes attrs) {
return attrs.getOptionalAttribute(SdkInternalExecutionAttribute.IS_DISCOVERED_ENDPOINT).orElse(false);
}

/**
* True if endpoint resolution should be skipped for the current request. This returns {@code true} if either
* {@link SdkInternalExecutionAttribute#SKIP_ENDPOINT_RESOLUTION} or
* {@link SdkInternalExecutionAttribute#IS_DISCOVERED_ENDPOINT} is set to {@code true}.
*/
public static boolean skipEndpointResolution(ExecutionAttributes attrs) {
return attrs.getOptionalAttribute(SdkInternalExecutionAttribute.SKIP_ENDPOINT_RESOLUTION).orElse(false)
|| attrs.getOptionalAttribute(SdkInternalExecutionAttribute.IS_DISCOVERED_ENDPOINT).orElse(false);
}

/**
* True if the the {@link SdkInternalExecutionAttribute#DISABLE_HOST_PREFIX_INJECTION} attribute is present and its
* value is {@code true}, {@code false} otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public QueryResolveEndpointInterceptor() {
@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
SdkRequest result = context.request();
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return result;
}
QueryEndpointProvider provider = (QueryEndpointProvider) executionAttributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public final class QueryResolveEndpointInterceptor implements ExecutionIntercept
@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
SdkRequest result = context.request();
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return result;
}
QueryEndpointProvider provider = (QueryEndpointProvider) executionAttributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public final class DatabaseResolveEndpointInterceptor implements ExecutionInterc
@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
SdkRequest result = context.request();
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return result;
}
DatabaseEndpointProvider provider = (DatabaseEndpointProvider) executionAttributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class SampleSvcResolveEndpointInterceptor implements ExecutionInter
@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
SdkRequest result = context.request();
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return result;
}
SampleSvcEndpointProvider provider = (SampleSvcEndpointProvider) executionAttributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public final class QueryResolveEndpointInterceptor implements ExecutionIntercept
@Override
public SdkRequest modifyRequest(Context.ModifyRequest context, ExecutionAttributes executionAttributes) {
SdkRequest result = context.request();
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return result;
}
QueryEndpointProvider provider = (QueryEndpointProvider) executionAttributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
public final class QueryRequestSetEndpointInterceptor implements ExecutionInterceptor {
@Override
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
if (AwsEndpointProviderUtils.endpointIsDiscovered(executionAttributes)) {
if (AwsEndpointProviderUtils.skipEndpointResolution(executionAttributes)) {
return context.httpRequest();
}
Endpoint endpoint = executionAttributes.getAttribute(SdkInternalExecutionAttribute.RESOLVED_ENDPOINT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,22 @@ public final class SdkInternalExecutionAttribute extends SdkExecutionAttribute {

/**
* Whether the endpoint on the request is the result of Endpoint Discovery.
*
* @deprecated This attribute is specific to the endpoint discovery feature and should not be used to skip endpoint
* resolution; use {@link #SKIP_ENDPOINT_RESOLUTION} instead.
*/
@Deprecated
public static final ExecutionAttribute<Boolean> IS_DISCOVERED_ENDPOINT =
new ExecutionAttribute<>("IsDiscoveredEndpoint");

/**
* Whether endpoint resolution should be skipped for the current request. When set to {@code true}, the SDK will not
* invoke the endpoint provider and will use the endpoint already set on the request. This is used for pre-signed URL
* operations and other scenarios where the endpoint is already fully resolved.
*/
public static final ExecutionAttribute<Boolean> SKIP_ENDPOINT_RESOLUTION =
new ExecutionAttribute<>("SkipEndpointResolution");

/**
* The nano time that the current API call attempt began.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private static SdkHttpFullRequest modifyEndpointHostIfNeeded(SdkHttpFullRequest
ClientExecutionParams executionParams) {
if (executionParams.discoveredEndpoint() != null) {
URI discoveredEndpoint = executionParams.discoveredEndpoint();
executionParams.putExecutionAttribute(SdkInternalExecutionAttribute.IS_DISCOVERED_ENDPOINT, true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we also keep the IS_DISCOVERED_ENDPOINT write for cross-module compatibility? If a customer pins new sdk-core with an older service module, that module's generated endpointIsDiscovered() only checks the old attribute, which would cause unexpected behavior.

executionParams.putExecutionAttribute(SdkInternalExecutionAttribute.SKIP_ENDPOINT_RESOLUTION, true);
return originalRequest.toBuilder().host(discoveredEndpoint.getHost()).port(discoveredEndpoint.getPort()).build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ public <ReturnT> CompletableFuture<ReturnT> getObject(
.withRequestConfiguration(clientConfiguration)
.withInput(internalRequest)
.withMetricCollector(apiCallMetricCollector)
// TODO: Deprecate IS_DISCOVERED_ENDPOINT, use new SKIP_ENDPOINT_RESOLUTION for better semantics
.putExecutionAttribute(SdkInternalExecutionAttribute.IS_DISCOVERED_ENDPOINT, true)
.putExecutionAttribute(SdkInternalExecutionAttribute.SKIP_ENDPOINT_RESOLUTION, true)
.putExecutionAttribute(SdkInternalExecutionAttribute.HTTP_CHECKSUM,
checksumConfigForUrl(presignedUrlDownloadRequest.presignedUrl()))
.withMarshaller(new PresignedUrlDownloadRequestMarshaller(protocolFactory)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ public void endpointIsDiscovered_attrIsTrue_returnsTrue() {
assertThat(AwsEndpointProviderUtils.endpointIsDiscovered(attrs)).isTrue();
}

@Test
public void skipEndpointResolution_skipAttrIsTrue_returnsTrue() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we add a test for missing attributes for this?

ExecutionAttributes attrs = new ExecutionAttributes();
attrs.putAttribute(SdkInternalExecutionAttribute.SKIP_ENDPOINT_RESOLUTION, true);
assertThat(AwsEndpointProviderUtils.skipEndpointResolution(attrs)).isTrue();
}

@Test
public void disableHostPrefixInjection_attrIsFalse_returnsFalse() {
ExecutionAttributes attrs = new ExecutionAttributes();
Expand Down
Loading