Skip to content

Commit a78e710

Browse files
mrgrainmrgrain
committed
PR feedback
1 parent e07c8be commit a78e710

File tree

10 files changed

+50
-37
lines changed

10 files changed

+50
-37
lines changed

‎packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts‎

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export class AwsCliCompatible {
6969
return makeCachingProvider(fromIni({
7070
profile: options.profile,
7171
ignoreCache: true,
72-
mfaCodeProvider: makeTokenCodeFn(this.ioHelper),
72+
mfaCodeProvider: this.tokenCodeFn.bind(this),
7373
clientConfig,
7474
parentClientConfig,
7575
logger: options.logger,
@@ -106,7 +106,7 @@ export class AwsCliCompatible {
106106
clientConfig,
107107
parentClientConfig,
108108
logger: options.logger,
109-
mfaCodeProvider: makeTokenCodeFn(this.ioHelper),
109+
mfaCodeProvider: this.tokenCodeFn.bind(this),
110110
ignoreCache: true,
111111
});
112112

@@ -214,6 +214,29 @@ export class AwsCliCompatible {
214214
private getRegionFromIniFile(profile: string, data?: any) {
215215
return data?.[profile]?.region;
216216
}
217+
218+
/**
219+
* Ask user for MFA token for given serial
220+
*
221+
* Result is send to callback function for SDK to authorize the request
222+
*/
223+
private async tokenCodeFn(serialArn: string): Promise<string> {
224+
const debugFn = (msg: string, ...args: any[]) => this.ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(format(msg, ...args)));
225+
await debugFn('Require MFA token for serial ARN', serialArn);
226+
try {
227+
const token: string = await promptly.prompt(`MFA token for ${serialArn}: `, {
228+
trim: true,
229+
default: '',
230+
});
231+
await debugFn('Successfully got MFA token from user');
232+
return token;
233+
} catch (err: any) {
234+
await debugFn('Failed to get MFA token', err);
235+
const e = new AuthenticationError(`Error fetching MFA token: ${err.message ?? err}`);
236+
e.name = 'SharedIniFileCredentialsProviderFailure';
237+
throw e;
238+
}
239+
}
217240
}
218241

219242
/**
@@ -245,28 +268,3 @@ export interface CredentialChainOptions {
245268
readonly httpOptions?: SdkHttpOptions;
246269
readonly logger?: Logger;
247270
}
248-
249-
/**
250-
* Ask user for MFA token for given serial
251-
*
252-
* Result is send to callback function for SDK to authorize the request
253-
*/
254-
function makeTokenCodeFn(ioHelper: IoHelper) {
255-
const debugFn = (msg: string, ...args: any[]) => ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(format(msg, ...args)));
256-
return async (serialArn: string): Promise<string> => {
257-
await debugFn('Require MFA token for serial ARN', serialArn);
258-
try {
259-
const token: string = await promptly.prompt(`MFA token for ${serialArn}: `, {
260-
trim: true,
261-
default: '',
262-
});
263-
await debugFn('Successfully got MFA token from user');
264-
return token;
265-
} catch (err: any) {
266-
await debugFn('Failed to get MFA token', err);
267-
const e = new AuthenticationError(`Error fetching MFA token: ${err.message ?? err}`);
268-
e.name = 'SharedIniFileCredentialsProviderFailure';
269-
throw e;
270-
}
271-
};
272-
}

‎packages/aws-cdk/lib/api/aws-auth/proxy-agent.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
import * as fs from 'fs-extra';
12
import { ProxyAgent } from 'proxy-agent';
23
import type { SdkHttpOptions } from './sdk-provider';
3-
import { readIfPossible } from './util';
44
import { IO, type IoHelper } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
55

66
export class ProxyAgentProvider {
@@ -27,7 +27,15 @@ export class ProxyAgentProvider {
2727
const path = bundlePath || this.caBundlePathFromEnvironment();
2828
if (path) {
2929
await this.ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(`Using CA bundle path: ${path}`));
30-
return readIfPossible(path);
30+
try {
31+
if (!fs.pathExistsSync(path)) {
32+
return undefined;
33+
}
34+
return fs.readFileSync(path, { encoding: 'utf-8' });
35+
} catch (e: any) {
36+
await this.ioHelper.notify(IO.DEFAULT_SDK_DEBUG.msg(String(e)));
37+
return undefined;
38+
}
3139
}
3240
return undefined;
3341
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ export class SDK {
10151015
if (!accountId) {
10161016
throw new AuthenticationError("STS didn't return an account ID");
10171017
}
1018-
await this.debug(`Default account ID ${accountId}`);
1018+
await this.debug(`Default account ID: ${accountId}`);
10191019

10201020
// Save another STS call later if this one already succeeded
10211021
this._credentialsValidated = true;

‎packages/aws-cdk/test/context-providers/amis.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ import { DescribeImagesCommand } from '@aws-sdk/client-ec2';
33
import { SDK, SdkForEnvironment } from '../../lib/api';
44
import { AmiContextProviderPlugin } from '../../lib/context-providers/ami';
55
import { FAKE_CREDENTIAL_CHAIN, MockSdkProvider, mockEC2Client } from '../util/mock-sdk';
6+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
67

78
const mockSDK = new (class extends MockSdkProvider {
89
public forEnvironment(): Promise<SdkForEnvironment> {
9-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
10+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
1011
}
1112
})();
1213

‎packages/aws-cdk/test/context-providers/availability-zones.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import { DescribeAvailabilityZonesCommand } from '@aws-sdk/client-ec2';
22
import { SDK, SdkForEnvironment } from '../../lib/api';
33
import { AZContextProviderPlugin } from '../../lib/context-providers/availability-zones';
44
import { FAKE_CREDENTIAL_CHAIN, mockEC2Client, MockSdkProvider } from '../util/mock-sdk';
5+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
56

67
const mockSDK = new (class extends MockSdkProvider {
78
public forEnvironment(): Promise<SdkForEnvironment> {
8-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
9+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
910
}
1011
})();
1112

‎packages/aws-cdk/test/context-providers/endpoint-service-availability-zones.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import {
44
EndpointServiceAZContextProviderPlugin,
55
} from '../../lib/context-providers/endpoint-service-availability-zones';
66
import { FAKE_CREDENTIAL_CHAIN, mockEC2Client, MockSdkProvider } from '../util/mock-sdk';
7+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
78

89
const mockSDK = new (class extends MockSdkProvider {
910
public forEnvironment(): Promise<SdkForEnvironment> {
10-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
11+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
1112
}
1213
})();
1314

‎packages/aws-cdk/test/context-providers/hosted-zones.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import { GetHostedZoneCommand, ListHostedZonesByNameCommand } from '@aws-sdk/cli
22
import { SDK, SdkForEnvironment } from '../../lib/api';
33
import { HostedZoneContextProviderPlugin } from '../../lib/context-providers/hosted-zones';
44
import { FAKE_CREDENTIAL_CHAIN, mockRoute53Client, MockSdkProvider } from '../util/mock-sdk';
5+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
56

67
const mockSDK = new (class extends MockSdkProvider {
78
public forEnvironment(): Promise<SdkForEnvironment> {
8-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
9+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
910
}
1011
})();
1112

‎packages/aws-cdk/test/context-providers/load-balancers.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import {
1515
mockElasticLoadBalancingV2Client,
1616
restoreSdkMocksToDefault,
1717
} from '../util/mock-sdk';
18+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
1819

1920
const mockSDK = new (class extends MockSdkProvider {
2021
public forEnvironment(): Promise<SdkForEnvironment> {
21-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
22+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
2223
}
2324
})();
2425

‎packages/aws-cdk/test/context-providers/security-groups.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import { DescribeSecurityGroupsCommand } from '@aws-sdk/client-ec2';
22
import { SDK, type SdkForEnvironment } from '../../lib/api';
33
import { hasAllTrafficEgress, SecurityGroupContextProviderPlugin } from '../../lib/context-providers/security-groups';
44
import { FAKE_CREDENTIAL_CHAIN, MockSdkProvider, mockEC2Client, restoreSdkMocksToDefault } from '../util/mock-sdk';
5+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
56

67
const mockSDK = new (class extends MockSdkProvider {
78
public forEnvironment(): Promise<SdkForEnvironment> {
8-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
9+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
910
}
1011
})();
1112

‎packages/aws-cdk/test/context-providers/ssm-parameters.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ import { GetParameterCommand } from '@aws-sdk/client-ssm';
22
import { SDK, SdkForEnvironment } from '../../lib/api';
33
import { SSMContextProviderPlugin } from '../../lib/context-providers/ssm-parameters';
44
import { FAKE_CREDENTIAL_CHAIN, MockSdkProvider, mockSSMClient, restoreSdkMocksToDefault } from '../util/mock-sdk';
5+
import { TestIoHost } from '../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
56

67
const mockSDK = new (class extends MockSdkProvider {
78
public forEnvironment(): Promise<SdkForEnvironment> {
8-
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}), didAssumeRole: false });
9+
return Promise.resolve({ sdk: new SDK(FAKE_CREDENTIAL_CHAIN, mockSDK.defaultRegion, {}, new TestIoHost().asHelper("deploy")), didAssumeRole: false });
910
}
1011
})();
1112

0 commit comments

Comments
 (0)