diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts index 0617aacf6a5a1..0afee0220a5f7 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy.ts @@ -96,7 +96,7 @@ export class Policy extends Resource implements IPolicy { // generatePolicyName will take the last 128 characters of the logical id since // policy names are limited to 128. the last 8 chars are a stack-unique hash, so // that shouod be sufficient to ensure uniqueness within a principal. - Lazy.stringValue({ produce: () => generatePolicyName(resource.logicalId) }) + Lazy.stringValue({ produce: () => generatePolicyName(scope, resource.logicalId) }) }); const resource = new CfnPolicy(this, 'Resource', { diff --git a/packages/@aws-cdk/aws-iam/lib/util.ts b/packages/@aws-cdk/aws-iam/lib/util.ts index 9203c0efbb6f7..cd1a29cd7d7c3 100644 --- a/packages/@aws-cdk/aws-iam/lib/util.ts +++ b/packages/@aws-cdk/aws-iam/lib/util.ts @@ -1,4 +1,4 @@ -import { Lazy } from '@aws-cdk/core'; +import { DefaultTokenResolver, IConstruct, Lazy, StringConcat, Tokenization } from '@aws-cdk/core'; import { IPolicy } from './policy'; const MAX_POLICY_NAME_LEN = 128; @@ -16,8 +16,25 @@ export function undefinedIfEmpty(f: () => string[]): string[] { * 128 characters, so we take the last 128 characters (in order to make sure the hash * is there). */ -export function generatePolicyName(logicalId: string) { - return logicalId.substring(Math.max(logicalId.length - MAX_POLICY_NAME_LEN, 0), logicalId.length); +export function generatePolicyName(scope: IConstruct, logicalId: string): string { + // as logicalId is itself a Token, resolve it first + const resolvedLogicalId = Tokenization.resolve(logicalId, { + scope, + resolver: new DefaultTokenResolver(new StringConcat()), + }); + return lastNCharacters(resolvedLogicalId, MAX_POLICY_NAME_LEN); +} + +/** + * Returns a string composed of the last n characters of str. + * If str is shorter than n, returns str. + * + * @param str the string to return the last n characters of + * @param n how many characters to return + */ +function lastNCharacters(str: string, n: number) { + const startIndex = Math.max(str.length - n, 0); + return str.substring(startIndex, str.length); } /** @@ -61,4 +78,4 @@ export function mergePrincipal(target: { [key: string]: string[] }, source: { [k } return target; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/test/test.policy.ts b/packages/@aws-cdk/aws-iam/test/test.policy.ts index 7fe0fb17affb6..5fbfd57c5d9c2 100644 --- a/packages/@aws-cdk/aws-iam/test/test.policy.ts +++ b/packages/@aws-cdk/aws-iam/test/test.policy.ts @@ -1,8 +1,9 @@ -import { expect } from '@aws-cdk/assert'; +import { expect, haveResourceLike } from '@aws-cdk/assert'; import { App, Stack } from '@aws-cdk/core'; import { Test } from 'nodeunit'; -import { Group, Policy, PolicyStatement, Role, ServicePrincipal, User } from '../lib'; -import { generatePolicyName } from '../lib/util'; +import { AnyPrincipal, CfnPolicy, Group, Policy, PolicyStatement, Role, ServicePrincipal, User } from '../lib'; + +// tslint:disable:object-literal-key-quotes export = { 'fails when policy is empty'(test: Test) { @@ -254,17 +255,29 @@ export = { test.done(); }, + "generated policy name is the same as the logical id if it's shorter than 128 characters"(test: Test) { + const stack = new Stack(); + + createPolicyWithLogicalId(stack, 'Foo'); + + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + "PolicyName": "Foo", + })); + + test.done(); + }, + 'generated policy name only uses the last 128 characters of the logical id'(test: Test) { - test.equal(generatePolicyName('Foo'), 'Foo'); + const stack = new Stack(); - const logicalId50 = '[' + dup(50 - 2) + ']'; - test.equal(generatePolicyName(logicalId50), logicalId50); + const logicalId128 = 'a' + dup(128 - 2) + 'a'; + const logicalIdOver128 = 'PREFIX' + logicalId128; - const logicalId128 = '[' + dup(128 - 2) + ']'; - test.equal(generatePolicyName(logicalId128), logicalId128); + createPolicyWithLogicalId(stack, logicalIdOver128); - const withPrefix = 'PREFIX' + logicalId128; - test.equal(generatePolicyName(withPrefix), logicalId128, 'ensure prefix is omitted'); + expect(stack).to(haveResourceLike('AWS::IAM::Policy', { + "PolicyName": logicalId128, + })); test.done(); @@ -275,5 +288,18 @@ export = { } return r; } - } + }, }; + +function createPolicyWithLogicalId(stack: Stack, logicalId: string): void { + const policy = new Policy(stack, logicalId); + const cfnPolicy = policy.node.defaultChild as CfnPolicy; + cfnPolicy.overrideLogicalId(logicalId); // force a particular logical ID + + // add statements & principal to satisfy validation + policy.addStatements(new PolicyStatement({ + actions: ['*'], + resources: ['*'], + })); + policy.attachToRole(new Role(stack, 'Role', { assumedBy: new AnyPrincipal() })); +}