From ccba71ed3b1b376a823980863273ae54c58c82c9 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Fri, 28 Aug 2026 08:16:45 -0700 Subject: [PATCH 1/3] [CredentialsCache] Revert changes to ProcessCredentials stale/prefetch times --- .../ProcessCredentialsProvider.java | 33 ++----- .../ProcessCredentialsProviderTest.java | 39 ++------ .../awssdk/utils/cache/CacheRefreshUtils.java | 67 +------------ .../utils/cache/CacheRefreshUtilsTest.java | 93 +------------------ 4 files changed, 23 insertions(+), 209 deletions(-) diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java index f365594291bc..d9e5c0e59787 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java +++ b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java @@ -40,7 +40,6 @@ import software.amazon.awssdk.utils.Validate; import software.amazon.awssdk.utils.builder.CopyableBuilder; import software.amazon.awssdk.utils.builder.ToCopyableBuilder; -import software.amazon.awssdk.utils.cache.CacheRefreshUtils; import software.amazon.awssdk.utils.cache.CachedSupplier; import software.amazon.awssdk.utils.cache.NonBlocking; import software.amazon.awssdk.utils.cache.RefreshResult; @@ -63,12 +62,12 @@ * (deprecated) or as a list of strings. *
  • StaleTime - The amount of time before credential expiration that defines the mandatory refresh window. When * credentials are within this window, all callers block until a refresh attempt completes. If the refresh fails, an - * exception is raised. Default: 1 minute.
    • + * exception is raised. Default: 0, i.e. the mandatory refresh window opens at expiration. *
  • PrefetchTime - The amount of time before credential expiration that defines the advisory refresh window. When * credentials are within this window, the provider proactively attempts to refresh them. If the refresh fails during the * advisory window, the existing cached credentials are returned without error. This replaces the deprecated * {@code credentialRefreshThreshold} setting; if that setting was explicitly configured, its value is honored as the - * prefetch time for backward compatibility. Default: 5 minutes.
    • + * prefetch time for backward compatibility. Default: 15 seconds. *
  • AsyncCredentialUpdateEnabled - Whether to refresh credentials asynchronously in a background thread during * the advisory refresh window, so that callers are less likely to block. Default: disabled.
    • *
  • ProcessOutputLimit - The maximum amount of data that can be returned by the external process before an @@ -87,7 +86,8 @@ public final class ProcessCredentialsProvider private static final JsonNodeParser PARSER = JsonNodeParser.builder() .removeErrorLocations(true) .build(); - private static final Duration DEFAULT_STALE_TIME = Duration.ofMinutes(1); + private static final Duration DEFAULT_STALE_TIME = Duration.ZERO; + private static final Duration DEFAULT_PREFETCH_TIME = Duration.ofSeconds(15); private final List executableCommand; private final long processOutputLimit; @@ -121,11 +121,9 @@ private ProcessCredentialsProvider(Builder builder) { ? PROVIDER_NAME : builder.sourceChain + "," + PROVIDER_NAME; this.staleTime = Optional.ofNullable(builder.staleTime).orElse(DEFAULT_STALE_TIME); - this.prefetchTime = builder.prefetchTime; - if (this.prefetchTime != null) { - Validate.isTrue(this.staleTime.compareTo(this.prefetchTime) <= 0, - "staleTime (%s) must be less than or equal to prefetchTime (%s).", this.staleTime, this.prefetchTime); - } + this.prefetchTime = Optional.ofNullable(builder.prefetchTime).orElse(DEFAULT_PREFETCH_TIME); + Validate.isTrue(this.staleTime.compareTo(this.prefetchTime) <= 0, + "staleTime (%s) must be less than or equal to prefetchTime (%s).", this.staleTime, this.prefetchTime); CachedSupplier.Builder cacheBuilder = CachedSupplier.builder(this::refreshCredentials) .cachedValueName(toString()) @@ -206,11 +204,7 @@ private Instant prefetchTime(Instant expiration) { if (expiration == null || expiration.equals(Instant.MAX)) { return Instant.MAX; } - Instant now = Instant.now(); - // Unlike the AWS credential services, the process decides its own expiration and may emit credentials that are - // shorter-lived than the smallest standard advisory refresh window, so the window has to adapt to the lifetime. - Duration dynamicWindow = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, prefetchTime, now); - return expiration.minus(dynamicWindow); + return expiration.minus(prefetchTime); } /** @@ -375,7 +369,7 @@ public Builder asyncCredentialUpdateEnabled(Boolean asyncCredentialUpdateEnabled *

    This value must be less than or equal to {@link #prefetchTime(Duration)}. Setting this equal to * {@code prefetchTime} effectively disables prefetch, causing all refreshes to be mandatory (blocking). * - *

    By default, this is 1 minute. + *

    By default, this is zero, so the mandatory refresh window opens when the credentials expire. * * @param staleTime the duration before expiration that triggers mandatory (blocking) refresh */ @@ -398,14 +392,7 @@ public Builder staleTime(Duration staleTime) { *

    This value must be greater than or equal to {@link #staleTime(Duration)}. Setting this equal to * {@code staleTime} effectively disables prefetch, causing all refreshes to be mandatory (blocking). * - *

    If not explicitly set, the advisory refresh window is computed dynamically based on the credential's - * remaining lifetime: half the remaining lifetime (but never less than 1 minute) for credentials with 10 minutes - * or less remaining, 5 minutes for 10-20 minutes remaining, 15 minutes for 20-90 minutes remaining, and 60 - * minutes for 90+ minutes remaining. This dynamic window is recomputed on each successful refresh. - * - *

    The halved window for short-lived credentials is specific to this provider. Because the process decides its - * own expiration, it may emit credentials that are shorter-lived than the smallest standard window, which would - * otherwise place them inside their advisory refresh window as soon as they are produced. + *

    By default, this is 15 seconds. * * @param prefetchTime the duration before expiration that triggers advisory (proactive) refresh */ diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java index fc5df6543696..ec9f08c5aa60 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java +++ b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java @@ -228,7 +228,7 @@ void resultsAreCached() { ProcessCredentialsProvider.builder() .command(String.format("%s %s %s token=%s exp=%s", scriptLocation, ACCESS_KEY_ID, SECRET_ACCESS_KEY, SESSION_TOKEN, - DateUtils.formatIso8601Date(Instant.now().plus(Duration.ofMinutes(30))))) + DateUtils.formatIso8601Date(Instant.now().plusSeconds(20)))) .build(); AwsCredentials request1 = credentialsProvider.resolveCredentials(); @@ -279,41 +279,18 @@ void resolveCredentials_advisoryWindowIsNotJittered() { assertThat(request1).isNotEqualTo(request2); } - /** - * The process decides its own expiration and may produce credentials shorter-lived than the smallest standard advisory - * refresh window. This provider therefore halves the lifetime instead of using that window, so freshly produced - * credentials are served from the cache rather than re-running the process on every call. This differs from the - * AWS-service-backed providers, which can rely on a 15 minute minimum session duration. - */ - @Test - void shortLivedCredentials_areNotRefreshedOnTheCallFollowingIssuance() { - // A 3 minute lifetime gives a 90 second advisory window, which opens 90 seconds after the process runs. - ProcessCredentialsProvider credentialsProvider = - ProcessCredentialsProvider.builder() - .command(String.format("%s %s %s token=%s exp=%s", - scriptLocation, ACCESS_KEY_ID, SECRET_ACCESS_KEY, - RANDOM_SESSION_TOKEN, - DateUtils.formatIso8601Date(Instant.now().plus(Duration.ofMinutes(3))))) - .build(); - - // The process emits a random session token on each run, so equal credentials mean it only ran once. - AwsCredentials request1 = credentialsProvider.resolveCredentials(); - AwsCredentials request2 = credentialsProvider.resolveCredentials(); - - assertThat(request1).isEqualTo(request2); - } - @Test - void defaultPrefetchTime_credentialsWithinFiveMinuteWindow_areRefreshed() { - // Credentials that expire in 30 seconds: staleTime = now+30s - 1min = now-30s (in the past, stale!) - // In STRICT mode, stale credentials force a synchronous refresh on every call + void defaultPrefetchTime_credentialsWithinFifteenSecondsOfExpiry_areRefreshed() { + // Credentials that expire in 10 seconds: prefetchTime = now+10s - 15s = now-5s (in the past), so the advisory + // refresh window is already open when the credentials are produced and the next call re-runs the process. ProcessCredentialsProvider credentialsProvider = ProcessCredentialsProvider.builder() .command(String.format("%s %s %s token=%s exp=%s", scriptLocation, ACCESS_KEY_ID, SECRET_ACCESS_KEY, RANDOM_SESSION_TOKEN, - DateUtils.formatIso8601Date(Instant.now().plusSeconds(30)))) + DateUtils.formatIso8601Date(Instant.now().plusSeconds(10)))) .build(); + // The process emits a random session token on each run, so unequal credentials mean it ran twice. AwsCredentials request1 = credentialsProvider.resolveCredentials(); AwsCredentials request2 = credentialsProvider.resolveCredentials(); @@ -322,8 +299,8 @@ void defaultPrefetchTime_credentialsWithinFiveMinuteWindow_areRefreshed() { @Test void defaultPrefetchTime_credentialsFarFromExpiry_areCached() { - // Credentials that expire in 30 minutes: prefetchTime = now+30min - 5min = now+25min (in the future) - // So the cache should NOT refresh + // Credentials that expire in 30 minutes: prefetchTime = now+30min - 15s (in the future), so the cache should + // NOT refresh. ProcessCredentialsProvider credentialsProvider = ProcessCredentialsProvider.builder() .command(String.format("%s %s %s token=%s exp=%s", diff --git a/utils/src/main/java/software/amazon/awssdk/utils/cache/CacheRefreshUtils.java b/utils/src/main/java/software/amazon/awssdk/utils/cache/CacheRefreshUtils.java index 60aae6708fbd..224ebf3abc35 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/cache/CacheRefreshUtils.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/cache/CacheRefreshUtils.java @@ -18,7 +18,6 @@ import java.time.Duration; import java.time.Instant; import software.amazon.awssdk.annotations.SdkProtectedApi; -import software.amazon.awssdk.utils.ComparableUtils; /** * Utility methods for credential cache refresh timing computation. @@ -30,13 +29,6 @@ public final class CacheRefreshUtils { private static final Duration WINDOW_MEDIUM = Duration.ofMinutes(15); private static final Duration WINDOW_LONG = Duration.ofMinutes(60); - /** - * The smallest window the halved-lifetime rule will produce. This matches the default mandatory refresh window, so that - * the advisory refresh window is never narrower than the mandatory one. - */ - private static final Duration WINDOW_MIN = Duration.ofMinutes(1); - - private static final Duration THRESHOLD_HALVED = Duration.ofMinutes(10); private static final Duration THRESHOLD_MEDIUM = Duration.ofMinutes(20); private static final Duration THRESHOLD_LONG = Duration.ofMinutes(90); @@ -57,9 +49,8 @@ private CacheRefreshUtils() { * * *

    This assumes the credential source does not vend credentials with a lifetime shorter than the smallest window - * above. That holds for AWS credential services, which have a minimum session duration of 15 minutes. Callers whose - * credential source offers no such guarantee should use - * {@link #computePrefetchWindowForArbitraryLifetime(Instant, Duration, Instant)} instead. + * above. That holds for AWS credential services, which have a minimum session duration of 15 minutes. A credential + * shorter-lived than its window is inside its advisory refresh window from the moment it is issued. * * @param expiration the credential's expiration time * @param prefetchTime the explicitly configured prefetch window, or {@code null} to compute dynamically @@ -77,58 +68,8 @@ public static Duration computePrefetchWindow(Instant expiration, Duration prefet return WINDOW_SHORT; } - return windowForLifetime(remainingLifetime); - } - - /** - * As {@link #computePrefetchWindow(Instant, Duration, Instant)}, but also handles credentials whose lifetime is shorter - * than the smallest window that method would assign. For those, the window is half the remaining lifetime, floored at 1 - * minute to match the default mandatory refresh window: - * - *

      - *
    • remaining lifetime <= 10 minutes → half the remaining lifetime, but never less than 1 minute
      • - *
    • 10 minutes < remaining lifetime <= 20 minutes → 5 minute window
      • - *
    • 20 minutes < remaining lifetime < 90 minutes → 15 minute window
      • - *
    • remaining lifetime >= 90 minutes → 60 minute window
      • - *
    - * - *

    Halving keeps the window inside the credential's lifetime, and joins the 5 minute window continuously at a lifetime - * of 10 minutes. Without it, a credential shorter-lived than its window is inside its advisory refresh window from the - * moment it is issued, so every subsequent request for it contacts the credential source. - * - *

    This is for credential sources whose expiration the SDK cannot make assumptions about, such as an external process. - * Callers backed by AWS credential services should use {@link #computePrefetchWindow(Instant, Duration, Instant)}, whose - * windows follow the credential refresh specification exactly. - * - * @param expiration the credential's expiration time - * @param prefetchTime the explicitly configured prefetch window, or {@code null} to compute dynamically - * @param now the current time - * @return the Duration to use as the advisory refresh window - */ - public static Duration computePrefetchWindowForArbitraryLifetime(Instant expiration, Duration prefetchTime, Instant now) { - if (prefetchTime != null) { - return prefetchTime; - } - - Duration remainingLifetime = Duration.between(now, expiration); - if (remainingLifetime.isNegative() || remainingLifetime.isZero()) { - // Already expired. Any window puts the prefetch time in the past, which refreshes on the next request. - return WINDOW_SHORT; - } - - if (remainingLifetime.compareTo(THRESHOLD_HALVED) <= 0) { - return ComparableUtils.maximum(remainingLifetime.dividedBy(2), WINDOW_MIN); - } - - return windowForLifetime(remainingLifetime); - } - - /** - * The window tiers defined by the credential refresh specification. Thresholds are compared as durations rather than - * whole minutes, so that a lifetime just over a boundary selects the larger window instead of being truncated down into - * the smaller one. - */ - private static Duration windowForLifetime(Duration remainingLifetime) { + // Thresholds are compared as durations rather than whole minutes, so that a lifetime just over a boundary selects + // the larger window instead of being truncated down into the smaller one. if (remainingLifetime.compareTo(THRESHOLD_MEDIUM) <= 0) { return WINDOW_SHORT; } else if (remainingLifetime.compareTo(THRESHOLD_LONG) < 0) { diff --git a/utils/src/test/java/software/amazon/awssdk/utils/cache/CacheRefreshUtilsTest.java b/utils/src/test/java/software/amazon/awssdk/utils/cache/CacheRefreshUtilsTest.java index 6d1a62781098..a9c5b86d2958 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/cache/CacheRefreshUtilsTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/cache/CacheRefreshUtilsTest.java @@ -45,8 +45,7 @@ public void remainingLifetimeExactly0_returns5MinuteWindow() { /** * AWS credential services do not issue sessions shorter than 15 minutes, so the specification's smallest window can - * exceed the lifetime only when the credential source is not an AWS service (or the host clock is skewed). Callers that - * have to tolerate that use {@link CacheRefreshUtils#computePrefetchWindowForArbitraryLifetime}. + * exceed the lifetime only when the credential source is not an AWS service (or the host clock is skewed). */ @Test public void remainingLifetimeShorterThanSmallestWindow_stillReturns5MinuteWindow() { @@ -151,94 +150,4 @@ public void explicitPrefetchTime_ignoresRemainingLifetime() { Duration window = CacheRefreshUtils.computePrefetchWindow(expiration, explicitPrefetch, NOW); assertThat(window).isEqualTo(Duration.ofMinutes(60)); } - - // computePrefetchWindowForArbitraryLifetime: adds the halved tier for credential sources that may vend credentials - // shorter-lived than the smallest specification window. - - @Test - public void arbitraryLifetime_remainingLifetime3Minutes_returnsHalfOfLifetime() { - Instant expiration = NOW.plus(Duration.ofMinutes(3)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofSeconds(90)); - } - - @Test - public void arbitraryLifetime_remainingLifetime5Minutes_returnsHalfOfLifetime() { - Instant expiration = NOW.plus(Duration.ofMinutes(5)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofSeconds(150)); - } - - @Test - public void arbitraryLifetime_remainingLifetimeExactly10Minutes_returnsHalfOfLifetime() { - // The halved tier joins the 5 minute tier continuously here. - Instant expiration = NOW.plus(Duration.ofMinutes(10)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(5)); - } - - @Test - public void arbitraryLifetime_remainingLifetimeJustOver10Minutes_returns5MinuteWindow() { - Instant expiration = NOW.plus(Duration.ofMinutes(10)).plusSeconds(1); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(5)); - } - - @Test - public void arbitraryLifetime_remainingLifetime2Minutes_returnsMandatoryWindowFloor() { - // Half of 2 minutes is exactly the 1 minute floor. - Instant expiration = NOW.plus(Duration.ofMinutes(2)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(1)); - } - - @Test - public void arbitraryLifetime_remainingLifetimeUnder2Minutes_isFlooredAtMandatoryWindow() { - // Half of 90 seconds is 45 seconds, which is narrower than the 1 minute mandatory refresh window. - Instant expiration = NOW.plus(Duration.ofSeconds(90)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(1)); - } - - @Test - public void arbitraryLifetime_remainingLifetimeNegative_returns5MinuteWindow() { - Instant expiration = NOW.minus(Duration.ofMinutes(5)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(5)); - } - - @Test - public void arbitraryLifetime_explicitPrefetchTime_returnsExplicitValue() { - Instant expiration = NOW.plus(Duration.ofMinutes(3)); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, Duration.ofMinutes(2), NOW); - assertThat(window).isEqualTo(Duration.ofMinutes(2)); - } - - /** - * The halved tier is the only difference between the two methods. Above 10 minutes they must agree, so that the - * specification's windows apply to every credential long-lived enough to have one. - */ - @Test - public void arbitraryLifetime_above10Minutes_matchesSpecificationWindows() { - for (long lifetimeSeconds = 601; lifetimeSeconds <= 12 * 60 * 60; lifetimeSeconds++) { - Instant expiration = NOW.plus(Duration.ofSeconds(lifetimeSeconds)); - assertThat(CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(expiration, null, NOW)) - .as("lifetime %s seconds", lifetimeSeconds) - .isEqualTo(CacheRefreshUtils.computePrefetchWindow(expiration, null, NOW)); - } - } - - /** - * The advisory refresh window must land strictly inside the credential's lifetime, otherwise the credential is inside its - * advisory window the moment it is issued and every subsequent request contacts the credential source. The 1 minute floor - * means this can only be guaranteed for credentials that outlive the mandatory refresh window. - */ - @Test - public void arbitraryLifetime_anyLifetimeLongerThanMandatoryWindow_windowIsShorterThanLifetime() { - for (long lifetimeSeconds = 61; lifetimeSeconds <= 12 * 60 * 60; lifetimeSeconds++) { - Duration lifetime = Duration.ofSeconds(lifetimeSeconds); - Duration window = CacheRefreshUtils.computePrefetchWindowForArbitraryLifetime(NOW.plus(lifetime), null, NOW); - assertThat(window).as("lifetime %s", lifetime).isLessThan(lifetime); - } - } } From d80106031f9829be3430c17139cd462ab7096c79 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Fri, 28 Aug 2026 10:28:08 -0700 Subject: [PATCH 2/3] Invalidate in the async refresh path as well (missed earlier) --- .../amazon/awssdk/spotbugs-suppressions.xml | 1 - .../utils/AuthErrorInvalidationHelper.java | 79 ++++++---- .../stages/utils/RetryableStageHelper.java | 4 + .../AuthErrorInvalidationHelperTest.java | 136 ++++++++++++++---- .../AuthErrorInvalidationFunctionalTest.java | 73 ++++++++++ 5 files changed, 239 insertions(+), 54 deletions(-) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index c4a837ae3f05..fb37121a02e5 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -346,7 +346,6 @@ - diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelper.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelper.java index e27c1f7176fd..10320bbf5c7e 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelper.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelper.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.core.internal.http.pipeline.stages.utils; +import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SelectedAuthScheme; import software.amazon.awssdk.core.exception.SdkServiceException; @@ -22,7 +23,6 @@ import software.amazon.awssdk.core.internal.http.RequestExecutionContext; import software.amazon.awssdk.identity.spi.Identity; import software.amazon.awssdk.identity.spi.IdentityProvider; -import software.amazon.awssdk.utils.CompletableFutureUtils; import software.amazon.awssdk.utils.Logger; /** @@ -32,7 +32,14 @@ *

    When a service returns an authentication error (as determined by * {@link SdkServiceException#isAuthenticationError()}), this helper retrieves the * {@link SelectedAuthScheme} from the execution context and calls - * {@link IdentityProvider#invalidate} so the next retry attempt resolves fresh credentials. + * {@link IdentityProvider#invalidate} so that the provider refreshes before it vends credentials again. + * + *

    Identity is resolved once per API call, by a stage that sits outside the retry loop, and every attempt of that + * call reuses it. Invalidation therefore does not affect the attempt currently being retried; it takes effect on the + * next API call that resolves credentials. + * + *

    Both the synchronous and asynchronous request paths must call + * {@link #invalidateIfAuthError(Throwable, RequestExecutionContext)} for the behavior to apply to both client types. * *

    All exceptions from the invalidation path are caught and logged at debug level. * Invalidation failures never disrupt the normal request/retry flow. @@ -50,49 +57,67 @@ private AuthErrorInvalidationHelper() { * credential invalidation. If so, retrieves the identity provider from the * {@link SelectedAuthScheme} and calls invalidate() on it. * + *

    This never blocks the calling thread: it composes on the resolved identity future rather than joining it, so + * it is safe to call from the async request path, which runs on I/O threads. In practice the identity is already + * resolved by the time a response has been received, so the invalidation completes inline. + * + *

    The returned future never completes exceptionally. Invalidation is best-effort, and any failure is logged at + * debug level instead of being propagated. Callers are not required to await it: identity is resolved outside the + * retry loop, so a pending invalidation cannot affect the attempt currently being retried. + * * @param exception The exception from the failed request attempt * @param context The request execution context containing auth scheme info + * @return A future completing when the invalidation attempt has finished. Never completes exceptionally. + */ + public static CompletableFuture invalidateIfAuthError(Throwable exception, RequestExecutionContext context) { + SelectedAuthScheme selectedAuthScheme = authSchemeToInvalidate(exception, context); + if (selectedAuthScheme == null) { + return CompletableFuture.completedFuture(null); + } + + try { + return doInvalidate(selectedAuthScheme); + } catch (Exception e) { + LOG.debug(() -> "Failed to invalidate identity provider after auth error: " + e.getMessage(), e); + return CompletableFuture.completedFuture(null); + } + } + + /** + * Returns the {@link SelectedAuthScheme} whose identity provider should be invalidated in response to the given + * exception, or null if the exception is not an authentication failure or there is no provider to invalidate. */ - public static void invalidateIfAuthError(Throwable exception, RequestExecutionContext context) { + private static SelectedAuthScheme authSchemeToInvalidate(Throwable exception, RequestExecutionContext context) { if (!(exception instanceof SdkServiceException)) { - return; + return null; } SdkServiceException serviceException = (SdkServiceException) exception; if (!serviceException.isAuthenticationError()) { - return; + return null; } SelectedAuthScheme selectedAuthScheme = context.executionAttributes().getAttribute(SdkInternalExecutionAttribute.SELECTED_AUTH_SCHEME); if (selectedAuthScheme == null || selectedAuthScheme.identityProvider() == null) { - return; + return null; } - try { - doInvalidate(selectedAuthScheme); - } catch (Exception e) { - LOG.debug(() -> "Failed to invalidate identity provider after auth error: " + e.getMessage(), e); - } + return selectedAuthScheme; } - private static void doInvalidate(SelectedAuthScheme selectedAuthScheme) { - T resolvedIdentity = CompletableFutureUtils.joinLikeSync(selectedAuthScheme.identity()); + private static CompletableFuture doInvalidate(SelectedAuthScheme selectedAuthScheme) { IdentityProvider provider = selectedAuthScheme.identityProvider(); - // Invalidation is best-effort and must not block the request/retry path. - // Most CredentialProvider invalidation implementations invalidate synchronously and return instantly - // but handle the future here. - try { - provider.invalidate(resolvedIdentity) - .exceptionally(e -> { - if (e != null) { - LOG.debug(() -> "Failed to invalidate identity provider: " + e.getMessage(), e); - } - return null; - }); - } catch (RuntimeException e) { - LOG.debug(() -> "Failed to invalidate identity provider: " + e.getMessage(), e); - } + // thenCompose rather than a join on the identity future: by the time a response has been received the identity + // is already resolved, so this completes inline, but it stays non-blocking if it ever is not. + // A synchronous throw from invalidate(), and a failed identity future, both complete the composed future + // exceptionally, so exceptionally() covers every failure mode. + return selectedAuthScheme.identity() + .thenCompose(provider::invalidate) + .exceptionally(e -> { + LOG.debug(() -> "Failed to invalidate identity provider: " + e.getMessage(), e); + return null; + }); } } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/RetryableStageHelper.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/RetryableStageHelper.java index 6d0418b262d7..ca52e10b6025 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/RetryableStageHelper.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/RetryableStageHelper.java @@ -197,6 +197,10 @@ private RefreshRetryTokenResponse doBlockingRefreshRetryToken(Duration suggested } public CompletableFuture> tryRefreshTokenAsync(Duration suggestedDelay) { + // Invalidate cached credentials if this failure is an auth error, before the retry strategy evaluates. + // Not awaited: invalidation is best-effort and must not delay the retry path. + AuthErrorInvalidationHelper.invalidateIfAuthError(this.lastException, context); + CompletableFuture> cf = new CompletableFuture<>(); RetryToken retryToken = context.executionAttributes().getAttribute(RETRY_TOKEN); diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelperTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelperTest.java index 62b8b6e4aca9..9f9298726d0f 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelperTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/http/pipeline/stages/utils/AuthErrorInvalidationHelperTest.java @@ -16,7 +16,6 @@ package software.amazon.awssdk.core.internal.http.pipeline.stages.utils; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatNoException; import static org.mockito.Mockito.mock; import java.io.IOException; @@ -24,6 +23,10 @@ import java.util.HashSet; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; @@ -109,21 +112,22 @@ static Stream nonServiceExceptions() { ); } + // --- Returned future contract --- + // Both request paths rely on the future never failing, and the async path additionally on it never blocking. + @Test - void invalidateIfAuthError_whenSelectedAuthSchemeIsNull_doesNotThrow() { + void invalidateIfAuthError_whenSelectedAuthSchemeIsNull_completesNormally() { RequestExecutionContext context = contextWithNoAuthScheme(); Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); - assertThatNoException().isThrownBy(() -> - AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context) - ); + assertThat(AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context)) + .isCompletedWithValue(null); } @Test - void invalidateIfAuthError_whenIdentityProviderIsNull_doesNotThrow() { - TestIdentity identity = new TestIdentity(); + void invalidateIfAuthError_whenIdentityProviderIsNull_completesNormally() { SelectedAuthScheme selectedAuthScheme = new SelectedAuthScheme<>( - CompletableFuture.completedFuture(identity), + CompletableFuture.completedFuture(new TestIdentity()), mockSigner(), AuthSchemeOption.builder().schemeId("test").build() ); @@ -131,35 +135,93 @@ void invalidateIfAuthError_whenIdentityProviderIsNull_doesNotThrow() { RequestExecutionContext context = contextWithSelectedAuthScheme(selectedAuthScheme); Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); - assertThatNoException().isThrownBy(() -> - AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context) - ); + assertThat(AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context)) + .isCompletedWithValue(null); } + /** + * A provider that throws synchronously from invalidate() must not fail the returned future, since callers on the + * async path do not handle it. + */ @Test - void invalidateIfAuthError_whenInvalidateThrowsException_doesNotPropagate() { - ThrowingIdentityProvider provider = new ThrowingIdentityProvider(); - TestIdentity identity = new TestIdentity(); - SelectedAuthScheme selectedAuthScheme = SelectedAuthScheme.builder() - .identity(CompletableFuture.completedFuture(identity)) - .signer(mockSigner()) - .authSchemeOption(AuthSchemeOption.builder().schemeId("test").build()) - .identityProvider(provider) - .build(); + void invalidateIfAuthError_whenInvalidateThrows_completesNormally() { + RequestExecutionContext context = contextWithProvider(new ThrowingIdentityProvider(), new TestIdentity()); + Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); - RequestExecutionContext context = contextWithSelectedAuthScheme(selectedAuthScheme); + CompletableFuture invalidation = AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context); + + assertThat(invalidation).isCompletedWithValue(null); + } + + @Test + void invalidateIfAuthError_whenInvalidateReturnsFailedFuture_completesNormally() { + RequestExecutionContext context = contextWithProvider(new FailedFutureIdentityProvider(), new TestIdentity()); Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); - assertThatNoException().isThrownBy(() -> - AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context) - ); + CompletableFuture invalidation = AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context); + + assertThat(invalidation).isCompletedWithValue(null); + } + + @Test + void invalidateIfAuthError_whenIdentityFutureFailed_completesNormallyWithoutInvalidating() { + TrackingIdentityProvider provider = new TrackingIdentityProvider(); + CompletableFuture identityFuture = new CompletableFuture<>(); + identityFuture.completeExceptionally(new RuntimeException("identity resolution failed")); + RequestExecutionContext context = contextWithProvider(provider, identityFuture); + Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); + + CompletableFuture invalidation = AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context); + + assertThat(invalidation).isCompletedWithValue(null); + assertThat(provider.invalidateCalled()).isFalse(); + } + + /** + * The async request path calls this from an I/O thread, so it must never block waiting on the identity. The call is + * made on a separate thread with a bounded get() so that a blocking implementation fails the test rather than + * hanging it. + */ + @Test + void invalidateIfAuthError_whenIdentityNotYetResolved_doesNotBlock() throws Exception { + TrackingIdentityProvider provider = new TrackingIdentityProvider(); + CompletableFuture identityFuture = new CompletableFuture<>(); + RequestExecutionContext context = contextWithProvider(provider, identityFuture); + Throwable exception = serviceExceptionWithErrorCode("ExpiredToken"); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + Future> call = + executor.submit(() -> AuthErrorInvalidationHelper.invalidateIfAuthError(exception, context)); + + CompletableFuture invalidation = call.get(5, TimeUnit.SECONDS); + + // Returned without waiting on the identity, so nothing has been invalidated yet. + assertThat(invalidation).isNotDone(); + assertThat(provider.invalidateCalled()).isFalse(); + + // Resolving the identity drives the invalidation to completion. + TestIdentity identity = new TestIdentity(); + identityFuture.complete(identity); + + invalidation.get(5, TimeUnit.SECONDS); + assertThat(provider.invalidateCalled()).isTrue(); + assertThat(provider.lastInvalidatedIdentity()).isSameAs(identity); + } finally { + executor.shutdownNow(); + } } // --- Helper methods --- - private RequestExecutionContext contextWithProvider(TrackingIdentityProvider provider, TestIdentity identity) { + private RequestExecutionContext contextWithProvider(IdentityProvider provider, TestIdentity identity) { + return contextWithProvider(provider, CompletableFuture.completedFuture(identity)); + } + + private RequestExecutionContext contextWithProvider(IdentityProvider provider, + CompletableFuture identityFuture) { SelectedAuthScheme selectedAuthScheme = SelectedAuthScheme.builder() - .identity(CompletableFuture.completedFuture(identity)) + .identity(identityFuture) .signer(mockSigner()) .authSchemeOption(AuthSchemeOption.builder().schemeId("test").build()) .identityProvider(provider) @@ -269,6 +331,28 @@ public CompletableFuture invalidate(TestIdentity identity) { } } + /** + * An identity provider whose invalidate() returns a failed future rather than throwing. + */ + private static class FailedFutureIdentityProvider implements IdentityProvider { + @Override + public Class identityType() { + return TestIdentity.class; + } + + @Override + public CompletableFuture resolveIdentity(ResolveIdentityRequest request) { + return CompletableFuture.completedFuture(new TestIdentity()); + } + + @Override + public CompletableFuture invalidate(TestIdentity identity) { + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(new RuntimeException("Simulated invalidation failure")); + return failed; + } + } + /** * Simulates an AwsServiceException that reports authentication errors via the * {@link SdkServiceException#isAuthenticationError()} virtual method. diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/AuthErrorInvalidationFunctionalTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/AuthErrorInvalidationFunctionalTest.java index bf18fc2a5e5f..468da6b6b120 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/AuthErrorInvalidationFunctionalTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/retry/AuthErrorInvalidationFunctionalTest.java @@ -34,7 +34,9 @@ import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.identity.spi.ResolveIdentityRequest; import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonAsyncClient; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonClient; +import software.amazon.awssdk.testutils.service.http.MockAsyncHttpClient; import software.amazon.awssdk.testutils.service.http.MockSyncHttpClient; import software.amazon.awssdk.utils.StringInputStream; @@ -99,6 +101,52 @@ public void expiredToken_invalidatesCredentials_nextCallUsesFreshCredentials() { } } + /** + * The async client must invalidate on an auth error just as the sync client does. The two client types drive + * retries through different helper methods, so this is exercised separately rather than assumed from the sync case. + */ + @Test + public void async_expiredToken_invalidatesCredentials_nextCallUsesFreshCredentials() { + MockAsyncHttpClient mockHttpClient = new MockAsyncHttpClient(); + InvalidationTrackingCredentialsProvider credentialsProvider = + new InvalidationTrackingCredentialsProvider("old-key", "old-secret", "new-key", "new-secret"); + + try (ProtocolRestJsonAsyncClient client = ProtocolRestJsonAsyncClient.builder() + .credentialsProvider(credentialsProvider) + .region(Region.US_EAST_1) + .endpointOverride(URI.create("http://localhost")) + .httpClient(mockHttpClient) + .build()) { + + // First API call: ExpiredToken (not retryable — fails immediately) + mockHttpClient.stubResponses(expiredTokenResponse()); + + assertThatThrownBy(() -> client.allTypes().join()) + .hasCauseInstanceOf(AwsServiceException.class) + .satisfies(e -> assertThat(((AwsServiceException) e.getCause()).awsErrorDetails().errorCode()) + .isEqualTo("ExpiredToken")); + + // Verify invalidate was called during the first API call + assertThat(credentialsProvider.invalidateCallCount()).isEqualTo(1); + + // Verify first call used "old-key" + List firstCallRequests = mockHttpClient.getRequests(); + assertThat(firstCallRequests).hasSize(1); + assertRequestUsedAccessKey(firstCallRequests.get(0), "old-key"); + + // Now make a second API call — this should resolve fresh credentials + mockHttpClient.reset(); + mockHttpClient.stubResponses(successResponse()); + + client.allTypes().join(); + + // Second call should use new credentials (provider was invalidated) + List secondCallRequests = mockHttpClient.getRequests(); + assertThat(secondCallRequests).hasSize(1); + assertRequestUsedAccessKey(secondCallRequests.get(0), "new-key"); + } + } + /** * Verify that AccessDenied does NOT trigger invalidation and the exception propagates. */ @@ -127,6 +175,31 @@ public void accessDenied_doesNotInvalidateCredentials() { } } + @Test + public void async_accessDenied_doesNotInvalidateCredentials() { + MockAsyncHttpClient mockHttpClient = new MockAsyncHttpClient(); + InvalidationTrackingCredentialsProvider credentialsProvider = + new InvalidationTrackingCredentialsProvider("my-key", "my-secret", "new-key", "new-secret"); + + try (ProtocolRestJsonAsyncClient client = ProtocolRestJsonAsyncClient.builder() + .credentialsProvider(credentialsProvider) + .region(Region.US_EAST_1) + .endpointOverride(URI.create("http://localhost")) + .httpClient(mockHttpClient) + .build()) { + + mockHttpClient.stubResponses(accessDeniedResponse()); + + assertThatThrownBy(() -> client.allTypes().join()) + .hasCauseInstanceOf(AwsServiceException.class) + .satisfies(e -> assertThat(((AwsServiceException) e.getCause()).awsErrorDetails().errorCode()) + .isEqualTo("AccessDenied")); + + // Verify invalidate was NOT called + assertThat(credentialsProvider.invalidateCallCount()).isEqualTo(0); + } + } + // --- Helper methods --- private void assertRequestUsedAccessKey(SdkHttpRequest request, String expectedAccessKeyId) { From 7f88cc4676a979330548596e5c381e6d5916046a Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Fri, 28 Aug 2026 11:20:40 -0700 Subject: [PATCH 3/3] Fix flaky test --- .../utils/cache/CachedSupplierTest.java | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java b/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java index ec4c0760f71a..bc0d34cf8259 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java @@ -62,12 +62,23 @@ public class CachedSupplierTest { /** Maximum static stability backoff duration in seconds (10 minutes). */ private static final long BACKOFF_MAX_SECONDS = 600; + /** Minimum duration (seconds) that a non-recoverable error stays cached. */ + private static final long NON_RECOVERABLE_ERROR_CACHE_MIN_SECONDS = 1; + /** Maximum duration (seconds) that a non-recoverable error stays cached. */ private static final long NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS = 5; /** A duration safely past the non-recoverable error cache max, guaranteeing the cache has expired. */ private static final long PAST_NON_RECOVERABLE_ERROR_CACHE = NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS + 1; + /** + * A duration guaranteed to fall strictly inside the non-recoverable error cache window, whichever duration the + * jitter picked. This must stay below the cache minimum: the cache expires exactly at its expiry instant, so + * advancing by the minimum itself lands on an already-expired cache whenever the jitter picks that minimum. + */ + private static final long WITHIN_NON_RECOVERABLE_ERROR_CACHE_MILLIS = + NON_RECOVERABLE_ERROR_CACHE_MIN_SECONDS * 1000 - 100; + /** A duration safely past the maximum backoff, guaranteeing the backoff has elapsed. */ private static final long PAST_MAX_BACKOFF = BACKOFF_MAX_SECONDS + 1; @@ -1233,8 +1244,8 @@ public void allowMode_nonRecoverableErrorCached_withinCacheWindow_reRaisesWithou assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); assertThat(supplierCallCount.get()).isEqualTo(1); - // Second call within the cache window (< 5 seconds) — should re-raise without calling source - clock.time = now.plusSeconds(1); + // Second call within the cache window — should re-raise without calling source + clock.time = now.plusMillis(WITHIN_NON_RECOVERABLE_ERROR_CACHE_MILLIS); assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); assertThat(supplierCallCount.get()).isEqualTo(1); // Still 1 — source was NOT contacted } @@ -1345,7 +1356,7 @@ public void allowMode_nonRecoverableErrorCached_staleWindow_reRaisesWithoutCalli assertThatThrownBy(cachedSupplier::get).isEqualTo(error); // Immediately retry (within cache window) — should re-raise the same error without calling source - clock.time = now.plusSeconds(62); + clock.time = now.plusSeconds(61).plusMillis(WITHIN_NON_RECOVERABLE_ERROR_CACHE_MILLIS); // Swap supplier to something that would succeed — if called, we'd get "new-creds" not an exception supplier.set(RefreshResult.builder("new-creds") .staleTime(Instant.MAX) @@ -1385,7 +1396,7 @@ public void allowMode_nonRecoverableErrorCached_prefetchWindow_reRaisesWithoutCa assertThatThrownBy(cachedSupplier::get).isEqualTo(error); // Immediately retry (within cache window) — should re-raise without calling source - clock.time = now.plusSeconds(62); + clock.time = now.plusSeconds(61).plusMillis(WITHIN_NON_RECOVERABLE_ERROR_CACHE_MILLIS); supplier.set(RefreshResult.builder("new-creds") .staleTime(Instant.MAX) .prefetchTime(Instant.MAX) @@ -1419,8 +1430,8 @@ public void allowMode_nonRecoverableErrorCached_cacheWindowIsJitteredBetween1And assertThatThrownBy(cachedSupplier::get).isEqualTo(error); assertThat(callCount.get()).isEqualTo(1); - // At 0.9s — should still be cached (cache min is 1s) - clock.time = now.plusMillis(900); + // Just under the cache minimum — should still be cached whatever the jitter picked + clock.time = now.plusMillis(WITHIN_NON_RECOVERABLE_ERROR_CACHE_MILLIS); assertThatThrownBy(cachedSupplier::get).isEqualTo(error); assertThat(callCount.get()).isEqualTo(1);