fix(cli): parameter value reuse is not configurable

The automatic parameter reuse which was introduced in #7041 has been causing issues for some people who use Default Values to classify between CI/CD deployments and CLI deployments (value supplied=CI/CD, value default=CLI). Introduce a switch to turn it off.
aws · May 6, 2020 · 44310c9 · 44310c9
1 parent 63d8cfe
commit 44310c9
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 2 deletions.
diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts
@@ -69,7 +69,8 @@ async function parseCommandLineArguments() {
       .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true })
       .option('force', { alias: 'f', type: 'boolean', desc: 'Always deploy stack even if templates are identical', default: false })
       .option('parameters', { type: 'array', desc: 'Additional parameters passed to CloudFormation at deploy time (STACK:KEY=VALUE)', nargs: 1, requiresArg: true, default: {} })
-      .option('outputs-file', { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true }),
+      .option('outputs-file', { type: 'string', alias: 'O', desc: 'Path to file where stack outputs will be written as JSON', requiresArg: true })
+      .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)' }),
     )
     .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs
       .option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only destroy requested stacks, don\'t include dependees' })
@@ -243,6 +244,7 @@ async function initCommandLine() {
           execute: args.execute,
           force: args.force,
           parameters: parameterMap,
+          usePreviousParameters: args['previous-parameters'],
           outputsFile: args.outputsFile,
         });
 

diff --git a/packages/aws-cdk/lib/api/cloudformation-deployments.ts b/packages/aws-cdk/lib/api/cloudformation-deployments.ts
@@ -77,6 +77,15 @@ export interface DeployStackOptions {
    * @default - no additional parameters will be passed to the template
    */
   parameters?: { [name: string]: string | undefined };
+
+  /**
+   * Use previous values for unspecified parameters
+   *
+   * If not set, all parameters must be specified for every deployment.
+   *
+   * @default true
+   */
+  usePreviousParameters?: boolean;
 }
 
 export interface DestroyStackOptions {
@@ -138,6 +147,7 @@ export class CloudFormationDeployments {
       execute: options.execute,
       force: options.force,
       parameters: options.parameters,
+      usePreviousParameters: options.usePreviousParameters,
     });
   }
 

diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -130,6 +130,15 @@ export interface DeployStackOptions {
    */
   parameters?: { [name: string]: string | undefined };
 
+  /**
+   * Use previous values for unspecified parameters
+   *
+   * If not set, all parameters must be specified for every deployment.
+   *
+   * @default true
+   */
+  usePreviousParameters?: boolean;
+
   /**
    * Deploy even if the deployed template is identical to the one we are about to deploy.
    * @default false
@@ -170,7 +179,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
   const apiParameters = TemplateParameters.fromTemplate(stackArtifact.template).makeApiParameters({
     ...options.parameters,
     ...assetParams,
-  }, cloudFormationStack.parameterNames);
+  }, options.usePreviousParameters ? cloudFormationStack.parameterNames : []);
 
   const executionId = uuid.v4();
   const bodyParameter = await makeBodyParameter(stackArtifact, options.resolvedEnvironment, legacyAssets, options.toolkitInfo);

diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -189,6 +189,7 @@ export class CdkToolkit {
           execute: options.execute,
           force: options.force,
           parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
+          usePreviousParameters: options.usePreviousParameters,
         });
 
         const message = result.noOp
@@ -563,6 +564,15 @@ export interface DeployOptions {
    */
   parameters?: { [name: string]: string | undefined };
 
+  /**
+   * Use previous values for unspecified parameters
+   *
+   * If not set, all parameters must be specified for every deployment.
+   *
+   * @default true
+   */
+  usePreviousParameters?: boolean;
+
   /**
    * Path to file where stack outputs will be written after a successful deploy as JSON
    * @default - Outputs are not written to any file

diff --git a/packages/aws-cdk/test/api/deploy-stack.test.ts b/packages/aws-cdk/test/api/deploy-stack.test.ts
@@ -9,6 +9,17 @@ const FAKE_STACK = testStack({
   template: FAKE_TEMPLATE,
 });
 
+const FAKE_STACK_WITH_PARAMETERS = testStack({
+  stackName: 'withparameters',
+  template: {
+    Parameters: {
+      HasValue: { Type: 'String' },
+      HasDefault: { Type: 'String', Default: 'TheDefault' },
+      OtherParameter: { Type: 'String' },
+    },
+  },
+});
+
 const FAKE_STACK_TERMINATION_PROTECTION = testStack({
   stackName: 'termination-protection',
   template: FAKE_TEMPLATE,
@@ -84,6 +95,88 @@ test('correctly passes CFN parameters, ignoring ones with empty values', async (
   }));
 });
 
+test('reuse previous parameters if requested', async () => {
+  // GIVEN
+  givenStackExists({
+    Parameters: [
+      { ParameterKey: 'HasValue', ParameterValue: 'TheValue' },
+      { ParameterKey: 'HasDefault', ParameterValue: 'TheOldValue' },
+    ],
+  });
+
+  // WHEN
+  await deployStack({
+    stack: FAKE_STACK_WITH_PARAMETERS,
+    sdk,
+    sdkProvider,
+    resolvedEnvironment: mockResolvedEnvironment(),
+    parameters: {
+      OtherParameter: 'SomeValue',
+    },
+    usePreviousParameters: true,
+  });
+
+  // THEN
+  expect(cfnMocks.createChangeSet).toHaveBeenCalledWith(expect.objectContaining({
+    Parameters: [
+      { ParameterKey: 'HasValue', UsePreviousValue: true },
+      { ParameterKey: 'HasDefault', UsePreviousValue: true },
+      { ParameterKey: 'OtherParameter', ParameterValue: 'SomeValue' },
+    ],
+  }));
+});
+
+test('do not reuse previous parameters if not requested', async () => {
+  // GIVEN
+  givenStackExists({
+    Parameters: [
+      { ParameterKey: 'HasValue', ParameterValue: 'TheValue' },
+      { ParameterKey: 'HasDefault', ParameterValue: 'TheOldValue' },
+    ],
+  });
+
+  // WHEN
+  await deployStack({
+    stack: FAKE_STACK_WITH_PARAMETERS,
+    sdk,
+    sdkProvider,
+    resolvedEnvironment: mockResolvedEnvironment(),
+    parameters: {
+      HasValue: 'SomeValue',
+      OtherParameter: 'SomeValue',
+    },
+  });
+
+  // THEN
+  expect(cfnMocks.createChangeSet).toHaveBeenCalledWith(expect.objectContaining({
+    Parameters: [
+      { ParameterKey: 'HasValue', ParameterValue: 'SomeValue' },
+      { ParameterKey: 'OtherParameter', ParameterValue: 'SomeValue' },
+    ],
+  }));
+});
+
+test('throw exception if not enough parameters supplied', async () => {
+  // GIVEN
+  givenStackExists({
+    Parameters: [
+      { ParameterKey: 'HasValue', ParameterValue: 'TheValue' },
+      { ParameterKey: 'HasDefault', ParameterValue: 'TheOldValue' },
+    ],
+  });
+
+  // WHEN
+  await expect(deployStack({
+    stack: FAKE_STACK_WITH_PARAMETERS,
+    sdk,
+    sdkProvider,
+    resolvedEnvironment: mockResolvedEnvironment(),
+    parameters: {
+      OtherParameter: 'SomeValue',
+    },
+  })).rejects.toThrow(/CloudFormation Parameters are missing a value/);
+});
+
 test('deploy is skipped if template did not change', async () => {
   // GIVEN
   givenStackExists();