Avoid retries on expired credentials #362
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gdavison
requested changes
Feb 14, 2023
v2/awsv1shim/session.go
Outdated
| @@ -145,6 +145,13 @@ func GetSession(ctx context.Context, awsC *awsv2.Config, c *awsbase.Config) (*se | |||
| }) | |||
| r.Retryable = aws.Bool(false) | |||
| } | |||
|
|
|||
| if r.IsErrorExpired() { | |||
There was a problem hiding this comment.
There should be a corresponding update to networkErrorShortcutter.RetryDelay for calls using the AWS SDK for Go v2
There was a problem hiding this comment.
Not needed since V2 won't retry 403 Forbidden with standard retryer (not retryable and not in the list of errors to be retried).
YakDriver
force-pushed
the
b-cut-expired-creds-retry
branch
from
259828e
to
c4d0ab0
Compare
This was referenced Feb 22, 2023
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Community Note
NOTE
Retrying 4xx errors was a bug that AWS fixed in AWS SDK for Go v2. Although this says it was fixed by #362, in reality the bulk of the problem was fixed when we moved to v2. Here, the only thing fixed involves the part of authentication that still takes place in v1, getting the session. Now, if you attempt to get a new session but receive an
Expired Tokenerror, it will not retry.Closes: #23
Closes: #363
Closes: hashicorp/terraform-provider-aws#6992
See also: hashicorp/terraform-provider-aws#29612 (using version with these changes)
To avoid retry/backoff causing long or terminal delays in reporting the expired credentials error, stop retrying when this error is encountered.
Note: #23 and hashicorp/terraform-provider-aws#6992 take the approach of adding a flag to avoid this retrying behavior. However, after discussions with the AWS Provider team, security, and a former AWS Provider engineer, there appears to be no situation in which an expired credential would ever become unexpired. In other words, long retrying efforts are a bug that can be fixed without adding a new flag (e.g.,
stop_on_expired_creds).