From 464c7e3944a2e382a2e3ce5fc207474970ef24f2 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Sat, 30 Jun 2018 04:45:55 -0700 Subject: [PATCH 1/3] Store default account in disk cache Given an access key ID, the account ID is fixed. This means that we don't need to look up the default account every time the toolkit is used. This change will store the mapping between access key and account ID in `~/.cdk/cache/accounts.json` and will fetch the account ID from there if it exists. --- .../aws-cdk/lib/api/util/account-cache.ts | 49 +++++++++++++++++++ packages/aws-cdk/lib/api/util/sdk.ts | 31 ++++++++++-- packages/aws-cdk/test/test.account-cache.ts | 49 +++++++++++++++++++ 3 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 packages/aws-cdk/lib/api/util/account-cache.ts create mode 100644 packages/aws-cdk/test/test.account-cache.ts diff --git a/packages/aws-cdk/lib/api/util/account-cache.ts b/packages/aws-cdk/lib/api/util/account-cache.ts new file mode 100644 index 0000000000000..b24e70e2a0746 --- /dev/null +++ b/packages/aws-cdk/lib/api/util/account-cache.ts @@ -0,0 +1,49 @@ +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from 'path'; + +/** + * Disk cache which maps access key IDs to account IDs. + * Usage: + * cache.get(accessKey) => accountId | undefined + * cache.put(accessKey, accountId) + */ +export class AccountAccessKeyCache { + private readonly cacheFile: string; + + /** + * @param filePath Path to the cache file + */ + constructor(filePath?: string) { + this.cacheFile = filePath || path.join(os.homedir(), '.cdk/cache/accounts.json'); + } + + /** Get the account ID from an access key or undefined if not in cache */ + public async get(accessKeyId: string) { + const map = await this.loadMap(); + return map[accessKeyId]; + } + + /** Put a mapping betweenn access key and account ID */ + public async put(accessKeyId: string, accountId: string) { + const map = await this.loadMap(); + map[accessKeyId] = accountId; + await this.saveMap(map); + } + + private async loadMap() { + if (!(await fs.pathExists(this.cacheFile))) { + return { }; + } + + return await fs.readJson(this.cacheFile); + } + + private async saveMap(map: { [accessKeyId: string]: string }) { + if (!(await fs.pathExists(this.cacheFile))) { + await fs.mkdirs(path.dirname(this.cacheFile)); + } + + await fs.writeJson(this.cacheFile, map, { spaces: 2 }); + } +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/api/util/sdk.ts b/packages/aws-cdk/lib/api/util/sdk.ts index fe513ffaefa16..2dfa6382f893c 100644 --- a/packages/aws-cdk/lib/api/util/sdk.ts +++ b/packages/aws-cdk/lib/api/util/sdk.ts @@ -1,8 +1,9 @@ import { Environment} from '@aws-cdk/cx-api'; -import { CloudFormation, config, CredentialProviderChain , EC2, S3, SSM, STS } from 'aws-sdk'; +import { CloudFormation, config, CredentialProviderChain, EC2, S3, SSM, STS } from 'aws-sdk'; import { debug } from '../../logging'; import { PluginHost } from '../../plugin'; import { CredentialProviderSource, Mode } from '../aws-auth/credentials'; +import { AccountAccessKeyCache } from './account-cache'; /** * Source for SDK client objects @@ -17,6 +18,7 @@ export class SDK { private defaultAccountFetched = false; private defaultAccountId?: string = undefined; private readonly userAgent: string; + private readonly accountCache = new AccountAccessKeyCache(); constructor() { // Find the package.json from the main toolkit @@ -70,11 +72,33 @@ export class SDK { private async lookupDefaultAccount() { try { + debug('Resolving default credentials'); + const chain = new CredentialProviderChain(); + const creds = await chain.resolvePromise(); + const accessKeyId = creds.accessKeyId; + if (!accessKeyId) { + throw new Error('Unable to resolve AWS credentials'); + } + + // try to get account ID based on this access key ID from disk. + const cached = await this.accountCache.get(creds.accessKeyId); + if (cached) { + debug(`Retrieved account ID ${cached} from disk cache`); + return cached; + } + + // if we don't have one, resolve from STS and store in cache. debug('Looking up default account ID from STS'); const result = await new STS().getCallerIdentity().promise(); - return result.Account; + const accountId = result.Account; + if (!accountId) { + debug('STS didn\'t return an account ID'); + return undefined; + } + await this.accountCache.put(accessKeyId, accountId); + return accountId; } catch (e) { - debug('Unable to retrieve default account from STS:', e); + debug('Unable to determine the default AWS account (use "aws configure" to set up):', e); return undefined; } } @@ -108,3 +132,4 @@ export class SDK { throw new Error(`Need to perform AWS calls for account ${awsAccountId}, but no credentials found. Tried: ${sourceNames}.`); } } + diff --git a/packages/aws-cdk/test/test.account-cache.ts b/packages/aws-cdk/test/test.account-cache.ts new file mode 100644 index 0000000000000..2be9922e51cd9 --- /dev/null +++ b/packages/aws-cdk/test/test.account-cache.ts @@ -0,0 +1,49 @@ +import * as fs from 'fs-extra'; +import { ICallbackFunction, Test } from 'nodeunit'; +import * as path from 'path'; +import { AccountAccessKeyCache } from '../lib/api/util/account-cache'; + +export = { + 'setUp'(cb: ICallbackFunction) { + const self = this as any; + fs.mkdtemp('/tmp/account-cache-test').then(dir => { + self.file = path.join(dir, 'cache.json'); + self.cache = new AccountAccessKeyCache(self.file); + return cb(); + }); + }, + + 'tearDown'(cb: ICallbackFunction) { + const self = this as any; + fs.remove(path.dirname(self.file)).then(cb); + }, + + async 'get(k) when cache is empty'(test: Test) { + const self = this as any; + const cache: AccountAccessKeyCache = self.cache; + test.equal(await cache.get('foo'), undefined, 'get returns undefined'); + test.equal(await fs.pathExists(self.file), false, 'cache file is not created'); + test.done(); + }, + + async 'put(k,v) and then get(k)'(test: Test) { + const self = this as any; + const cache: AccountAccessKeyCache = self.cache; + + await cache.put('key', 'value'); + await cache.put('boo', 'bar'); + test.deepEqual(await cache.get('key'), 'value', '"key" is mapped to "value"'); + + // create another cache instance on the same file, should still work + const cache2 = new AccountAccessKeyCache(self.file); + test.deepEqual(await cache2.get('boo'), 'bar', '"boo" is mapped to "bar"'); + + // whitebox: read the file + test.deepEqual(await fs.readJson(self.file), { + key: 'value', + boo: 'bar' + }); + + test.done(); + } +}; \ No newline at end of file From 6870dcfc8970ee84da8abbba3c6a0e5f93986058 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 3 Jul 2018 03:09:28 -0700 Subject: [PATCH 2/3] Add fetch API --- .../aws-cdk/lib/api/util/account-cache.ts | 38 +++++++++++++++++-- packages/aws-cdk/lib/api/util/sdk.ts | 32 +++++++--------- packages/aws-cdk/test/test.account-cache.ts | 10 +++++ 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/account-cache.ts b/packages/aws-cdk/lib/api/util/account-cache.ts index b24e70e2a0746..a99f4e5e71e00 100644 --- a/packages/aws-cdk/lib/api/util/account-cache.ts +++ b/packages/aws-cdk/lib/api/util/account-cache.ts @@ -1,6 +1,7 @@ import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; +import { debug } from '../../logging'; /** * Disk cache which maps access key IDs to account IDs. @@ -15,11 +16,42 @@ export class AccountAccessKeyCache { * @param filePath Path to the cache file */ constructor(filePath?: string) { - this.cacheFile = filePath || path.join(os.homedir(), '.cdk/cache/accounts.json'); + this.cacheFile = filePath || path.join(os.homedir(), '.cdk', 'cache', 'accounts.json'); + } + + /** + * Tries to fetch the account ID from cache. If it's not in the cache, invokes + * the resolver function which should retrieve the account ID and return it. + * Then, it will be stored into disk cache returned. + * + * Example: + * + * const accountId = cache.fetch(accessKey, async () => { + * return await fetchAccountIdFromSomewhere(accessKey); + * }); + * + * @param accessKeyId + * @param resolver + */ + public async fetch(accessKeyId: string, resolver: () => Promise) { + // try to get account ID based on this access key ID from disk. + const cached = await this.get(accessKeyId); + if (cached) { + debug(`Retrieved account ID ${cached} from disk cache`); + return cached; + } + + // if it's not in the cache, resolve and put in cache. + const accountId = await resolver(); + if (accountId) { + await this.put(accessKeyId, accountId); + } + + return accountId; } /** Get the account ID from an access key or undefined if not in cache */ - public async get(accessKeyId: string) { + public async get(accessKeyId: string): Promise { const map = await this.loadMap(); return map[accessKeyId]; } @@ -31,7 +63,7 @@ export class AccountAccessKeyCache { await this.saveMap(map); } - private async loadMap() { + private async loadMap(): Promise<{ [accessKeyId: string]: string }> { if (!(await fs.pathExists(this.cacheFile))) { return { }; } diff --git a/packages/aws-cdk/lib/api/util/sdk.ts b/packages/aws-cdk/lib/api/util/sdk.ts index 2dfa6382f893c..2f12c5db467dc 100644 --- a/packages/aws-cdk/lib/api/util/sdk.ts +++ b/packages/aws-cdk/lib/api/util/sdk.ts @@ -77,28 +77,25 @@ export class SDK { const creds = await chain.resolvePromise(); const accessKeyId = creds.accessKeyId; if (!accessKeyId) { - throw new Error('Unable to resolve AWS credentials'); + throw new Error('Unable to resolve AWS credentials (setup with "aws configure")'); } - // try to get account ID based on this access key ID from disk. - const cached = await this.accountCache.get(creds.accessKeyId); - if (cached) { - debug(`Retrieved account ID ${cached} from disk cache`); - return cached; - } + const accountId = await this.accountCache.fetch(creds.accessKeyId, async () => { + // if we don't have one, resolve from STS and store in cache. + debug('Looking up default account ID from STS'); + const result = await new STS().getCallerIdentity().promise(); + const aid = result.Account; + if (!aid) { + debug('STS didn\'t return an account ID'); + return undefined; + } + debug('Default account ID:', aid); + return aid; + }); - // if we don't have one, resolve from STS and store in cache. - debug('Looking up default account ID from STS'); - const result = await new STS().getCallerIdentity().promise(); - const accountId = result.Account; - if (!accountId) { - debug('STS didn\'t return an account ID'); - return undefined; - } - await this.accountCache.put(accessKeyId, accountId); return accountId; } catch (e) { - debug('Unable to determine the default AWS account (use "aws configure" to set up):', e); + debug('Unable to determine the default AWS account (did you configure "aws configure"?):', e); return undefined; } } @@ -132,4 +129,3 @@ export class SDK { throw new Error(`Need to perform AWS calls for account ${awsAccountId}, but no credentials found. Tried: ${sourceNames}.`); } } - diff --git a/packages/aws-cdk/test/test.account-cache.ts b/packages/aws-cdk/test/test.account-cache.ts index 2be9922e51cd9..608b68994cf82 100644 --- a/packages/aws-cdk/test/test.account-cache.ts +++ b/packages/aws-cdk/test/test.account-cache.ts @@ -44,6 +44,16 @@ export = { boo: 'bar' }); + test.done(); + }, + + async 'fetch(k, resolver) can be used to "atomically" get + resolve + put'(test: Test) { + const self = this as any; + const cache: AccountAccessKeyCache = self.cache; + + test.deepEqual(await cache.get('foo'), undefined, 'no value for "foo" yet'); + test.deepEqual(await cache.fetch('foo', async () => 'bar'), 'bar', 'resolved by fetch and returned'); + test.deepEqual(await cache.get('foo'), 'bar', 'stored in cache by fetch()'); test.done(); } }; \ No newline at end of file From 8ca6f63613127f7f95dae03acc21c47648d267f1 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 4 Jul 2018 00:42:23 -0700 Subject: [PATCH 3/3] Nuke cache if exceeds 1000 entries --- .../aws-cdk/lib/api/util/account-cache.ts | 14 +++++++++- packages/aws-cdk/package.json | 3 ++- packages/aws-cdk/test/test.account-cache.ts | 26 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/account-cache.ts b/packages/aws-cdk/lib/api/util/account-cache.ts index a99f4e5e71e00..e453cb0a38847 100644 --- a/packages/aws-cdk/lib/api/util/account-cache.ts +++ b/packages/aws-cdk/lib/api/util/account-cache.ts @@ -3,6 +3,7 @@ import * as os from 'os'; import * as path from 'path'; import { debug } from '../../logging'; + /** * Disk cache which maps access key IDs to account IDs. * Usage: @@ -10,6 +11,11 @@ import { debug } from '../../logging'; * cache.put(accessKey, accountId) */ export class AccountAccessKeyCache { + /** + * Max number of entries in the cache, after which the cache will be reset. + */ + public static readonly MAX_ENTRIES = 1000; + private readonly cacheFile: string; /** @@ -58,7 +64,13 @@ export class AccountAccessKeyCache { /** Put a mapping betweenn access key and account ID */ public async put(accessKeyId: string, accountId: string) { - const map = await this.loadMap(); + let map = await this.loadMap(); + + // nuke cache if it's too big. + if (Object.keys(map).length >= AccountAccessKeyCache.MAX_ENTRIES) { + map = { }; + } + map[accessKeyId] = accountId; await this.saveMap(map); } diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index b467a704111bb..9eb7a1d6351b4 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -11,7 +11,8 @@ "prepare": "/bin/bash generate.sh && tslint -p . && tsc && chmod +x bin/cdk && pkglint", "watch": "tsc -w", "lint": "tsc && tslint -p . --force", - "pkglint": "pkglint -f" + "pkglint": "pkglint -f", + "test": "nodeunit test/test.*.js" }, "author": { "name": "Amazon Web Services", diff --git a/packages/aws-cdk/test/test.account-cache.ts b/packages/aws-cdk/test/test.account-cache.ts index 608b68994cf82..dd36b1657023a 100644 --- a/packages/aws-cdk/test/test.account-cache.ts +++ b/packages/aws-cdk/test/test.account-cache.ts @@ -54,6 +54,32 @@ export = { test.deepEqual(await cache.get('foo'), undefined, 'no value for "foo" yet'); test.deepEqual(await cache.fetch('foo', async () => 'bar'), 'bar', 'resolved by fetch and returned'); test.deepEqual(await cache.get('foo'), 'bar', 'stored in cache by fetch()'); + test.done(); + }, + + async 'cache is nuked if it exceeds 1000 entries'(test: Test) { + const self = this as any; + const cache: AccountAccessKeyCache = self.cache; + + for (let i = 0; i < AccountAccessKeyCache.MAX_ENTRIES; ++i) { + await cache.put(`key${i}`, `value${i}`); + } + + // verify all 1000 values are on disk + const otherCache = new AccountAccessKeyCache(self.file); + for (let i = 0; i < AccountAccessKeyCache.MAX_ENTRIES; ++i) { + test.equal(await otherCache.get(`key${i}`), `value${i}`); + } + + // add another value + await cache.put('nuke-me', 'genesis'); + + // now, we expect only `nuke-me` to exist on disk + test.equal(await otherCache.get('nuke-me'), 'genesis'); + for (let i = 0; i < AccountAccessKeyCache.MAX_ENTRIES; ++i) { + test.equal(await otherCache.get(`key${i}`), undefined); + } + test.done(); } }; \ No newline at end of file