Merge branch 'main' into mergify/bp/main/pr-19320

packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts

@@ -33,7 +33,8 @@ export interface EcrSourceVariables {
 export interface EcrSourceActionProps extends codepipeline.CommonAwsActionProps {
   /**
    * The image tag that will be checked for changes.
-   * Provide an empty string to trigger on changes to any tag.
+   *
+   * It is not possible to trigger on changes to more than one tag.
    *
    * @default 'latest'
    */

packages/@aws-cdk/aws-iam/lib/policy-document.ts

@@ -76,6 +76,7 @@ export class PolicyDocument implements cdk.IResolvable {
   }
 
   public resolve(context: cdk.IResolveContext): any {
+    this.freezeStatements();
     this._maybeMergeStatements(context.scope);
 
     // In the previous implementation of 'merge', sorting of actions/resources on
@@ -212,6 +213,7 @@ export class PolicyDocument implements cdk.IResolvable {
     const newDocs: PolicyDocument[] = [];
 
     // Maps final statements to original statements
+    this.freezeStatements();
     let statementsToOriginals = new Map(this.statements.map(s => [s, [s]]));
 
     // We always run 'mergeStatements' to minimize the policy before splitting.
@@ -298,4 +300,13 @@ export class PolicyDocument implements cdk.IResolvable {
   private shouldMerge(scope: IConstruct) {
     return this.minimize ?? cdk.FeatureFlags.of(scope).isEnabled(cxapi.IAM_MINIMIZE_POLICIES) ?? false;
   }
+
+  /**
+   * Freeze all statements
+   */
+  private freezeStatements() {
+    for (const statement of this.statements) {
+      statement.freeze();
+    }
+  }
 }

packages/@aws-cdk/aws-iam/lib/policy-statement.ts

@@ -69,28 +69,21 @@ export class PolicyStatement {
     return ret;
   }
 
-  /**
-   * Statement ID for this statement
-   */
-  public sid?: string;
-
-  /**
-   * Whether to allow or deny the actions in this statement
-   */
-  public effect: Effect;
-
   private readonly _action = new Array<string>();
   private readonly _notAction = new Array<string>();
   private readonly _principal: { [key: string]: any[] } = {};
   private readonly _notPrincipal: { [key: string]: any[] } = {};
   private readonly _resource = new Array<string>();
   private readonly _notResource = new Array<string>();
   private readonly _condition: { [key: string]: any } = { };
+  private _sid?: string;
+  private _effect: Effect;
   private principalConditionsJson?: string;
 
   // Hold on to those principals
   private readonly _principals = new Array<IPrincipal>();
   private readonly _notPrincipals = new Array<IPrincipal>();
+  private _frozen = false;
 
   constructor(props: PolicyStatementProps = {}) {
     // Validate actions
@@ -101,8 +94,8 @@ export class PolicyStatement {
       }
     }
 
-    this.sid = props.sid;
-    this.effect = props.effect || Effect.ALLOW;
+    this._sid = props.sid;
+    this._effect = props.effect || Effect.ALLOW;
 
     this.addActions(...props.actions || []);
     this.addNotActions(...props.notActions || []);
@@ -115,6 +108,36 @@ export class PolicyStatement {
     }
   }
 
+  /**
+   * Statement ID for this statement
+   */
+  public get sid(): string | undefined {
+    return this._sid;
+  }
+
+  /**
+   * Set Statement ID for this statement
+   */
+  public set sid(sid: string | undefined) {
+    this.assertNotFrozen('sid');
+    this._sid = sid;
+  }
+
+  /**
+   * Whether to allow or deny the actions in this statement
+   */
+  public get effect(): Effect {
+    return this._effect;
+  }
+
+  /**
+   * Set effect for this statement
+   */
+  public set effect(effect: Effect) {
+    this.assertNotFrozen('effect');
+    this._effect = effect;
+  }
+
   //
   // Actions
   //
@@ -127,6 +150,7 @@ export class PolicyStatement {
    * @param actions actions that will be allowed.
    */
   public addActions(...actions: string[]) {
+    this.assertNotFrozen('addActions');
     if (actions.length > 0 && this._notAction.length > 0) {
       throw new Error('Cannot add \'Actions\' to policy statement if \'NotActions\' have been added');
     }
@@ -142,6 +166,7 @@ export class PolicyStatement {
    * @param notActions actions that will be denied. All other actions will be permitted.
    */
   public addNotActions(...notActions: string[]) {
+    this.assertNotFrozen('addNotActions');
     if (notActions.length > 0 && this._action.length > 0) {
       throw new Error('Cannot add \'NotActions\' to policy statement if \'Actions\' have been added');
     }
@@ -167,6 +192,7 @@ export class PolicyStatement {
    * @param principals IAM principals that will be added
    */
   public addPrincipals(...principals: IPrincipal[]) {
+    this.assertNotFrozen('addPrincipals');
     this._principals.push(...principals);
     if (Object.keys(principals).length > 0 && Object.keys(this._notPrincipal).length > 0) {
       throw new Error('Cannot add \'Principals\' to policy statement if \'NotPrincipals\' have been added');
@@ -188,6 +214,7 @@ export class PolicyStatement {
    * @param notPrincipals IAM principals that will be denied access
    */
   public addNotPrincipals(...notPrincipals: IPrincipal[]) {
+    this.assertNotFrozen('addNotPrincipals');
     this._notPrincipals.push(...notPrincipals);
     if (Object.keys(notPrincipals).length > 0 && Object.keys(this._principal).length > 0) {
       throw new Error('Cannot add \'NotPrincipals\' to policy statement if \'Principals\' have been added');
@@ -280,6 +307,7 @@ export class PolicyStatement {
    * @param arns Amazon Resource Names (ARNs) of the resources that this policy statement applies to
    */
   public addResources(...arns: string[]) {
+    this.assertNotFrozen('addResources');
     if (arns.length > 0 && this._notResource.length > 0) {
       throw new Error('Cannot add \'Resources\' to policy statement if \'NotResources\' have been added');
     }
@@ -295,6 +323,7 @@ export class PolicyStatement {
    * @param arns Amazon Resource Names (ARNs) of the resources that this policy statement does not apply to
    */
   public addNotResources(...arns: string[]) {
+    this.assertNotFrozen('addNotResources');
     if (arns.length > 0 && this._resource.length > 0) {
       throw new Error('Cannot add \'NotResources\' to policy statement if \'Resources\' have been added');
     }
@@ -344,6 +373,7 @@ export class PolicyStatement {
    * ```
    */
   public addCondition(key: string, value: Condition) {
+    this.assertNotFrozen('addCondition');
     const existingValue = this._condition[key];
     this._condition[key] = existingValue ? { ...existingValue, ...value } : value;
   }
@@ -544,6 +574,29 @@ export class PolicyStatement {
     return { ...this._condition };
   }
 
+  /**
+   * Make the PolicyStatement immutable
+   *
+   * After calling this, any of the `addXxx()` methods will throw an exception.
+   *
+   * Libraries that lazily generate statement bodies can override this method to
+   * fill the actual PolicyStatement fields. Be aware that this method may be called
+   * multiple times.
+   */
+  public freeze(): PolicyStatement {
+    this._frozen = true;
+    return this;
+  }
+
+  /**
+   * Whether the PolicyStatement has been frozen
+   *
+   * The statement object is frozen when `freeze()` is called.
+   */
+  public get frozen(): boolean {
+    return this._frozen;
+  }
+
   /**
    * Estimate the size of this policy statement
    *
@@ -577,6 +630,15 @@ export class PolicyStatement {
       }
     }
   }
+
+  /**
+   * Throw an exception when the object is frozen
+   */
+  private assertNotFrozen(method: string) {
+    if (this._frozen) {
+      throw new Error(`${method}: freeze() has been called on this PolicyStatement previously, so it can no longer be modified`);
+    }
+  }
 }
 
 /**

packages/@aws-cdk/aws-iam/test/merge-statements.test.ts

@@ -471,6 +471,25 @@ test('keep merging even if it requires multiple passes', () => {
   ]);
 });
 
+test('lazily generated statements are merged correctly', () => {
+  assertMerged([
+    new LazyStatement((s) => {
+      s.addActions('service:A');
+      s.addResources('R1');
+    }),
+    new LazyStatement((s) => {
+      s.addActions('service:B');
+      s.addResources('R1');
+    }),
+  ], [
+    {
+      Effect: 'Allow',
+      Action: ['service:A', 'service:B'],
+      Resource: 'R1',
+    },
+  ]);
+});
+
 function assertNoMerge(statements: iam.PolicyStatement[]) {
   const app = new App();
   const stack = new Stack(app, 'Stack');
@@ -499,3 +518,17 @@ function assertMerged(statements: iam.PolicyStatement[], expected: any[]) {
 function assertMergedC(doMerge: boolean, statements: iam.PolicyStatement[], expected: any[]) {
   return doMerge ? assertMerged(statements, expected) : assertNoMerge(statements);
 }
+
+/**
+ * A statement that fills itself only when freeze() is called.
+ */
+class LazyStatement extends iam.PolicyStatement {
+  constructor(private readonly modifyMe: (x: iam.PolicyStatement) => void) {
+    super();
+  }
+
+  public freeze() {
+    this.modifyMe(this);
+    return super.freeze();
+  }
+}

packages/@aws-cdk/aws-iam/test/policy-statement.test.ts

@@ -1,5 +1,5 @@
 import { Stack } from '@aws-cdk/core';
-import { AnyPrincipal, Group, PolicyDocument, PolicyStatement } from '../lib';
+import { AnyPrincipal, Group, PolicyDocument, PolicyStatement, Effect } from '../lib';
 
 describe('IAM policy statement', () => {
 
@@ -214,4 +214,31 @@ describe('IAM policy statement', () => {
     expect(() => policyStatement.addNotPrincipals(group))
       .toThrow(/Cannot use an IAM Group as the 'Principal' or 'NotPrincipal' in an IAM Policy/);
   });
+
+
+  test('a frozen policy statement cannot be modified any more', () => {
+    // GIVEN
+    const statement = new PolicyStatement({
+      actions: ['action:a'],
+      resources: ['*'],
+    });
+    statement.freeze();
+
+    // WHEN
+    const modifications = [
+      () => statement.sid = 'asdf',
+      () => statement.effect = Effect.DENY,
+      () => statement.addActions('abc:def'),
+      () => statement.addNotActions('abc:def'),
+      () => statement.addResources('*'),
+      () => statement.addNotResources('*'),
+      () => statement.addPrincipals(new AnyPrincipal()),
+      () => statement.addNotPrincipals(new AnyPrincipal()),
+      () => statement.addCondition('key', 'value'),
+    ];
+
+    for (const mod of modifications) {
+      expect(mod).toThrow(/can no longer be modified/);
+    }
+  });
 });

packages/@aws-cdk/cfnspec/.npmignore

@@ -12,3 +12,4 @@ test/
 jest.config.js
 **/*.integ.snapshot
 **/*.integ.snapshot
+coverage/

packages/@aws-cdk/pipelines/README.md

@@ -1379,6 +1379,22 @@ After turning on `privilegedMode: true`, you will need to do a one-time manual c
 pipeline to get it going again (as with a broken 'synth' the pipeline will not be able to self
 update to the right state).
 
+### IAM policies: Cannot exceed quota for PoliciesPerRole / Maximum policy size exceeded
+
+This happens as a result of having a lot of targets in the Pipeline: the IAM policies that
+get generated enumerate all required roles and grow too large.
+
+Make sure you are on version `2.26.0` or higher, and that your `cdk.json` contains the
+following:
+
+```json
+{
+  "context": {
+    "@aws-cdk/aws-iam:minimizePolicies": true
+  }
+}
+```
+
 ### S3 error: Access Denied
 
 An "S3 Access Denied" error can have two causes: