diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 4b19fb7a93c99..0308899dc10ec 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -277,15 +277,20 @@ export class Role extends Resource implements IRole { throw new Error('\'addGrantsToResources\' can only be passed if \'mutable: false\''); } - const importedRole = new Import(scope, id); - const roleArnAndScopeStackAccountComparison = Token.compareStrings(importedRole.env.account, scopeStack.account); + const roleArnAndScopeStackAccountComparison = Token.compareStrings(roleAccount ?? '', scopeStack.account); const equalOrAnyUnresolved = roleArnAndScopeStackAccountComparison === TokenComparison.SAME || roleArnAndScopeStackAccountComparison === TokenComparison.BOTH_UNRESOLVED || roleArnAndScopeStackAccountComparison === TokenComparison.ONE_UNRESOLVED; + + // if we are returning an immutable role then the 'importedRole' is just a throwaway construct + // so give it a different id + const mutableRoleId = (options.mutable !== false && equalOrAnyUnresolved) ? id : `MutableRole${id}`; + const importedRole = new Import(scope, mutableRoleId); + // we only return an immutable Role if both accounts were explicitly provided, and different return options.mutable !== false && equalOrAnyUnresolved ? importedRole - : new ImmutableRole(scope, `ImmutableRole${id}`, importedRole, options.addGrantsToResources ?? false); + : new ImmutableRole(scope, id, importedRole, options.addGrantsToResources ?? false); } /** @@ -655,4 +660,4 @@ export interface WithoutPolicyUpdatesOptions { * @default false */ readonly addGrantsToResources?: boolean; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts index 12cb38dc9542b..b79dd3a55c23d 100644 --- a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts +++ b/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts @@ -1,4 +1,4 @@ -import { Template } from '@aws-cdk/assertions'; +import { Template, Match } from '@aws-cdk/assertions'; import { Stack } from '@aws-cdk/core'; import * as iam from '../lib'; @@ -53,6 +53,57 @@ describe('ImmutableRole', () => { }); }); + test('id of mutable role remains unchanged', () => { + iam.Role.fromRoleName(stack, 'TestRole123', 'my-role'); + expect(stack.node.tryFindChild('TestRole123')).not.toBeUndefined(); + expect(stack.node.tryFindChild('MutableRoleTestRole123')).toBeUndefined(); + }); + + test('remains mutable when called multiple times', () => { + const user = new iam.User(stack, 'User'); + const policy = new iam.Policy(stack, 'Policy', { + statements: [new iam.PolicyStatement({ + resources: ['*'], + actions: ['s3:*'], + })], + users: [user], + }); + + function findRole(): iam.IRole { + const foundRole = stack.node.tryFindChild('MyRole') as iam.IRole; + if (foundRole) { + return foundRole; + } + return iam.Role.fromRoleArn(stack, 'MyRole', 'arn:aws:iam::12345:role/role-name', { mutable: false }); + } + + let foundRole = findRole(); + foundRole.attachInlinePolicy(policy); + foundRole = findRole(); + foundRole.attachInlinePolicy(policy); + + expect(stack.node.tryFindChild('MutableRoleMyRole')).not.toBeUndefined(); + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', { + 'PolicyDocument': { + 'Statement': [ + { + 'Action': 's3:*', + 'Resource': '*', + 'Effect': 'Allow', + }, + ], + 'Version': '2012-10-17', + }, + 'PolicyName': 'Policy23B91518', + 'Roles': Match.absent(), + 'Users': [ + { + 'Ref': 'User00B015A1', + }, + ], + }); + }); + test('ignores calls to addManagedPolicy', () => { mutableRole.addManagedPolicy({ managedPolicyArn: 'Arn1' }); @@ -117,4 +168,4 @@ describe('ImmutableRole', () => { new Construct(immutableRole as unknown as Construct, 'Child'); new Construct(mutableRole.withoutPolicyUpdates() as unknown as Construct, 'Child2'); }); -}); \ No newline at end of file +});