Skip to content

Commit 7299254

Browse files
committed
fix: clear credential cache on InvalidClientTokenId to allow retry on next reconcile
After the AWS SDK v1 to v2 migration, aws.NewCredentialsCache caches failed AssumeRole results. When a freshly-created IAM access key has not yet propagated (AWS IAM eventual consistency), the first AssumeRole call fails with InvalidClientTokenId and this error is cached indefinitely. Unlike SDK v1 where IsExpired() returned true after failure (triggering a fresh attempt), SDK v2's CredentialsCache retains the error. This fix clears p.credentials when InvalidClientTokenId is returned, so the next controller-runtime reconcile loop creates a fresh credentials cache and retries the AssumeRole call, restoring the pre-migration recovery behavior. Fixes #6123
1 parent af0e5ca commit 7299254

2 files changed

Lines changed: 142 additions & 1 deletion

File tree

pkg/cloud/identity/identity.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,16 @@ import (
2222
"context"
2323
"crypto/sha256"
2424
"encoding/gob"
25+
"errors"
26+
"strings"
2527
"time"
2628

2729
"github.com/aws/aws-sdk-go-v2/aws"
2830
"github.com/aws/aws-sdk-go-v2/config"
2931
"github.com/aws/aws-sdk-go-v2/credentials"
3032
"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
3133
"github.com/aws/aws-sdk-go-v2/service/sts"
34+
"github.com/aws/smithy-go"
3235
corev1 "k8s.io/api/core/v1"
3336

3437
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
@@ -180,5 +183,30 @@ func (p *AWSRolePrincipalTypeProvider) Retrieve(ctx context.Context) (aws.Creden
180183
// Update credentials
181184
p.credentials = creds
182185
}
183-
return p.credentials.Retrieve(ctx)
186+
result, err := p.credentials.Retrieve(ctx)
187+
if err != nil && IsInvalidClientTokenIDError(err) {
188+
// Clear the cached credentials so the next reconcile creates a fresh
189+
// cache and retries AssumeRole. This handles AWS IAM eventual
190+
// consistency for freshly-created access keys, where the first
191+
// AssumeRole call may fail with InvalidClientTokenId before the key
192+
// has propagated. Without this, the CredentialsCache retains the
193+
// failed result and subsequent reconciles never recover.
194+
p.log.Info("Transient InvalidClientTokenId error from AssumeRole, clearing credential cache to allow retry on next reconcile")
195+
p.credentials = nil
196+
}
197+
return result, err
198+
}
199+
200+
// IsInvalidClientTokenIDError reports whether err is an AWS STS
201+
// InvalidClientTokenId error, which is raised when an access key has not yet
202+
// propagated through IAM's eventual consistency.
203+
func IsInvalidClientTokenIDError(err error) bool {
204+
if err == nil {
205+
return false
206+
}
207+
var apiErr smithy.APIError
208+
if errors.As(err, &apiErr) && apiErr.ErrorCode() == "InvalidClientTokenId" {
209+
return true
210+
}
211+
return strings.Contains(err.Error(), "InvalidClientTokenId")
184212
}

pkg/cloud/identity/identity_test.go

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,23 @@ package identity
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"testing"
2223
"time"
2324

2425
"github.com/aws/aws-sdk-go-v2/aws"
2526
"github.com/aws/aws-sdk-go-v2/service/sts"
2627
ststypes "github.com/aws/aws-sdk-go-v2/service/sts/types"
28+
"github.com/aws/smithy-go"
2729
"github.com/golang/mock/gomock"
2830
"github.com/google/go-cmp/cmp"
2931
. "github.com/onsi/gomega"
3032
corev1 "k8s.io/api/core/v1"
33+
"k8s.io/klog/v2"
3134

3235
infrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/api/v1beta2"
3336
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/services/sts/mock_stsiface"
37+
"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
3438
)
3539

3640
func TestAWSStaticPrincipalTypeProvider(t *testing.T) {
@@ -193,3 +197,112 @@ func TestAWSStaticPrincipalTypeProvider(t *testing.T) {
193197
})
194198
}
195199
}
200+
201+
func TestAWSRolePrincipalTypeProvider_ClearsCredentialCacheOnInvalidClientTokenId(t *testing.T) {
202+
mockCtrl := gomock.NewController(t)
203+
defer mockCtrl.Finish()
204+
205+
g := NewWithT(t)
206+
207+
secret := &corev1.Secret{
208+
Data: map[string][]byte{
209+
"AccessKeyID": []byte("test-key"),
210+
"SecretAccessKey": []byte("test-secret"),
211+
},
212+
}
213+
staticProvider := NewAWSStaticPrincipalTypeProvider(&infrav1.AWSClusterStaticIdentity{}, secret)
214+
215+
stsMock := mock_stsiface.NewMockSTSClient(mockCtrl)
216+
roleIdentity := &infrav1.AWSClusterRoleIdentity{
217+
Spec: infrav1.AWSClusterRoleIdentitySpec{
218+
AWSRoleSpec: infrav1.AWSRoleSpec{
219+
RoleArn: "arn:aws:iam::123456789012:role/test-role",
220+
SessionName: "test-session",
221+
DurationSeconds: 900,
222+
},
223+
},
224+
}
225+
226+
testLog := logger.NewLogger(klog.NewKlogr())
227+
roleProvider := &AWSRolePrincipalTypeProvider{
228+
credentials: nil,
229+
Principal: roleIdentity,
230+
region: "us-east-1",
231+
sourceProvider: staticProvider,
232+
stsClient: stsMock,
233+
log: testLog.WithName("test"),
234+
}
235+
236+
// First call: return InvalidClientTokenId error
237+
invalidTokenErr := &smithy.GenericAPIError{
238+
Code: "InvalidClientTokenId",
239+
Message: "The security token included in the request is invalid",
240+
}
241+
stsMock.EXPECT().AssumeRole(gomock.Any(), gomock.Any()).Return(nil, invalidTokenErr)
242+
243+
_, err := roleProvider.Retrieve(context.TODO())
244+
g.Expect(err).To(HaveOccurred())
245+
g.Expect(err.Error()).To(ContainSubstring("InvalidClientTokenId"))
246+
// Credentials cache should be cleared
247+
g.Expect(roleProvider.credentials).To(BeNil())
248+
249+
// Second call: succeed — proves the cache was cleared and a fresh attempt is made
250+
stsMock.EXPECT().AssumeRole(gomock.Any(), gomock.Any()).Return(&sts.AssumeRoleOutput{
251+
Credentials: &ststypes.Credentials{
252+
AccessKeyId: aws.String("new-key"),
253+
SecretAccessKey: aws.String("new-secret"),
254+
SessionToken: aws.String("new-token"),
255+
Expiration: aws.Time(time.Now().Add(1 * time.Hour)),
256+
},
257+
}, nil)
258+
259+
creds, err := roleProvider.Retrieve(context.TODO())
260+
g.Expect(err).To(BeNil())
261+
g.Expect(creds.AccessKeyID).To(Equal("new-key"))
262+
}
263+
264+
func TestIsInvalidClientTokenIDError(t *testing.T) {
265+
tests := []struct {
266+
name string
267+
err error
268+
expect bool
269+
}{
270+
{
271+
name: "nil error",
272+
err: nil,
273+
expect: false,
274+
},
275+
{
276+
name: "typed smithy APIError with InvalidClientTokenId",
277+
err: &smithy.GenericAPIError{
278+
Code: "InvalidClientTokenId",
279+
Message: "The security token included in the request is invalid",
280+
},
281+
expect: true,
282+
},
283+
{
284+
name: "error containing InvalidClientTokenId in message",
285+
err: fmt.Errorf("operation failed: InvalidClientTokenId: token not valid"),
286+
expect: true,
287+
},
288+
{
289+
name: "unrelated error",
290+
err: fmt.Errorf("connection timeout"),
291+
expect: false,
292+
},
293+
{
294+
name: "different API error code",
295+
err: &smithy.GenericAPIError{
296+
Code: "AccessDenied",
297+
Message: "not authorized",
298+
},
299+
expect: false,
300+
},
301+
}
302+
for _, tt := range tests {
303+
t.Run(tt.name, func(t *testing.T) {
304+
g := NewWithT(t)
305+
g.Expect(IsInvalidClientTokenIDError(tt.err)).To(Equal(tt.expect))
306+
})
307+
}
308+
}

0 commit comments

Comments
 (0)