diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 6dad03744c8e7..8b3819f8067c8 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -708,13 +708,7 @@ export class Pipeline extends PipelineBase { } // the pipeline role needs assumeRole permissions to the action role - if (actionRole) { - this.role.addToPrincipalPolicy(new iam.PolicyStatement({ - actions: ['sts:AssumeRole'], - resources: [actionRole.roleArn], - })); - } - + actionRole?.grantAssumeRole(this.role); return actionRole; } diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index f5973d663e399..0d23f01614e2e 100644 --- a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts @@ -119,6 +119,13 @@ export class LazyRole extends cdk.Resource implements IRole { return this.instantiate().grantPassRole(identity); } + /** + * Grant permissions to the given principal to assume this role. + */ + public grantAssumeRole(identity: IPrincipal): Grant { + return this.instantiate().grantAssumeRole(identity); + } + private instantiate(): Role { if (!this.role) { const role = new Role(this, 'Default', this.props); diff --git a/packages/@aws-cdk/aws-iam/lib/policy-document.ts b/packages/@aws-cdk/aws-iam/lib/policy-document.ts index 9d73acb4693ac..b46286be72f7c 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-document.ts @@ -1,6 +1,8 @@ import * as cdk from '@aws-cdk/core'; import * as cxapi from '@aws-cdk/cx-api'; +import { IConstruct } from 'constructs'; import { PolicyStatement } from './policy-statement'; +import { mergeStatements } from './private/merge-statements'; import { PostProcessPolicyDocument } from './private/postprocess-policy-document'; /** @@ -64,6 +66,7 @@ export class PolicyDocument implements cdk.IResolvable { private readonly statements = new Array(); private readonly autoAssignSids: boolean; private readonly minimize?: boolean; + private _minimized = false; constructor(props: PolicyDocumentProps = {}) { this.creationStack = cdk.captureStackTrace(); @@ -74,10 +77,8 @@ export class PolicyDocument implements cdk.IResolvable { } public resolve(context: cdk.IResolveContext): any { - context.registerPostProcessor(new PostProcessPolicyDocument( - this.autoAssignSids, - this.minimize ?? cdk.FeatureFlags.of(context.scope).isEnabled(cxapi.IAM_MINIMIZE_POLICIES) ?? false, - )); + this._maybeMergeStatements(context.scope); + context.registerPostProcessor(new PostProcessPolicyDocument(this.autoAssignSids)); return this.render(); } @@ -165,6 +166,98 @@ export class PolicyDocument implements cdk.IResolvable { return errors; } + /** + * Perform statement merging (if enabled and not done yet) + * + * Returns a mapping of merged statements to original statements on the first invocation, + * `undefined` on subsequent invocations. + * + * @internal + */ + public _maybeMergeStatements(scope: IConstruct): Map | undefined { + const minimize = this.minimize ?? cdk.FeatureFlags.of(scope).isEnabled(cxapi.IAM_MINIMIZE_POLICIES) ?? false; + if (minimize) { + if (this._minimized) { + return undefined; + } + const result = mergeStatements(this.statements); + this.statements.splice(0, this.statements.length, ...result.mergedStatements); + this._minimized = true; + return result.originsMap; + } + return new Map(this.statements.map(s => [s, [s]])); + } + + /** + * Split the statements of the PolicyDocument into multiple groups, limited by their size + * + * Returns the policy documents created to hold statements that were split off. + * + * @internal + */ + public _splitDocument(selfMaximumSize: number, splitMaximumSize: number): PolicyDocument[] { + const self = this; + const newDocs: PolicyDocument[] = []; + + // Cache statement sizes to avoid recomputing them based on the fields + const statementSizes = new Map(this.statements.map(s => [s, s._estimateSize()])); + + // Keep some size counters so we can avoid recomputing them based on the statements in each + let selfSize = 0; + const polSizes = new Map(); + // Getter with a default to save some syntactic noise + const polSize = (x: PolicyDocument) => polSizes.get(x) ?? 0; + + let i = 0; + while (i < this.statements.length) { + const statement = this.statements[i]; + + const statementSize = statementSizes.get(statement) ?? 0; + if (selfSize + statementSize < selfMaximumSize) { + // Fits in self + selfSize += statementSize; + i++; + continue; + } + + // Split off to new PolicyDocument. Find the PolicyDocument we can add this to, + // or add a fresh one. + const addToDoc = findDocWithSpace(statementSize); + addToDoc.addStatements(statement); + polSizes.set(addToDoc, polSize(addToDoc) + statementSize); + this.statements.splice(i, 1); + } + + return newDocs; + + function findDocWithSpace(size: number) { + let j = 0; + while (j < newDocs.length && polSize(newDocs[j]) + size > splitMaximumSize) { + j++; + } + if (j < newDocs.length) { + return newDocs[j]; + } + + const newDoc = new PolicyDocument({ + assignSids: self.autoAssignSids, + // Minimizing has already been done + minimize: false, + }); + newDocs.push(newDoc); + return newDoc; + } + } + + /** + * The statements in this doc + * + * @internal + */ + public _statements() { + return [...this.statements]; + } + private render(): any { if (this.isEmpty) { return undefined; diff --git a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts index 80ff191613e0e..210bcab227aae 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy-statement.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy-statement.ts @@ -5,7 +5,7 @@ import { FederatedPrincipal, IPrincipal, PrincipalBase, PrincipalPolicyFragment, ServicePrincipal, ServicePrincipalOpts, } from './principals'; import { normalizeStatement } from './private/postprocess-policy-document'; -import { LITERAL_STRING_KEY, mergePrincipal } from './util'; +import { LITERAL_STRING_KEY, mergePrincipal, sum } from './util'; const ensureArrayOrUndefined = (field: any) => { if (field === undefined) { @@ -56,22 +56,24 @@ export class PolicyStatement { * 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[] } = {}; - private readonly notPrincipal: { [key: string]: any[] } = {}; - private readonly resource = new Array(); - private readonly notResource = new Array(); - private readonly condition: { [key: string]: any } = { }; + private readonly _action = new Array(); + private readonly _notAction = new Array(); + private readonly _principal: { [key: string]: any[] } = {}; + private readonly _notPrincipal: { [key: string]: any[] } = {}; + private readonly _resource = new Array(); + private readonly _notResource = new Array(); + private readonly _condition: { [key: string]: any } = { }; private principalConditionsJson?: string; // Hold on to those principals private readonly _principals = new Array(); + private readonly _notPrincipals = new Array(); constructor(props: PolicyStatementProps = {}) { // Validate actions @@ -108,10 +110,10 @@ export class PolicyStatement { * @param actions actions that will be allowed. */ public addActions(...actions: string[]) { - if (actions.length > 0 && this.notAction.length > 0) { + if (actions.length > 0 && this._notAction.length > 0) { throw new Error('Cannot add \'Actions\' to policy statement if \'NotActions\' have been added'); } - this.action.push(...actions); + this._action.push(...actions); } /** @@ -123,10 +125,10 @@ export class PolicyStatement { * @param notActions actions that will be denied. All other actions will be permitted. */ public addNotActions(...notActions: string[]) { - if (notActions.length > 0 && this.action.length > 0) { + if (notActions.length > 0 && this._action.length > 0) { throw new Error('Cannot add \'NotActions\' to policy statement if \'Actions\' have been added'); } - this.notAction.push(...notActions); + this._notAction.push(...notActions); } // @@ -137,7 +139,7 @@ export class PolicyStatement { * Indicates if this permission has a "Principal" section. */ public get hasPrincipal() { - return Object.keys(this.principal).length > 0 || Object.keys(this.notPrincipal).length > 0; + return this._principals.length + this._notPrincipals.length > 0; } /** @@ -149,26 +151,17 @@ export class PolicyStatement { */ public addPrincipals(...principals: IPrincipal[]) { this._principals.push(...principals); - if (Object.keys(principals).length > 0 && Object.keys(this.notPrincipal).length > 0) { + 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'); } for (const principal of principals) { this.validatePolicyPrincipal(principal); const fragment = principal.policyFragment; - mergePrincipal(this.principal, fragment.principalJson); + mergePrincipal(this._principal, fragment.principalJson); this.addPrincipalConditions(fragment.conditions); } } - /** - * Expose principals to allow their ARNs to be replaced by account ID strings - * in policy statements for resources policies that don't allow full account ARNs, - * such as AWS::Logs::ResourcePolicy. - */ - public get principals(): IPrincipal[] { - return [...this._principals]; - } - /** * Specify principals that is not allowed or denied access to the "NotPrincipal" section of * a policy statement. @@ -178,13 +171,14 @@ export class PolicyStatement { * @param notPrincipals IAM principals that will be denied access */ public addNotPrincipals(...notPrincipals: IPrincipal[]) { - if (Object.keys(notPrincipals).length > 0 && Object.keys(this.principal).length > 0) { + 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'); } for (const notPrincipal of notPrincipals) { this.validatePolicyPrincipal(notPrincipal); const fragment = notPrincipal.policyFragment; - mergePrincipal(this.notPrincipal, fragment.principalJson); + mergePrincipal(this._notPrincipal, fragment.principalJson); this.addPrincipalConditions(fragment.conditions); } } @@ -269,10 +263,10 @@ export class PolicyStatement { * @param arns Amazon Resource Names (ARNs) of the resources that this policy statement applies to */ public addResources(...arns: string[]) { - if (arns.length > 0 && this.notResource.length > 0) { + if (arns.length > 0 && this._notResource.length > 0) { throw new Error('Cannot add \'Resources\' to policy statement if \'NotResources\' have been added'); } - this.resource.push(...arns); + this._resource.push(...arns); } /** @@ -284,10 +278,10 @@ export class PolicyStatement { * @param arns Amazon Resource Names (ARNs) of the resources that this policy statement does not apply to */ public addNotResources(...arns: string[]) { - if (arns.length > 0 && this.resource.length > 0) { + if (arns.length > 0 && this._resource.length > 0) { throw new Error('Cannot add \'NotResources\' to policy statement if \'Resources\' have been added'); } - this.notResource.push(...arns); + this._notResource.push(...arns); } /** @@ -301,7 +295,7 @@ export class PolicyStatement { * Indicates if this permission has at least one resource associated with it. */ public get hasResource() { - return this.resource && this.resource.length > 0; + return this._resource && this._resource.length > 0; } // @@ -312,8 +306,8 @@ export class PolicyStatement { * Add a condition to the Policy */ public addCondition(key: string, value: Condition) { - const existingValue = this.condition[key]; - this.condition[key] = existingValue ? { ...existingValue, ...value } : value; + const existingValue = this._condition[key]; + this._condition[key] = existingValue ? { ...existingValue, ...value } : value; } /** @@ -340,14 +334,16 @@ export class PolicyStatement { return new PolicyStatement({ sid: overrides.sid ?? this.sid, effect: overrides.effect ?? this.effect, - actions: overrides.actions ?? this.action, - notActions: overrides.notActions ?? this.notAction, + actions: overrides.actions ?? this.actions, + notActions: overrides.notActions ?? this.notActions, - principals: overrides.principals, - notPrincipals: overrides.notPrincipals, + principals: overrides.principals ?? this.principals, + notPrincipals: overrides.notPrincipals ?? this.notPrincipals, - resources: overrides.resources ?? this.resource, - notResources: overrides.notResources ?? this.notResource, + resources: overrides.resources ?? this.resources, + notResources: overrides.notResources ?? this.notResources, + + conditions: this.conditions, }); } @@ -358,14 +354,14 @@ export class PolicyStatement { */ public toStatementJson(): any { return normalizeStatement({ - Action: this.action, - NotAction: this.notAction, - Condition: this.condition, + Action: this._action, + NotAction: this._notAction, + Condition: this._condition, Effect: this.effect, - Principal: this.principal, - NotPrincipal: this.notPrincipal, - Resource: this.resource, - NotResource: this.notResource, + Principal: this._principal, + NotPrincipal: this._notPrincipal, + Resource: this._resource, + NotResource: this._notResource, Sid: this.sid, }); } @@ -420,7 +416,7 @@ export class PolicyStatement { */ public validateForAnyPolicy(): string[] { const errors = new Array(); - if (this.action.length === 0 && this.notAction.length === 0) { + if (this._action.length === 0 && this._notAction.length === 0) { errors.push('A PolicyStatement must specify at least one \'action\' or \'notAction\'.'); } return errors; @@ -431,7 +427,7 @@ export class PolicyStatement { */ public validateForResourcePolicy(): string[] { const errors = this.validateForAnyPolicy(); - if (Object.keys(this.principal).length === 0 && Object.keys(this.notPrincipal).length === 0) { + if (Object.keys(this._principal).length === 0 && Object.keys(this._notPrincipal).length === 0) { errors.push('A PolicyStatement used in a resource-based policy must specify at least one IAM principal.'); } return errors; @@ -442,14 +438,98 @@ export class PolicyStatement { */ public validateForIdentityPolicy(): string[] { const errors = this.validateForAnyPolicy(); - if (Object.keys(this.principal).length > 0 || Object.keys(this.notPrincipal).length > 0) { + if (Object.keys(this._principal).length > 0 || Object.keys(this._notPrincipal).length > 0) { errors.push('A PolicyStatement used in an identity-based policy cannot specify any IAM principals.'); } - if (Object.keys(this.resource).length === 0 && Object.keys(this.notResource).length === 0) { + if (Object.keys(this._resource).length === 0 && Object.keys(this._notResource).length === 0) { errors.push('A PolicyStatement used in an identity-based policy must specify at least one resource.'); } return errors; } + + /** + * The Actions added to this statement + */ + public get actions() { + return [...this._action]; + } + + /** + * The NotActions added to this statement + */ + public get notActions() { + return [...this._notAction]; + } + + /** + * The Principals added to this statement + */ + public get principals(): IPrincipal[] { + return [...this._principals]; + } + + /** + * The NotPrincipals added to this statement + */ + public get notPrincipals(): IPrincipal[] { + return [...this._notPrincipals]; + } + + /** + * The Resources added to this statement + */ + public get resources() { + return [...this._resource]; + } + + /** + * The NotResources added to this statement + */ + public get notResources() { + return [...this._notResource]; + } + + /** + * The conditions added to this statement + */ + public get conditions(): any { + return { ...this._condition }; + } + + /** + * Estimate the size of this policy statement + * + * By necessity, this will not be accurate. We'll do our best to overestimate + * so we won't have nasty surprises. + * + * @internal + */ + public _estimateSize(): number { + let ret = 0; + + const actionEstimate = 20; + const arnEstimate = 120; // A safe (over)estimate on how long ARNs typically are + + ret += `"Effect": "${this.effect}",`.length; + + count('Action', this.actions, actionEstimate); + count('NotAction', this.notActions, actionEstimate); + count('Resource', this.resources, arnEstimate); + count('NotResource', this.notResources, arnEstimate); + + ret += this.principals.length * arnEstimate; + ret += this.notPrincipals.length * arnEstimate; + + ret += JSON.stringify(this.conditions).length; + return ret; + + function count(key: string, values: string[], tokenSize: number) { + if (values.length > 0) { + ret += key.length + 5 /* quotes, colon, brackets */ + + sum(values.map(v => (cdk.Token.isUnresolved(v) ? tokenSize : v.length) + 3 /* quotes, separator */)); + } + } + } } /** diff --git a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts index 663cc601da281..8d0b454ffb97d 100644 --- a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts @@ -67,4 +67,8 @@ export class ImmutableRole extends Resource implements IRole { public grantPassRole(grantee: IPrincipal): Grant { return this.role.grantPassRole(grantee); } + + public grantAssumeRole(identity: IPrincipal): Grant { + return this.role.grantAssumeRole(identity); + } } diff --git a/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts b/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts index c79ecd6a8a814..17a33f5c10788 100644 --- a/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts +++ b/packages/@aws-cdk/aws-iam/lib/private/merge-statements.ts @@ -4,8 +4,18 @@ // implemented here. +import { PolicyStatement } from '../policy-statement'; +import { IPrincipal } from '../principals'; import { LITERAL_STRING_KEY } from '../util'; -import { StatementSchema, normalizeStatement, IamValue } from './postprocess-policy-document'; + + +/* + * Don't produce any merged statements larger than this. + * + * They will become impossible to divide across managed policies if we do, + * and this is the maximum size for User policies. + */ +const MAX_MERGE_SIZE = 2000; /** * Merge as many statements as possible to shrink the total policy doc, modifying the input array in place @@ -15,12 +25,21 @@ import { StatementSchema, normalizeStatement, IamValue } from './postprocess-pol * Good Enough(tm). If it merges anything, it's at least going to produce a smaller output * than the input. */ -export function mergeStatements(statements: StatementSchema[]): StatementSchema[] { +export function mergeStatements(statements: PolicyStatement[]): MergeStatementResult { const compStatements = statements.map(makeComparable); // Keep trying until nothing changes anymore while (onePass()) { /* again */ } - return compStatements.map(renderComparable); + + const mergedStatements = new Array(); + const originsMap = new Map(); + for (const comp of compStatements) { + const statement = renderComparable(comp); + mergedStatements.push(statement); + originsMap.set(statement, comp.originals); + } + + return { mergedStatements, originsMap }; // Do one optimization pass, return 'true' if we merged anything function onePass() { @@ -47,6 +66,18 @@ export function mergeStatements(statements: StatementSchema[]): StatementSchema[ } } +export interface MergeStatementResult { + /** + * The list of maximally merged statements + */ + readonly mergedStatements: PolicyStatement[]; + + /** + * Mapping of old to new statements + */ + readonly originsMap: Map; +} + /** * Given two statements, return their merging (if possible) * @@ -61,32 +92,40 @@ export function mergeStatements(statements: StatementSchema[]): StatementSchema[ */ function tryMerge(a: ComparableStatement, b: ComparableStatement): ComparableStatement | undefined { // Effects must be the same - if (a.effect !== b.effect) { return; } + if (a.statement.effect !== b.statement.effect) { return; } // We don't merge Sids (for now) - if (a.sid || b.sid) { return; } + if (a.statement.sid || b.statement.sid) { return; } + // Not if the combination grows too large + if (a.sizeEstimate + b.sizeEstimate > MAX_MERGE_SIZE) { return; } if (a.conditionString !== b.conditionString) { return; } - if (!setEqual(a.notAction, b.notAction) || !setEqual(a.notResource, b.notResource) || !setEqual(a.notPrincipal, b.notPrincipal)) { return; } + if ( + !setEqual(a.statement.notActions, b.statement.notActions) || + !setEqual(a.statement.notResources, b.statement.notResources) || + !setEqual(a.notPrincipalStrings, b.notPrincipalStrings) + ) { + return; + } // We can merge these statements if 2 out of the 3 sets of Action, Resource, Principal // are the same. - const setsEqual = (setEqual(a.action, b.action) ? 1 : 0) + - (setEqual(a.resource, b.resource) ? 1 : 0) + - (setEqual(a.principal, b.principal) ? 1 : 0); + const setsEqual = (setEqual(a.statement.actions, b.statement.actions) ? 1 : 0) + + (setEqual(a.statement.resources, b.statement.resources) ? 1 : 0) + + (setEqual(a.principalStrings, b.principalStrings) ? 1 : 0); if (setsEqual < 2 || unmergeablePrincipals(a, b)) { return; } return { - effect: a.effect, + originals: [...a.originals, ...b.originals], + statement: a.statement.copy({ + actions: setMerge(a.statement.actions, b.statement.actions), + resources: setMerge(a.statement.resources, b.statement.resources), + principals: setMerge(a.statement.principals, b.statement.principals), + }), + principalStrings: setMerge(a.principalStrings, b.principalStrings), + notPrincipalStrings: a.notPrincipalStrings, conditionString: a.conditionString, - conditionValue: b.conditionValue, - notAction: a.notAction, - notPrincipal: a.notPrincipal, - notResource: a.notResource, - - action: setMerge(a.action, b.action), - resource: setMerge(a.resource, b.resource), - principal: setMerge(a.principal, b.principal), + sizeEstimate: a.sizeEstimate + b.sizeEstimate, }; } @@ -95,42 +134,18 @@ function tryMerge(a: ComparableStatement, b: ComparableStatement): ComparableSta * * This is to be able to do comparisons on these sets quickly. */ -function makeComparable(s: StatementSchema): ComparableStatement { +function makeComparable(s: PolicyStatement): ComparableStatement { return { - effect: s.Effect, - sid: s.Sid, - action: iamSet(s.Action), - notAction: iamSet(s.NotAction), - resource: iamSet(s.Resource), - notResource: iamSet(s.NotResource), - principal: principalIamSet(s.Principal), - notPrincipal: principalIamSet(s.NotPrincipal), - conditionString: JSON.stringify(s.Condition), - conditionValue: s.Condition, + originals: [s], + statement: s, + conditionString: JSON.stringify(s.conditions), + principalStrings: s.principals.map(renderPrincipal), + notPrincipalStrings: s.notPrincipals.map(renderPrincipal), + sizeEstimate: s._estimateSize(), }; - function forceArray(x: A | Array): Array { - return Array.isArray(x) ? x : [x]; - } - - function iamSet(x: IamValue | undefined): IamValueSet { - if (x == undefined) { return {}; } - return mkdict(forceArray(x).map(e => [JSON.stringify(e), e])); - } - - function principalIamSet(x: IamValue | Record | undefined): IamValueSet { - if (x === undefined) { return {}; } - - if (Array.isArray(x) || typeof x === 'string') { - x = { [LITERAL_STRING_KEY]: x }; - } - - if (typeof x === 'object' && x !== null) { - // Turn { AWS: [a, b], Service: [c] } into [{ AWS: a }, { AWS: b }, { Service: c }] - const individualPrincipals = Object.entries(x).flatMap(([principalType, value]) => forceArray(value).map(v => ({ [principalType]: v }))); - return iamSet(individualPrincipals); - } - return {}; + function renderPrincipal(x: IPrincipal) { + return JSON.stringify(x.policyFragment.principalJson); } } @@ -144,106 +159,41 @@ function makeComparable(s: StatementSchema): ComparableStatement { * therefore be preserved. */ function unmergeablePrincipals(a: ComparableStatement, b: ComparableStatement) { - const aHasLiteral = Object.values(a.principal).some(v => LITERAL_STRING_KEY in v); - const bHasLiteral = Object.values(b.principal).some(v => LITERAL_STRING_KEY in v); + const aHasLiteral = a.statement.principals.some(v => LITERAL_STRING_KEY in v.policyFragment.principalJson); + const bHasLiteral = b.statement.principals.some(v => LITERAL_STRING_KEY in v.policyFragment.principalJson); return aHasLiteral !== bHasLiteral; } /** - * Turn a ComparableStatement back into a StatementSchema + * Turn a ComparableStatement back into a Statement */ -function renderComparable(s: ComparableStatement): StatementSchema { - return normalizeStatement({ - Effect: s.effect, - Sid: s.sid, - Condition: s.conditionValue, - Action: renderSet(s.action), - NotAction: renderSet(s.notAction), - Resource: renderSet(s.resource), - NotResource: renderSet(s.notResource), - Principal: renderPrincipalSet(s.principal), - NotPrincipal: renderPrincipalSet(s.notPrincipal), - }); - - function renderSet(x: IamValueSet): IamValue | undefined { - // Return as sorted array so that we normalize - const keys = Object.keys(x).sort(); - return keys.length > 0 ? keys.map(key => x[key]) : undefined; - } - - function renderPrincipalSet(x: IamValueSet): Record { - const keys = Object.keys(x).sort(); - // The first level will be an object - const ret: Record = {}; - for (const key of keys) { - const principal = x[key]; - if (principal == null || typeof principal !== 'object') { - throw new Error(`Principal should be an object with a principal type, got: ${principal}`); - } - const principalKeys = Object.keys(principal); - if (principalKeys.length !== 1) { - throw new Error(`Principal should be an object with 1 key, found keys: ${principalKeys}`); - } - const pk = principalKeys[0]; - if (!ret[pk]) { - ret[pk] = []; - } - (ret[pk] as IamValue[]).push(principal[pk]); - } - return ret; - } +function renderComparable(s: ComparableStatement): PolicyStatement { + return s.statement; } /** * An analyzed version of a statement that makes it easier to do comparisons and merging on - * - * We will stringify parts of the statement: comparisons are done on the strings, the original - * values are retained so we can stitch them back together into a real policy. */ interface ComparableStatement { - readonly effect?: string; - readonly sid?: string; - - readonly principal: IamValueSet; - readonly notPrincipal: IamValueSet; - readonly action: IamValueSet; - readonly notAction: IamValueSet; - readonly resource: IamValueSet; - readonly notResource: IamValueSet; - + readonly statement: PolicyStatement; + readonly originals: PolicyStatement[]; + readonly principalStrings: string[]; + readonly notPrincipalStrings: string[]; readonly conditionString: string; - readonly conditionValue: any; + readonly sizeEstimate: number; } -/** - * A collection of comparable IAM values - * - * Each value is indexed by its stringified value, mapping to its original value. - * This allows us to compare values quickly and easily (even if they are complex), - * while also being able to deduplicate the originals. - */ -type IamValueSet = Record; - /** * Whether the given sets are equal */ -function setEqual(a: IamValueSet, b: IamValueSet) { - const keysA = Object.keys(a); - const keysB = Object.keys(b); - return keysA.length === keysB.length && keysA.every(k => k in b); +function setEqual(a: A[], b: A[]) { + const bSet = new Set(b); + return a.length === b.length && a.every(k => bSet.has(k)); } /** * Merge two IAM value sets */ -function setMerge(x: IamValueSet, y: IamValueSet): IamValueSet { - return { ...x, ...y }; -} - -function mkdict(xs: Array<[string, A]>): Record { - const ret: Record = {}; - for (const x of xs) { - ret[x[0]] = x[1]; - } - return ret; -} +function setMerge(x: A[], y: A[]): A[] { + return Array.from(new Set([...x, ...y])).sort(); +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/lib/private/postprocess-policy-document.ts b/packages/@aws-cdk/aws-iam/lib/private/postprocess-policy-document.ts index f54873aa7340c..05193e8467051 100644 --- a/packages/@aws-cdk/aws-iam/lib/private/postprocess-policy-document.ts +++ b/packages/@aws-cdk/aws-iam/lib/private/postprocess-policy-document.ts @@ -1,18 +1,17 @@ import * as cdk from '@aws-cdk/core'; import { LITERAL_STRING_KEY } from '../util'; -import { mergeStatements } from './merge-statements'; /** * A Token postprocesser for policy documents * - * Removes duplicate statements, merges statements, and assign Sids if necessary + * Removes duplicate statements, and assign Sids if necessary * * Because policy documents can contain all kinds of crazy things, * we do all the necessary work here after the document has been mostly resolved * into a predictable CloudFormation form. */ export class PostProcessPolicyDocument implements cdk.IPostProcessor { - constructor(private readonly autoAssignSids: boolean, private readonly minimize: boolean) { + constructor(private readonly autoAssignSids: boolean) { } public postProcess(input: any, _context: cdk.IResolveContext): any { @@ -20,10 +19,6 @@ export class PostProcessPolicyDocument implements cdk.IPostProcessor { return input; } - if (this.minimize) { - input.Statement = mergeStatements(input.Statement); - } - // Also remove full-on duplicates (this will not be necessary if // we minimized, but it might still dedupe statements we didn't // minimize like 'Deny' statements, and definitely is still necessary diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 73ba42c051800..4a84fb8600696 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -1,9 +1,9 @@ -import { ArnFormat, Duration, Resource, Stack, Token, TokenComparison } from '@aws-cdk/core'; +import { ArnFormat, IConstruct, Duration, Resource, Stack, Token, TokenComparison, Aspects, ConcreteDependable, Annotations } from '@aws-cdk/core'; import { Construct, Node } from 'constructs'; import { Grant } from './grant'; import { CfnRole } from './iam.generated'; import { IIdentity } from './identity-base'; -import { IManagedPolicy } from './managed-policy'; +import { IManagedPolicy, ManagedPolicy } from './managed-policy'; import { Policy } from './policy'; import { PolicyDocument } from './policy-document'; import { PolicyStatement } from './policy-statement'; @@ -13,6 +13,9 @@ import { ImmutableRole } from './private/immutable-role'; import { MutatingPolicyDocumentAdapter } from './private/policydoc-adapter'; import { AttachedPolicies, UniqueStringSet } from './util'; +const MAX_INLINE_SIZE = 10000; +const MAX_MANAGEDPOL_SIZE = 6000; + /** * Properties for defining an IAM Role */ @@ -246,6 +249,13 @@ export class Role extends Resource implements IRole { return this.grant(identity, 'iam:PassRole'); } + /** + * Grant permissions to the given principal to pass this role. + */ + public grantAssumeRole(identity: IPrincipal): Grant { + return this.grant(identity, 'sts:AssumeRole'); + } + /** * Grant the actions defined in actions to the identity Principal on this resource. */ @@ -331,7 +341,9 @@ export class Role extends Resource implements IRole { private readonly managedPolicies: IManagedPolicy[] = []; private readonly attachedPolicies = new AttachedPolicies(); private readonly inlinePolicies: { [name: string]: PolicyDocument }; + private readonly dependables = new Map(); private immutableRole?: IRole; + private _didSplit = false; constructor(scope: Construct, id: string, props: RoleProps) { super(scope, id, { @@ -387,6 +399,14 @@ export class Role extends Resource implements IRole { } return result; } + + Aspects.of(this).add({ + visit: (c) => { + if (c === this) { + this.splitLargePolicy(); + } + }, + }); } /** @@ -400,7 +420,13 @@ export class Role extends Resource implements IRole { this.attachInlinePolicy(this.defaultPolicy); } this.defaultPolicy.addStatements(statement); - return { statementAdded: true, policyDependable: this.defaultPolicy }; + + // We might split this statement off into a different policy, so we'll need to + // late-bind the dependable. + const policyDependable = new ConcreteDependable(); + this.dependables.set(statement, policyDependable); + + return { statementAdded: true, policyDependable }; } public addToPolicy(statement: PolicyStatement): boolean { @@ -444,6 +470,14 @@ export class Role extends Resource implements IRole { return this.grant(identity, 'iam:PassRole'); } + /** + * Grant permissions to the given principal to assume this role. + */ + public grantAssumeRole(identity: IPrincipal) { + return this.grant(identity, 'sts:AssumeRole'); + } + + /** * Return a copy of this Role object whose Policies will not be updated * @@ -469,6 +503,59 @@ export class Role extends Resource implements IRole { } return errors; } + + /** + * Split large inline policies into managed policies + * + * This gets around the 10k bytes limit on role policies. + */ + private splitLargePolicy() { + if (!this.defaultPolicy || this._didSplit) { + return; + } + this._didSplit = true; + + const self = this; + const splitDocument = this.defaultPolicy.document; + + const mergeMap = splitDocument._maybeMergeStatements(this); + if (mergeMap === undefined) { + throw new Error('Unexpected operation order: splitLargePolicy() called on already-merged policy document'); + } + const splitOffDocs = splitDocument._splitDocument(MAX_INLINE_SIZE, MAX_MANAGEDPOL_SIZE); + + const mpCount = this.managedPolicies.length + splitOffDocs.length; + if (mpCount > 20) { + Annotations.of(this).addWarning(`Policy too large: ${mpCount} exceeds the maximum of 20 managed policies attached to a Role`); + } else if (mpCount > 10) { + Annotations.of(this).addWarning(`Policy large: ${mpCount} exceeds 10 managed policies attached to a Role, this requires a quota increase`); + } + + // Create the managed policies and fix up the dependencies + markDeclaringConstruct(splitDocument, this.defaultPolicy); + + let i = 1; + for (const splitDoc of splitOffDocs) { + const mp = new ManagedPolicy(this, `OverflowPolicy${i++}`, { + description: `Part of the policies for ${this.node.path}`, + document: splitDoc, + roles: [this], + }); + markDeclaringConstruct(splitDoc, mp); + } + + /** + * Update the Dependables for the statements in the given PolicyDocument to point to the actual declaring construct + */ + function markDeclaringConstruct(doc: PolicyDocument, declaringConstruct: IConstruct) { + for (const statement of doc._statements()) { + const originalStatements = mergeMap!.get(statement) ?? []; + for (const original of originalStatements) { + self.dependables.get(original)?.add(declaringConstruct); + } + } + } + } } /** @@ -498,6 +585,11 @@ export interface IRole extends IIdentity { * Grant permissions to the given principal to pass this role. */ grantPassRole(grantee: IPrincipal): Grant; + + /** + * Grant permissions to the given principal to assume this role. + */ + grantAssumeRole(grantee: IPrincipal): Grant; } function createAssumeRolePolicy(principal: IPrincipal, externalIds: string[]) { diff --git a/packages/@aws-cdk/aws-iam/lib/util.ts b/packages/@aws-cdk/aws-iam/lib/util.ts index 831f625a1fdcf..ae4825ffa0ee0 100644 --- a/packages/@aws-cdk/aws-iam/lib/util.ts +++ b/packages/@aws-cdk/aws-iam/lib/util.ts @@ -74,12 +74,15 @@ export class AttachedPolicies { */ export function mergePrincipal(target: { [key: string]: string[] }, source: { [key: string]: string[] }) { // If one represents a literal string, the other one must be empty - if ((LITERAL_STRING_KEY in source && !isEmptyObject(target)) || - (LITERAL_STRING_KEY in target && !isEmptyObject(source))) { + const sourceKeys = Object.keys(source); + const targetKeys = Object.keys(target); + + if ((LITERAL_STRING_KEY in source && targetKeys.some(k => k !== LITERAL_STRING_KEY)) || + (LITERAL_STRING_KEY in target && sourceKeys.some(k => k !== LITERAL_STRING_KEY))) { throw new Error(`Cannot merge principals ${JSON.stringify(target)} and ${JSON.stringify(source)}; if one uses a literal principal string the other one must be empty`); } - for (const key of Object.keys(source)) { + for (const key of sourceKeys) { target[key] = target[key] ?? []; let value = source[key]; @@ -135,6 +138,6 @@ export class UniqueStringSet implements IResolvable, IPostProcessor { } } -function isEmptyObject(x: { [key: string]: any }): boolean { - return Object.keys(x).length === 0; -} +export function sum(xs: number[]) { + return xs.reduce((a, b) => a + b, 0); +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-iam/test/role.test.ts b/packages/@aws-cdk/aws-iam/test/role.test.ts index e0927e14b80ad..fd0b024d60c22 100644 --- a/packages/@aws-cdk/aws-iam/test/role.test.ts +++ b/packages/@aws-cdk/aws-iam/test/role.test.ts @@ -1,6 +1,6 @@ -import { Template } from '@aws-cdk/assertions'; +import { Template, Match } from '@aws-cdk/assertions'; import { testDeprecated } from '@aws-cdk/cdk-build-tools'; -import { Duration, Stack, App } from '@aws-cdk/core'; +import { Duration, Stack, App, CfnResource } from '@aws-cdk/core'; import { AnyPrincipal, ArnPrincipal, CompositePrincipal, FederatedPrincipal, ManagedPolicy, PolicyStatement, Role, ServicePrincipal, User, Policy, PolicyDocument } from '../lib'; describe('IAM role', () => { @@ -59,6 +59,30 @@ describe('IAM role', () => { }); }); + test('a role can grant AssumeRole permissions', () => { + // GIVEN + const stack = new Stack(); + const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('henk.amazonaws.com') }); + const user = new User(stack, 'User'); + + // WHEN + role.grantAssumeRole(user); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'sts:AssumeRole', + Effect: 'Allow', + Resource: { 'Fn::GetAtt': ['Role1ABCC5F0', 'Arn'] }, + }, + ], + Version: '2012-10-17', + }, + }); + }); + testDeprecated('can supply externalId', () => { // GIVEN const stack = new Stack(); @@ -569,4 +593,64 @@ test('managed policy ARNs are deduplicated', () => { }, ], }); +}); + +describe('role with too large inline policy', () => { + const N = 100; + + let app: App; + let stack: Stack; + let role: Role; + beforeEach(() => { + app = new App(); + stack = new Stack(app, 'my-stack'); + role = new Role(stack, 'MyRole', { + assumedBy: new ServicePrincipal('service.amazonaws.com'), + }); + + for (let i = 0; i < N; i++) { + role.addToPrincipalPolicy(new PolicyStatement({ + actions: ['aws:DoAThing'], + resources: [`arn:aws:service:us-east-1:111122223333:someResource/SomeSpecificResource${i}`], + })); + } + }); + + test('excess gets split off into ManagedPolicies', () => { + // THEN + const template = Template.fromStack(stack); + template.hasResourceProperties('AWS::IAM::ManagedPolicy', { + PolicyDocument: { + Statement: Match.arrayWith([ + Match.objectLike({ + Resource: `arn:aws:service:us-east-1:111122223333:someResource/SomeSpecificResource${N - 1}`, + }), + ]), + }, + Roles: [{ Ref: 'MyRoleF48FFE04' }], + }); + }); + + test('Dependables track the final declaring construct', () => { + // WHEN + const result = role.addToPrincipalPolicy(new PolicyStatement({ + actions: ['aws:DoAThing'], + resources: [`arn:aws:service:us-east-1:111122223333:someResource/SomeSpecificResource${N}`], + })); + + const res = new CfnResource(stack, 'Depender', { + type: 'AWS::Some::Resource', + }); + + expect(result.policyDependable).toBeTruthy(); + res.node.addDependency(result.policyDependable!); + + // THEN + const template = Template.fromStack(stack); + template.hasResource('AWS::Some::Resource', { + DependsOn: [ + 'MyRoleOverflowPolicy13EF5596A', + ], + }); + }); }); \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 91410813fa176..4e6287f72fc2b 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/packages/@aws-cdk/core/lib/stack.ts @@ -791,7 +791,8 @@ export class Stack extends CoreConstruct implements ITaggable { const numberOfResources = Object.keys(resources).length; if (numberOfResources > this.maxResources) { - throw new Error(`Number of resources in stack '${this.node.path}': ${numberOfResources} is greater than allowed maximum of ${this.maxResources}`); + const counts = Object.entries(count(Object.values(resources).map((r: any) => `${r?.Type}`))).map(([type, c]) => `${type} (${c})`).join(', '); + throw new Error(`Number of resources in stack '${this.node.path}': ${numberOfResources} is greater than allowed maximum of ${this.maxResources}: ${counts}`); } else if (numberOfResources >= (this.maxResources * 0.8)) { Annotations.of(this).addInfo(`Number of resources: ${numberOfResources} is approaching allowed maximum of ${this.maxResources}`); } @@ -1357,6 +1358,18 @@ export interface ExportValueOptions { readonly name?: string; } +function count(xs: string[]): Record { + const ret: Record = {}; + for (const x of xs) { + if (x in ret) { + ret[x] += 1; + } else { + ret[x] = 1; + } + } + return ret; +} + // These imports have to be at the end to prevent circular imports import { CfnOutput } from './cfn-output'; import { addDependency } from './deps'; diff --git a/packages/@aws-cdk/pipelines/lib/codepipeline/private/codebuild-factory.ts b/packages/@aws-cdk/pipelines/lib/codepipeline/private/codebuild-factory.ts index 3103586f71546..36ae8728d3376 100644 --- a/packages/@aws-cdk/pipelines/lib/codepipeline/private/codebuild-factory.ts +++ b/packages/@aws-cdk/pipelines/lib/codepipeline/private/codebuild-factory.ts @@ -315,6 +315,14 @@ export class CodeBuildFactory implements ICodePipelineActionFactory { ? { _PROJECT_CONFIG_HASH: projectConfigHash } : {}; + + // Start all CodeBuild projects from a single (shared) Action Role, so that we don't have to generate an Action Role for each + // individual CodeBuild Project and blow out the pipeline policy size (and potentially # of resources in the stack). + const actionRoleCid = 'CodeBuildActionRole'; + const actionRole = options.pipeline.node.tryFindChild(actionRoleCid) as iam.IRole ?? new iam.Role(options.pipeline, actionRoleCid, { + assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'), + }); + stage.addAction(new codepipeline_actions.CodeBuildAction({ actionName: actionName, input: inputArtifact, @@ -323,6 +331,7 @@ export class CodeBuildFactory implements ICodePipelineActionFactory { project, runOrder: options.runOrder, variablesNamespace: options.variablesNamespace, + role: actionRole, // Inclusion of the hash here will lead to the pipeline structure for any changes // made the config of the underlying CodeBuild Project. diff --git a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-graph.test.ts b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-graph.test.ts index a933b4887cb97..e53775e84f382 100644 --- a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-graph.test.ts +++ b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-graph.test.ts @@ -1,5 +1,4 @@ /* eslint-disable import/no-extraneous-dependencies */ -import '@aws-cdk/assert-internal/jest'; import * as cdkp from '../../../lib'; import { ManualApprovalStep, Step } from '../../../lib'; import { Graph, GraphNode, PipelineGraph } from '../../../lib/helpers-internal'; diff --git a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-queries.test.ts b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-queries.test.ts index a859cf98d1bce..f452ff27e5b76 100644 --- a/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-queries.test.ts +++ b/packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-queries.test.ts @@ -1,5 +1,4 @@ /* eslint-disable import/no-extraneous-dependencies */ -import '@aws-cdk/assert-internal/jest'; import * as cdkp from '../../../lib'; import { PipelineQueries } from '../../../lib/helpers-internal/pipeline-queries'; import { AppWithOutput, TestApp } from '../../testhelpers/test-app'; diff --git a/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts b/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts index 3c7a134a5118f..60ba70efaf15a 100644 --- a/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts +++ b/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts @@ -1,9 +1,10 @@ +import { Template, Annotations, Match } from '@aws-cdk/assertions'; import * as ccommit from '@aws-cdk/aws-codecommit'; import * as sqs from '@aws-cdk/aws-sqs'; import * as cdk from '@aws-cdk/core'; import { Construct } from 'constructs'; import * as cdkp from '../../lib'; -import { PIPELINE_ENV, TestApp } from '../testhelpers'; +import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp } from '../testhelpers'; let app: TestApp; @@ -52,24 +53,91 @@ describe('CodePipeline support stack reuse', () => { const supportStackAArtifact = assembly.getStackByName(`PipelineStackA-support-${testStageEnv.region}`); const supportStackBArtifact = assembly.getStackByName(`PipelineStackB-support-${testStageEnv.region}`); - const supportStackATemplate = supportStackAArtifact.template; - expect(supportStackATemplate).toHaveResourceLike('AWS::S3::Bucket', { + const supportStackATemplate = Template.fromJSON(supportStackAArtifact.template); + supportStackATemplate.hasResourceProperties('AWS::S3::Bucket', { BucketName: 'pipelinestacka-support-useplicationbucket80db3753a0ebbf052279', }); - expect(supportStackATemplate).toHaveResourceLike('AWS::KMS::Alias', { + supportStackATemplate.hasResourceProperties('AWS::KMS::Alias', { AliasName: 'alias/pport-ustencryptionalias5cad45754e1ff088476b', }); - const supportStackBTemplate = supportStackBArtifact.template; - expect(supportStackBTemplate).toHaveResourceLike('AWS::S3::Bucket', { + const supportStackBTemplate = Template.fromJSON(supportStackBArtifact.template); + supportStackBTemplate.hasResourceProperties('AWS::S3::Bucket', { BucketName: 'pipelinestackb-support-useplicationbucket1d556ec7f959b336abf8', }); - expect(supportStackBTemplate).toHaveResourceLike('AWS::KMS::Alias', { + supportStackBTemplate.hasResourceProperties('AWS::KMS::Alias', { AliasName: 'alias/pport-ustencryptionalias668c7ffd0de17c9867b0', }); }); }); +test('Policy sizes do not exceed the maximum size', () => { + const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV }); + pipelineStack.node.setContext('@aws-cdk/aws-iam:minimizePolicies', true); + const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', { + crossAccountKeys: true, + }); + + // WHEN + const regions = ['us-east-1', 'us-east-2', 'eu-west-1', 'eu-west-2', 'somethingelse1', 'somethingelse-2', 'yapregion', 'more-region']; + for (let i = 0; i < 70; i++) { + pipeline.addStage(new FileAssetApp(pipelineStack, `App${i}`, { + env: { + account: `account${i}`, + region: regions[i % regions.length], + }, + }), { + post: [ + new cdkp.ShellStep('DoAThing', { commands: ['true'] }), + new cdkp.ShellStep('DoASecondThing', { commands: ['false'] }), + ], + }); + } + + // THEN + const template = Template.fromStack(pipelineStack); + + // Collect policies by role + const rolePolicies: Record = {}; + for (const pol of Object.values(template.findResources('AWS::IAM::Policy'))) { + for (const roleName of pol.Properties?.Roles ?? []) { + const roleLogicalId = roleName.Ref; // Roles: [ { Ref: MyRole } ] + if (!roleLogicalId) { continue; } + + if (!rolePolicies[roleLogicalId]) { + rolePolicies[roleLogicalId] = []; + } + + rolePolicies[roleLogicalId].push(pol.Properties.PolicyDocument); + } + } + + + // Validate sizes + // + // Not entirely accurate, because our "Ref"s and "Fn::GetAtt"s actually need to be evaluated + // to ARNs... but it gives an order-of-magnitude indication. + // 10% of margin for CFN intrinsics like { Fn::Join } and { Ref: 'AWS::Partition' } which don't contribute to + // the ACTUAL size, but do contribute to the measured size here. + const cfnOverheadMargin = 1.10; + + for (const [logId, poldoc] of Object.entries(rolePolicies)) { + const totalJson = JSON.stringify(poldoc); + if (totalJson.length > 10000 * cfnOverheadMargin) { + throw new Error(`Policy for Role ${logId} is too large (${totalJson.length} bytes): ${JSON.stringify(poldoc, undefined, 2)}`); + } + } + + for (const [logId, poldoc] of Object.entries(template.findResources('AWS::IAM::ManagedPolicy'))) { + const totalJson = JSON.stringify(poldoc); + if (totalJson.length > 6000 * cfnOverheadMargin) { + throw new Error(`Managed Policy ${logId} is too large (${totalJson.length} bytes): ${JSON.stringify(poldoc, undefined, 2)}`); + } + } + + Annotations.fromStack(pipelineStack).hasNoWarning('*', Match.anyValue()); +}); + interface ReuseCodePipelineStackProps extends cdk.StackProps { reuseCrossRegionSupportStacks?: boolean; } diff --git a/packages/@aws-cdk/pipelines/test/compliance/basic-behavior.test.ts b/packages/@aws-cdk/pipelines/test/compliance/basic-behavior.test.ts index 85d7d5911dd0a..26a570d602f0d 100644 --- a/packages/@aws-cdk/pipelines/test/compliance/basic-behavior.test.ts +++ b/packages/@aws-cdk/pipelines/test/compliance/basic-behavior.test.ts @@ -2,7 +2,6 @@ import * as fs from 'fs'; import * as path from 'path'; import { Capture, Match, Template } from '@aws-cdk/assertions'; -import '@aws-cdk/assert-internal/jest'; import { Stack, Stage, StageProps, Tags } from '@aws-cdk/core'; import { Construct } from 'constructs'; import { behavior, LegacyTestGitHubNpmPipeline, OneStackApp, BucketStack, PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, stringLike } from '../testhelpers'; diff --git a/packages/@aws-cdk/pipelines/test/testhelpers/test-app.ts b/packages/@aws-cdk/pipelines/test/testhelpers/test-app.ts index a534866d19b47..c240cf1fde841 100644 --- a/packages/@aws-cdk/pipelines/test/testhelpers/test-app.ts +++ b/packages/@aws-cdk/pipelines/test/testhelpers/test-app.ts @@ -1,5 +1,4 @@ /* eslint-disable import/no-extraneous-dependencies */ -import '@aws-cdk/assert-internal/jest'; import * as fs from 'fs'; import * as path from 'path'; import * as ecr_assets from '@aws-cdk/aws-ecr-assets';