diff --git a/generator/.DevConfigs/fe9da473-d386-4772-8d42-aec0583e4e02.json b/generator/.DevConfigs/fe9da473-d386-4772-8d42-aec0583e4e02.json new file mode 100644 index 000000000000..60258fd73f75 --- /dev/null +++ b/generator/.DevConfigs/fe9da473-d386-4772-8d42-aec0583e4e02.json @@ -0,0 +1,9 @@ +{ + "core": { + "updateMinimum": true, + "type": "Patch", + "changeLogMessages": [ + "Revert [background refresh of credentials during preempt expiry period](https://github.com/aws/aws-sdk-net/commit/ab7f535b92fcfb07c92bf7a0aa91853f3114e46d) as it has caused the SDK to return expired credentials in some scenarios" + ] + } +} \ No newline at end of file diff --git a/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs b/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs index de6e50d627dd..afe79d0b5af6 100644 --- a/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs +++ b/sdk/src/Core/Amazon.Runtime/Credentials/RefreshingAWSCredentials.cs @@ -26,7 +26,7 @@ namespace Amazon.Runtime /// public abstract class RefreshingAWSCredentials : AWSCredentials, IDisposable { - private Logger _logger = Logger.GetLogger(typeof(RefreshingAWSCredentials)); + private readonly Logger _logger = Logger.GetLogger(typeof(RefreshingAWSCredentials)); /// public override DateTime? Expiration @@ -68,14 +68,6 @@ internal bool IsExpiredWithin(TimeSpan preemptExpiryTime) var exp = Expiration.ToUniversalTime(); return now > exp - preemptExpiryTime; } - - internal TimeSpan GetTimeToLive(TimeSpan preemptExpiryTime) - { - var now = AWSSDKUtils.CorrectedUtcNow; - var exp = Expiration.ToUniversalTime(); - - return exp - now + preemptExpiryTime; - } } /// @@ -127,105 +119,51 @@ public TimeSpan PreemptExpiryTime /// public override sealed ImmutableCredentials GetCredentials() { - // We save the currentState as it might be modified or cleared. - var tempState = currentState; - - var ttl = tempState?.GetTimeToLive(PreemptExpiryTime); + _updateGeneratedCredentialsSemaphore.Wait(); - if (ttl > TimeSpan.Zero) + try { - if (ttl < PreemptExpiryTime) + // We save the currentState as it might be modified or cleared. + var tempState = currentState; + + // If credentials are expired or we don't have any state yet, update + if (ShouldUpdateState(tempState)) { - // background refresh (fire & forget) - if (_updateGeneratedCredentialsSemaphore.Wait(0)) - { - _ = System.Threading.Tasks.Task.Run(GenerateCredentialsAndUpdateState); - } + tempState = GenerateNewCredentials(); + UpdateToGeneratedCredentials(tempState); + currentState = tempState; } + + return tempState.Credentials; } - else + finally { - // If credentials are expired, update - _updateGeneratedCredentialsSemaphore.Wait(); - tempState = GenerateCredentialsAndUpdateState(); - } - - return tempState.Credentials; - - CredentialsRefreshState GenerateCredentialsAndUpdateState() - { - System.Diagnostics.Debug.Assert(_updateGeneratedCredentialsSemaphore.CurrentCount == 0); - - try - { - var tempState = currentState; - // double-check that the credentials still need updating - // as it's possible that multiple requests were queued acquiring the semaphore - if (ShouldUpdateState(tempState)) - { - tempState = GenerateNewCredentials(); - UpdateToGeneratedCredentials(tempState); - currentState = tempState; - } - - return tempState; - } - finally - { - _updateGeneratedCredentialsSemaphore.Release(); - } + _updateGeneratedCredentialsSemaphore.Release(); } } public override sealed async System.Threading.Tasks.Task GetCredentialsAsync() { - // We save the currentState as it might be modified or cleared. - var tempState = currentState; + await _updateGeneratedCredentialsSemaphore.WaitAsync().ConfigureAwait(false); - var ttl = tempState?.GetTimeToLive(PreemptExpiryTime); - - if (ttl > TimeSpan.Zero) + try { - if (ttl < PreemptExpiryTime) + // We save the currentState as it might be modified or cleared. + var tempState = currentState; + + // If credentials are expired, update + if (ShouldUpdateState(tempState)) { - // background refresh (fire & forget) - if (_updateGeneratedCredentialsSemaphore.Wait(0)) - { - _ = GenerateCredentialsAndUpdateStateAsync(); - } + tempState = await GenerateNewCredentialsAsync().ConfigureAwait(false); + UpdateToGeneratedCredentials(tempState); + currentState = tempState; } - } - else - { - // If credentials are expired, update - await _updateGeneratedCredentialsSemaphore.WaitAsync().ConfigureAwait(false); - tempState = await GenerateCredentialsAndUpdateStateAsync().ConfigureAwait(false); - } - - return tempState.Credentials; - async System.Threading.Tasks.Task GenerateCredentialsAndUpdateStateAsync() + return tempState.Credentials; + } + finally { - System.Diagnostics.Debug.Assert(_updateGeneratedCredentialsSemaphore.CurrentCount == 0); - - try - { - var tempState = currentState; - // double-check that the credentials still need updating - // as it's possible that multiple requests were queued acquiring the semaphore - if (ShouldUpdateState(tempState)) - { - tempState = await GenerateNewCredentialsAsync().ConfigureAwait(false); - UpdateToGeneratedCredentials(tempState); - currentState = tempState; - } - - return tempState; - } - finally - { - _updateGeneratedCredentialsSemaphore.Release(); - } + _updateGeneratedCredentialsSemaphore.Release(); } } @@ -261,8 +199,7 @@ private void UpdateToGeneratedCredentials(CredentialsRefreshState state) { // This could happen if the default value of PreemptExpiryTime is // overridden and set too high such that ShouldUpdate returns true. - var logger = Logger.GetLogger(typeof(RefreshingAWSCredentials)); - logger.InfoFormat( + _logger.InfoFormat( "The preempt expiry time is set too high: Current time = {0}, Credentials expiry time = {1}, Preempt expiry time = {2}.", AWSSDKUtils.CorrectedUtcNow, state.Expiration, PreemptExpiryTime); @@ -301,8 +238,7 @@ private bool ShouldUpdateState(CredentialsRefreshState state) var isExpired = state?.IsExpiredWithin(TimeSpan.Zero); if (isExpired == true) { - var logger = Logger.GetLogger(typeof(RefreshingAWSCredentials)); - logger.InfoFormat("Determined refreshing credentials should update. Expiration time: {0}, Current time: {1}", + _logger.InfoFormat("Determined refreshing credentials should update. Expiration time: {0}, Current time: {1}", state.Expiration.Add(PreemptExpiryTime).ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ss.f ffffffK", CultureInfo.InvariantCulture), AWSSDKUtils.CorrectedUtcNow.ToString("yyyy-MM-ddTHH:mm:ss.fffffffK", CultureInfo.InvariantCulture)); } diff --git a/sdk/test/NetStandard/UnitTests/Core/Credentials/RefreshingAWSCredentialsTests.cs b/sdk/test/NetStandard/UnitTests/Core/Credentials/RefreshingAWSCredentialsTests.cs deleted file mode 100644 index 3c1f2a7db29d..000000000000 --- a/sdk/test/NetStandard/UnitTests/Core/Credentials/RefreshingAWSCredentialsTests.cs +++ /dev/null @@ -1,232 +0,0 @@ -using System; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Amazon; -using Amazon.Runtime; -using Xunit; - -namespace UnitTests.NetStandard.Core.Credentials -{ - public sealed class RefreshingAWSCredentialsTests : IDisposable - { - private readonly Func _resetUtcNowSource; - - public RefreshingAWSCredentialsTests() - { - _resetUtcNowSource = AWSConfigs.utcNowSource; - } - - [Fact] - public void ConcurrentCallsToGetExpiredCrendentialsOnlyGeneratesNewCredentialsOnce() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = mockCredentials.GetCredentials(); - - AWSConfigs.utcNowSource = () => baseTimeUtc + lifetime; - - mockCredentials.CloseGenerateCredentialsGate(); // prevent GenerateNewCredentials from returning - - var concurrentCredentialTasks = Task.WhenAll( - Enumerable.Range(1, 5) - .Select(i => Task.Run(() => mockCredentials.GetCredentials())) - ); - - mockCredentials.OpenGenerateCredentialsGate(); // allow GenerateNewCredentials to complete - - var allCreds = concurrentCredentialTasks.Result; - Assert.NotEqual(initialCreds, allCreds[0]); - - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - for (var i = 1; i < allCreds.Length; i++) - { - Assert.Equal(allCreds[0], allCreds[i]); - } - } - - [Fact] - public void CredentialsAreRefreshedInImmediatelyWhenExpired() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = mockCredentials.GetCredentials(); - - AWSConfigs.utcNowSource = () => baseTimeUtc + lifetime; - var credsAfterExpiration = mockCredentials.GetCredentials(); - Assert.NotEqual(initialCreds, credsAfterExpiration); - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - } - - [Fact] - public async Task CredentialsAreRefreshedInImmediatelyWhenExpiredAsync() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = await mockCredentials.GetCredentialsAsync().ConfigureAwait(false); - - AWSConfigs.utcNowSource = () => baseTimeUtc + lifetime; - var credsAfterExpiration = await mockCredentials.GetCredentialsAsync().ConfigureAwait(false); - Assert.NotEqual(initialCreds, credsAfterExpiration); - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - } - - [Fact] - public async Task ConcurrentCallsToGetExpiredCrendentialsOnlyGeneratesNewCredentialsOnceAsync() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = mockCredentials.GetCredentials(); - - AWSConfigs.utcNowSource = () => baseTimeUtc + lifetime; - - mockCredentials.CloseGenerateCredentialsGate(); // prevent GenerateNewCredentials from returning - - var concurrentCredentialTasks = Task.WhenAll( - Enumerable.Range(1, 5) - .Select(i => mockCredentials.GetCredentialsAsync()) - ); - - mockCredentials.OpenGenerateCredentialsGate(); // allow GenerateNewCredentials to complete - - var allCreds = await concurrentCredentialTasks; - - Assert.NotEqual(initialCreds, allCreds[0]); - - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - for (var i = 1; i < allCreds.Length; i++) - { - Assert.Equal(allCreds[0], allCreds[i]); - } - } - - [Fact] - public void CredentialsAreRefreshedInBackgroundDuringPreemptyExpiryPeriod() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = mockCredentials.GetCredentials(); - - AWSConfigs.utcNowSource = () => baseTimeUtc + TimeSpan.FromMinutes(50); - var previousState = mockCredentials.CurrentState; - var credsDuringPreemptExpiry = mockCredentials.GetCredentials(); - Assert.Equal(initialCreds, credsDuringPreemptExpiry); - - // wait for background refresh to complete - Assert.True(SpinWait.SpinUntil(() => !ReferenceEquals(mockCredentials.CurrentState, previousState), 1_000)); - - var credsAfterRefresh = mockCredentials.GetCredentials(); - Assert.NotEqual(credsAfterRefresh, credsDuringPreemptExpiry); - Assert.Equal(AWSConfigs.utcNowSource() + lifetime - mockCredentials.PreemptExpiryTime, mockCredentials.CurrentState.Expiration); - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - } - - [Fact] - public async Task CredentialsAreRefreshedInBackgroundDuringPreemptyExpiryPeriodAsync() - { - var baseTimeUtc = new DateTime(1970, 1, 1).ToUniversalTime(); - var lifetime = TimeSpan.FromMinutes(60); - var mockCredentials = new MockRefreshingAWSCredentials(lifetime) - { - PreemptExpiryTime = TimeSpan.FromMinutes(15), - }; - - AWSConfigs.utcNowSource = () => baseTimeUtc; - var initialCreds = await mockCredentials.GetCredentialsAsync().ConfigureAwait(false); - - AWSConfigs.utcNowSource = () => baseTimeUtc + TimeSpan.FromMinutes(50); - var previousState = mockCredentials.CurrentState; - var credsDuringPreemptExpiry = await mockCredentials.GetCredentialsAsync().ConfigureAwait(false); - Assert.Equal(initialCreds, credsDuringPreemptExpiry); - - // wait for background refresh to complete - Assert.True(SpinWait.SpinUntil(() => !ReferenceEquals(mockCredentials.CurrentState, previousState), 1_000)); - - var credsAfterRefresh = await mockCredentials.GetCredentialsAsync().ConfigureAwait(false); - Assert.NotEqual(credsAfterRefresh, credsDuringPreemptExpiry); - Assert.Equal(AWSConfigs.utcNowSource() + lifetime - mockCredentials.PreemptExpiryTime, mockCredentials.CurrentState.Expiration); - Assert.Equal(2, mockCredentials.GeneratedTokenCount); - } - - public void Dispose() - { - AWSConfigs.utcNowSource = _resetUtcNowSource; - } - - // using a hand-written mock in order to have access to the protected fields - private sealed class MockRefreshingAWSCredentials : RefreshingAWSCredentials - { - private readonly TimeSpan _credentialsLifetime; - private readonly ManualResetEventSlim _generateCredsEvent; - private int _tokenCounter; - - public MockRefreshingAWSCredentials(TimeSpan credentialsLifetime) - { - _credentialsLifetime = credentialsLifetime; - _generateCredsEvent = new ManualResetEventSlim(initialState: true); - _tokenCounter = 0; - } - - public CredentialsRefreshState CurrentState => base.currentState; - - public int GeneratedTokenCount => _tokenCounter; - - public bool IsGenerateCredentialsGateClosed => !_generateCredsEvent.IsSet; - - public void OpenGenerateCredentialsGate() - { - _generateCredsEvent.Set(); - } - - public void CloseGenerateCredentialsGate() - { - _generateCredsEvent.Reset(); - } - - protected override CredentialsRefreshState GenerateNewCredentials() - { - _generateCredsEvent.Wait(); - return new CredentialsRefreshState - { - Credentials = new ImmutableCredentials("access_key_id", "secret_access_key", $"token_{Interlocked.Increment(ref _tokenCounter)}"), - Expiration = AWSConfigs.utcNowSource() + _credentialsLifetime, - }; - } - - protected override Task GenerateNewCredentialsAsync() - { - return Task.Run(GenerateNewCredentials); - } - } - } -}