fix(cloudformation-include): drops unknown policy attributes (#32321)

Unknown fields in `CreationPolicy`/`UpdatePolicy` were dropped while loading resources using `CfnInclude`.

In this PR, handle unknown fields in those places like unknown fields in other places: retain them during parsing and add them as overrides.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

packages/aws-cdk-lib/assertions/lib/matcher.ts

@@ -182,7 +182,6 @@ export class MatchResult {
    */
   public toHumanStrings(): string[] {
     const failures = new Array<MatchFailure>();
-    debugger;
     recurse(this, []);
 
     return failures.map(r => {

packages/aws-cdk-lib/cloudformation-include/test/test-templates/non-existent-policy-attribute.json

@@ -0,0 +1,17 @@
+{
+  "Resources": {
+    "ASG": {
+      "Type": "AWS::AutoScaling::AutoScalingGroup",
+      "Properties": {
+        "MinSize": "1",
+        "MaxSize": "5"
+      },
+      "CreationPolicy": {
+        "NonExistentResourceAttribute": "Bucket1"
+      },
+      "UpdatePolicy": {
+        "NonExistentResourceAttribute": "Bucket1"
+      }
+    }
+  }
+}

packages/aws-cdk-lib/cloudformation-include/test/valid-templates.test.ts

@@ -707,6 +707,13 @@ describe('CDK Include', () => {
     );
   });
 
+  test('preserves unknown policy attributes', () => {
+    const cfnTemplate = includeTestTemplate(stack, 'non-existent-policy-attribute.json');
+    Template.fromStack(stack).templateMatches(
+      loadTestFileToJsObject('non-existent-policy-attribute.json'),
+    );
+  });
+
   test("correctly handles referencing the ingested template's resources across Stacks", () => {
     // for cross-stack sharing to work, we need an App
     const app = new core.App();

packages/aws-cdk-lib/core/lib/helpers-internal/cfn-parse.ts

@@ -356,14 +356,23 @@ export class CfnParser {
     const cfnOptions = resource.cfnOptions;
     this.stack = Stack.of(resource);
 
-    cfnOptions.creationPolicy = this.parseCreationPolicy(resourceAttributes.CreationPolicy, logicalId);
-    cfnOptions.updatePolicy = this.parseUpdatePolicy(resourceAttributes.UpdatePolicy, logicalId);
+    const creationPolicy = this.parseCreationPolicy(resourceAttributes.CreationPolicy, logicalId);
+    const updatePolicy = this.parseUpdatePolicy(resourceAttributes.UpdatePolicy, logicalId);
+    cfnOptions.creationPolicy = creationPolicy.value;
+    cfnOptions.updatePolicy = updatePolicy.value;
     cfnOptions.deletionPolicy = this.parseDeletionPolicy(resourceAttributes.DeletionPolicy);
     cfnOptions.updateReplacePolicy = this.parseDeletionPolicy(resourceAttributes.UpdateReplacePolicy);
     cfnOptions.version = this.parseValue(resourceAttributes.Version);
     cfnOptions.description = this.parseValue(resourceAttributes.Description);
     cfnOptions.metadata = this.parseValue(resourceAttributes.Metadata);
 
+    for (const [key, value] of Object.entries(creationPolicy.extraProperties)) {
+      resource.addOverride(`CreationPolicy.${key}`, value);
+    }
+    for (const [key, value] of Object.entries(updatePolicy.extraProperties)) {
+      resource.addOverride(`UpdatePolicy.${key}`, value);
+    }
+
     // handle Condition
     if (resourceAttributes.Condition) {
       const condition = this.finder.findCondition(resourceAttributes.Condition);
@@ -386,98 +395,93 @@ export class CfnParser {
     }
   }
 
-  private parseCreationPolicy(policy: any, logicalId: string): CfnCreationPolicy | undefined {
-    if (typeof policy !== 'object') { return undefined; }
+  private parseCreationPolicy(policy: any, logicalId: string): FromCloudFormationResult<CfnCreationPolicy | undefined> {
+    if (typeof policy !== 'object') { return new FromCloudFormationResult(undefined); }
     this.throwIfIsIntrinsic(policy, logicalId);
     const self = this;
 
-    // change simple JS values to their CDK equivalents
-    policy = this.parseValue(policy);
-
-    return undefinedIfAllValuesAreEmpty({
-      autoScalingCreationPolicy: parseAutoScalingCreationPolicy(policy.AutoScalingCreationPolicy),
-      resourceSignal: parseResourceSignal(policy.ResourceSignal),
-    });
+    const creaPol = new ObjectParser<CfnCreationPolicy>(this.parseValue(policy));
+    creaPol.parseCase('autoScalingCreationPolicy', parseAutoScalingCreationPolicy);
+    creaPol.parseCase('resourceSignal', parseResourceSignal);
+    return creaPol.toResult();
 
-    function parseAutoScalingCreationPolicy(p: any): CfnResourceAutoScalingCreationPolicy | undefined {
+    function parseAutoScalingCreationPolicy(p: any): FromCloudFormationResult<CfnResourceAutoScalingCreationPolicy | undefined> {
       self.throwIfIsIntrinsic(p, logicalId);
-      if (typeof p !== 'object') { return undefined; }
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
 
-      return undefinedIfAllValuesAreEmpty({
-        minSuccessfulInstancesPercent: FromCloudFormation.getNumber(p.MinSuccessfulInstancesPercent).value,
-      });
+      const autoPol = new ObjectParser<CfnResourceAutoScalingCreationPolicy>(p);
+      autoPol.parseCase('minSuccessfulInstancesPercent', FromCloudFormation.getNumber);
+      return autoPol.toResult();
     }
 
-    function parseResourceSignal(p: any): CfnResourceSignal | undefined {
-      if (typeof p !== 'object') { return undefined; }
+    function parseResourceSignal(p: any): FromCloudFormationResult<CfnResourceSignal | undefined> {
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
       self.throwIfIsIntrinsic(p, logicalId);
 
-      return undefinedIfAllValuesAreEmpty({
-        count: FromCloudFormation.getNumber(p.Count).value,
-        timeout: FromCloudFormation.getString(p.Timeout).value,
-      });
+      const sig = new ObjectParser<CfnResourceSignal>(p);
+      sig.parseCase('count', FromCloudFormation.getNumber);
+      sig.parseCase('timeout', FromCloudFormation.getString);
+      return sig.toResult();
     }
   }
 
-  private parseUpdatePolicy(policy: any, logicalId: string): CfnUpdatePolicy | undefined {
-    if (typeof policy !== 'object') { return undefined; }
+  private parseUpdatePolicy(policy: any, logicalId: string): FromCloudFormationResult<CfnUpdatePolicy | undefined> {
+    if (typeof policy !== 'object') { return new FromCloudFormationResult(undefined); }
     this.throwIfIsIntrinsic(policy, logicalId);
     const self = this;
 
     // change simple JS values to their CDK equivalents
-    policy = this.parseValue(policy);
-
-    return undefinedIfAllValuesAreEmpty({
-      autoScalingReplacingUpdate: parseAutoScalingReplacingUpdate(policy.AutoScalingReplacingUpdate),
-      autoScalingRollingUpdate: parseAutoScalingRollingUpdate(policy.AutoScalingRollingUpdate),
-      autoScalingScheduledAction: parseAutoScalingScheduledAction(policy.AutoScalingScheduledAction),
-      codeDeployLambdaAliasUpdate: parseCodeDeployLambdaAliasUpdate(policy.CodeDeployLambdaAliasUpdate),
-      enableVersionUpgrade: FromCloudFormation.getBoolean(policy.EnableVersionUpgrade).value,
-      useOnlineResharding: FromCloudFormation.getBoolean(policy.UseOnlineResharding).value,
-    });
-
-    function parseAutoScalingReplacingUpdate(p: any): CfnAutoScalingReplacingUpdate | undefined {
-      if (typeof p !== 'object') { return undefined; }
+    const uppol = new ObjectParser<CfnUpdatePolicy>(this.parseValue(policy));
+    uppol.parseCase('autoScalingReplacingUpdate', parseAutoScalingReplacingUpdate);
+    uppol.parseCase('autoScalingRollingUpdate', parseAutoScalingRollingUpdate);
+    uppol.parseCase('autoScalingScheduledAction', parseAutoScalingScheduledAction);
+    uppol.parseCase('codeDeployLambdaAliasUpdate', parseCodeDeployLambdaAliasUpdate);
+    uppol.parseCase('enableVersionUpgrade', (x) => FromCloudFormation.getBoolean(x) as any);
+    uppol.parseCase('useOnlineResharding', (x) => FromCloudFormation.getBoolean(x) as any);
+    return uppol.toResult();
+
+    function parseAutoScalingReplacingUpdate(p: any): FromCloudFormationResult<CfnAutoScalingReplacingUpdate | undefined> {
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
       self.throwIfIsIntrinsic(p, logicalId);
 
-      return undefinedIfAllValuesAreEmpty({
-        willReplace: p.WillReplace,
-      });
+      const repUp = new ObjectParser<CfnAutoScalingReplacingUpdate>(p);
+      repUp.parseCase('willReplace', (x) => x);
+      return repUp.toResult();
     }
 
-    function parseAutoScalingRollingUpdate(p: any): CfnAutoScalingRollingUpdate | undefined {
-      if (typeof p !== 'object') { return undefined; }
+    function parseAutoScalingRollingUpdate(p: any): FromCloudFormationResult<CfnAutoScalingRollingUpdate | undefined> {
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
       self.throwIfIsIntrinsic(p, logicalId);
 
-      return undefinedIfAllValuesAreEmpty({
-        maxBatchSize: FromCloudFormation.getNumber(p.MaxBatchSize).value,
-        minInstancesInService: FromCloudFormation.getNumber(p.MinInstancesInService).value,
-        minSuccessfulInstancesPercent: FromCloudFormation.getNumber(p.MinSuccessfulInstancesPercent).value,
-        pauseTime: FromCloudFormation.getString(p.PauseTime).value,
-        suspendProcesses: FromCloudFormation.getStringArray(p.SuspendProcesses).value,
-        waitOnResourceSignals: FromCloudFormation.getBoolean(p.WaitOnResourceSignals).value,
-      });
+      const rollUp = new ObjectParser<CfnAutoScalingRollingUpdate>(p);
+      rollUp.parseCase('maxBatchSize', FromCloudFormation.getNumber);
+      rollUp.parseCase('minInstancesInService', FromCloudFormation.getNumber);
+      rollUp.parseCase('minSuccessfulInstancesPercent', FromCloudFormation.getNumber);
+      rollUp.parseCase('pauseTime', FromCloudFormation.getString);
+      rollUp.parseCase('suspendProcesses', FromCloudFormation.getStringArray);
+      rollUp.parseCase('waitOnResourceSignals', FromCloudFormation.getBoolean);
+      return rollUp.toResult();
     }
 
-    function parseCodeDeployLambdaAliasUpdate(p: any): CfnCodeDeployLambdaAliasUpdate | undefined {
+    function parseCodeDeployLambdaAliasUpdate(p: any): FromCloudFormationResult<CfnCodeDeployLambdaAliasUpdate | undefined> {
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
       self.throwIfIsIntrinsic(p, logicalId);
-      if (typeof p !== 'object') { return undefined; }
 
-      return {
-        beforeAllowTrafficHook: FromCloudFormation.getString(p.BeforeAllowTrafficHook).value,
-        afterAllowTrafficHook: FromCloudFormation.getString(p.AfterAllowTrafficHook).value,
-        applicationName: FromCloudFormation.getString(p.ApplicationName).value,
-        deploymentGroupName: FromCloudFormation.getString(p.DeploymentGroupName).value,
-      };
+      const cdUp = new ObjectParser<CfnCodeDeployLambdaAliasUpdate>(p);
+      cdUp.parseCase('beforeAllowTrafficHook', FromCloudFormation.getString);
+      cdUp.parseCase('afterAllowTrafficHook', FromCloudFormation.getString);
+      cdUp.parseCase('applicationName', FromCloudFormation.getString);
+      cdUp.parseCase('deploymentGroupName', FromCloudFormation.getString);
+      return cdUp.toResult();
     }
 
-    function parseAutoScalingScheduledAction(p: any): CfnAutoScalingScheduledAction | undefined {
+    function parseAutoScalingScheduledAction(p: any): FromCloudFormationResult<CfnAutoScalingScheduledAction | undefined> {
+      if (typeof p !== 'object') { return new FromCloudFormationResult(undefined); }
       self.throwIfIsIntrinsic(p, logicalId);
-      if (typeof p !== 'object') { return undefined; }
 
-      return undefinedIfAllValuesAreEmpty({
-        ignoreUnmodifiedGroupSizeProperties: FromCloudFormation.getBoolean(p.IgnoreUnmodifiedGroupSizeProperties).value,
-      });
+      const schedUp = new ObjectParser<CfnAutoScalingScheduledAction>(p);
+      schedUp.parseCase('ignoreUnmodifiedGroupSizeProperties', FromCloudFormation.getBoolean);
+      return schedUp.toResult();
     }
   }
 
@@ -853,3 +857,70 @@ export class CfnParser {
     return this.options.parameters || {};
   }
 }
+
+class ObjectParser<T extends object> {
+  private readonly parsed: Record<string, unknown> = {};
+  private readonly unparsed: Record<string, unknown> = {};
+
+  constructor(input: Record<string, unknown>) {
+    this.unparsed = { ...input };
+  }
+
+  /**
+   * Parse a single field from the object into the target object
+   *
+   * The source key will be assumed to be the exact same as the
+   * target key, but with an uppercase first letter.
+   */
+  public parseCase<K extends keyof T>(targetKey: K, parser: (x: any) => T[K] | FromCloudFormationResult<T[K] | IResolvable>) {
+    const sourceKey = ucfirst(String(targetKey));
+    this.parse(targetKey, sourceKey, parser);
+  }
+
+  public parse<K extends keyof T>(targetKey: K, sourceKey: string, parser: (x: any) => T[K] | FromCloudFormationResult<T[K] | IResolvable>) {
+    if (!(sourceKey in this.unparsed)) {
+      return;
+    }
+
+    const value = parser(this.unparsed[sourceKey]);
+    delete this.unparsed[sourceKey];
+
+    if (value instanceof FromCloudFormationResult) {
+      for (const [k, v] of Object.entries(value.extraProperties ?? {})) {
+        this.unparsed[`${sourceKey}.${k}`] = v;
+      }
+      this.parsed[targetKey as any] = value.value;
+    } else {
+      this.parsed[targetKey as any] = value;
+    }
+  }
+
+  public toResult(): FromCloudFormationResult<T | undefined> {
+    const ret = new FromCloudFormationResult(undefinedIfAllValuesAreEmpty(this.parsed as any));
+    for (const [k, v] of Object.entries(this.unparsedKeys)) {
+      ret.extraProperties[k] = v;
+    }
+    return ret;
+  }
+
+  private get unparsedKeys(): Record<string, unknown> {
+    const unparsed = { ...this.unparsed };
+
+    for (const [k, v] of Object.entries(this.unparsed)) {
+      if (v instanceof FromCloudFormationResult) {
+        for (const [k2, v2] of Object.entries(v.extraProperties ?? {})) {
+          unparsed[`${k}.${k2}`] = v2;
+        }
+      } else {
+        unparsed[k] = v;
+      }
+    }
+
+    return unparsed;
+  }
+}
+
+function ucfirst(x: string) {
+  return x.slice(0, 1).toUpperCase() + x.slice(1);
+}
+

packages/aws-cdk-lib/core/lib/util.ts

@@ -121,6 +121,6 @@ export function findLastCommonElement<T>(path1: T[], path2: T[]): T | undefined
   return path1[i - 1];
 }
 
-export function undefinedIfAllValuesAreEmpty(object: object): object | undefined {
+export function undefinedIfAllValuesAreEmpty<A extends object>(object: A): A | undefined {
   return Object.values(object).some(v => v !== undefined) ? object : undefined;
 }

packages/aws-cdk-lib/jest.config.js

@@ -8,6 +8,8 @@ module.exports = {
   testMatch: [
     '<rootDir>/**/test/**/?(*.)+(test).ts',
   ],
+  // Massive parallellism leads to common timeouts
+  testTimeout: 60_000,
 
   coverageThreshold: {
     global: {

packages/aws-cdk/lib/util/npm.ts

@@ -5,6 +5,7 @@ import { debug } from '../../lib/logging';
 
 const exec = promisify(_exec);
 
+/* istanbul ignore next: not called during unit tests */
 export async function getLatestVersionFromNpm(): Promise<string> {
   const { stdout, stderr } = await exec('npm view aws-cdk version', { timeout: 3000 });
   if (stderr && stderr.trim().length > 0) {