From f001f7e2989254c2ceae6ec22486ad6aee5ee66c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 25 Aug 2022 15:51:22 +0200 Subject: [PATCH 1/5] feat(cli): re-introduce `--concurrency` option (#21681) Re-rolls #20345, after it had to be reverted in #21664. Includes the fix necessary to make sure that `--exclusively` works (#21663), and includes `cdk watch --concurrency` as well (#21598). Closes #21663. --- packages/aws-cdk/README.md | 11 + packages/aws-cdk/THIRD_PARTY_LICENSES | 80 +++++++ packages/aws-cdk/lib/cdk-toolkit.ts | 85 ++++++-- packages/aws-cdk/lib/cli.ts | 8 +- packages/aws-cdk/lib/deploy.ts | 69 ++++++ packages/aws-cdk/package.json | 1 + packages/aws-cdk/test/cdk-toolkit.test.ts | 12 ++ packages/aws-cdk/test/deploy.test.ts | 199 ++++++++++++++++++ packages/aws-cdk/test/integ/cli/app/app.js | 9 +- .../aws-cdk/test/integ/cli/cli.integtest.ts | 43 +++- packages/aws-cdk/tsconfig.json | 4 +- 11 files changed, 494 insertions(+), 27 deletions(-) create mode 100644 packages/aws-cdk/lib/deploy.ts create mode 100644 packages/aws-cdk/test/deploy.test.ts diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md index a9a35f644d2fa..d6e29a2e3e5dc 100644 --- a/packages/aws-cdk/README.md +++ b/packages/aws-cdk/README.md @@ -191,6 +191,8 @@ In order to deploy them, you can list the stacks you want to deploy. If your app If you want to deploy all of them, you can use the flag `--all` or the wildcard `*` to deploy all stacks in an app. Please note that, if you have a hierarchy of stacks as described above, `--all` and `*` will only match the stacks on the top level. If you want to match all the stacks in the hierarchy, use `**`. You can also combine these patterns. For example, if you want to deploy all stacks in the `Prod` stage, you can use `cdk deploy PipelineStack/Prod/**`. +`--concurrency N` allows deploying multiple stacks in parallel while respecting inter-stack dependencies to speed up deployments. It does not protect against CloudFormation and other AWS account rate limiting. + #### Parameters Pass parameters to your template during deployment by using `--parameters @@ -456,6 +458,15 @@ locally to your terminal. To disable this feature you can pass the `--no-logs` o $ cdk watch --no-logs ``` +You can increase the concurrency by which `watch` will deploy and hotswap +your stacks by specifying `--concurrency N`. `--concurrency` for `watch` +acts the same as `--concurrency` for `deploy`, in that it will deploy or +hotswap your stacks while respecting inter-stack dependencies. + +```console +$ cdk watch --concurrency 5 +``` + **Note**: This command is considered experimental, and might have breaking changes in the future. The same limitations apply to to `watch` deployments as do to `--hotswap` deployments. See the *Hotswap deployments for faster development* section for more information. diff --git a/packages/aws-cdk/THIRD_PARTY_LICENSES b/packages/aws-cdk/THIRD_PARTY_LICENSES index d9c641e10cb4d..71e2287774c85 100644 --- a/packages/aws-cdk/THIRD_PARTY_LICENSES +++ b/packages/aws-cdk/THIRD_PARTY_LICENSES @@ -1143,6 +1143,32 @@ ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +---------------- + +** eventemitter3@4.0.7 - https://www.npmjs.com/package/eventemitter3/v/4.0.7 | MIT +The MIT License (MIT) + +Copyright (c) 2014 Arnout Kazemier + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. + + ---------------- ** fast-deep-equal@3.1.3 - https://www.npmjs.com/package/fast-deep-equal/v/3.1.3 | MIT @@ -2274,6 +2300,60 @@ ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +---------------- + +** p-finally@1.0.0 - https://www.npmjs.com/package/p-finally/v/1.0.0 | MIT +The MIT License (MIT) + +Copyright (c) Sindre Sorhus (sindresorhus.com) + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. + + +---------------- + +** p-queue@6.6.2 - https://www.npmjs.com/package/p-queue/v/6.6.2 | MIT +MIT License + +Copyright (c) Sindre Sorhus (https://sindresorhus.com) + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + +---------------- + +** p-timeout@3.2.0 - https://www.npmjs.com/package/p-timeout/v/3.2.0 | MIT +MIT License + +Copyright (c) Sindre Sorhus (sindresorhus.com) + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + + ---------------- ** pac-proxy-agent@5.0.0 - https://www.npmjs.com/package/pac-proxy-agent/v/5.0.0 | MIT diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index debd5a32fa998..4d12e2613d237 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -15,6 +15,7 @@ import { findCloudWatchLogGroups } from './api/logs/find-cloudwatch-logs'; import { CloudWatchLogEventMonitor } from './api/logs/logs-monitor'; import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor'; import { buildAllStackAssets } from './build'; +import { deployStacks } from './deploy'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { ResourceImporter } from './import'; import { data, debug, error, highlight, print, success, warning } from './logging'; @@ -138,7 +139,7 @@ export class CdkToolkit { } const startSynthTime = new Date().getTime(); - const stacks = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly); + const stackCollection = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly); const elapsedSynthTime = new Date().getTime() - startSynthTime; print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime)); @@ -164,33 +165,23 @@ export class CdkToolkit { warning('⚠️ It should only be used for development - never use it for your production Stacks!'); } + const stacks = stackCollection.stackArtifacts; + const stackOutputs: { [key: string]: any } = { }; const outputsFile = options.outputsFile; - const buildStackAssets = async (stack: cxapi.CloudFormationStackArtifact) => { - // Check whether the stack has an asset manifest before trying to build and publish. - if (!stack.dependencies.some(cxapi.AssetManifestArtifact.isAssetManifestArtifact)) { - return; - } - - print('%s: building assets...\n', chalk.bold(stack.displayName)); - await this.props.cloudFormation.buildStackAssets({ - stack, - roleArn: options.roleArn, - toolkitStackName: options.toolkitStackName, - }); - print('\n%s: assets built\n', chalk.bold(stack.displayName)); - }; - try { - await buildAllStackAssets(stacks.stackArtifacts, { buildStackAssets }); + await buildAllStackAssets(stackCollection.stackArtifacts, { + buildStackAssets: (a) => this.buildAllAssetsForSingleStack(a, options), + }); } catch (e) { error('\n ❌ Building assets failed: %s', e); throw e; } - for (const stack of stacks.stackArtifacts) { - if (stacks.stackCount !== 1) { highlight(stack.displayName); } + const deployStack = async (stack: cxapi.CloudFormationStackArtifact) => { + if (stackCollection.stackCount !== 1) { highlight(stack.displayName); } + if (!stack.environment) { // eslint-disable-next-line max-len throw new Error(`Stack ${stack.displayName} does not define an environment, and AWS credentials could not be obtained from standard locations or no region was configured.`); @@ -210,7 +201,7 @@ export class CdkToolkit { ci: options.ci, }); } - continue; + return; } if (requireApproval !== RequireApproval.Never) { @@ -224,6 +215,13 @@ export class CdkToolkit { 'but terminal (TTY) is not attached so we are unable to get a confirmation from the user'); } + // only talk to user if concurreny is 1 (otherwise, fail) + if (concurrency > 1) { + throw new Error( + '"--require-approval" is enabled and stack includes security-sensitive updates, ' + + 'but concurrency is greater than 1 so we are unable to get a confirmation from the user'); + } + const confirmed = await promptly.confirm('Do you wish to deploy these changes (y/n)?'); if (!confirmed) { throw new Error('Aborted by user'); } } @@ -252,7 +250,7 @@ export class CdkToolkit { force: options.force, parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]), usePreviousParameters: options.usePreviousParameters, - progress: options.progress, + progress, ci: options.ci, rollback: options.rollback, hotswap: options.hotswap, @@ -302,6 +300,19 @@ export class CdkToolkit { } } print('\n✨ Total time: %ss\n', formatTime(elapsedSynthTime + elapsedDeployTime)); + }; + + const concurrency = options.concurrency || 1; + const progress = concurrency > 1 ? StackActivityProgress.EVENTS : options.progress; + if (concurrency > 1 && options.progress && options.progress != StackActivityProgress.EVENTS) { + warning('⚠️ The --concurrency flag only supports --progress "events". Switching to "events".'); + } + + try { + await deployStacks(stacks, { concurrency, deployStack }); + } catch (e) { + error('\n ❌ Deployment failed: %s', e); + throw e; } } @@ -738,6 +749,7 @@ export class CdkToolkit { cacheCloudAssembly: false, hotswap: hotswap, extraUserAgent: `cdk-watch/hotswap-${hotswap ? 'on' : 'off'}`, + concurrency: options.concurrency, }; try { @@ -746,6 +758,21 @@ export class CdkToolkit { // just continue - deploy will show the error } } + + private async buildAllAssetsForSingleStack(stack: cxapi.CloudFormationStackArtifact, options: Pick): Promise { + // Check whether the stack has an asset manifest before trying to build and publish. + if (!stack.dependencies.some(cxapi.AssetManifestArtifact.isAssetManifestArtifact)) { + return; + } + + print('%s: building assets...\n', chalk.bold(stack.displayName)); + await this.props.cloudFormation.buildStackAssets({ + stack, + roleArn: options.roleArn, + toolkitStackName: options.toolkitStackName, + }); + print('\n%s: assets built\n', chalk.bold(stack.displayName)); + } } export interface DiffOptions { @@ -901,6 +928,14 @@ interface WatchOptions extends Omit { * @default - false */ readonly traceLogs?: boolean; + + /** + * Maximum number of simulatenous deployments (dependency permitting) to execute. + * The default is '1', which executes all deployments serially. + * + * @default 1 + */ + readonly concurrency?: number; } export interface DeployOptions extends CfnDeployOptions, WatchOptions { @@ -972,6 +1007,14 @@ export interface DeployOptions extends CfnDeployOptions, WatchOptions { * @default - not monitoring CloudWatch logs */ readonly cloudWatchLogMonitor?: CloudWatchLogEventMonitor; + + /** + * Maximum number of simulatenous deployments (dependency permitting) to execute. + * The default is '1', which executes all deployments serially. + * + * @default 1 + */ + readonly concurrency?: number; } export interface ImportOptions extends CfnDeployOptions { diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 86317d8a1f100..94fa333b41009 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -146,7 +146,8 @@ async function parseCommandLineArguments() { desc: 'Show CloudWatch log events from all resources in the selected Stacks in the terminal. ' + "'true' by default, use --no-logs to turn off. " + "Only in effect if specified alongside the '--watch' option", - }), + }) + .option('concurrency', { type: 'number', desc: 'Maximum number of simulatenous deployments (dependency permitting) to execute.', default: 1, requiresArg: true }), ) .command('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) => yargs .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true }) @@ -215,7 +216,8 @@ async function parseCommandLineArguments() { default: true, desc: 'Show CloudWatch log events from all resources in the selected Stacks in the terminal. ' + "'true' by default, use --no-logs to turn off", - }), + }) + .option('concurrency', { type: 'number', desc: 'Maximum number of simulatenous deployments (dependency permitting) to execute.', default: 1, requiresArg: true }), ) .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', (yargs: Argv) => yargs .option('all', { type: 'boolean', default: false, desc: 'Destroy all available stacks' }) @@ -480,6 +482,7 @@ async function initCommandLine() { hotswap: args.hotswap, watch: args.watch, traceLogs: args.logs, + concurrency: args.concurrency, }); case 'import': @@ -514,6 +517,7 @@ async function initCommandLine() { rollback: configuration.settings.get(['rollback']), hotswap: args.hotswap, traceLogs: args.logs, + concurrency: args.concurrency, }); case 'destroy': diff --git a/packages/aws-cdk/lib/deploy.ts b/packages/aws-cdk/lib/deploy.ts new file mode 100644 index 0000000000000..76bd1b579664f --- /dev/null +++ b/packages/aws-cdk/lib/deploy.ts @@ -0,0 +1,69 @@ +import * as cxapi from '@aws-cdk/cx-api'; +import PQueue from 'p-queue'; + +type Options = { + concurrency: number; + deployStack: (stack: cxapi.CloudFormationStackArtifact) => Promise; +}; + +type DeploymentState = 'pending' | 'queued' | 'deploying' | 'completed' | 'failed' | 'skipped'; + +export const deployStacks = async (stacks: cxapi.CloudFormationStackArtifact[], { concurrency, deployStack }: Options): Promise => { + const queue = new PQueue({ concurrency }); + const deploymentStates = stacks.reduce((acc, stack) => ({ ...acc, [stack.id]: 'pending' as const }), {} as Record); + + const isStackUnblocked = (stack: cxapi.CloudFormationStackArtifact) => + stack.dependencies + .map(({ id }) => id) + .filter((id) => !id.endsWith('.assets')) + .every((id) => !deploymentStates[id] || deploymentStates[id] === 'completed'); // Dependency not selected or already finished + + const hasAnyStackFailed = (states: Record) => Object.values(states).includes('failed'); + + const deploymentErrors: Error[] = []; + + const enqueueStackDeploys = () => { + stacks.forEach(async (stack) => { + if (deploymentStates[stack.id] === 'pending' && isStackUnblocked(stack)) { + deploymentStates[stack.id] = 'queued'; + + await queue.add(async () => { + // Do not start new deployments if any has already failed + if (hasAnyStackFailed(deploymentStates)) { + deploymentStates[stack.id] = 'skipped'; + return; + } + + deploymentStates[stack.id] = 'deploying'; + + await deployStack(stack).catch((err) => { + // By recording the failure immediately as the queued task exits, we prevent the next + // queued task from starting (its 'hasAnyStackFailed' will return 'true'). + deploymentStates[stack.id] = 'failed'; + throw err; + }); + + deploymentStates[stack.id] = 'completed'; + enqueueStackDeploys(); + }).catch((err) => { + deploymentStates[stack.id] = 'failed'; + deploymentErrors.push(err); + }); + } + }); + }; + + enqueueStackDeploys(); + + await queue.onIdle(); + + if (deploymentErrors.length) { + throw Error(`Stack Deployments Failed: ${deploymentErrors}`); + } + + // We shouldn't be able to get here, but check it anyway + const neverUnblocked = Object.entries(deploymentStates).filter(([_, s]) => s === 'pending').map(([n, _]) => n); + if (neverUnblocked.length > 0) { + throw new Error(`The following stacks never became unblocked: ${neverUnblocked.join(', ')}. Please report this at https://github.com/aws/aws-cdk/issues`); + } +}; diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index ef903908b5000..9fddc07471060 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -107,6 +107,7 @@ "glob": "^7.2.3", "json-diff": "^0.9.0", "minimatch": ">=3.1", + "p-queue": "^6.6.2", "promptly": "^3.2.0", "proxy-agent": "^5.0.0", "semver": "^7.3.7", diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts index 341775b08b38a..b0359fbb6dbc7 100644 --- a/packages/aws-cdk/test/cdk-toolkit.test.ts +++ b/packages/aws-cdk/test/cdk-toolkit.test.ts @@ -657,6 +657,18 @@ describe('watch', () => { expect(excludeArgs[1]).toBe('**/my-dir2'); }); + test('allows watching with deploy concurrency', async () => { + cloudExecutable.configuration.settings.set(['watch'], {}); + const toolkit = defaultToolkitSetup(); + const cdkDeployMock = jest.fn(); + toolkit.deploy = cdkDeployMock; + + await toolkit.watch({ selector: { patterns: [] }, concurrency: 3 }); + fakeChokidarWatcherOn.readyCallback(); + + expect(cdkDeployMock).toBeCalledWith(expect.objectContaining({ concurrency: 3 })); + }); + describe('with file change events', () => { let toolkit: CdkToolkit; let cdkDeployMock: jest.Mock; diff --git a/packages/aws-cdk/test/deploy.test.ts b/packages/aws-cdk/test/deploy.test.ts new file mode 100644 index 0000000000000..a6121d7f5d1a5 --- /dev/null +++ b/packages/aws-cdk/test/deploy.test.ts @@ -0,0 +1,199 @@ +import * as cxapi from '@aws-cdk/cx-api'; +import { deployStacks } from '../lib/deploy'; + +type Stack = cxapi.CloudFormationStackArtifact; + +const sleep = async (duration: number) => new Promise((resolve) => setTimeout(() => resolve(), duration)); + +// Not great to have actual sleeps in the tests, but they mostly just exist to give 'p-queue' +// a chance to start new tasks. +const SLOW = 200; + +/** + * Repurposing unused stack attributes to create specific test scenarios + * - stack.name = deployment duration + * - stack.displayName = error message + */ +describe('DeployStacks', () => { + const deployedStacks: string[] = []; + const deployStack = async ({ id, displayName, name }: Stack) => { + const errorMessage = displayName; + const timeout = Number(name) || 0; + + await sleep(timeout); + + if (errorMessage) { + throw Error(errorMessage); + } + + deployedStacks.push(id); + }; + + beforeEach(() => { + deployedStacks.splice(0); + }); + + // Success + test.each([ + // Concurrency 1 + { scenario: 'No Stacks', concurrency: 1, toDeploy: [], expected: [] }, + { scenario: 'A', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }], expected: ['A'] }, + { scenario: 'A, B', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [] }], expected: ['A', 'B'] }, + { scenario: 'A -> B', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }], expected: ['A', 'B'] }, + { scenario: '[unsorted] A -> B', concurrency: 1, toDeploy: [{ id: 'B', dependencies: [{ id: 'A' }] }, { id: 'A', dependencies: [] }], expected: ['A', 'B'] }, + { scenario: 'A -> B -> C', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }, { id: 'C', dependencies: [{ id: 'B' }] }], expected: ['A', 'B', 'C'] }, + { scenario: 'A -> B, A -> C', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }, { id: 'C', dependencies: [{ id: 'A' }] }], expected: ['A', 'B', 'C'] }, + { + scenario: 'A (slow), B', + concurrency: 1, + toDeploy: [ + { id: 'A', dependencies: [], name: SLOW }, + { id: 'B', dependencies: [] }, + ], + expected: ['A', 'B'], + }, + { + scenario: 'A -> B, C -> D', + concurrency: 1, + toDeploy: [ + { id: 'A', dependencies: [] }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expected: ['A', 'C', 'B', 'D'], + }, + { + scenario: 'A (slow) -> B, C -> D', + concurrency: 1, + toDeploy: [ + { id: 'A', dependencies: [], name: SLOW }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expected: ['A', 'C', 'B', 'D'], + }, + + // Concurrency 2 + { scenario: 'No Stacks', concurrency: 2, toDeploy: [], expected: [] }, + { scenario: 'A', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }], expected: ['A'] }, + { scenario: 'A, B', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [] }], expected: ['A', 'B'] }, + { scenario: 'A -> B', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }], expected: ['A', 'B'] }, + { scenario: '[unsorted] A -> B', concurrency: 2, toDeploy: [{ id: 'B', dependencies: [{ id: 'A' }] }, { id: 'A', dependencies: [] }], expected: ['A', 'B'] }, + { scenario: 'A -> B -> C', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }, { id: 'C', dependencies: [{ id: 'B' }] }], expected: ['A', 'B', 'C'] }, + { scenario: 'A -> B, A -> C', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [{ id: 'A' }] }, { id: 'C', dependencies: [{ id: 'A' }] }], expected: ['A', 'B', 'C'] }, + { + scenario: 'A, B', + concurrency: 2, + toDeploy: [ + { id: 'A', dependencies: [], name: SLOW }, + { id: 'B', dependencies: [] }, + ], + expected: ['B', 'A'], + }, + { + scenario: 'A -> B, C -> D', + concurrency: 2, + toDeploy: [ + { id: 'A', dependencies: [] }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expected: ['A', 'C', 'B', 'D'], + }, + { + scenario: 'A (slow) -> B, C -> D', + concurrency: 2, + toDeploy: [ + { id: 'A', dependencies: [], name: SLOW }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expected: ['C', 'D', 'A', 'B'], + }, + { + scenario: 'A -> B, A not selected', + concurrency: 1, + toDeploy: [ + { id: 'B', dependencies: [{ id: 'A' }] }, + ], + expected: ['B'], + }, + ])('Success - Concurrency: $concurrency - $scenario', async ({ concurrency, expected, toDeploy }) => { + await expect(deployStacks(toDeploy as unknown as Stack[], { concurrency, deployStack })).resolves.toBeUndefined(); + + expect(deployedStacks).toStrictEqual(expected); + }); + + // Failure + test.each([ + // Concurrency 1 + { scenario: 'A (error)', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }], expectedError: 'A', expectedStacks: [] }, + { scenario: 'A (error), B', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }, { id: 'B', dependencies: [] }], expectedError: 'A', expectedStacks: [] }, + { scenario: 'A, B (error)', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [], displayName: 'B' }], expectedError: 'B', expectedStacks: ['A'] }, + { scenario: 'A (error) -> B', concurrency: 1, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }, { id: 'B', dependencies: [{ id: 'A' }] }], expectedError: 'A', expectedStacks: [] }, + { scenario: '[unsorted] A (error) -> B', concurrency: 1, toDeploy: [{ id: 'B', dependencies: [{ id: 'A' }] }, { id: 'A', dependencies: [], displayName: 'A' }], expectedError: 'A', expectedStacks: [] }, + { + scenario: 'A (error) -> B, C -> D', + concurrency: 1, + toDeploy: [ + { id: 'A', dependencies: [], displayName: 'A' }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expectedError: 'A', + expectedStacks: [], + }, + { + scenario: 'A -> B, C (error) -> D', + concurrency: 1, + toDeploy: [ + { id: 'A', dependencies: [] }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [], displayName: 'C', name: SLOW }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expectedError: 'C', + expectedStacks: ['A'], + }, + + // Concurrency 2 + { scenario: 'A (error)', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }], expectedError: 'A', expectedStacks: [] }, + { scenario: 'A (error), B', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }, { id: 'B', dependencies: [] }], expectedError: 'A', expectedStacks: ['B'] }, + { scenario: 'A, B (error)', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [] }, { id: 'B', dependencies: [], displayName: 'B' }], expectedError: 'B', expectedStacks: ['A'] }, + { scenario: 'A (error) -> B', concurrency: 2, toDeploy: [{ id: 'A', dependencies: [], displayName: 'A' }, { id: 'B', dependencies: [{ id: 'A' }] }], expectedError: 'A', expectedStacks: [] }, + { scenario: '[unsorted] A (error) -> B', concurrency: 2, toDeploy: [{ id: 'B', dependencies: [{ id: 'A' }] }, { id: 'A', dependencies: [], displayName: 'A' }], expectedError: 'A', expectedStacks: [] }, + { + scenario: 'A (error) -> B, C -> D', + concurrency: 2, + toDeploy: [ + { id: 'A', dependencies: [], displayName: 'A' }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [] }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expectedError: 'A', + expectedStacks: ['C'], + }, + { + scenario: 'A -> B, C (error) -> D', + concurrency: 2, + toDeploy: [ + { id: 'A', dependencies: [] }, + { id: 'B', dependencies: [{ id: 'A' }] }, + { id: 'C', dependencies: [], displayName: 'C', name: SLOW }, + { id: 'D', dependencies: [{ id: 'C' }] }, + ], + expectedError: 'C', + expectedStacks: ['A', 'B'], + }, + ])('Failure - Concurrency: $concurrency - $scenario', async ({ concurrency, expectedError, toDeploy, expectedStacks }) => { + await expect(deployStacks(toDeploy as unknown as Stack[], { concurrency, deployStack })).rejects.toThrowError(expectedError); + + expect(deployedStacks).toStrictEqual(expectedStacks); + }); +}); \ No newline at end of file diff --git a/packages/aws-cdk/test/integ/cli/app/app.js b/packages/aws-cdk/test/integ/cli/app/app.js index 8c6a722bdb674..131e759e10495 100755 --- a/packages/aws-cdk/test/integ/cli/app/app.js +++ b/packages/aws-cdk/test/integ/cli/app/app.js @@ -279,6 +279,9 @@ class DockerStackWithCustomFile extends cdk.Stack { } } +/** + * A stack that will never succeed deploying (done in a way that CDK cannot detect but CFN will complain about) + */ class FailedStack extends cdk.Stack { constructor(parent, id, props) { @@ -400,7 +403,11 @@ switch (stackSet) { new LambdaHotswapStack(app, `${stackPrefix}-lambda-hotswap`); new DockerStack(app, `${stackPrefix}-docker`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); - new FailedStack(app, `${stackPrefix}-failed`) + const failed = new FailedStack(app, `${stackPrefix}-failed`) + + // A stack that depends on the failed stack -- used to test that '-e' does not deploy the failing stack + const dependsOnFailed = new OutputsStack(app, `${stackPrefix}-depends-on-failed`); + dependsOnFailed.addDependency(failed); if (process.env.ENABLE_VPC_TESTING) { // Gating so we don't do context fetching unless that's what we are here for const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }; diff --git a/packages/aws-cdk/test/integ/cli/cli.integtest.ts b/packages/aws-cdk/test/integ/cli/cli.integtest.ts index ba3519481e883..89477861966d1 100644 --- a/packages/aws-cdk/test/integ/cli/cli.integtest.ts +++ b/packages/aws-cdk/test/integ/cli/cli.integtest.ts @@ -135,6 +135,37 @@ integTest('automatic ordering', withDefaultFixture(async (fixture) => { await fixture.cdkDestroy('order-providing'); })); +integTest('automatic ordering with concurrency', withDefaultFixture(async (fixture) => { + // Deploy the consuming stack which will include the producing stack + await fixture.cdkDeploy('order-consuming', { options: ['--concurrency', '2'] }); + + // Destroy the providing stack which will include the consuming stack + await fixture.cdkDestroy('order-providing'); +})); + +integTest('--exclusively selects only selected stack', withDefaultFixture(async (fixture) => { + // Deploy the "depends-on-failed" stack, with --exclusively. It will NOT fail (because + // of --exclusively) and it WILL create an output we can check for to confirm that it did + // get deployed. + const outputsFile = path.join(fixture.integTestDir, 'outputs', 'outputs.json'); + await fs.mkdir(path.dirname(outputsFile), { recursive: true }); + + await fixture.cdkDeploy('depends-on-failed', { + options: [ + '--exclusively', + '--outputs-file', outputsFile, + ], + }); + + // Verify the output to see that the stack deployed + const outputs = JSON.parse((await fs.readFile(outputsFile, { encoding: 'utf-8' })).toString()); + expect(outputs).toEqual({ + [`${fixture.stackNamePrefix}-depends-on-failed`]: { + TopicName: `${fixture.stackNamePrefix}-depends-on-failedMyTopic`, + }, + }); +})); + integTest('context setting', withDefaultFixture(async (fixture) => { await fs.writeFile(path.join(fixture.integTestDir, 'cdk.context.json'), JSON.stringify({ contextkey: 'this is the context value', @@ -178,7 +209,17 @@ integTest('deploy', withDefaultFixture(async (fixture) => { integTest('deploy all', withDefaultFixture(async (fixture) => { const arns = await fixture.cdkDeploy('test-*', { captureStderr: false }); - // verify that we only deployed a single stack (there's a single ARN in the output) + // verify that we only deployed both stacks (there are 2 ARNs in the output) + expect(arns.split('\n').length).toEqual(2); +})); + +integTest('deploy all concurrently', withDefaultFixture(async (fixture) => { + const arns = await fixture.cdkDeploy('test-*', { + captureStderr: false, + options: ['--concurrency', '2'], + }); + + // verify that we only deployed both stacks (there are 2 ARNs in the output) expect(arns.split('\n').length).toEqual(2); })); diff --git a/packages/aws-cdk/tsconfig.json b/packages/aws-cdk/tsconfig.json index 674269068d4c5..2f37a56b99090 100644 --- a/packages/aws-cdk/tsconfig.json +++ b/packages/aws-cdk/tsconfig.json @@ -1,8 +1,8 @@ { "compilerOptions": { - "target": "ES2019", + "target": "ES2020", "module": "commonjs", - "lib": ["es2019", "dom"], + "lib": ["es2019", "es2020", "dom"], "strict": true, "alwaysStrict": true, "declaration": true, From 3d22f70a8e4e81e2e9056fa76a4c932f3305fd4b Mon Sep 17 00:00:00 2001 From: Masashi Tomooka Date: Thu, 25 Aug 2022 23:28:24 +0900 Subject: [PATCH 2/5] fix(cli): ECS hotswap breaks Firelens configuration (#21748) closes #21692 Now we can specify keys that will not be transformed by `transformObjectKeys` function. ---- ### All Submissions: * [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/lib/api/hotswap/common.ts | 17 ++- .../aws-cdk/lib/api/hotswap/ecs-services.ts | 20 ++- .../ecs-services-hotswap-deployments.test.ts | 118 ++++++++++++++++++ 3 files changed, 151 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts index 03f78df4ddac6..8f4c17ccc21b2 100644 --- a/packages/aws-cdk/lib/api/hotswap/common.ts +++ b/packages/aws-cdk/lib/api/hotswap/common.ts @@ -61,23 +61,34 @@ export class HotswappableChangeCandidate { } } +type Exclude = { [key: string]: Exclude | true } + /** * This function transforms all keys (recursively) in the provided `val` object. * * @param val The object whose keys need to be transformed. * @param transform The function that will be applied to each key. + * @param exclude The keys that will not be transformed and copied to output directly * @returns A new object with the same values as `val`, but with all keys transformed according to `transform`. */ -export function transformObjectKeys(val: any, transform: (str: string) => string): any { +export function transformObjectKeys(val: any, transform: (str: string) => string, exclude: Exclude = {}): any { if (val == null || typeof val !== 'object') { return val; } if (Array.isArray(val)) { - return val.map((input: any) => transformObjectKeys(input, transform)); + // For arrays we just pass parent's exclude object directly + // since it makes no sense to specify different exclude options for each array element + return val.map((input: any) => transformObjectKeys(input, transform, exclude)); } const ret: { [k: string]: any; } = {}; for (const [k, v] of Object.entries(val)) { - ret[transform(k)] = transformObjectKeys(v, transform); + const childExclude = exclude[k]; + if (childExclude === true) { + // we don't transform this object if the key is specified in exclude + ret[transform(k)] = v; + } else { + ret[transform(k)] = transformObjectKeys(v, transform, childExclude); + } } return ret; } diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 1794457d86c9f..baa7383fd3d62 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -91,7 +91,25 @@ class EcsServiceHotswapOperation implements HotswapOperation { // Step 1 - update the changed TaskDefinition, creating a new TaskDefinition Revision // we need to lowercase the evaluated TaskDef from CloudFormation, // as the AWS SDK uses lowercase property names for these - const lowercasedTaskDef = transformObjectKeys(this.taskDefinitionResource, lowerCaseFirstCharacter); + const lowercasedTaskDef = transformObjectKeys(this.taskDefinitionResource, lowerCaseFirstCharacter, { + // All the properties that take arbitrary string as keys i.e. { "string" : "string" } + // https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_RegisterTaskDefinition.html#API_RegisterTaskDefinition_RequestSyntax + ContainerDefinitions: { + DockerLabels: true, + FirelensConfiguration: { + Options: true, + }, + LogConfiguration: { + Options: true, + }, + }, + Volumes: { + DockerVolumeConfiguration: { + DriverOpts: true, + Labels: true, + }, + }, + }); const registerTaskDefResponse = await sdk.ecs().registerTaskDefinition(lowercasedTaskDef).promise(); const taskDefRevArn = registerTaskDefResponse.taskDefinition?.taskDefinitionArn; diff --git a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts index 717f68ecd5b29..dda972f5a90b9 100644 --- a/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/ecs-services-hotswap-deployments.test.ts @@ -362,3 +362,121 @@ test('if anything besides an ECS Service references the changed TaskDefinition, expect(deployStackResult).toBeUndefined(); expect(mockRegisterTaskDef).not.toHaveBeenCalled(); }); + +test('should call registerTaskDefinition with certain properties not lowercased', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { Image: 'image1' }, + ], + Volumes: [ + { + DockerVolumeConfiguration: { + DriverOpts: { Option1: 'option1' }, + Labels: { Label1: 'label1' }, + }, + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }); + setup.pushStackResourceSummaries( + setup.stackSummaryOf('Service', 'AWS::ECS::Service', + 'arn:aws:ecs:region:account:service/my-cluster/my-service'), + ); + mockRegisterTaskDef.mockReturnValue({ + taskDefinition: { + taskDefinitionArn: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + TaskDef: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + Family: 'my-task-def', + ContainerDefinitions: [ + { + Image: 'image2', + DockerLabels: { Label1: 'label1' }, + FirelensConfiguration: { + Options: { Name: 'cloudwatch' }, + }, + LogConfiguration: { + Options: { Option1: 'option1' }, + }, + }, + ], + Volumes: [ + { + DockerVolumeConfiguration: { + DriverOpts: { Option1: 'option1' }, + Labels: { Label1: 'label1' }, + }, + }, + ], + }, + }, + Service: { + Type: 'AWS::ECS::Service', + Properties: { + TaskDefinition: { Ref: 'TaskDef' }, + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockRegisterTaskDef).toBeCalledWith({ + family: 'my-task-def', + containerDefinitions: [ + { + image: 'image2', + dockerLabels: { Label1: 'label1' }, + firelensConfiguration: { + options: { + Name: 'cloudwatch', + }, + }, + logConfiguration: { + options: { Option1: 'option1' }, + }, + }, + ], + volumes: [ + { + dockerVolumeConfiguration: { + driverOpts: { Option1: 'option1' }, + labels: { Label1: 'label1' }, + }, + }, + ], + }); + expect(mockUpdateService).toBeCalledWith({ + service: 'arn:aws:ecs:region:account:service/my-cluster/my-service', + cluster: 'my-cluster', + taskDefinition: 'arn:aws:ecs:region:account:task-definition/my-task-def:3', + deploymentConfiguration: { + minimumHealthyPercent: 0, + }, + forceNewDeployment: true, + }); +}); From 5e7662c3dd5cdb7ea3e8d21456e36874dbafc0e3 Mon Sep 17 00:00:00 2001 From: Evgeny Karasik Date: Thu, 25 Aug 2022 18:37:49 +0300 Subject: [PATCH 3/5] chore(docs): add deprecation message for languages (#21754) ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index b91cc2e44fad7..a911cc9c574b9 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,8 @@ The CDK is available in the following languages: * .NET ([.NET Core ≥ 3.1](https://dotnet.microsoft.com/download)) * Go ([Go ≥ 1.16.4](https://golang.org/)) +Third-party Language Deprecation: language version is only supported until its EOL (End Of Life) shared by the vendor or community and is subject to change with prior notice. + \ Jump To: [Developer Guide](https://docs.aws.amazon.com/cdk/latest/guide) | From da61adc1bfadea4e541d34f4eb082f280cc289e1 Mon Sep 17 00:00:00 2001 From: Peter Woodworth <44349620+peterwoodworth@users.noreply.github.com> Date: Thu, 25 Aug 2022 09:28:34 -0700 Subject: [PATCH 4/5] fix(autoscaling): error not thrown when associatePublicIpAddress is set to false when specifying launchTemplate (#21714) Setting `associatePublicIpAddress` prop when also setting `launchTemplate` prop on a new ASG is supposed to throw an error, but doesn't due to `associatePublicIpAddress` being a boolean, and the error message not triggering if a falsy value is used for the prop. This PR will ensure an error is thrown when `associatePublicIpAddress` is set, even when the value passed is falsy. It also updates the documentation on all props which conflict with `launchTemplate` or `mixedInstancesPolicy` fixes: #21576 ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-autoscaling/lib/auto-scaling-group.ts | 22 ++++++++++++++----- .../test/auto-scaling-group.test.ts | 10 ++++++++- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 3a5061fc0ff58..0c5006c29f514 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -80,6 +80,8 @@ export interface CommonAutoScalingGroupProps { /** * Name of SSH keypair to grant access to instances * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default - No SSH access will be possible. */ readonly keyName?: string; @@ -190,6 +192,8 @@ export interface CommonAutoScalingGroupProps { * Whether instances in the Auto Scaling Group should have public * IP addresses associated with them. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default - Use subnet setting. */ readonly associatePublicIpAddress?: boolean; @@ -198,6 +202,8 @@ export interface CommonAutoScalingGroupProps { * The maximum hourly price (in USD) to be paid for any Spot Instance launched to fulfill the request. Spot Instances are * launched when the price you specify exceeds the current Spot market price. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @default none */ readonly spotPrice?: string; @@ -217,6 +223,8 @@ export interface CommonAutoScalingGroupProps { * You can use block device mappings to specify additional EBS volumes or * instance store volumes to attach to an instance when it is launched. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html * * @default - Uses the block device mapping of the AMI @@ -243,6 +251,8 @@ export interface CommonAutoScalingGroupProps { * When detailed monitoring is enabled, Amazon CloudWatch generates metrics every minute and your account * is charged a fee. When you disable detailed monitoring, CloudWatch generates metrics every 5 minutes. * + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified + * * @see https://docs.aws.amazon.com/autoscaling/latest/userguide/as-instance-monitoring.html#enable-as-instance-metrics * * @default - Monitoring.DETAILED @@ -543,7 +553,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * Type of instance to launch * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - Do not provide any instance type */ @@ -552,7 +562,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * AMI to launch * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - Do not provide any machine image */ @@ -561,7 +571,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { /** * Security group to launch the instances in. * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - A SecurityGroup will be created if none is specified. */ @@ -572,7 +582,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { * * The UserData may still be mutated after creation. * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @default - A UserData object appropriate for the MachineImage's * Operating System is created. @@ -584,7 +594,7 @@ export interface AutoScalingGroupProps extends CommonAutoScalingGroupProps { * * The role must be assumable by the service principal `ec2.amazonaws.com`: * - * `launchTemplate` must not be specified when this property is specified. + * `launchTemplate` and `mixedInstancesPolicy` must not be specified when this property is specified * * @example * @@ -1523,7 +1533,7 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements if (props.instanceMonitoring) { throw new Error('Setting \'instanceMonitoring\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); } - if (props.associatePublicIpAddress) { + if (props.associatePublicIpAddress !== undefined) { throw new Error('Setting \'associatePublicIpAddress\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); } if (props.spotPrice) { diff --git a/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts b/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts index cde7f73e86c31..0f836518ea204 100644 --- a/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts +++ b/packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts @@ -1525,6 +1525,7 @@ describe('auto scaling group', () => { launchTemplateId: 'test-lt-id', versionNumber: '0', }); + const vpc = mockVpc(stack); // THEN expect(() => { @@ -1535,9 +1536,16 @@ describe('auto scaling group', () => { generation: AmazonLinuxGeneration.AMAZON_LINUX_2, cpuType: AmazonLinuxCpuType.X86_64, }), - vpc: mockVpc(stack), + vpc, }); }).toThrow('Setting \'machineImage\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); + expect(() => { + new autoscaling.AutoScalingGroup(stack, 'imported-lt-asg-2', { + launchTemplate: lt, + associatePublicIpAddress: true, + vpc, + }); + }).toThrow('Setting \'associatePublicIpAddress\' must not be set when \'launchTemplate\' or \'mixedInstancesPolicy\' is set'); }); test('Cannot specify Launch Template without instance type', () => { From daf885ff8d20088c93e214dbb07d163cfaa28089 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Thu, 25 Aug 2022 14:04:28 -0400 Subject: [PATCH 5/5] fix(core): feature flag values should be booleans (#21759) `FeatureFlags.isEnabled()` returns `boolean | undefined`, but if the value of the feature flag is something other than a boolean, it will actually return that type instead. For example, ```ts node.setContext(someFeatureFlag, 'true'); const enabled = FeatureFlags.of(node).isEnabled(someFeatureFlag); if (enabled) { // will work } if (enabled === true) { // will not work because enabled === 'true' } ``` It looks like the only place this bug crops up is in the `FunctionVersionUpgrade` aspect. https://github.com/aws/aws-cdk/blob/8b0832334afee496355e7aeb684773d8f939f058/packages/@aws-cdk/aws-lambda/lib/function.ts#L1306 ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/core/lib/feature-flags.ts | 2 +- packages/@aws-cdk/core/test/feature-flags.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/core/lib/feature-flags.ts b/packages/@aws-cdk/core/lib/feature-flags.ts index 2da5397b8ec23..95ff50be4d040 100644 --- a/packages/@aws-cdk/core/lib/feature-flags.ts +++ b/packages/@aws-cdk/core/lib/feature-flags.ts @@ -32,6 +32,6 @@ export class FeatureFlags { } return true; } - return context ?? cxapi.futureFlagDefault(featureFlag); + return context !== undefined ? Boolean(context) : cxapi.futureFlagDefault(featureFlag); } } diff --git a/packages/@aws-cdk/core/test/feature-flags.test.ts b/packages/@aws-cdk/core/test/feature-flags.test.ts index a65490afc375c..0e4a267452dd6 100644 --- a/packages/@aws-cdk/core/test/feature-flags.test.ts +++ b/packages/@aws-cdk/core/test/feature-flags.test.ts @@ -23,5 +23,13 @@ describe('feature flags', () => { expect(FeatureFlags.of(stack).isEnabled('non-existent-flag')).toEqual(false); }); + + test('strings are evaluated as boolean', () => { + const stack = new Stack(); + stack.node.setContext(cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT, 'true'); + + const actual = FeatureFlags.of(stack).isEnabled(cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT); + expect(actual).toEqual(true); + }); }); });