cdk-cli: allow --no-previous-parameters to be configured in cdk.json alongside other config

[0xdevalias]

Describe the feature

When using CDK aspects like StringParameter.fromStringParameterAttributes / StringParameter.valueForStringParameter; by default, they use CloudFormation Stack Parameters; which are fixed at the point of first deploy, unless we deploy with cdk deploy --no-previous-parameters.

This is often confusing and unexpected, as outlined in this issue:

• SSM - parameter path and value changes are not updated in the stack. #7722

Use Case

I want to enforce --no-previous-parameters to always be used in my cdk.json, so that I can commit it into my project repo.

Proposed Solution

Allow --no-previous-parameters to be configured in cdk.json alongside existing config values.

Other Information

You can see some of my deepdive comments on the following issue:

• SSM - parameter path and value changes are not updated in the stack. #7722 (comment)
• SSM - parameter path and value changes are not updated in the stack. #7722 (comment)

Notably in that 2nd comment, it appears that some very recent changes (~2 weeks ago) made in CDK 2.87.0 would sort of allow the root issue to be worked around in a different way if I made use of forceDynamicReference:

Edit 3: Looking closer at the CDK docs for StringParameter.fromStringParameterAttributes(scope, id, attrs), and particularly StringParameterAttributes, there appears to be a forceDynamicReference option:

• docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ssm.StringParameter.html#static-fromwbrstringwbrparameterwbrattributesscope-id-attrs

• StringParameterAttributes: docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ssm.StringParameterAttributes.html#forcedynamicreference

  • Use a dynamic reference as the representation in CloudFormation template level.
    By default, CDK tries to deduce an appropriate representation based on the parameter value (a CfnParameter or a dynamic reference). Use this flag to override the representation when it does not work.

Ha.. so apparently forceDynamicReference and the underlying functionality related to it is brand new as of CDK 2.87.0 (released ~2 weeks ago):

• https://github.com/aws/aws-cdk/releases/tag/v2.87.0

  • ssm: cannot import a ssm parameter with a name containing unresolved token (1f1b642)

    • fix(ssm): cannot import a ssm parameter with a name containing unresolved token #25749
    • closes (ssm): can not import parameter name from another stack #17094

Previously, when we import a SSM parameter by ssm.StringParameter.fromStringParameterAttributes, we use CfnParameter to get the value.

  "Parameters": {
    "importsqsstringparamParameter": {
      "Type": "AWS::SSM::Parameter::Value<String>",
      "Default": {
        "Fn::ImportValue": "some-exported-value-holding-the-param-name"
      }
    },

However, Parameters.<Name>.Default only allows a concrete string value. If it contains e.g. intrinsic functions, we get an error like this from CFn: Template format error: Every Default member must be a string.

This PR changes the behavior of fromStringParameterAttributes method. Now it uses CfnDynamicReference instead of CfnParameter if a parameter name contains unresolved tokens.

Originally posted by @tmokmss in #25749

Another thing we can say about ssm parameters is that it doesn't differ much between CfnParameters and dynamic references. Reading through the document, it seems that most of the characteristics are the same, such as when it's resolved and updated, where it can be used in a template, etc. There are of course some differences e.g. max num of references (200 vs 60), but they seems trivial.

Originally posted by @tmokmss in #25749 (comment)

I'm now wondering whether switching a parameter to a dynamic reference should really be considered as a breaking change. As far as I read the docs, there seems to be no remarkable difference between them. Given it's also very rare to use a lazy token for parameter names, we can tolerate the change, maybe under a feature flag.

Originally posted by @tmokmss in #25749 (comment)

If only we could stop using CfnParameter and use CfnDynamicReference instead for all cases... I see no point to use CfnParameter here. (Actually, isn't that the purpose of feature flags?)

Originally posted by @tmokmss in #25749 (comment)

There are some limitations #22239 (comment)

Originally posted by @corymhall in #25749 (comment)

So how about letting users choose which they use, parameter or dynamic reference? We'll add a property like forceDynamicReference?: boolean (default to false) to CommonStringParameterAttributes. This is kind of a leaky abstraction, but it should at least solve all the problem above. Plus we can easily ensure there is no breaking change, without adding any feature flag.

Originally posted by @tmokmss in #25749 (comment)

Originally posted by @0xdevalias in #7722 (comment)

---

Then, on an internal issue, I noted the following while researching whether it was possible to configure this in cdk.json:

Avoiding this issue coming up again in future

Since I don't believe we really make use of CDK / CloudFormation parameters directly ourselves (and it's apparently not really a recommended thing to do either), I wonder if there would be any harm in us forcing CDK to always update them so that we avoid this issue in future?

Looking in AWS web console -> CloudFormation, for each of our stacks:

• REDACTED: ✅ Seems to only use the BootstrapVersion parameter + SSM parameters
• REDACTED: ✅ Seems to only use the BootstrapVersion parameter + SSM parameters
• REDACTED: ✅ Seems to only use the BootstrapVersion parameter
• REDACTED: ✅ Seems to only use the BootstrapVersion parameter
• REDACTED: ✅ Seems to only use the BootstrapVersion parameter

✅ Based on the above, it looks like it should be safe to always deploy our stacks with --no-previous-parameters!

---

Looking at the cdk deploy --help, here are the relevant parts that mention parameters:

⇒ yarn cdk-dev deploy --help
yarn run v1.22.19
$ yarn -s awsvault-dev:nosession cdk deploy --help
cdk deploy [STACKS..]

Deploys the stack(s) named STACKS into your AWS account

Options:
  ..snip..

  -c, --context              Add contextual string parameter (KEY=VALUE) [array]

  ..snip..

      --parameters           Additional parameters passed to CloudFormation at
                             deploy time (STACK:KEY=VALUE) [array] [default: {}]
  ..snip..

      --previous-parameters  Use previous values for existing parameters (you
                             must specify all parameters on every deployment if
                             this is disabled)         [boolean] [default: true]
  ..snip..

✨  Done in 6.61s.

--previous-parameters defaults to true, which is why we have to use --no-previous-parameters to disable it.

It seems that at least some of the CDK CLI flags can be configured in the project's cdk.json file:

• https://docs.aws.amazon.com/cdk/v2/guide/cli.html#cli-config

Unfortunately this doesn't seem to include the previous-parameters / no-previous-parameters option.

We can confirm that by looking at CDK's Configuration class:

aws-cdk/packages/aws-cdk/lib/settings.ts

Lines 68 to 158 in 6c75581

	/**
	* All sources of settings combined
	*/
	export class Configuration {
	public settings = new Settings();
	public context = new Context();
	public readonly defaultConfig = new Settings({
	versionReporting: true,
	assetMetadata: true,
	pathMetadata: true,
	output: 'cdk.out',
	});
	private readonly commandLineArguments: Settings;
	private readonly commandLineContext: Settings;
	private _projectConfig?: Settings;
	private _projectContext?: Settings;
	private loaded = false;
	constructor(private readonly props: ConfigurationProps = {}) {
	this.commandLineArguments = props.commandLineArguments
	? Settings.fromCommandLineArguments(props.commandLineArguments)
	: new Settings();
	this.commandLineContext = this.commandLineArguments.subSettings([CONTEXT_KEY]).makeReadOnly();
	}
	private get projectConfig() {
	if (!this._projectConfig) {
	throw new Error('#load has not been called yet!');
	}
	return this._projectConfig;
	}
	private get projectContext() {
	if (!this._projectContext) {
	throw new Error('#load has not been called yet!');
	}
	return this._projectContext;
	}
	/**
	* Load all config
	*/
	public async load(): Promise<this> {
	const userConfig = await loadAndLog(USER_DEFAULTS);
	this._projectConfig = await loadAndLog(PROJECT_CONFIG);
	this._projectContext = await loadAndLog(PROJECT_CONTEXT);
	const readUserContext = this.props.readUserContext ?? true;
	if (userConfig.get(['build'])) {
	throw new Error('The `build` key cannot be specified in the user config (~/.cdk.json), specify it in the project config (cdk.json) instead');
	}
	const contextSources = [
	this.commandLineContext,
	this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(),
	this.projectContext,
	];
	if (readUserContext) {
	contextSources.push(userConfig.subSettings([CONTEXT_KEY]).makeReadOnly());
	}
	this.context = new Context(...contextSources);
	// Build settings from what's left
	this.settings = this.defaultConfig
	.merge(userConfig)
	.merge(this.projectConfig)
	.merge(this.commandLineArguments)
	.makeReadOnly();
	debug('merged settings:', this.settings.all);
	this.loaded = true;
	return this;
	}
	/**
	* Save the project context
	*/
	public async saveContext(): Promise<this> {
	if (!this.loaded) { return this; } // Avoid overwriting files with nothing
	await this.projectContext.save(PROJECT_CONTEXT);
	return this;
	}
	}

and Settings class:

aws-cdk/packages/aws-cdk/lib/settings.ts

Lines 232 to 445 in 6c75581

	/**
	* A single bag of settings
	*/
	export class Settings {
	/**
	* Parse Settings out of CLI arguments.
	*
	* CLI arguments in must be accessed in the CLI code via
	* `configuration.settings.get(['argName'])` instead of via `args.argName`.
	*
	* The advantage is that they can be configured via `cdk.json` and
	* `$HOME/.cdk.json`. Arguments not listed below and accessed via this object
	* can only be specified on the command line.
	*
	* @param argv the received CLI arguments.
	* @returns a new Settings object.
	*/
	public static fromCommandLineArguments(argv: Arguments): Settings {
	const context = this.parseStringContextListToObject(argv);
	const tags = this.parseStringTagsListToObject(expectStringList(argv.tags));
	// Determine bundling stacks
	let bundlingStacks: string[];
	if (BUNDLING_COMMANDS.includes(argv._[0])) {
	// If we deploy, diff, synth or watch a list of stacks exclusively we skip
	// bundling for all other stacks.
	bundlingStacks = argv.exclusively
	? argv.STACKS ?? ['**']
	: ['**'];
	} else { // Skip bundling for all stacks
	bundlingStacks = [];
	}
	return new Settings({
	app: argv.app,
	browser: argv.browser,
	build: argv.build,
	context,
	debug: argv.debug,
	tags,
	language: argv.language,
	pathMetadata: argv.pathMetadata,
	assetMetadata: argv.assetMetadata,
	profile: argv.profile,
	plugin: argv.plugin,
	requireApproval: argv.requireApproval,
	toolkitStackName: argv.toolkitStackName,
	toolkitBucket: {
	bucketName: argv.bootstrapBucketName,
	kmsKeyId: argv.bootstrapKmsKeyId,
	},
	versionReporting: argv.versionReporting,
	staging: argv.staging,
	output: argv.output,
	outputsFile: argv.outputsFile,
	progress: argv.progress,
	bundlingStacks,
	lookups: argv.lookups,
	rollback: argv.rollback,
	notices: argv.notices,
	assetParallelism: argv['asset-parallelism'],
	assetPrebuild: argv['asset-prebuild'],
	});
	}
	public static mergeAll(...settings: Settings[]): Settings {
	let ret = new Settings();
	for (const setting of settings) {
	ret = ret.merge(setting);
	}
	return ret;
	}
	private static parseStringContextListToObject(argv: Arguments): any {
	const context: any = {};
	for (const assignment of ((argv as any).context || [])) {
	const parts = assignment.split(/=(.*)/, 2);
	if (parts.length === 2) {
	debug('CLI argument context: %s=%s', parts[0], parts[1]);
	if (parts[0].match(/^aws:.+/)) {
	throw new Error(`User-provided context cannot use keys prefixed with 'aws:', but ${parts[0]} was provided.`);
	}
	context[parts[0]] = parts[1];
	} else {
	warning('Context argument is not an assignment (key=value): %s', assignment);
	}
	}
	return context;
	}
	/**
	* Parse tags out of arguments
	*
	* Return undefined if no tags were provided, return an empty array if only empty
	* strings were provided
	*/
	private static parseStringTagsListToObject(argTags: string[] | undefined): Tag[] | undefined {
	if (argTags === undefined) { return undefined; }
	if (argTags.length === 0) { return undefined; }
	const nonEmptyTags = argTags.filter(t => t !== '');
	if (nonEmptyTags.length === 0) { return []; }
	const tags: Tag[] = [];
	for (const assignment of nonEmptyTags) {
	const parts = assignment.split('=', 2);
	if (parts.length === 2) {
	debug('CLI argument tags: %s=%s', parts[0], parts[1]);
	tags.push({
	Key: parts[0],
	Value: parts[1],
	});
	} else {
	warning('Tags argument is not an assignment (key=value): %s', assignment);
	}
	}
	return tags.length > 0 ? tags : undefined;
	}
	constructor(private settings: SettingsMap = {}, public readonly readOnly = false) {}
	public async load(fileName: string): Promise<this> {
	if (this.readOnly) {
	throw new Error(`Can't load ${fileName}: settings object is readonly`);
	}
	this.settings = {};
	const expanded = expandHomeDir(fileName);
	if (await fs.pathExists(expanded)) {
	this.settings = await fs.readJson(expanded);
	}
	// See https://github.com/aws/aws-cdk/issues/59
	this.prohibitContextKey('default-account', fileName);
	this.prohibitContextKey('default-region', fileName);
	this.warnAboutContextKey('aws:', fileName);
	return this;
	}
	public async save(fileName: string): Promise<this> {
	const expanded = expandHomeDir(fileName);
	await fs.writeJson(expanded, stripTransientValues(this.settings), { spaces: 2 });
	return this;
	}
	public get all(): any {
	return this.get([]);
	}
	public merge(other: Settings): Settings {
	return new Settings(util.deepMerge(this.settings, other.settings));
	}
	public subSettings(keyPrefix: string[]) {
	return new Settings(this.get(keyPrefix) || {}, false);
	}
	public makeReadOnly(): Settings {
	return new Settings(this.settings, true);
	}
	public clear() {
	if (this.readOnly) {
	throw new Error('Cannot clear(): settings are readonly');
	}
	this.settings = {};
	}
	public get empty(): boolean {
	return Object.keys(this.settings).length === 0;
	}
	public get(path: string[]): any {
	return util.deepClone(util.deepGet(this.settings, path));
	}
	public set(path: string[], value: any): Settings {
	if (this.readOnly) {
	throw new Error(`Can't set ${path}: settings object is readonly`);
	}
	if (path.length === 0) {
	// deepSet can't handle this case
	this.settings = value;
	} else {
	util.deepSet(this.settings, path, value);
	}
	return this;
	}
	public unset(path: string[]) {
	this.set(path, undefined);
	}
	private prohibitContextKey(key: string, fileName: string) {
	if (!this.settings.context) { return; }
	if (key in this.settings.context) {
	// eslint-disable-next-line max-len
	throw new Error(`The 'context.${key}' key was found in ${fs_path.resolve(fileName)}, but it is no longer supported. Please remove it.`);
	}
	}
	private warnAboutContextKey(prefix: string, fileName: string) {
	if (!this.settings.context) { return; }
	for (const contextKey of Object.keys(this.settings.context)) {
	if (contextKey.startsWith(prefix)) {
	// eslint-disable-next-line max-len
	warning(`A reserved context key ('context.${prefix}') key was found in ${fs_path.resolve(fileName)}, it might cause surprising behavior and should be removed.`);
	}
	}
	}
	}

Neither of which seem to contain entries for previous-parameters / previousParameters, which only seem to be meaningfully used by:

The cdk bootstrap command:

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

Line 510 in 6c75581

usePreviousParameters: args['previous-parameters'],

The cdk deploy command:

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

Line 569 in 6c75581

usePreviousParameters: args['previous-parameters'],

And a commented out reference in the cdk watch command:

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

Line 603 in 6c75581

// usePreviousParameters: args['previous-parameters'],

---

Based on the above, it seems like our simplest/best option right now might be to just hardcode --no-previous-parameters into our deploy helper script in package.json or similar (⚠️ TODO: and probably also raise an upstream issue on the AWS CDK repository to explore whether this is something that can be added to the CDK)

Originally posted by @0xdevalias in https://github.com/PsychNEXUS/REMDR/issues/278#issuecomment-1637500082

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

2.84.0 (build f7c792f)

Environment details (OS name and version, etc.)

macOS / N/A

[indrora]

Thanks. This is something that occasionally bites people, you're right. This is why we have --no-previous-paramters, but there should be a way to specify a workaround per-parameter at least.

[0xdevalias]

but there should be a way to specify a workaround per-parameter at least.

A workaround per parameter would be useful for sure; but also a way to blanket designate a workaround at a project level as well. Like, for my use case, there is literally no time that I don't want the re-deploy to use the latest version of the parameter in the SSM parameter store. That's the whole reason I put the config in there in the first place.

So what this really ends up feeling like is having to hack/work around a problem introduced by an implementation choice the CDK made, to achieve what I suspect would actually be the default expectation of most users using things from SSM parameter store.

I don't expect the default would be changed since it would be a pretty breaking change. Though perhaps a better solution even than allowing --no-previous-parameters to be configured in cdk.json would be to update all functions within StringParameter/etc to have an equivalent of forceDynamicReference; and ideally a feature flag that let's us just switch that on as the default case across our project as a whole.