Skip to content

Commit 6250b39

Browse files
kumsmritmrgrain
andauthored
fix(cli): retain type of context values and not convert them to string (#595)
Fixes [#30583](aws/aws-cdk#30583) CLI’s context-passing mechanism relied on serialising every entry to a "KEY=VALUE" string, leading CLI to only allows passing string context, so even JSON booleans (true/false) and numbers became plain strings. This resulted in losing the typed context, like boolean feature-flags are always being passed as string "true"/"false". This incorrectly passed feature flags caused certain integration tests snapshot in the [aws-cdk](https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk-testing/framework-integ) library to be updated with incorrect values causing the integration tests to fail. This fix retains the type of context values when passed via CLI, by correctly JSON parsing the context while building user configuration. Values that aren’t valid JSON will fall back to plain strings, so existing workflows should continue working as is. This will restore correct typing for feature flags without disrupting existing tests. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Co-authored-by: Momo Kornher <[email protected]>
1 parent bdf1f66 commit 6250b39

File tree

3 files changed

+31
-3
lines changed

3 files changed

+31
-3
lines changed

packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,8 @@ class LambdaStack extends cdk.Stack {
453453
constructor(parent, id, props) {
454454
// sometimes we need to specify the custom bootstrap bucket to use
455455
// see the 'upgrade legacy bootstrap stack' test
456-
const synthesizer = parent.node.tryGetContext('legacySynth') === 'true' ?
456+
const useLegacy = [true, 'true'].includes(parent.node.tryGetContext('legacySynth'));
457+
const synthesizer = useLegacy ?
457458
new LegacyStackSynthesizer({
458459
fileAssetsBucketName: parent.node.tryGetContext('bootstrapBucket'),
459460
})
@@ -477,7 +478,8 @@ class LambdaStack extends cdk.Stack {
477478

478479
class DriftableStack extends cdk.Stack {
479480
constructor(parent, id, props) {
480-
const synthesizer = parent.node.tryGetContext('legacySynth') === 'true' ?
481+
const useLegacy = [true, 'true'].includes(parent.node.tryGetContext('legacySynth'));
482+
const synthesizer = useLegacy ?
481483
new LegacyStackSynthesizer({
482484
fileAssetsBucketName: parent.node.tryGetContext('bootstrapBucket'),
483485
})

packages/aws-cdk/lib/cli/user-configuration.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,13 @@ function parseStringContextListToObject(argv: Arguments): any {
341341
`User-provided context cannot use keys prefixed with 'aws:', but ${parts[0]} was provided.`,
342342
);
343343
}
344-
context[parts[0]] = parts[1];
344+
let parsedValue: any = parts[1];
345+
try {
346+
parsedValue = JSON.parse(parts[1]);
347+
} catch {
348+
debug('Non-JSON context value for %s, leaving as string: %s', parts[0], parts[1]);
349+
}
350+
context[parts[0]] = parsedValue;
345351
} else {
346352
warning(
347353
'Context argument is not an assignment (key=value): %s',

packages/aws-cdk/test/cli/configuration.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,23 @@ test('providing a build arg', () => {
116116
// THEN
117117
expect(settings.get(['build'])).toEqual('mvn package');
118118
});
119+
120+
test('can parse boolean context from command line arguments', () => {
121+
// GIVEN
122+
const settings1 = commandLineArgumentsToSettings({ context: ['foo=true'], _: [Command.DEPLOY] });
123+
const settings2 = commandLineArgumentsToSettings({ context: ['foo=false'], _: [Command.DEPLOY] });
124+
125+
// THEN
126+
expect(settings1.get(['context']).foo).toEqual( true );
127+
expect(settings2.get(['context']).foo).toEqual( false );
128+
});
129+
130+
test('can parse number context from command line arguments', () => {
131+
// GIVEN
132+
const settings1 = commandLineArgumentsToSettings({ context: ['foo=5'], _: [Command.DEPLOY] });
133+
const settings2 = commandLineArgumentsToSettings({ context: ['foo=3.14'], _: [Command.DEPLOY] });
134+
135+
// THEN
136+
expect(settings1.get(['context']).foo).toEqual( 5 );
137+
expect(settings2.get(['context']).foo).toEqual( 3.14 );
138+
});

0 commit comments

Comments
 (0)