diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts index c48ce3dcb4281..0485d450e82d8 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts +++ b/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' */ diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index f41ef46812a92..c11e41312b1ce 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/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(); + } + } } diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 19cf7cff8b85b..f912a4c180182 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -69,16 +69,6 @@ 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(); private readonly _notAction = new Array(); private readonly _principal: { [key: string]: any[] } = {}; @@ -86,11 +76,14 @@ export class PolicyStatement { private readonly _resource = new Array(); private readonly _notResource = new Array(); 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(); private readonly _notPrincipals = new Array(); + 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`); + } + } } /** diff --git a/packages/@aws-cdk/aws-iam/test/merge-statements.test.ts b/packages/@aws-cdk/aws-iam/test/merge-statements.test.ts index 061db0e134d02..c4d84204352f6 100644 --- a/packages/@aws-cdk/aws-iam/test/merge-statements.test.ts +++ b/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(); + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/test/policy-statement.test.ts b/packages/@aws-cdk/aws-iam/test/policy-statement.test.ts index 7498ee2814d80..ec1f9f691f727 100644 --- a/packages/@aws-cdk/aws-iam/test/policy-statement.test.ts +++ b/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/); + } + }); }); diff --git a/packages/@aws-cdk/cfnspec/.npmignore b/packages/@aws-cdk/cfnspec/.npmignore index 673239340aba1..623692bba67de 100644 --- a/packages/@aws-cdk/cfnspec/.npmignore +++ b/packages/@aws-cdk/cfnspec/.npmignore @@ -12,3 +12,4 @@ test/ jest.config.js **/*.integ.snapshot **/*.integ.snapshot +coverage/ diff --git a/packages/@aws-cdk/pipelines/README.md b/packages/@aws-cdk/pipelines/README.md index c6a8ed91928e5..e0490385b3aec 100644 --- a/packages/@aws-cdk/pipelines/README.md +++ b/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: