From 912839031b26123287ffc38d9a6e95986e0a5c90 Mon Sep 17 00:00:00 2001 From: Romain Marcadier-Muller Date: Tue, 22 Jan 2019 09:45:03 -0800 Subject: [PATCH] feat: Make `roleName` available on `IRole` (#1589) The `iam.Policy` class expected to be provided (concrete) `Role` instances, making it unusable when all that is available is an `IRole` (as should be in most situations). This changes the API to accept an `IRole` instead, increasing the usability. --- .../assert/lib/assertions/have-resource.ts | 2 +- packages/@aws-cdk/aws-iam/lib/lazy-role.ts | 4 ++ packages/@aws-cdk/aws-iam/lib/policy.ts | 8 +-- packages/@aws-cdk/aws-iam/lib/role.ts | 11 +++- .../@aws-cdk/aws-iam/test/test.lazy-role.ts | 59 +++++++++++++++++++ packages/@aws-cdk/aws-iam/test/test.role.ts | 11 ++++ 6 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 packages/@aws-cdk/aws-iam/test/test.lazy-role.ts diff --git a/packages/@aws-cdk/assert/lib/assertions/have-resource.ts b/packages/@aws-cdk/assert/lib/assertions/have-resource.ts index 89ec97f2d1d29..324a83b2f10a1 100644 --- a/packages/@aws-cdk/assert/lib/assertions/have-resource.ts +++ b/packages/@aws-cdk/assert/lib/assertions/have-resource.ts @@ -46,7 +46,7 @@ class HaveResourceAssertion extends Assertion { } public assertUsing(inspector: StackInspector): boolean { - for (const logicalId of Object.keys(inspector.value.Resources)) { + for (const logicalId of Object.keys(inspector.value.Resources || {})) { const resource = inspector.value.Resources[logicalId]; if (resource.Type === this.resourceType) { const propsToCheck = this.part === ResourcePart.Properties ? resource.Properties : resource; diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index 429c7fa4f14cd..fcf8307fadb39 100644 --- a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts @@ -81,6 +81,10 @@ export class LazyRole extends cdk.Construct implements IRole { return this.instantiate().roleId; } + public get roleName(): string { + return this.instantiate().roleName; + } + /** * Returns a Principal object representing the ARN of this role. */ diff --git a/packages/@aws-cdk/aws-iam/lib/policy.ts b/packages/@aws-cdk/aws-iam/lib/policy.ts index 6c5208a7b9dd9..2121441970a85 100644 --- a/packages/@aws-cdk/aws-iam/lib/policy.ts +++ b/packages/@aws-cdk/aws-iam/lib/policy.ts @@ -2,7 +2,7 @@ import { Construct, IDependable, Token } from '@aws-cdk/cdk'; import { Group } from './group'; import { CfnPolicy } from './iam.generated'; import { PolicyDocument, PolicyPrincipal, PolicyStatement } from './policy-document'; -import { Role } from './role'; +import { IRole } from './role'; import { User } from './user'; import { generatePolicyName, undefinedIfEmpty } from './util'; @@ -62,7 +62,7 @@ export interface PolicyProps { * Roles to attach this policy to. * You can also use `attachToRole(role)` to attach this policy to a role. */ - roles?: Role[]; + roles?: IRole[]; /** * Groups to attach this policy to. @@ -99,7 +99,7 @@ export class Policy extends Construct implements IDependable { */ public readonly dependencyElements: IDependable[]; - private readonly roles = new Array(); + private readonly roles = new Array(); private readonly users = new Array(); private readonly groups = new Array(); @@ -156,7 +156,7 @@ export class Policy extends Construct implements IDependable { /** * Attaches this policy to a role. */ - public attachToRole(role: Role) { + public attachToRole(role: IRole) { if (this.roles.find(r => r === role)) { return; } this.roles.push(role); role.attachInlinePolicy(this); diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 11fb37a834529..3f83f67c401e4 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -1,4 +1,4 @@ -import { Construct, IConstruct, IDependable, Output } from '@aws-cdk/cdk'; +import { Construct, IConstruct, IDependable, Output, Stack } from '@aws-cdk/cdk'; import { CfnRole } from './iam.generated'; import { IPrincipal, Policy } from './policy'; import { ArnPrincipal, PolicyDocument, PolicyPrincipal, PolicyStatement } from './policy-document'; @@ -223,6 +223,11 @@ export interface IRole extends IConstruct, IPrincipal, IDependable { */ readonly roleId: string; + /** + * Returns the name of this role. + */ + readonly roleName: string; + /** * Export this role to another stack. */ @@ -295,6 +300,10 @@ class ImportedRole extends Construct implements IRole { return this._roleId; } + public get roleName() { + return Stack.find(this).parseArn(this.roleArn).resourceName!; + } + public export() { return this.props; } diff --git a/packages/@aws-cdk/aws-iam/test/test.lazy-role.ts b/packages/@aws-cdk/aws-iam/test/test.lazy-role.ts new file mode 100644 index 0000000000000..83804807ebeea --- /dev/null +++ b/packages/@aws-cdk/aws-iam/test/test.lazy-role.ts @@ -0,0 +1,59 @@ +import { expect, haveResource, haveResourceLike } from '@aws-cdk/assert'; +import cdk = require('@aws-cdk/cdk'); +import nodeunit = require('nodeunit'); +import iam = require('../lib'); + +export = nodeunit.testCase({ + 'creates no resource when unused'(test: nodeunit.Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new iam.LazyRole(stack, 'Lazy', { + assumedBy: new iam.ServicePrincipal('test.amazonaws.com') + }); + + // THEN + expect(stack).notTo(haveResourceLike('AWS::IAM::Role')); + test.done(); + }, + + 'creates the resource when a property is read'(test: nodeunit.Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const roleArn = new iam.LazyRole(stack, 'Lazy', { + assumedBy: new iam.ServicePrincipal('test.amazonaws.com') + }).roleArn; + + // THEN + test.notEqual(roleArn, null); + expect(stack).to(haveResource('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Version: '2012-10-17', + Statement: [{ + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { Service: 'test.amazonaws.com' } + }] + } + })); + test.done(); + }, + + 'returns appropriate roleName'(test: nodeunit.Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const role = new iam.LazyRole(stack, 'Lazy', { + assumedBy: new iam.ServicePrincipal('test.amazonaws.com') + }); + + // THEN + test.deepEqual(stack.node.resolve(role.roleName), + { Ref: 'Lazy399F7F48'}); + test.done(); + } +}); diff --git a/packages/@aws-cdk/aws-iam/test/test.role.ts b/packages/@aws-cdk/aws-iam/test/test.role.ts index 54ea02a6a3121..73f92671383ee 100644 --- a/packages/@aws-cdk/aws-iam/test/test.role.ts +++ b/packages/@aws-cdk/aws-iam/test/test.role.ts @@ -256,6 +256,17 @@ export = { test.deepEqual(stack.node.resolve(importedRole.roleArn), { 'Fn::ImportValue': 'MyRoleRoleArn3388B7E2' }); test.deepEqual(stack.node.resolve(importedRole.roleId), { 'Fn::ImportValue': 'MyRoleRoleIdF7B258D8' }); + test.deepEqual(stack.node.resolve(importedRole.roleName), { + 'Fn::Select': [ 1, { + 'Fn::Split': [ '/', { + 'Fn::Select': [ 5, { + 'Fn::Split': [ ':', { + 'Fn::ImportValue': 'MyRoleRoleArn3388B7E2' + } ] + } ] + } ] + } ] + }); test.done(); } };