Skip to content

Commit ea09e4c

Browse files
committed
Correct order of operations
1 parent 503670f commit ea09e4c

File tree

2 files changed

+102
-11
lines changed

2 files changed

+102
-11
lines changed

v2/awsv1shim/session.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,17 @@ func GetSession(ctx context.Context, awsC *awsv2.Config, c *awsbase.Config) (*se
129129
sess.Handlers.Retry.PushBack(func(r *request.Request) {
130130
logger := logging.RetrieveLogger(r.Context())
131131

132+
if r.IsErrorExpired() {
133+
logger.Warn(ctx, "Disabling retries after next request due to expired credentials", map[string]any{
134+
"error": r.Error,
135+
})
136+
r.Retryable = aws.Bool(false)
137+
}
138+
132139
if r.RetryCount < constants.MaxNetworkRetryCount {
133140
return
134141
}
142+
135143
// RequestError: send request failed
136144
// caused by: Post https://FQDN/: dial tcp: lookup FQDN: no such host
137145
if tfawserr.ErrMessageAndOrigErrContain(r.Error, request.ErrCodeRequestError, "send request failed", "no such host") {
@@ -148,13 +156,6 @@ func GetSession(ctx context.Context, awsC *awsv2.Config, c *awsbase.Config) (*se
148156
})
149157
r.Retryable = aws.Bool(false)
150158
}
151-
152-
if r.IsErrorExpired() {
153-
logger.Warn(ctx, "Disabling retries after next request due to expired credentials", map[string]any{
154-
"error": r.Error,
155-
})
156-
r.Retryable = aws.Bool(false)
157-
}
158159
})
159160

160161
return sess, nil

v2/awsv1shim/session_test.go

Lines changed: 94 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -878,15 +878,84 @@ region = us-east-1
878878
Region: "us-east-1",
879879
SecretKey: servicemocks.MockStaticSecretKey,
880880
},
881-
Description: "expired token error",
881+
Description: "ExpiredToken invalid body",
882882
ExpectedError: func(err error) bool {
883883
return strings.Contains(err.Error(), "ExpiredToken")
884884
},
885885
MockStsEndpoints: []*servicemocks.MockEndpoint{
886886
servicemocks.MockStsGetCallerIdentityInvalidBodyExpiredToken,
887887
},
888888
},
889-
889+
{
890+
Config: &awsbase.Config{
891+
AccessKey: servicemocks.MockStaticAccessKey,
892+
Region: "us-east-1",
893+
SecretKey: servicemocks.MockStaticSecretKey,
894+
},
895+
Description: "ExpiredToken valid body", // in case they change it
896+
ExpectedError: func(err error) bool {
897+
return strings.Contains(err.Error(), "ExpiredToken")
898+
},
899+
MockStsEndpoints: []*servicemocks.MockEndpoint{
900+
servicemocks.MockStsGetCallerIdentityValidBodyExpiredToken,
901+
},
902+
},
903+
{
904+
Config: &awsbase.Config{
905+
AccessKey: servicemocks.MockStaticAccessKey,
906+
Region: "us-east-1",
907+
SecretKey: servicemocks.MockStaticSecretKey,
908+
},
909+
Description: "ExpiredTokenException invalid body",
910+
ExpectedError: func(err error) bool {
911+
return strings.Contains(err.Error(), "ExpiredTokenException")
912+
},
913+
MockStsEndpoints: []*servicemocks.MockEndpoint{
914+
servicemocks.MockStsGetCallerIdentityInvalidBodyExpiredTokenException,
915+
},
916+
},
917+
{
918+
Config: &awsbase.Config{
919+
AccessKey: servicemocks.MockStaticAccessKey,
920+
Region: "us-east-1",
921+
SecretKey: servicemocks.MockStaticSecretKey,
922+
},
923+
Description: "ExpiredTokenException valid body", // in case they change it
924+
ExpectedError: func(err error) bool {
925+
return strings.Contains(err.Error(), "ExpiredTokenException")
926+
},
927+
MockStsEndpoints: []*servicemocks.MockEndpoint{
928+
servicemocks.MockStsGetCallerIdentityValidBodyExpiredTokenException,
929+
},
930+
},
931+
{
932+
Config: &awsbase.Config{
933+
AccessKey: servicemocks.MockStaticAccessKey,
934+
Region: "us-east-1",
935+
SecretKey: servicemocks.MockStaticSecretKey,
936+
},
937+
Description: "RequestExpired invalid body",
938+
ExpectedError: func(err error) bool {
939+
return strings.Contains(err.Error(), "RequestExpired")
940+
},
941+
MockStsEndpoints: []*servicemocks.MockEndpoint{
942+
servicemocks.MockStsGetCallerIdentityInvalidBodyRequestExpired,
943+
},
944+
},
945+
{
946+
Config: &awsbase.Config{
947+
AccessKey: servicemocks.MockStaticAccessKey,
948+
Region: "us-east-1",
949+
SecretKey: servicemocks.MockStaticSecretKey,
950+
},
951+
Description: "RequestExpired valid body", // in case they change it
952+
ExpectedError: func(err error) bool {
953+
return strings.Contains(err.Error(), "RequestExpired")
954+
},
955+
MockStsEndpoints: []*servicemocks.MockEndpoint{
956+
servicemocks.MockStsGetCallerIdentityValidBodyRequestExpired,
957+
},
958+
},
890959
// {
891960
// Config: &awsbase.Config{
892961
// AccessKey: servicemocks.MockStaticAccessKey,
@@ -1133,7 +1202,7 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
11331202
t.Fatalf("unexpected GetAwsConfig() '%[1]T' error: %[1]s", err)
11341203
}
11351204

1136-
t.Logf("received expected error: %s", err)
1205+
t.Logf("received expected error (awsbase.GetAwsConfig): %s", err)
11371206
return
11381207
}
11391208
actualSession, err := GetSession(ctx, &awsConfig, testCase.Config)
@@ -1146,7 +1215,7 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
11461215
t.Fatalf("unexpected GetSession() '%[1]T' error: %[1]s", err)
11471216
}
11481217

1149-
t.Logf("received expected error: %s", err)
1218+
t.Logf("received expected error (GetSession): %s", err)
11501219
return
11511220
}
11521221

@@ -2300,6 +2369,27 @@ func TestSessionRetryHandlers(t *testing.T) {
23002369
ExpectedRetryableValue: true, // defaults to true for non-AWS errors
23012370
ExpectRetryToBeAttempted: false, // Does not actually get retried, because over max retry limit
23022371
},
2372+
{
2373+
Description: "ExpiredToken error no retries",
2374+
RetryCount: maxRetries,
2375+
Error: awserr.New("ExpiredToken", "The security token included in the request is expired", nil),
2376+
ExpectedRetryableValue: false,
2377+
ExpectRetryToBeAttempted: false,
2378+
},
2379+
{
2380+
Description: "ExpiredTokenException error no retries",
2381+
RetryCount: maxRetries,
2382+
Error: awserr.New("ExpiredTokenException", "The security token included in the request is expired", nil),
2383+
ExpectedRetryableValue: false,
2384+
ExpectRetryToBeAttempted: false,
2385+
},
2386+
{
2387+
Description: "RequestExpired error no retries",
2388+
RetryCount: maxRetries,
2389+
Error: awserr.New("RequestExpired", "The security token included in the request is expired", nil),
2390+
ExpectedRetryableValue: false,
2391+
ExpectRetryToBeAttempted: false,
2392+
},
23032393
{
23042394
Description: "send request no such host failed under MaxNetworkRetryCount",
23052395
RetryCount: constants.MaxNetworkRetryCount - 1,

0 commit comments

Comments
 (0)