Skip to content

Commit 506d210

Browse files
authored
chore(cli): simplify config with helper functions (#32547)
### Reason for this change Simplify the codegen for CLI config. Previously the generated function expected a number of arguments at call time. Instead we now require a single helpers object at build time. This holds typewriter references to any function called during config parsing. Additionally, some the helper functions for init and migrate are now just called at build time. This is fine because these values don't change dynamically unless a code change is made to the CLI. ### Description of changes - Moved all helper functions into a single file - Created a typewriter proxy class for that file - Use the module proxy instead passing arguments at runtime - This allows us to get rid of the `DynamicValue` stuff all-together and instead we can just check if a value is a typewriter expression. ### Description of how you validated changes Existing tests cover this. Run additional manual verification as well. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 38116b0 commit 506d210

File tree

8 files changed

+173
-139
lines changed

8 files changed

+173
-139
lines changed

packages/aws-cdk/lib/cli.ts

+3-13
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,13 @@ import { CdkToolkit, AssetBuildTime } from '../lib/cdk-toolkit';
2020
import { realHandler as context } from '../lib/commands/context';
2121
import { realHandler as docs } from '../lib/commands/docs';
2222
import { realHandler as doctor } from '../lib/commands/doctor';
23-
import { MIGRATE_SUPPORTED_LANGUAGES, getMigrateScanType } from '../lib/commands/migrate';
24-
import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init';
23+
import { getMigrateScanType } from '../lib/commands/migrate';
24+
import { cliInit, printAvailableTemplates } from '../lib/init';
2525
import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/logging';
2626
import { Notices } from '../lib/notices';
2727
import { Command, Configuration, Settings } from '../lib/settings';
2828
import * as version from '../lib/version';
2929
import { SdkToCliLogger } from './api/aws-auth/sdk-logger';
30-
import { yargsNegativeAlias } from './util/yargs-helpers';
3130

3231
/* eslint-disable max-len */
3332
/* eslint-disable @typescript-eslint/no-shadow */ // yargs
@@ -38,17 +37,8 @@ if (!process.stdout.isTTY) {
3837
}
3938

4039
export async function exec(args: string[], synthesizer?: Synthesizer): Promise<number | void> {
41-
function makeBrowserDefault(): string {
42-
const defaultBrowserCommand: { [key in NodeJS.Platform]?: string } = {
43-
darwin: 'open %u',
44-
win32: 'start %u',
45-
};
46-
47-
const cmd = defaultBrowserCommand[process.platform];
48-
return cmd ?? 'xdg-open %u';
49-
}
5040

51-
const argv = await parseCommandLineArguments(args, makeBrowserDefault(), await availableInitLanguages(), MIGRATE_SUPPORTED_LANGUAGES as string[], version.DISPLAY_VERSION, yargsNegativeAlias);
41+
const argv = await parseCommandLineArguments(args);
5242

5343
// if one -v, log at a DEBUG level
5444
// if 2 -v, log at a TRACE level

packages/aws-cdk/lib/config.ts

+27-61
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1-
import type { CliConfig, DynamicResult } from '@aws-cdk/yargs-gen';
1+
// eslint-disable-next-line import/no-extraneous-dependencies
2+
import { CliHelpers, type CliConfig } from '@aws-cdk/yargs-gen';
23
import { StackActivityProgress } from './api/util/cloudformation/stack-activity-monitor';
4+
import { MIGRATE_SUPPORTED_LANGUAGES } from './commands/migrate';
35
import { RequireApproval } from './diff';
6+
import { availableInitLanguages } from './init';
47

5-
/* eslint-disable quote-props */
8+
export const YARGS_HELPERS = new CliHelpers('./util/yargs-helpers');
69

710
/**
811
* Source of truth for all CDK CLI commands. `yargs-gen` translates this into the `yargs` definition
912
* in `lib/parse-command-line-arguments.ts`.
1013
*/
11-
export function makeConfig(): CliConfig {
14+
export async function makeConfig(): Promise<CliConfig> {
1215
return {
1316
globalOptions: {
1417
'app': { type: 'string', alias: 'a', desc: 'REQUIRED WHEN RUNNING APP: command-line for executing your app or a cloud assembly directory (e.g. "node bin/my-app.js"). Can also be specified in cdk.json or ~/.cdk.json', requiresArg: true },
@@ -34,11 +37,11 @@ export function makeConfig(): CliConfig {
3437
'output': { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true },
3538
'notices': { type: 'boolean', desc: 'Show relevant notices' },
3639
'no-color': { type: 'boolean', desc: 'Removes colors and other style from console output', default: false },
37-
'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: DynamicValue.fromInline(() => process.env.CI !== undefined) },
40+
'ci': { type: 'boolean', desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr', default: YARGS_HELPERS.isCI() },
3841
'unstable': { type: 'array', desc: 'Opt in to unstable features. The flag indicates that the scope and API of a feature might still change. Otherwise the feature is generally production ready and fully supported. Can be specified multiple times.', default: [] },
3942
},
4043
commands: {
41-
'list': {
44+
list: {
4245
arg: {
4346
name: 'STACKS',
4447
variadic: true,
@@ -50,17 +53,17 @@ export function makeConfig(): CliConfig {
5053
'show-dependencies': { type: 'boolean', default: false, alias: 'd', desc: 'Display stack dependency information for each stack' },
5154
},
5255
},
53-
'synthesize': {
56+
synthesize: {
5457
arg: {
5558
name: 'STACKS',
5659
variadic: true,
5760
},
5861
aliases: ['synth'],
5962
description: 'Synthesizes and prints the CloudFormation template for this stack',
6063
options: {
61-
'exclusively': { type: 'boolean', alias: 'e', desc: 'Only synthesize requested stacks, don\'t include dependencies' },
62-
'validation': { type: 'boolean', desc: 'After synthesis, validate stacks with the "validateOnSynth" attribute set (can also be controlled with CDK_VALIDATION)', default: true },
63-
'quiet': { type: 'boolean', alias: 'q', desc: 'Do not output CloudFormation Template to stdout', default: false },
64+
exclusively: { type: 'boolean', alias: 'e', desc: 'Only synthesize requested stacks, don\'t include dependencies' },
65+
validation: { type: 'boolean', desc: 'After synthesis, validate stacks with the "validateOnSynth" attribute set (can also be controlled with CDK_VALIDATION)', default: true },
66+
quiet: { type: 'boolean', alias: 'q', desc: 'Do not output CloudFormation Template to stdout', default: false },
6467
},
6568
},
6669
bootstrap: {
@@ -242,18 +245,6 @@ export function makeConfig(): CliConfig {
242245
variadic: true,
243246
},
244247
options: {
245-
// I'm fairly certain none of these options, present for 'deploy', make sense for 'watch':
246-
// .option('all', { type: 'boolean', default: false, desc: 'Deploy all available stacks' })
247-
// .option('ci', { type: 'boolean', desc: 'Force CI detection', default: process.env.CI !== undefined })
248-
// @deprecated(v2) -- tags are part of the Cloud Assembly and tags specified here will be overwritten on the next deployment
249-
// .option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE), overrides tags from Cloud Assembly (deprecated)', nargs: 1, requiresArg: true })
250-
// .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true })
251-
// These options, however, are more subtle - I could be convinced some of these should also be available for 'watch':
252-
// .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' })
253-
// .option('parameters', { type: 'array', desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)', nargs: 1, requiresArg: true, default: {} })
254-
// .option('previous-parameters', { type: 'boolean', default: true, desc: 'Use previous values for existing parameters (you must specify all parameters on every deployment if this is disabled)' })
255-
// .option('outputs-file', { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true })
256-
// .option('notification-arns', { type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true })
257248
'build-exclude': { type: 'array', alias: 'E', desc: 'Do not rebuild asset with the given ID. Can be specified multiple times', default: [] },
258249
'exclusively': { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' },
259250
'change-set-name': { type: 'string', desc: 'Name of the CloudFormation change set to create' },
@@ -295,9 +286,9 @@ export function makeConfig(): CliConfig {
295286
variadic: true,
296287
},
297288
options: {
298-
'all': { type: 'boolean', default: false, desc: 'Destroy all available stacks' },
299-
'exclusively': { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' },
300-
'force': { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' },
289+
all: { type: 'boolean', default: false, desc: 'Destroy all available stacks' },
290+
exclusively: { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' },
291+
force: { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' },
301292
},
302293
},
303294
diff: {
@@ -336,7 +327,7 @@ export function makeConfig(): CliConfig {
336327
notices: {
337328
description: 'Returns a list of relevant notices',
338329
options: {
339-
'unacknowledged': { type: 'boolean', alias: 'u', default: false, desc: 'Returns a list of unacknowledged notices' },
330+
unacknowledged: { type: 'boolean', alias: 'u', default: false, desc: 'Returns a list of unacknowledged notices' },
340331
},
341332
},
342333
init: {
@@ -346,16 +337,16 @@ export function makeConfig(): CliConfig {
346337
variadic: false,
347338
},
348339
options: {
349-
'language': { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: DynamicValue.fromParameter('availableInitLanguages') } as any, // TODO: preamble, this initTemplateLanguages variable needs to go as a statement there.
340+
'language': { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: await availableInitLanguages() },
350341
'list': { type: 'boolean', desc: 'List the available templates' },
351342
'generate-only': { type: 'boolean', default: false, desc: 'If true, only generates project files, without executing additional operations such as setting up a git repo, installing dependencies or compiling the project' },
352343
},
353344
},
354-
'migrate': {
345+
migrate: {
355346
description: false as any,
356347
options: {
357348
'stack-name': { type: 'string', alias: 'n', desc: 'The name assigned to the stack created in the new project. The name of the app will be based off this name as well.', requiresArg: true },
358-
'language': { type: 'string', default: 'typescript', alias: 'l', desc: 'The language to be used for the new project', choices: DynamicValue.fromParameter('migrateSupportedLanguages') as any },
349+
'language': { type: 'string', default: 'typescript', alias: 'l', desc: 'The language to be used for the new project', choices: MIGRATE_SUPPORTED_LANGUAGES },
359350
'account': { type: 'string', desc: 'The account to retrieve the CloudFormation stack template from' },
360351
'region': { type: 'string', desc: 'The region to retrieve the CloudFormation stack template from' },
361352
'from-path': { type: 'string', desc: 'The path to the CloudFormation template to migrate. Use this for locally stored templates' },
@@ -379,54 +370,29 @@ export function makeConfig(): CliConfig {
379370
'compress': { type: 'boolean', desc: 'Use this flag to zip the generated CDK app' },
380371
},
381372
},
382-
'context': {
373+
context: {
383374
description: 'Manage cached context values',
384375
options: {
385-
'reset': { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true },
386-
'force': { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false },
387-
'clear': { desc: 'Clear all context', type: 'boolean' },
376+
reset: { alias: 'e', desc: 'The context key (or its index) to reset', type: 'string', requiresArg: true },
377+
force: { alias: 'f', desc: 'Ignore missing key error', type: 'boolean', default: false },
378+
clear: { desc: 'Clear all context', type: 'boolean' },
388379
},
389380
},
390-
'docs': {
381+
docs: {
391382
aliases: ['doc'],
392383
description: 'Opens the reference documentation in a browser',
393384
options: {
394-
'browser': {
385+
browser: {
395386
alias: 'b',
396387
desc: 'the command to use to open the browser, using %u as a placeholder for the path of the file to open',
397388
type: 'string',
398-
default: DynamicValue.fromParameter('browserDefault'),
389+
default: YARGS_HELPERS.browserForPlatform(),
399390
},
400391
},
401392
},
402-
'doctor': {
393+
doctor: {
403394
description: 'Check your set-up for potential problems',
404395
},
405396
},
406397
};
407398
}
408-
409-
/**
410-
* Informs the code library, `@aws-cdk/yargs-gen`, that
411-
* this value references an entity not defined in this configuration file.
412-
*/
413-
export class DynamicValue {
414-
/**
415-
* Instructs `yargs-gen` to retrieve this value from the parameter with passed name.
416-
*/
417-
public static fromParameter(parameterName: string): DynamicResult {
418-
return {
419-
dynamicType: 'parameter',
420-
dynamicValue: parameterName,
421-
};
422-
}
423-
424-
public static fromInline(f: () => any): DynamicResult {
425-
const ARROW = '=>';
426-
const body = f.toString();
427-
return {
428-
dynamicType: 'function',
429-
dynamicValue: body.substring(body.indexOf(ARROW) + ARROW.length).trim(),
430-
};
431-
}
432-
}

packages/aws-cdk/lib/parse-command-line-arguments.ts

+9-15
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,10 @@
44
// -------------------------------------------------------------------------------------------
55
/* eslint-disable @typescript-eslint/comma-dangle, comma-spacing, max-len, quotes, quote-props */
66
import { Argv } from 'yargs';
7+
import * as helpers from './util/yargs-helpers';
78

89
// @ts-ignore TS6133
9-
export function parseCommandLineArguments(
10-
args: Array<string>,
11-
browserDefault: string,
12-
availableInitLanguages: Array<string>,
13-
migrateSupportedLanguages: Array<string>,
14-
version: string,
15-
yargsNegativeAlias: any
16-
): any {
10+
export function parseCommandLineArguments(args: Array<string>): any {
1711
return yargs
1812
.env('CDK')
1913
.usage('Usage: cdk -a <cdk-app> COMMAND')
@@ -143,7 +137,7 @@ export function parseCommandLineArguments(
143137
.option('ci', {
144138
type: 'boolean',
145139
desc: 'Force CI detection. If CI=true then logs will be sent to stdout instead of stderr',
146-
default: process.env.CI !== undefined,
140+
default: helpers.isCI(),
147141
})
148142
.option('unstable', {
149143
type: 'array',
@@ -424,8 +418,8 @@ export function parseCommandLineArguments(
424418
type: 'boolean',
425419
desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. Note: do **not** disable this flag for deployments with resource replacements, as that will always fail",
426420
})
427-
.middleware(yargsNegativeAlias('R', 'rollback'), true)
428421
.option('R', { type: 'boolean', hidden: true })
422+
.middleware(helpers.yargsNegativeAlias('R', 'rollback'), true)
429423
.option('hotswap', {
430424
type: 'boolean',
431425
desc: "Attempts to perform a 'hotswap' deployment, but does not fall back to a full deployment if that is not possible. Instead, changes to any non-hotswappable properties are ignored.Do not use this in production environments",
@@ -570,8 +564,8 @@ export function parseCommandLineArguments(
570564
type: 'boolean',
571565
desc: "Rollback stack to stable state on failure. Defaults to 'true', iterate more rapidly with --no-rollback or -R. Note: do **not** disable this flag for deployments with resource replacements, as that will always fail",
572566
})
573-
.middleware(yargsNegativeAlias('R', 'rollback'), true)
574567
.option('R', { type: 'boolean', hidden: true })
568+
.middleware(helpers.yargsNegativeAlias('R', 'rollback'), true)
575569
.option('hotswap', {
576570
type: 'boolean',
577571
desc: "Attempts to perform a 'hotswap' deployment, but does not fall back to a full deployment if that is not possible. Instead, changes to any non-hotswappable properties are ignored.'true' by default, use --no-hotswap to turn off",
@@ -679,7 +673,7 @@ export function parseCommandLineArguments(
679673
type: 'string',
680674
alias: 'l',
681675
desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)',
682-
choices: availableInitLanguages,
676+
choices: ['csharp', 'fsharp', 'go', 'java', 'javascript', 'python', 'typescript'],
683677
})
684678
.option('list', {
685679
type: 'boolean',
@@ -704,7 +698,7 @@ export function parseCommandLineArguments(
704698
default: 'typescript',
705699
alias: 'l',
706700
desc: 'The language to be used for the new project',
707-
choices: migrateSupportedLanguages,
701+
choices: ['typescript', 'go', 'java', 'python', 'csharp'],
708702
})
709703
.option('account', {
710704
type: 'string',
@@ -765,11 +759,11 @@ export function parseCommandLineArguments(
765759
alias: 'b',
766760
desc: 'the command to use to open the browser, using %u as a placeholder for the path of the file to open',
767761
type: 'string',
768-
default: browserDefault,
762+
default: helpers.browserForPlatform(),
769763
})
770764
)
771765
.command('doctor', 'Check your set-up for potential problems')
772-
.version(version)
766+
.version(helpers.cliVersion())
773767
.demandCommand(1, '')
774768
.recommendCommands()
775769
.help()

packages/aws-cdk/lib/util/yargs-helpers.ts

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
import * as version from '../../lib/version';
2+
13
/**
24
* yargs middleware to negate an option if a negative alias is provided
35
* E.g. `-R` will imply `--rollback=false`
46
*
57
* @param optionToNegate The name of the option to negate, e.g. `rollback`
68
* @param negativeAlias The alias that should negate the option, e.g. `R`
7-
* @returns
9+
* @returns a middleware function that can be passed to yargs
810
*/
911
export function yargsNegativeAlias<T extends { [x in S | L]: boolean | undefined }, S extends string, L extends string>(
1012
negativeAlias: S,
@@ -19,3 +21,35 @@ export function yargsNegativeAlias<T extends { [x in S | L]: boolean | undefined
1921
return argv;
2022
};
2123
}
24+
25+
/**
26+
* Returns true if the current process is running in a CI environment
27+
* @returns true if the current process is running in a CI environment
28+
*/
29+
export function isCI(): boolean {
30+
return process.env.CI !== undefined;
31+
}
32+
33+
/**
34+
* Returns the current version of the CLI
35+
* @returns the current version of the CLI
36+
*/
37+
export function cliVersion(): string {
38+
return version.DISPLAY_VERSION;
39+
}
40+
41+
/**
42+
* Returns the default browser command for the current platform
43+
* @returns the default browser command for the current platform
44+
*/
45+
export function browserForPlatform(): string {
46+
switch (process.platform) {
47+
case 'darwin':
48+
return 'open %u';
49+
case 'win32':
50+
return 'start %u';
51+
default:
52+
return 'xdg-open %u';
53+
}
54+
}
55+

packages/aws-cdk/scripts/yargs-gen.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import * as fs from 'fs';
22
// eslint-disable-next-line import/no-extraneous-dependencies
33
import { renderYargs } from '@aws-cdk/yargs-gen';
4-
import { makeConfig } from '../lib/config';
4+
import { makeConfig, YARGS_HELPERS } from '../lib/config';
55

66
async function main() {
7-
fs.writeFileSync('./lib/parse-command-line-arguments.ts', await renderYargs(makeConfig()));
7+
fs.writeFileSync('./lib/parse-command-line-arguments.ts', await renderYargs(await makeConfig(), YARGS_HELPERS));
88
}
99

1010
main().then(() => {

0 commit comments

Comments
 (0)