From a105c8976f4f91912d9a8983465b749057ad2263 Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Wed, 14 Aug 2024 12:27:11 -0400 Subject: [PATCH 1/6] Add stale time config to InstanceProfileCredentialsProvider to allow for refreshing credentials earlier due to stale value. This will help prevent returning invalid credentials when an error is encountered during asynchronous refresh. --- .../InstanceProfileCredentialsProvider.java | 24 ++++++++- .../ContainerCredentialsRetryPolicy.java | 5 ++ .../StaticResourcesEndpointProvider.java | 6 +++ ...nstanceProfileCredentialsProviderTest.java | 49 +++++++++++++++++++ .../auth/src/test/resources/log4j2.properties | 6 +++ 5 files changed, 89 insertions(+), 1 deletion(-) diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java index 72c825c9fdcf..add9aedd5f91 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java @@ -90,6 +90,8 @@ public final class InstanceProfileCredentialsProvider private final String profileName; + private final Duration staleTime; + /** * @see #builder() */ @@ -111,6 +113,8 @@ private InstanceProfileCredentialsProvider(BuilderImpl builder) { .build(); this.ec2MetadataDisableV1Resolver = Ec2MetadataDisableV1Resolver.create(profileFile, profileName); + this.staleTime = Validate.getOrDefault(builder.staleTime, () -> Duration.ofSeconds(1)); + if (Boolean.TRUE.equals(builder.asyncCredentialUpdateEnabled)) { Validate.paramNotBlank(builder.asyncThreadName, "asyncThreadName"); this.credentialsCache = CachedSupplier.builder(this::refreshCredentials) @@ -177,7 +181,7 @@ private Instant staleTime(Instant expiration) { return null; } - return expiration.minusSeconds(1); + return expiration.minus(staleTime); } private Instant prefetchTime(Instant expiration) { @@ -331,6 +335,13 @@ public interface Builder extends HttpCredentialsProvider.Builder profileFile; private String profileName; + private Duration staleTime; private BuilderImpl() { asyncThreadName("instance-profile-credentials-provider"); @@ -426,6 +438,16 @@ public void setProfileName(String profileName) { profileName(profileName); } + @Override + public Builder staleTime(Duration duration) { + this.staleTime = staleTime; + return this; + } + + public void setStaleTime(Duration duration) { + staleTime(duration); + } + @Override public InstanceProfileCredentialsProvider build() { return new InstanceProfileCredentialsProvider(this); diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/ContainerCredentialsRetryPolicy.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/ContainerCredentialsRetryPolicy.java index 448d19aee53c..8d3179dcd5eb 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/ContainerCredentialsRetryPolicy.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/ContainerCredentialsRetryPolicy.java @@ -17,10 +17,15 @@ import java.io.IOException; import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.auth.credentials.ContainerCredentialsProvider; +import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider; import software.amazon.awssdk.http.HttpStatusFamily; import software.amazon.awssdk.regions.util.ResourcesEndpointRetryParameters; import software.amazon.awssdk.regions.util.ResourcesEndpointRetryPolicy; +/** + * Retry policy shared by {@link InstanceProfileCredentialsProvider} and {@link ContainerCredentialsProvider#}. + */ @SdkInternalApi public final class ContainerCredentialsRetryPolicy implements ResourcesEndpointRetryPolicy { diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java index 0e81d151f401..09722b6f10e1 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java @@ -21,6 +21,7 @@ import java.util.Map; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.regions.util.ResourcesEndpointProvider; +import software.amazon.awssdk.regions.util.ResourcesEndpointRetryPolicy; import software.amazon.awssdk.utils.Validate; @SdkInternalApi @@ -46,4 +47,9 @@ public URI endpoint() throws IOException { public Map headers() { return Collections.unmodifiableMap(headers); } + + @Override + public ResourcesEndpointRetryPolicy retryPolicy() { + return new ContainerCredentialsRetryPolicy(); + } } diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java index c54a2ca3d4d1..ffa851cd6789 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java @@ -544,6 +544,46 @@ public void imdsCallFrequencyIsLimited() { } } + @Test + void testErrorWhileCacheIsStale_shouldRecover() { + AdjustableClock clock = new AdjustableClock(); + + Instant now = Instant.now(); + Instant expiration = now.plus(Duration.ofHours(6)); + + String successfulCredentialsResponse = + "{" + + "\"AccessKeyId\":\"ACCESS_KEY_ID\"," + + "\"SecretAccessKey\":\"SECRET_ACCESS_KEY\"," + + "\"Expiration\":\"" + DateUtils.formatIso8601Date(expiration) + '"' + + "}"; + + String staleResponse = + "{" + + "\"AccessKeyId\":\"ACCESS_KEY_ID_2\"," + + "\"SecretAccessKey\":\"SECRET_ACCESS_KEY_2\"," + + "\"Expiration\":\"" + DateUtils.formatIso8601Date(Instant.now()) + '"' + + "}"; + + + Duration staleTime = Duration.ofMinutes(5); + AwsCredentialsProvider provider = credentialsProviderWithClock(clock, staleTime); + + // cache expiration with expiration = 6 hours + clock.time = now; + stubSecureCredentialsResponse(aResponse().withBody(successfulCredentialsResponse)); + AwsCredentials validCreds = provider.resolveCredentials(); + + // failure while cache is stale + clock.time = expiration.minus(staleTime.minus(Duration.ofMinutes(2))); + stubTokenFetchErrorResponse(aResponse().withFixedDelay(2000).withBody(STUB_CREDENTIALS), 500); + stubSecureCredentialsResponse(aResponse().withBody(staleResponse)); + AwsCredentials refreshedWhileStale = provider.resolveCredentials(); + + assertThat(refreshedWhileStale).isNotEqualTo(validCreds); + assertThat(refreshedWhileStale.secretAccessKey()).isEqualTo("SECRET_ACCESS_KEY_2"); + } + private AwsCredentialsProvider credentialsProviderWithClock(Clock clock) { InstanceProfileCredentialsProvider.BuilderImpl builder = (InstanceProfileCredentialsProvider.BuilderImpl) InstanceProfileCredentialsProvider.builder(); @@ -551,6 +591,15 @@ private AwsCredentialsProvider credentialsProviderWithClock(Clock clock) { return builder.build(); } + private AwsCredentialsProvider credentialsProviderWithClock(Clock clock, Duration staleTime) { + InstanceProfileCredentialsProvider.BuilderImpl builder = + (InstanceProfileCredentialsProvider.BuilderImpl) InstanceProfileCredentialsProvider.builder(); + builder.clock(clock); + builder.staleTime(staleTime); + return builder.build(); + } + + private static class AdjustableClock extends Clock { private Instant time; diff --git a/core/auth/src/test/resources/log4j2.properties b/core/auth/src/test/resources/log4j2.properties index e5e68dd2faa4..c63fc7025376 100644 --- a/core/auth/src/test/resources/log4j2.properties +++ b/core/auth/src/test/resources/log4j2.properties @@ -36,3 +36,9 @@ rootLogger.appenderRef.stdout.ref = ConsoleAppender # #logger.netty.name = io.netty.handler.logging #logger.netty.level = debug + +#logger.cache.name = software.amazon.awssdk.utils.cache.CachedSupplier +#logger.cache.level = DEBUG + +#logger.instance.name = software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider +#logger.instance.level = DEBUG From 199713cdfd1fd70410d48e16c27ca988b79fbe14 Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Wed, 18 Dec 2024 12:56:16 -0500 Subject: [PATCH 2/6] fix spotbug --- .../auth/credentials/InstanceProfileCredentialsProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java index 096361efcf6c..2a148adfeb87 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java @@ -379,6 +379,7 @@ private BuilderImpl(InstanceProfileCredentialsProvider provider) { this.asyncThreadName = provider.asyncThreadName; this.profileFile = provider.profileFile; this.profileName = provider.profileName; + this.staleTime = provider.staleTime; } Builder clock(Clock clock) { @@ -449,7 +450,7 @@ public void setProfileName(String profileName) { @Override public Builder staleTime(Duration duration) { - this.staleTime = staleTime; + this.staleTime = duration; return this; } From a7612b2c9661396839a5535f94e738d565552b46 Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Mon, 6 Jan 2025 10:33:02 -0500 Subject: [PATCH 3/6] add configurable retry policy --- .../InstanceProfileCredentialsProvider.java | 33 ++++++++ ...InstanceProfileCredentialsRetryPolicy.java | 39 +++++++++ .../StaticResourcesEndpointProvider.java | 17 +++- ...nstanceProfileCredentialsProviderTest.java | 80 +++++++++++++++++++ 4 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java index 2a148adfeb87..bd0f6a886302 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java @@ -90,6 +90,8 @@ public final class InstanceProfileCredentialsProvider private final Duration staleTime; + private final InstanceProfileCredentialsRetryPolicy retryPolicy; + /** * @see #builder() */ @@ -112,6 +114,8 @@ private InstanceProfileCredentialsProvider(BuilderImpl builder) { this.staleTime = Validate.getOrDefault(builder.staleTime, () -> Duration.ofSeconds(1)); + this.retryPolicy = Validate.getOrDefault(builder.retryPolicy, () -> InstanceProfileCredentialsRetryPolicy.NO_RETRY); + if (Boolean.TRUE.equals(builder.asyncCredentialUpdateEnabled)) { Validate.paramNotBlank(builder.asyncThreadName, "asyncThreadName"); this.credentialsCache = CachedSupplier.builder(this::refreshCredentials) @@ -213,6 +217,9 @@ private ResourcesEndpointProvider createEndpointProvider() { String[] securityCredentials = getSecurityCredentials(imdsHostname, token); return StaticResourcesEndpointProvider.builder() + .retryPolicy( + (retriesAttempted, param) -> retryPolicy.shouldRetry( + retriesAttempted, param.getStatusCode(), param.getException())) .endpoint(URI.create(imdsHostname + SECURITY_CREDENTIALS_RESOURCE + securityCredentials[0])) .headers(getTokenHeaders(token)) @@ -234,6 +241,9 @@ private String getToken(String imdsHostname) { ResourcesEndpointProvider tokenEndpoint = StaticResourcesEndpointProvider.builder() .endpoint(getTokenEndpoint(imdsHostname)) + .retryPolicy( + (retriesAttempted, param) -> retryPolicy.shouldRetry( + retriesAttempted, param.getStatusCode(), param.getException())) .headers(tokenTtlHeaders) .connectionTimeout(Duration.ofMillis( this.configProvider.serviceTimeout())) @@ -287,6 +297,9 @@ private boolean isInsecureFallbackDisabled() { private String[] getSecurityCredentials(String imdsHostname, String metadataToken) { ResourcesEndpointProvider securityCredentialsEndpoint = StaticResourcesEndpointProvider.builder() + .retryPolicy( + (retriesAttempted, param) -> retryPolicy.shouldRetry( + retriesAttempted, param.getStatusCode(), param.getException())) .endpoint(URI.create(imdsHostname + SECURITY_CREDENTIALS_RESOURCE)) .headers(getTokenHeaders(metadataToken)) .connectionTimeout(Duration.ofMillis(this.configProvider.serviceTimeout())) @@ -351,6 +364,13 @@ public interface Builder extends HttpCredentialsProvider.Builder profileFile; private String profileName; private Duration staleTime; + private InstanceProfileCredentialsRetryPolicy retryPolicy; private BuilderImpl() { asyncThreadName("instance-profile-credentials-provider"); @@ -380,6 +401,7 @@ private BuilderImpl(InstanceProfileCredentialsProvider provider) { this.profileFile = provider.profileFile; this.profileName = provider.profileName; this.staleTime = provider.staleTime; + this.retryPolicy = provider.retryPolicy; } Builder clock(Clock clock) { @@ -458,6 +480,17 @@ public void setStaleTime(Duration duration) { staleTime(duration); } + @Override + public Builder retryPolicy(InstanceProfileCredentialsRetryPolicy retryPolicy) { + this.retryPolicy = retryPolicy; + return this; + } + + public void setRetryPolicy(InstanceProfileCredentialsRetryPolicy retryPolicy) { + retryPolicy(retryPolicy); + } + + @Override public InstanceProfileCredentialsProvider build() { return new InstanceProfileCredentialsProvider(this); diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java new file mode 100644 index 000000000000..59b518499623 --- /dev/null +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java @@ -0,0 +1,39 @@ +/* + * 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.auth.credentials; + +import software.amazon.awssdk.annotations.SdkPublicApi; + +/** + * Custom retry policy for {@link InstanceProfileCredentialsProvider} calls to retrieve credentials + * from the Amazon EC2 Instance Metadata Service. + */ +@SdkPublicApi +public interface InstanceProfileCredentialsRetryPolicy { + + InstanceProfileCredentialsRetryPolicy NO_RETRY = (retriesAttempted, statusCode, exception) -> false; + + /** + * Returns whether a failed request should be retried. + * + * @param retriesAttempted The number of times the current request has been attempted. + * @param statusCode The http status code returned by the call to Instance Metadata Service + * @param exception The error that caused the request to fail. + * + * @return True if the failed request should be retried. + */ + boolean shouldRetry(Integer retriesAttempted, Integer statusCode, Exception exception); +} diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java index 9a3aedf0798b..7d705e76944f 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java @@ -32,16 +32,19 @@ public final class StaticResourcesEndpointProvider implements ResourcesEndpointP private final URI endpoint; private final Map headers; private final Duration connectionTimeout; + private final ResourcesEndpointRetryPolicy retryPolicy; private StaticResourcesEndpointProvider(URI endpoint, - Map additionalHeaders, - Duration customTimeout) { + Map additionalHeaders, + Duration customTimeout, + ResourcesEndpointRetryPolicy retryPolicy) { this.endpoint = Validate.paramNotNull(endpoint, "endpoint"); this.headers = ResourcesEndpointProvider.super.headers(); if (additionalHeaders != null) { this.headers.putAll(additionalHeaders); } this.connectionTimeout = customTimeout; + this.retryPolicy = retryPolicy; } @Override @@ -61,13 +64,14 @@ public Map headers() { @Override public ResourcesEndpointRetryPolicy retryPolicy() { - return new ContainerCredentialsRetryPolicy(); + return this.retryPolicy; } public static class Builder { private URI endpoint; private Map additionalHeaders = new HashMap<>(); private Duration customTimeout; + private ResourcesEndpointRetryPolicy retryPolicy = ResourcesEndpointRetryPolicy.NO_RETRY; public Builder endpoint(URI endpoint) { this.endpoint = Validate.paramNotNull(endpoint, "endpoint"); @@ -86,8 +90,13 @@ public Builder connectionTimeout(Duration timeout) { return this; } + public Builder retryPolicy(ResourcesEndpointRetryPolicy retryPolicy) { + this.retryPolicy = retryPolicy; + return this; + } + public StaticResourcesEndpointProvider build() { - return new StaticResourcesEndpointProvider(endpoint, additionalHeaders, customTimeout); + return new StaticResourcesEndpointProvider(endpoint, additionalHeaders, customTimeout, retryPolicy); } } diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java index 7d8a4982e66c..fe67eaf87adf 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java @@ -17,6 +17,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.exactly; import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.put; @@ -24,6 +25,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.github.tomakehurst.wiremock.stubbing.Scenario.STARTED; import static java.time.temporal.ChronoUnit.HOURS; import static java.time.temporal.ChronoUnit.MINUTES; import static java.time.temporal.ChronoUnit.SECONDS; @@ -55,6 +57,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; import software.amazon.awssdk.core.SdkSystemSetting; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.util.SdkUserAgent; @@ -636,6 +639,83 @@ void testErrorWhileCacheIsStale_shouldRecover() { assertThat(refreshedWhileStale.secretAccessKey()).isEqualTo("SECRET_ACCESS_KEY_2"); } + @Test + void shouldNotRetry_whenSucceeds() { + stubSecureCredentialsResponse(aResponse().withBody(STUB_CREDENTIALS)); + InstanceProfileCredentialsProvider provider = + InstanceProfileCredentialsProvider.builder() + .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < 3 && statusCode != 200) + .build(); + AwsCredentials credentials = provider.resolveCredentials(); + assertThat(credentials.accessKeyId()).isEqualTo("ACCESS_KEY_ID"); + assertThat(credentials.secretAccessKey()).isEqualTo("SECRET_ACCESS_KEY"); + assertThat(credentials.providerName()).isPresent().contains("InstanceProfileCredentialsProvider"); + verifyImdsCallWithToken(); + WireMock.verify(exactly(1), getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); + } + + @ParameterizedTest + @ValueSource(ints = {0, 1, 2, 3, 4, 5}) + void shouldRetryAndFail_whenFails_basedOnRetryPolicy(int retries) { + InstanceProfileCredentialsProvider provider = + InstanceProfileCredentialsProvider.builder() + .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < retries && statusCode != 200) + .build(); + + String errorMessage = "some error msg"; + String credentialsResponse = + "{" + + "\"code\": \"InternalServiceException\"," + + "\"message\": \"" + errorMessage + "\"" + + "}"; + + stubSecureCredentialsResponse(aResponse().withStatus(500) + .withBody(credentialsResponse)); + assertThatThrownBy(provider::resolveCredentials).hasRootCauseMessage("some error msg"); + WireMock.verify(retries + 1, getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); + } + + @Test + void shouldRetryThenSucceed_basedOnRetryPolicy() { + String errorMessage = "some error msg"; + String credentialsResponse = + "{" + + "\"code\": \"InternalServiceException\"," + + "\"message\": \"" + errorMessage + "\"" + + "}"; + + wireMockServer.stubFor(put(urlPathEqualTo(TOKEN_RESOURCE_PATH)) + .inScenario("Retry") + .whenScenarioStateIs(STARTED) + .willReturn(aResponse().withBody(TOKEN_STUB).withStatus(200))); + wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH)) + .inScenario("Retry") + .willReturn(aResponse().withBody(PROFILE_NAME).withStatus(200)) + .whenScenarioStateIs(STARTED) + .willSetStateTo("error")); + wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + PROFILE_NAME)) + .inScenario("Retry") + .whenScenarioStateIs("error") + .willReturn(aResponse().withBody(credentialsResponse).withStatus(500)) + .willSetStateTo("success")); + + wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + PROFILE_NAME)) + .inScenario("Retry") + .willReturn(aResponse().withBody(STUB_CREDENTIALS).withStatus(200)) + .whenScenarioStateIs("success")); + + InstanceProfileCredentialsProvider provider = + InstanceProfileCredentialsProvider.builder() + .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < 3 && statusCode != 200) + .build(); + + provider.resolveCredentials(); + + WireMock.verify(putRequestedFor(urlPathEqualTo(TOKEN_RESOURCE_PATH))); + WireMock.verify(getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH))); + WireMock.verify(2, getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); + } + private AwsCredentialsProvider credentialsProviderWithClock(Clock clock) { InstanceProfileCredentialsProvider.BuilderImpl builder = (InstanceProfileCredentialsProvider.BuilderImpl) InstanceProfileCredentialsProvider.builder(); From 360591afa7a7a39b67219be5d81bda3c7226015f Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Tue, 7 Jan 2025 12:45:03 -0500 Subject: [PATCH 4/6] javadoc --- .../credentials/InstanceProfileCredentialsProvider.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java index bd0f6a886302..cbefc6208955 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java @@ -359,7 +359,12 @@ public interface Builder extends HttpCredentialsProvider.BuilderIncreasing this value to a higher value (10s or more) may help with situations where a higher load on the instance + * metadata service causes it to return 503s error, for which the SDK may not be able to recover fast enough and + * returns expired credentials. + * * @param duration the amount of time before expiration for when to consider the credentials to be stale and need refresh */ Builder staleTime(Duration duration); From 17226f18ca023cb072ce098f2f292d4c7041f4ec Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Wed, 22 Jan 2025 12:25:24 -0500 Subject: [PATCH 5/6] remove InstanceProfileCredentialsRetryPolicy --- .../InstanceProfileCredentialsProvider.java | 33 --------- ...InstanceProfileCredentialsRetryPolicy.java | 39 ----------- ...nstanceProfileCredentialsProviderTest.java | 67 +------------------ 3 files changed, 1 insertion(+), 138 deletions(-) delete mode 100644 core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java index cbefc6208955..b1ddc5d7faef 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProvider.java @@ -90,8 +90,6 @@ public final class InstanceProfileCredentialsProvider private final Duration staleTime; - private final InstanceProfileCredentialsRetryPolicy retryPolicy; - /** * @see #builder() */ @@ -114,8 +112,6 @@ private InstanceProfileCredentialsProvider(BuilderImpl builder) { this.staleTime = Validate.getOrDefault(builder.staleTime, () -> Duration.ofSeconds(1)); - this.retryPolicy = Validate.getOrDefault(builder.retryPolicy, () -> InstanceProfileCredentialsRetryPolicy.NO_RETRY); - if (Boolean.TRUE.equals(builder.asyncCredentialUpdateEnabled)) { Validate.paramNotBlank(builder.asyncThreadName, "asyncThreadName"); this.credentialsCache = CachedSupplier.builder(this::refreshCredentials) @@ -217,9 +213,6 @@ private ResourcesEndpointProvider createEndpointProvider() { String[] securityCredentials = getSecurityCredentials(imdsHostname, token); return StaticResourcesEndpointProvider.builder() - .retryPolicy( - (retriesAttempted, param) -> retryPolicy.shouldRetry( - retriesAttempted, param.getStatusCode(), param.getException())) .endpoint(URI.create(imdsHostname + SECURITY_CREDENTIALS_RESOURCE + securityCredentials[0])) .headers(getTokenHeaders(token)) @@ -241,9 +234,6 @@ private String getToken(String imdsHostname) { ResourcesEndpointProvider tokenEndpoint = StaticResourcesEndpointProvider.builder() .endpoint(getTokenEndpoint(imdsHostname)) - .retryPolicy( - (retriesAttempted, param) -> retryPolicy.shouldRetry( - retriesAttempted, param.getStatusCode(), param.getException())) .headers(tokenTtlHeaders) .connectionTimeout(Duration.ofMillis( this.configProvider.serviceTimeout())) @@ -297,9 +287,6 @@ private boolean isInsecureFallbackDisabled() { private String[] getSecurityCredentials(String imdsHostname, String metadataToken) { ResourcesEndpointProvider securityCredentialsEndpoint = StaticResourcesEndpointProvider.builder() - .retryPolicy( - (retriesAttempted, param) -> retryPolicy.shouldRetry( - retriesAttempted, param.getStatusCode(), param.getException())) .endpoint(URI.create(imdsHostname + SECURITY_CREDENTIALS_RESOURCE)) .headers(getTokenHeaders(metadataToken)) .connectionTimeout(Duration.ofMillis(this.configProvider.serviceTimeout())) @@ -369,13 +356,6 @@ public interface Builder extends HttpCredentialsProvider.Builder profileFile; private String profileName; private Duration staleTime; - private InstanceProfileCredentialsRetryPolicy retryPolicy; private BuilderImpl() { asyncThreadName("instance-profile-credentials-provider"); @@ -406,7 +385,6 @@ private BuilderImpl(InstanceProfileCredentialsProvider provider) { this.profileFile = provider.profileFile; this.profileName = provider.profileName; this.staleTime = provider.staleTime; - this.retryPolicy = provider.retryPolicy; } Builder clock(Clock clock) { @@ -485,17 +463,6 @@ public void setStaleTime(Duration duration) { staleTime(duration); } - @Override - public Builder retryPolicy(InstanceProfileCredentialsRetryPolicy retryPolicy) { - this.retryPolicy = retryPolicy; - return this; - } - - public void setRetryPolicy(InstanceProfileCredentialsRetryPolicy retryPolicy) { - retryPolicy(retryPolicy); - } - - @Override public InstanceProfileCredentialsProvider build() { return new InstanceProfileCredentialsProvider(this); diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java deleted file mode 100644 index 59b518499623..000000000000 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsRetryPolicy.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * 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.auth.credentials; - -import software.amazon.awssdk.annotations.SdkPublicApi; - -/** - * Custom retry policy for {@link InstanceProfileCredentialsProvider} calls to retrieve credentials - * from the Amazon EC2 Instance Metadata Service. - */ -@SdkPublicApi -public interface InstanceProfileCredentialsRetryPolicy { - - InstanceProfileCredentialsRetryPolicy NO_RETRY = (retriesAttempted, statusCode, exception) -> false; - - /** - * Returns whether a failed request should be retried. - * - * @param retriesAttempted The number of times the current request has been attempted. - * @param statusCode The http status code returned by the call to Instance Metadata Service - * @param exception The error that caused the request to fail. - * - * @return True if the failed request should be retried. - */ - boolean shouldRetry(Integer retriesAttempted, Integer statusCode, Exception exception); -} diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java index fe67eaf87adf..023f4e4cec8d 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/InstanceProfileCredentialsProviderTest.java @@ -642,10 +642,7 @@ void testErrorWhileCacheIsStale_shouldRecover() { @Test void shouldNotRetry_whenSucceeds() { stubSecureCredentialsResponse(aResponse().withBody(STUB_CREDENTIALS)); - InstanceProfileCredentialsProvider provider = - InstanceProfileCredentialsProvider.builder() - .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < 3 && statusCode != 200) - .build(); + InstanceProfileCredentialsProvider provider = InstanceProfileCredentialsProvider.builder().build(); AwsCredentials credentials = provider.resolveCredentials(); assertThat(credentials.accessKeyId()).isEqualTo("ACCESS_KEY_ID"); assertThat(credentials.secretAccessKey()).isEqualTo("SECRET_ACCESS_KEY"); @@ -654,68 +651,6 @@ void shouldNotRetry_whenSucceeds() { WireMock.verify(exactly(1), getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); } - @ParameterizedTest - @ValueSource(ints = {0, 1, 2, 3, 4, 5}) - void shouldRetryAndFail_whenFails_basedOnRetryPolicy(int retries) { - InstanceProfileCredentialsProvider provider = - InstanceProfileCredentialsProvider.builder() - .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < retries && statusCode != 200) - .build(); - - String errorMessage = "some error msg"; - String credentialsResponse = - "{" - + "\"code\": \"InternalServiceException\"," - + "\"message\": \"" + errorMessage + "\"" - + "}"; - - stubSecureCredentialsResponse(aResponse().withStatus(500) - .withBody(credentialsResponse)); - assertThatThrownBy(provider::resolveCredentials).hasRootCauseMessage("some error msg"); - WireMock.verify(retries + 1, getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); - } - - @Test - void shouldRetryThenSucceed_basedOnRetryPolicy() { - String errorMessage = "some error msg"; - String credentialsResponse = - "{" - + "\"code\": \"InternalServiceException\"," - + "\"message\": \"" + errorMessage + "\"" - + "}"; - - wireMockServer.stubFor(put(urlPathEqualTo(TOKEN_RESOURCE_PATH)) - .inScenario("Retry") - .whenScenarioStateIs(STARTED) - .willReturn(aResponse().withBody(TOKEN_STUB).withStatus(200))); - wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH)) - .inScenario("Retry") - .willReturn(aResponse().withBody(PROFILE_NAME).withStatus(200)) - .whenScenarioStateIs(STARTED) - .willSetStateTo("error")); - wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + PROFILE_NAME)) - .inScenario("Retry") - .whenScenarioStateIs("error") - .willReturn(aResponse().withBody(credentialsResponse).withStatus(500)) - .willSetStateTo("success")); - - wireMockServer.stubFor(get(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + PROFILE_NAME)) - .inScenario("Retry") - .willReturn(aResponse().withBody(STUB_CREDENTIALS).withStatus(200)) - .whenScenarioStateIs("success")); - - InstanceProfileCredentialsProvider provider = - InstanceProfileCredentialsProvider.builder() - .retryPolicy((retriesAttempted, statusCode, exception) -> retriesAttempted < 3 && statusCode != 200) - .build(); - - provider.resolveCredentials(); - - WireMock.verify(putRequestedFor(urlPathEqualTo(TOKEN_RESOURCE_PATH))); - WireMock.verify(getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH))); - WireMock.verify(2, getRequestedFor(urlPathEqualTo(CREDENTIALS_RESOURCE_PATH + "some-profile"))); - } - private AwsCredentialsProvider credentialsProviderWithClock(Clock clock) { InstanceProfileCredentialsProvider.BuilderImpl builder = (InstanceProfileCredentialsProvider.BuilderImpl) InstanceProfileCredentialsProvider.builder(); From 806ce7776d58a03ff93c4ff0f64ea9d32b3ec47d Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Thu, 23 Jan 2025 15:00:37 -0500 Subject: [PATCH 6/6] default retryPolicy set in StaticResourcesEndpointProvider ctor --- .../credentials/internal/StaticResourcesEndpointProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java index 7d705e76944f..e2e56695e968 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/internal/StaticResourcesEndpointProvider.java @@ -44,7 +44,7 @@ private StaticResourcesEndpointProvider(URI endpoint, this.headers.putAll(additionalHeaders); } this.connectionTimeout = customTimeout; - this.retryPolicy = retryPolicy; + this.retryPolicy = Validate.getOrDefault(retryPolicy, () -> ResourcesEndpointRetryPolicy.NO_RETRY); } @Override @@ -71,7 +71,7 @@ public static class Builder { private URI endpoint; private Map additionalHeaders = new HashMap<>(); private Duration customTimeout; - private ResourcesEndpointRetryPolicy retryPolicy = ResourcesEndpointRetryPolicy.NO_RETRY; + private ResourcesEndpointRetryPolicy retryPolicy; public Builder endpoint(URI endpoint) { this.endpoint = Validate.paramNotNull(endpoint, "endpoint");