Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Duplicate concurrency argument causes the CLI to silently exit with code 0 after the synth phase #30752

Open
cmaster11 opened this issue Jul 4, 2024 · 1 comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI

Comments

@cmaster11
Copy link

cmaster11 commented Jul 4, 2024

Describe the bug

Running the cdk deploy command and passing multiple instances of the --concurrency argument causes the CLI to halt after the synth phase and exit with code 0 and no output.

This causes deployments to silently fail. Nothing happens after the synth phase, which means the CDK does not communicate with CloudFormation, and stacks are not deployed. When used in a CI/CD context, this can cause transparent failures: exit code 0, the CI job is marked as successful, but no software has been deployed.

I fully understand this problem can be avoided by not using the same parameter multiple times, but the lack of validation can cause unexpected behaviors.

Expected Behavior

Either:

  1. The parsed args would "ignore" the first entry of the argument and use the second entry as the final value.
  2. The CLI would throw an error because of an invalid argument type.

Current Behavior

The CLI exits with code 0 and no error output straight after the synth phase.

Reproduction Steps

On any CDK project I've tried, run:

npm run cdk -- deploy --concurrency 1 --concurrency 10  --all

The last line of the output is:

✨  Synthesis time: 14.7s

The exit code is 0, and nothing else happens. With verbose mode enabled, I inspected the parsed arguments, and I can see the concurrency arg is turned into an array (this is the default behavior of the yargs library):

 concurrency: [ 1, 10 ],

Possible Solution

Validation

The yargs library lacks proper validation for the arguments (from what I've seen), but this issue could arise with an unpredictable amount of other CLI arguments. I feel like there should be a degree of validation of the arguments directly in the AWS CDK CLI.

It could also be a per-command Joi validation done on the incoming command arguments (e.g. put the validation of a Joi schema here).

This specific issue could be solved by disabling the duplicate-arguments-array config option, but this would also disable the ability to specify the same parameter multiple times to populate an array, which does not sound like the right way to go.

Another lib?

Have you considered using commander.js? It has a similar approach to yargs but a more predictable behavior.

I ran a simple test with both libraries, and commander.js works as I'd normally expect an args-parsing library to do.

# node index.js --str "a string" --arr "array string 1"
### yargs
args: { str: 'a string', arr: [ 'array string 1' ] }
### commander
args: { str: 'a string', arr: [ 'array string 1' ] }

# node index.js --str "a string" --arr "array string 1" --str "another string" --arr "array string 2"
### yargs
args: {
  str: [ 'a string', 'another string' ],
  arr: [ 'array string 1', 'array string 2' ]
}
### commander
args: { 
	str: 'another string', 
	arr: [ 'array string 1', 'array string 2' ] 
}

Additional Information/Context

There are some open issues about the lack of validation in the yargs repo:

yargs/yargs#110
yargs/yargs#1079

CDK CLI Version

2.147.3 (build 32f0fdb)

Framework Version

No response

Node.js Version

Node.js v20.10.0

OS

MacOS 14.4.1 (23E224)

Language

TypeScript

Language Version

TypeScript 4.9.5

@cmaster11 cmaster11 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 4, 2024
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jul 4, 2024
@cmaster11 cmaster11 changed the title CLI: Duplicate concurrency argument causes the CLI to crash CLI: Duplicate concurrency argument causes the CLI to silently exit Jul 4, 2024
@cmaster11 cmaster11 changed the title CLI: Duplicate concurrency argument causes the CLI to silently exit CLI: Duplicate concurrency argument causes the CLI to silently exit with code 0 after synth phase Jul 4, 2024
@cmaster11 cmaster11 changed the title CLI: Duplicate concurrency argument causes the CLI to silently exit with code 0 after synth phase CLI: Duplicate concurrency argument causes the CLI to silently exit with code 0 after the synth phase Jul 4, 2024
@ashishdhingra ashishdhingra self-assigned this Jul 5, 2024
@ashishdhingra ashishdhingra added needs-reproduction This issue needs reproduction. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 5, 2024
@ashishdhingra
Copy link
Contributor

Reproducible. Perhaps throwing error in case of duplicate --concurrency is a better option since we should not blindly consume 1st or 2nd option. May be this validation should be expanded to other parameters accepting a single value (this could be as simple check if the argument is an array rather than single value).

@ashishdhingra ashishdhingra added p2 effort/small Small work item – less than a day of effort and removed needs-reproduction This issue needs reproduction. labels Jul 5, 2024
@ashishdhingra ashishdhingra removed their assignment Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

2 participants