Skip to content

Commit

Permalink
fix(cli): synthesis stops on expired AWS credentials (#22861)
Browse files Browse the repository at this point in the history
#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*
  • Loading branch information
rix0rrr authored Nov 10, 2022
1 parent 0698cb2 commit 0a55e91
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 16 deletions.
12 changes: 9 additions & 3 deletions packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as cxapi from '@aws-cdk/cx-api';
import * as AWS from 'aws-sdk';
import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
import * as fs from 'fs-extra';
import { traceMethods } from '../../util/tracing';
import { debug, warning } from './_env';
import { AwsCliCompatible } from './awscli-compatible';
import { cached } from './cached';
Expand Down Expand Up @@ -127,6 +128,7 @@ export interface SdkForEnvironment {
* - Seeded terminal with `ReadOnly` credentials in order to do `cdk diff`--the `ReadOnly`
* role doesn't have `sts:AssumeRole` and will fail for no real good reason.
*/
@traceMethods
export class SdkProvider {
/**
* Create a new SdkProvider which gets its defaults in a way that behaves like the AWS CLI does
Expand Down Expand Up @@ -279,11 +281,15 @@ export class SdkProvider {

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

debug('Unable to determine the default AWS account:', e);
debug(`Unable to determine the default AWS account (${e.code}): ${e.message}`);
return undefined;
}
});
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as AWS from 'aws-sdk';
import type { ConfigurationOptions } from 'aws-sdk/lib/config-base';
import { traceMethods } from '../../util/tracing';
import { debug, trace } from './_env';
import { AccountAccessKeyCache } from './account-cache';
import { cached } from './cached';
Expand Down Expand Up @@ -81,6 +82,7 @@ export interface SdkOptions {
/**
* Base functionality of SDK without credential fetching
*/
@traceMethods
export class SDK implements ISDK {
private static readonly accountCache = new AccountAccessKeyCache();

Expand Down
5 changes: 5 additions & 0 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { displayNotices, refreshNotices } from '../lib/notices';
import { Command, Configuration, Settings } from '../lib/settings';
import * as version from '../lib/version';
import { DeploymentMethod } from './api';
import { enableTracing } from './util/tracing';

// https://github.com/yargs/yargs/issues/1929
// https://github.com/evanw/esbuild/issues/1492
Expand Down Expand Up @@ -281,6 +282,10 @@ async function initCommandLine() {
const argv = await parseCommandLineArguments();
if (argv.verbose) {
setLogLevel(argv.verbose);

if (argv.verbose > 2) {
enableTracing(true);
}
}

if (argv.ci) {
Expand Down
53 changes: 53 additions & 0 deletions packages/aws-cdk/lib/util/tracing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { debug } from '../logging';

let ENABLED = false;
let INDENT = 0;

export function enableTracing(enabled: boolean) {
ENABLED = enabled;
}

/**
* Method decorator to trace a single static or member method, any time it's called
*/
export function traceCall(receiver: object, _propertyKey: string, descriptor: PropertyDescriptor, parentClassName?: string) {
const fn = descriptor.value;
const className = typeof receiver === 'function' ? receiver.name : parentClassName;

descriptor.value = function (...args: any[]) {
if (!ENABLED) { return fn.apply(this, args); }

debug(`[trace] ${' '.repeat(INDENT)}${className || this.constructor.name || '(anonymous)'}#${fn.name}()`);
INDENT += 2;

const ret = fn.apply(this, args);
if (ret instanceof Promise) {
return ret.finally(() => {
INDENT -= 2;
});
} else {
INDENT -= 2;
return ret;
}
};
return descriptor;
}

/**
* Class decorator, enable tracing for all methods on this class
*/
export function traceMethods(constructor: Function) {
// Statics
for (const [name, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(constructor))) {
if (typeof descriptor.value !== 'function') { continue; }
const newDescriptor = traceCall(constructor, name, descriptor, constructor.name) ?? descriptor;
Object.defineProperty(constructor, name, newDescriptor);
}

// Instancne members
for (const [name, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(constructor.prototype))) {
if (typeof descriptor.value !== 'function') { continue; }
const newDescriptor = traceCall(constructor.prototype, name, descriptor, constructor.name) ?? descriptor;
Object.defineProperty(constructor.prototype, name, newDescriptor);
}
}
31 changes: 18 additions & 13 deletions packages/aws-cdk/test/api/fake-sts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,14 @@ export class FakeSts {

private handleRequest(mockRequest: MockRequest): Record<string, any> {
const response = (() => {
const identity = this.identity(mockRequest);

switch (mockRequest.parsedBody.Action) {
case 'GetCallerIdentity':
return this.handleGetCallerIdentity(mockRequest);
return this.handleGetCallerIdentity(identity);

case 'AssumeRole':
return this.handleAssumeRole(mockRequest);
return this.handleAssumeRole(identity, mockRequest);
}

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

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

private handleAssumeRole(mockRequest: MockRequest): Record<string, any> {
const identity = this.identity(mockRequest);

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

this.assumedRoles.push({
roleArn: mockRequest.parsedBody.RoleArn,
Expand Down Expand Up @@ -213,8 +207,19 @@ export class FakeSts {
};
}

private checkForFailure(s: string) {
const failureRequested = s.match(/<FAIL:([^>]+)>/);
if (failureRequested) {
const err = new Error(`STS failing by user request: ${failureRequested[1]}`);
(err as any).code = failureRequested[1];
throw err;
}
}

private identity(mockRequest: MockRequest) {
const keyId = this.accessKeyId(mockRequest);
this.checkForFailure(keyId);

const ret = this.identities[keyId];
if (!ret) { throw new Error(`Unrecognized access key used: ${keyId}`); }
return ret;
Expand Down
12 changes: 12 additions & 0 deletions packages/aws-cdk/test/api/sdk-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,18 @@ describe('with intercepted network calls', () => {
// THEN
await expect(provider.defaultAccount()).resolves.toBe(undefined);
});

test('defaultAccount returns undefined, event if STS call fails with ExpiredToken', async () => {
// GIVEN
process.env.AWS_ACCESS_KEY_ID = `${uid}'<FAIL:ExpiredToken>'`;
process.env.AWS_SECRET_ACCESS_KEY = 'sekrit';

// WHEN
const provider = await providerFromProfile(undefined);

// THEN
await expect(provider.defaultAccount()).resolves.toBe(undefined);
});
});

test('even when using a profile to assume another profile, STS calls goes through the proxy', async () => {
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"experimentalDecorators": true,
"resolveJsonModule": true,
"composite": true,
"incremental": true
Expand Down

0 comments on commit 0a55e91

Please sign in to comment.