Skip to content

Commit 0a55e91

Browse files
authored
fix(cli): synthesis stops on expired AWS credentials (#22861)
#21052 tried to fix the situation where we would keep on doing retries if AWS credentials were expired. However, this is now failing too hard for people that commonly have expired credentials in their environment but still want to have `cdk synth` complete successfully. Catch and swallow the error (but do complain with a warning) if we encounter an `ExpiredToken` during the `defaultAccount` operation. That's the only place where it's used, and the only place where the value is optional -- it behaves the same as if no credentials were configured. Also in this PR: add some TypeScript decorators to trace through a bunch of async method calls to come up with a reasonable trace of where errors originate. Not complete, not intended to be. But it is a nice basis for debugging SDK call behavior, and can be used more in the future. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 0698cb2 commit 0a55e91

File tree

7 files changed

+100
-16
lines changed

7 files changed

+100
-16
lines changed

packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as cxapi from '@aws-cdk/cx-api';
44
import * as AWS from 'aws-sdk';
55
import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
66
import * as fs from 'fs-extra';
7+
import { traceMethods } from '../../util/tracing';
78
import { debug, warning } from './_env';
89
import { AwsCliCompatible } from './awscli-compatible';
910
import { cached } from './cached';
@@ -127,6 +128,7 @@ export interface SdkForEnvironment {
127128
* - Seeded terminal with `ReadOnly` credentials in order to do `cdk diff`--the `ReadOnly`
128129
* role doesn't have `sts:AssumeRole` and will fail for no real good reason.
129130
*/
131+
@traceMethods
130132
export class SdkProvider {
131133
/**
132134
* Create a new SdkProvider which gets its defaults in a way that behaves like the AWS CLI does
@@ -279,11 +281,15 @@ export class SdkProvider {
279281

280282
return await new SDK(creds, this.defaultRegion, this.sdkOptions).currentAccount();
281283
} catch (e) {
282-
if (isUnrecoverableAwsError(e)) {
283-
throw e;
284+
// Treat 'ExpiredToken' specially. This is a common situation that people may find themselves in, and
285+
// they are complaining about if we fail 'cdk synth' on them. We loudly complain in order to show that
286+
// the current situation is probably undesirable, but we don't fail.
287+
if ((e as any).code === 'ExpiredToken') {
288+
warning('There are expired AWS credentials in your environment. The CDK app will synth without current account information.');
289+
return undefined;
284290
}
285291

286-
debug('Unable to determine the default AWS account:', e);
292+
debug(`Unable to determine the default AWS account (${e.code}): ${e.message}`);
287293
return undefined;
288294
}
289295
});

packages/aws-cdk/lib/api/aws-auth/sdk.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as AWS from 'aws-sdk';
22
import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
3+
import { traceMethods } from '../../util/tracing';
34
import { debug, trace } from './_env';
45
import { AccountAccessKeyCache } from './account-cache';
56
import { cached } from './cached';
@@ -81,6 +82,7 @@ export interface SdkOptions {
8182
/**
8283
* Base functionality of SDK without credential fetching
8384
*/
85+
@traceMethods
8486
export class SDK implements ISDK {
8587
private static readonly accountCache = new AccountAccessKeyCache();
8688

packages/aws-cdk/lib/cli.ts

+5
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { displayNotices, refreshNotices } from '../lib/notices';
2424
import { Command, Configuration, Settings } from '../lib/settings';
2525
import * as version from '../lib/version';
2626
import { DeploymentMethod } from './api';
27+
import { enableTracing } from './util/tracing';
2728

2829
// https://github.com/yargs/yargs/issues/1929
2930
// https://github.com/evanw/esbuild/issues/1492
@@ -281,6 +282,10 @@ async function initCommandLine() {
281282
const argv = await parseCommandLineArguments();
282283
if (argv.verbose) {
283284
setLogLevel(argv.verbose);
285+
286+
if (argv.verbose > 2) {
287+
enableTracing(true);
288+
}
284289
}
285290

286291
if (argv.ci) {

packages/aws-cdk/lib/util/tracing.ts

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { debug } from '../logging';
2+
3+
let ENABLED = false;
4+
let INDENT = 0;
5+
6+
export function enableTracing(enabled: boolean) {
7+
ENABLED = enabled;
8+
}
9+
10+
/**
11+
* Method decorator to trace a single static or member method, any time it's called
12+
*/
13+
export function traceCall(receiver: object, _propertyKey: string, descriptor: PropertyDescriptor, parentClassName?: string) {
14+
const fn = descriptor.value;
15+
const className = typeof receiver === 'function' ? receiver.name : parentClassName;
16+
17+
descriptor.value = function (...args: any[]) {
18+
if (!ENABLED) { return fn.apply(this, args); }
19+
20+
debug(`[trace] ${' '.repeat(INDENT)}${className || this.constructor.name || '(anonymous)'}#${fn.name}()`);
21+
INDENT += 2;
22+
23+
const ret = fn.apply(this, args);
24+
if (ret instanceof Promise) {
25+
return ret.finally(() => {
26+
INDENT -= 2;
27+
});
28+
} else {
29+
INDENT -= 2;
30+
return ret;
31+
}
32+
};
33+
return descriptor;
34+
}
35+
36+
/**
37+
* Class decorator, enable tracing for all methods on this class
38+
*/
39+
export function traceMethods(constructor: Function) {
40+
// Statics
41+
for (const [name, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(constructor))) {
42+
if (typeof descriptor.value !== 'function') { continue; }
43+
const newDescriptor = traceCall(constructor, name, descriptor, constructor.name) ?? descriptor;
44+
Object.defineProperty(constructor, name, newDescriptor);
45+
}
46+
47+
// Instancne members
48+
for (const [name, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(constructor.prototype))) {
49+
if (typeof descriptor.value !== 'function') { continue; }
50+
const newDescriptor = traceCall(constructor.prototype, name, descriptor, constructor.name) ?? descriptor;
51+
Object.defineProperty(constructor.prototype, name, newDescriptor);
52+
}
53+
}

packages/aws-cdk/test/api/fake-sts.ts

+18-13
Original file line numberDiff line numberDiff line change
@@ -125,12 +125,14 @@ export class FakeSts {
125125

126126
private handleRequest(mockRequest: MockRequest): Record<string, any> {
127127
const response = (() => {
128+
const identity = this.identity(mockRequest);
129+
128130
switch (mockRequest.parsedBody.Action) {
129131
case 'GetCallerIdentity':
130-
return this.handleGetCallerIdentity(mockRequest);
132+
return this.handleGetCallerIdentity(identity);
131133

132134
case 'AssumeRole':
133-
return this.handleAssumeRole(mockRequest);
135+
return this.handleAssumeRole(identity, mockRequest);
134136
}
135137

136138
throw new Error(`Unrecognized Action in MockAwsHttp: ${mockRequest.parsedBody.Action}`);
@@ -139,8 +141,7 @@ export class FakeSts {
139141
return response;
140142
}
141143

142-
private handleGetCallerIdentity(mockRequest: MockRequest): Record<string, any> {
143-
const identity = this.identity(mockRequest);
144+
private handleGetCallerIdentity(identity: RegisteredIdentity): Record<string, any> {
144145
return {
145146
GetCallerIdentityResponse: {
146147
_attributes: { xmlns: 'https://sts.amazonaws.com/doc/2011-06-15/' },
@@ -156,15 +157,8 @@ export class FakeSts {
156157
};
157158
}
158159

159-
private handleAssumeRole(mockRequest: MockRequest): Record<string, any> {
160-
const identity = this.identity(mockRequest);
161-
162-
const failureRequested = mockRequest.parsedBody.RoleArn.match(/<FAIL:([^>]+)>/);
163-
if (failureRequested) {
164-
const err = new Error(`STS failing by user request: ${failureRequested[1]}`);
165-
(err as any).code = failureRequested[1];
166-
throw err;
167-
}
160+
private handleAssumeRole(identity: RegisteredIdentity, mockRequest: MockRequest): Record<string, any> {
161+
this.checkForFailure(mockRequest.parsedBody.RoleArn);
168162

169163
this.assumedRoles.push({
170164
roleArn: mockRequest.parsedBody.RoleArn,
@@ -213,8 +207,19 @@ export class FakeSts {
213207
};
214208
}
215209

210+
private checkForFailure(s: string) {
211+
const failureRequested = s.match(/<FAIL:([^>]+)>/);
212+
if (failureRequested) {
213+
const err = new Error(`STS failing by user request: ${failureRequested[1]}`);
214+
(err as any).code = failureRequested[1];
215+
throw err;
216+
}
217+
}
218+
216219
private identity(mockRequest: MockRequest) {
217220
const keyId = this.accessKeyId(mockRequest);
221+
this.checkForFailure(keyId);
222+
218223
const ret = this.identities[keyId];
219224
if (!ret) { throw new Error(`Unrecognized access key used: ${keyId}`); }
220225
return ret;

packages/aws-cdk/test/api/sdk-provider.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,18 @@ describe('with intercepted network calls', () => {
561561
// THEN
562562
await expect(provider.defaultAccount()).resolves.toBe(undefined);
563563
});
564+
565+
test('defaultAccount returns undefined, event if STS call fails with ExpiredToken', async () => {
566+
// GIVEN
567+
process.env.AWS_ACCESS_KEY_ID = `${uid}'<FAIL:ExpiredToken>'`;
568+
process.env.AWS_SECRET_ACCESS_KEY = 'sekrit';
569+
570+
// WHEN
571+
const provider = await providerFromProfile(undefined);
572+
573+
// THEN
574+
await expect(provider.defaultAccount()).resolves.toBe(undefined);
575+
});
564576
});
565577

566578
test('even when using a profile to assume another profile, STS calls goes through the proxy', async () => {

packages/aws-cdk/tsconfig.json

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"noUnusedParameters": true,
1313
"noImplicitReturns": true,
1414
"noFallthroughCasesInSwitch": true,
15+
"experimentalDecorators": true,
1516
"resolveJsonModule": true,
1617
"composite": true,
1718
"incremental": true

0 commit comments

Comments
 (0)