Skip to content

Commit

Permalink
fix: gracefully handle absence fo the ~/.aws/credentials file (#541)
Browse files Browse the repository at this point in the history
The JS SDK assumes the file exists when it is passed as an argument, so
this change makes the CDK Toolkit check for file existence before
passing down to the SDK layer. Additionally, the SDK "ini file"
re-implementation failed to check for file existece before attempting to
load file contents.

Fixes #540
  • Loading branch information
RomainMuller authored Aug 14, 2018
1 parent 327f5e1 commit 5dfa785
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 17 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ async function initCommandLine() {
*/
async function populateDefaultEnvironmentIfNeeded(context: any) {
if (!(cxapi.DEFAULT_REGION_CONTEXT_KEY in context)) {
context[cxapi.DEFAULT_REGION_CONTEXT_KEY] = aws.defaultRegion();
context[cxapi.DEFAULT_REGION_CONTEXT_KEY] = await aws.defaultRegion();
debug(`Setting "${cxapi.DEFAULT_REGION_CONTEXT_KEY}" context to`, context[cxapi.DEFAULT_REGION_CONTEXT_KEY]);
}
Expand Down
21 changes: 12 additions & 9 deletions packages/aws-cdk/lib/api/util/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Environment} from '@aws-cdk/cx-api';
import AWS = require('aws-sdk');
import fs = require('fs-extra');
import os = require('os');
import path = require('path');
import { debug } from '../../logging';
Expand All @@ -22,13 +23,12 @@ export class SDK {
private defaultAccountId?: string = undefined;
private readonly userAgent: string;
private readonly accountCache = new AccountAccessKeyCache();
private readonly defaultCredentialProvider: AWS.CredentialProviderChain;
private defaultCredentialProvider?: AWS.CredentialProviderChain;

constructor(private readonly profile: string | undefined) {
// Find the package.json from the main toolkit
const pkg = (require.main as any).require('../package.json');
this.userAgent = `${pkg.name}/${pkg.version}`;
this.defaultCredentialProvider = makeCLICompatibleCredentialProvider(profile);
}

public async cloudFormation(environment: Environment, mode: Mode): Promise<AWS.CloudFormation> {
Expand Down Expand Up @@ -63,8 +63,8 @@ export class SDK {
});
}

public defaultRegion(): string | undefined {
return getCLICompatibleDefaultRegion(this.profile);
public async defaultRegion(): Promise<string | undefined> {
return await getCLICompatibleDefaultRegion(this.profile);
}

public async defaultAccount(): Promise<string | undefined> {
Expand All @@ -78,6 +78,9 @@ export class SDK {
private async lookupDefaultAccount() {
try {
debug('Resolving default credentials');
if (!this.defaultCredentialProvider) {
this.defaultCredentialProvider = await makeCLICompatibleCredentialProvider(this.profile);
}
const creds = await this.defaultCredentialProvider.resolvePromise();
const accessKeyId = creds.accessKeyId;
if (!accessKeyId) {
Expand Down Expand Up @@ -147,7 +150,7 @@ export class SDK {
* file location is not given (SDK expects explicit environment variable with name).
* - AWS_DEFAULT_PROFILE is also inspected for profile name (not just AWS_PROFILE).
*/
function makeCLICompatibleCredentialProvider(profile: string | undefined) {
async function makeCLICompatibleCredentialProvider(profile: string | undefined) {
profile = profile || process.env.AWS_PROFILE || process.env.AWS_DEFAULT_PROFILE || 'default';

// Need to construct filename ourselves, without appropriate environment variables
Expand All @@ -157,7 +160,7 @@ function makeCLICompatibleCredentialProvider(profile: string | undefined) {
return new AWS.CredentialProviderChain([
() => new AWS.EnvironmentCredentials('AWS'),
() => new AWS.EnvironmentCredentials('AMAZON'),
() => new AWS.SharedIniFileCredentials({ profile, filename }),
...(await fs.pathExists(filename) ? [() => new AWS.SharedIniFileCredentials({ profile, filename })] : []),
() => {
// Calling private API
if ((AWS.ECSCredentials.prototype as any).isConfiguredForEcsCredentials()) {
Expand All @@ -181,7 +184,7 @@ function makeCLICompatibleCredentialProvider(profile: string | undefined) {
* - AWS_DEFAULT_PROFILE and AWS_DEFAULT_REGION are also used as environment
* variables to be used to determine the region.
*/
function getCLICompatibleDefaultRegion(profile: string | undefined): string | undefined {
async function getCLICompatibleDefaultRegion(profile: string | undefined): Promise<string | undefined> {
profile = profile || process.env.AWS_PROFILE || process.env.AWS_DEFAULT_PROFILE || 'default';

// Defaults inside constructor
Expand All @@ -195,9 +198,9 @@ function getCLICompatibleDefaultRegion(profile: string | undefined): string | un

while (!region && toCheck.length > 0) {
const configFile = new SharedIniFile(toCheck.shift());
const section = configFile.getProfile(profile);
const section = await configFile.getProfile(profile);
region = section && section.region;
}

return region;
}
}
15 changes: 8 additions & 7 deletions packages/aws-cdk/lib/api/util/sdk_ini_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import AWS = require('aws-sdk');
import fs = require('fs-extra');
import os = require('os');
import path = require('path');

Expand All @@ -25,8 +26,8 @@ export class SharedIniFile {
this.filename = options.filename || this.getDefaultFilepath();
}

public getProfile(profile: string) {
this.ensureFileLoaded();
public async getProfile(profile: string) {
await this.ensureFileLoaded();

const profileIndex = profile !== (AWS as any).util.defaultProfile && this.isConfig ?
'profile ' + profile : profile;
Expand All @@ -42,11 +43,11 @@ export class SharedIniFile {
);
}

private ensureFileLoaded() {
private async ensureFileLoaded() {
if (!this.parsedContents) {
this.parsedContents = (AWS as any).util.ini.parse(
(AWS as any).util.readFileSync(this.filename)
);
this.parsedContents = await fs.pathExists(this.filename)
? (AWS as any).util.ini.parse(await fs.readFile(this.filename))
: {};
}
}
}
}

0 comments on commit 5dfa785

Please sign in to comment.