fix(cfn-diff): handle Fn::If inside policies and statements (#12975)

cloudformation-diff assumes the policies and statements it encounters are simple JSON objects.
However, in reality, everywhere that simple object can be used,
the Fn::If function can be used as well.
Add code that handles that eventuality.

Fixes #12887

----

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

Author: skinny85

packages/@aws-cdk/cloudformation-diff/lib/diffable.ts

@@ -44,13 +44,13 @@ interface Eq<T> {
 /**
  * Whether a collection contains some element (by value)
  */
-function contains<T extends Eq<T>>(element: T, xs: T[]) {
+function contains<T extends Eq<T>>(element: T, xs: T[]): boolean {
   return xs.some(x => x.equal(element));
 }
 
 /**
  * Return collection except for elements
  */
-function difference<T extends Eq<T>>(collection: T[], elements: T[]) {
+function difference<T extends Eq<T>>(collection: T[], elements: T[]): T[] {
   return collection.filter(x => !contains(x, elements));
 }

packages/@aws-cdk/cloudformation-diff/lib/iam/iam-changes.ts

@@ -4,8 +4,8 @@ import { PropertyChange, PropertyMap, ResourceChange } from '../diff/types';
 import { DiffableCollection } from '../diffable';
 import { renderIntrinsics } from '../render-intrinsics';
 import { deepRemoveUndefined, dropIfEmpty, flatMap, makeComparator } from '../util';
-import { ManagedPolicyAttachment, ManagedPolicyJson, parseManagedPolicies } from './managed-policy';
-import { parseLambdaPermission, parseStatements, renderCondition, Statement, StatementJson, Targets } from './statement';
+import { ManagedPolicyAttachment, ManagedPolicyJson } from './managed-policy';
+import { parseLambdaPermission, parseStatements, Statement, StatementJson } from './statement';
 
 export interface IamChangesProps {
   propertyChanges: PropertyChange[];
@@ -69,23 +69,25 @@ export class IamChanges {
 
     // First generate all lines, then sort on Resource so that similar resources are together
     for (const statement of this.statements.additions) {
+      const renderedStatement = statement.render();
       ret.push([
         '+',
-        renderTargets(statement.resources),
-        statement.effect,
-        renderTargets(statement.actions),
-        renderTargets(statement.principals),
-        renderCondition(statement.condition),
+        renderedStatement.resource,
+        renderedStatement.effect,
+        renderedStatement.action,
+        renderedStatement.principal,
+        renderedStatement.condition,
       ].map(s => colors.green(s)));
     }
     for (const statement of this.statements.removals) {
+      const renderedStatement = statement.render();
       ret.push([
         colors.red('-'),
-        renderTargets(statement.resources),
-        statement.effect,
-        renderTargets(statement.actions),
-        renderTargets(statement.principals),
-        renderCondition(statement.condition),
+        renderedStatement.resource,
+        renderedStatement.effect,
+        renderedStatement.action,
+        renderedStatement.principal,
+        renderedStatement.condition,
       ].map(s => colors.red(s)));
     }
 
@@ -125,14 +127,17 @@ export class IamChanges {
   }
 
   /**
-   * Return a machine-readable version of the changes
+   * Return a machine-readable version of the changes.
+   * This is only used in tests.
+   *
+   * @internal
    */
-  public toJson(): IamChangesJson {
+  public _toJson(): IamChangesJson {
     return deepRemoveUndefined({
-      statementAdditions: dropIfEmpty(this.statements.additions.map(s => s.toJson())),
-      statementRemovals: dropIfEmpty(this.statements.removals.map(s => s.toJson())),
-      managedPolicyAdditions: dropIfEmpty(this.managedPolicies.additions.map(s => s.toJson())),
-      managedPolicyRemovals: dropIfEmpty(this.managedPolicies.removals.map(s => s.toJson())),
+      statementAdditions: dropIfEmpty(this.statements.additions.map(s => s._toJson())),
+      statementRemovals: dropIfEmpty(this.statements.removals.map(s => s._toJson())),
+      managedPolicyAdditions: dropIfEmpty(this.managedPolicies.additions.map(s => s._toJson())),
+      managedPolicyRemovals: dropIfEmpty(this.managedPolicies.removals.map(s => s._toJson())),
     });
   }
 
@@ -184,7 +189,11 @@ export class IamChanges {
     const appliesToPrincipal = 'AWS:${' + logicalId + '}';
 
     return flatMap(policies, (policy: any) => {
-      return defaultPrincipal(appliesToPrincipal, parseStatements(renderIntrinsics(policy.PolicyDocument.Statement)));
+      // check if the Policy itself is not an intrinsic, like an Fn::If
+      const unparsedStatement = policy.PolicyDocument?.Statement
+        ? policy.PolicyDocument.Statement
+        : policy;
+      return defaultPrincipal(appliesToPrincipal, parseStatements(renderIntrinsics(unparsedStatement)));
     });
   }
 
@@ -234,11 +243,11 @@ export class IamChanges {
     });
   }
 
-  private readManagedPolicies(policyArns: string[] | undefined, logicalId: string): ManagedPolicyAttachment[] {
+  private readManagedPolicies(policyArns: any, logicalId: string): ManagedPolicyAttachment[] {
     if (!policyArns) { return []; }
 
     const rep = '${' + logicalId + '}';
-    return parseManagedPolicies(rep, renderIntrinsics(policyArns));
+    return ManagedPolicyAttachment.parseManagedPolicies(rep, renderIntrinsics(policyArns));
   }
 
   private readLambdaStatements(properties?: PropertyMap): Statement[] {
@@ -266,16 +275,6 @@ function defaultResource(resource: string, statements: Statement[]) {
   return statements;
 }
 
-/**
- * Render into a summary table cell
- */
-function renderTargets(targets: Targets): string {
-  if (targets.not) {
-    return targets.values.map(s => `NOT ${s}`).join('\n');
-  }
-  return targets.values.join('\n');
-}
-
 export interface IamChangesJson {
   statementAdditions?: StatementJson[];
   statementRemovals?: StatementJson[];

packages/@aws-cdk/cloudformation-diff/lib/iam/managed-policy.ts

@@ -1,13 +1,25 @@
 export class ManagedPolicyAttachment {
+  public static parseManagedPolicies(identityArn: string, arns: string | string[]): ManagedPolicyAttachment[] {
+    return typeof arns === 'string'
+      ? [new ManagedPolicyAttachment(identityArn, arns)]
+      : arns.map((arn: string) => new ManagedPolicyAttachment(identityArn, arn));
+  }
+
   constructor(public readonly identityArn: string, public readonly managedPolicyArn: string) {
   }
 
-  public equal(other: ManagedPolicyAttachment) {
+  public equal(other: ManagedPolicyAttachment): boolean {
     return this.identityArn === other.identityArn
         && this.managedPolicyArn === other.managedPolicyArn;
   }
 
-  public toJson() {
+  /**
+   * Return a machine-readable version of the changes.
+   * This is only used in tests.
+   *
+   * @internal
+   */
+  public _toJson(): ManagedPolicyJson {
     return { identityArn: this.identityArn, managedPolicyArn: this.managedPolicyArn };
   }
 }
@@ -16,7 +28,3 @@ export interface ManagedPolicyJson {
   identityArn: string;
   managedPolicyArn: string;
 }
-
-export function parseManagedPolicies(identityArn: string, arns: string[]): ManagedPolicyAttachment[] {
-  return arns.map((arn: string) => new ManagedPolicyAttachment(identityArn, arn));
-}

packages/@aws-cdk/cloudformation-diff/lib/iam/statement.ts

@@ -32,36 +32,76 @@ export class Statement {
    */
   public readonly condition?: any;
 
-  constructor(statement: UnknownMap) {
-    this.sid = expectString(statement.Sid);
-    this.effect = expectEffect(statement.Effect);
-    this.resources = new Targets(statement, 'Resource', 'NotResource');
-    this.actions = new Targets(statement, 'Action', 'NotAction');
-    this.principals = new Targets(statement, 'Principal', 'NotPrincipal');
-    this.condition = statement.Condition;
+  private readonly serializedIntrinsic: string | undefined;
+
+  constructor(statement: UnknownMap | string) {
+    if (typeof statement === 'string') {
+      this.sid = undefined;
+      this.effect = Effect.Unknown;
+      this.resources = new Targets({}, '', '');
+      this.actions = new Targets({}, '', '');
+      this.principals = new Targets({}, '', '');
+      this.condition = undefined;
+      this.serializedIntrinsic = statement;
+    } else {
+      this.sid = expectString(statement.Sid);
+      this.effect = expectEffect(statement.Effect);
+      this.resources = new Targets(statement, 'Resource', 'NotResource');
+      this.actions = new Targets(statement, 'Action', 'NotAction');
+      this.principals = new Targets(statement, 'Principal', 'NotPrincipal');
+      this.condition = statement.Condition;
+      this.serializedIntrinsic = undefined;
+    }
   }
 
   /**
    * Whether this statement is equal to the other statement
    */
-  public equal(other: Statement) {
+  public equal(other: Statement): boolean {
     return (this.sid === other.sid
       && this.effect === other.effect
+      && this.serializedIntrinsic === other.serializedIntrinsic
       && this.resources.equal(other.resources)
       && this.actions.equal(other.actions)
       && this.principals.equal(other.principals)
       && deepEqual(this.condition, other.condition));
   }
 
-  public toJson(): StatementJson {
-    return deepRemoveUndefined({
-      sid: this.sid,
-      effect: this.effect,
-      resources: this.resources.toJson(),
-      principals: this.principals.toJson(),
-      actions: this.actions.toJson(),
-      condition: this.condition,
-    });
+  public render(): RenderedStatement {
+    return this.serializedIntrinsic
+      ? {
+        resource: this.serializedIntrinsic,
+        effect: '',
+        action: '',
+        principal: this.principals.render(), // these will be replaced by the call to replaceEmpty() from IamChanges
+        condition: '',
+      }
+      : {
+        resource: this.resources.render(),
+        effect: this.effect,
+        action: this.actions.render(),
+        principal: this.principals.render(),
+        condition: renderCondition(this.condition),
+      };
+  }
+
+  /**
+   * Return a machine-readable version of the changes.
+   * This is only used in tests.
+   *
+   * @internal
+   */
+  public _toJson(): StatementJson {
+    return this.serializedIntrinsic
+      ? this.serializedIntrinsic
+      : deepRemoveUndefined({
+        sid: this.sid,
+        effect: this.effect,
+        resources: this.resources._toJson(),
+        principals: this.principals._toJson(),
+        actions: this.actions._toJson(),
+        condition: this.condition,
+      });
   }
 
   /**
@@ -76,6 +116,14 @@ export class Statement {
   }
 }
 
+export interface RenderedStatement {
+  readonly resource: string;
+  readonly effect: string;
+  readonly action: string;
+  readonly principal: string;
+  readonly condition: string;
+}
+
 export interface StatementJson {
   sid?: string;
   effect: string;
@@ -199,7 +247,22 @@ export class Targets {
     this.values.sort();
   }
 
-  public toJson(): TargetsJson {
+  /**
+   * Render into a summary table cell
+   */
+  public render(): string {
+    return this.not
+      ? this.values.map(s => `NOT ${s}`).join('\n')
+      : this.values.join('\n');
+  }
+
+  /**
+   * Return a machine-readable version of the changes.
+   * This is only used in tests.
+   *
+   * @internal
+   */
+  public _toJson(): TargetsJson {
     return { not: this.not, values: this.values };
   }
 }
@@ -243,7 +306,7 @@ function forceListOfStrings(x: unknown): string[] {
 /**
  * Render the Condition column
  */
-export function renderCondition(condition: any) {
+export function renderCondition(condition: any): string {
   if (!condition || Object.keys(condition).length === 0) { return ''; }
   const jsonRepresentation = JSON.stringify(condition, undefined, 2);