Skip to content

Commit

Permalink
fix(iam): immutable role cannot be used as a construct (#6920)
Browse files Browse the repository at this point in the history
* fix(dam): immutable role cannot be used as a construct

Due to a change in how `ConstructNode`s are associated with `Construct`s in 1.29.0, `ImmutableRole`'s "impersonation to a construct" -- by reflecting the construct's `node` property -- no longer works.

This change simply turns `ImmutableRole` into a real construct by extending the `Construct` base class.

This fixes the use case in #6885

* memoize immutable role so it can be called any number of times
  • Loading branch information
Elad Ben-Israel committed Mar 23, 2020
1 parent 4501b8b commit 56be032
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
9 changes: 5 additions & 4 deletions packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
@@ -1,4 +1,4 @@
import { DependableTrait } from '@aws-cdk/core';
import { Construct, DependableTrait } from '@aws-cdk/core';
import { Grant } from '../grant';
import { IManagedPolicy } from '../managed-policy';
import { Policy } from '../policy';
Expand All @@ -19,16 +19,17 @@ import { IRole } from '../role';
* which was imported into the CDK with {@link Role.fromRoleArn}, you don't have to use this class -
* simply pass the property mutable = false when calling {@link Role.fromRoleArn}.
*/
export class ImmutableRole implements IRole {
export class ImmutableRole extends Construct implements IRole {
public readonly assumeRoleAction = this.role.assumeRoleAction;
public readonly policyFragment = this.role.policyFragment;
public readonly grantPrincipal = this;
public readonly roleArn = this.role.roleArn;
public readonly roleName = this.role.roleName;
public readonly node = this.role.node;
public readonly stack = this.role.stack;

constructor(private readonly role: IRole) {
constructor(scope: Construct, id: string, private readonly role: IRole) {
super(scope, id);

// implement IDependable privately
DependableTrait.implement(this, {
dependencyRoots: [ role ]
Expand Down
9 changes: 7 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Expand Up @@ -235,7 +235,7 @@ export class Role extends Resource implements IRole {

return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount)
? new Import(scope, id)
: new ImmutableRole(new Import(scope, id));
: new ImmutableRole(scope, `ImmutableRole${id}`, new Import(scope, id));

function accountsAreEqualOrOneIsUnresolved(account1: string | undefined,
account2: string | undefined): boolean {
Expand Down Expand Up @@ -284,6 +284,7 @@ export class Role extends Resource implements IRole {
private defaultPolicy?: Policy;
private readonly managedPolicies: IManagedPolicy[] = [];
private readonly attachedPolicies = new AttachedPolicies();
private immutableRole?: IRole;

constructor(scope: Construct, id: string, props: RoleProps) {
super(scope, id, {
Expand Down Expand Up @@ -401,7 +402,11 @@ export class Role extends Resource implements IRole {
* Role's policies yourself.
*/
public withoutPolicyUpdates(): IRole {
return new ImmutableRole(this);
if (!this.immutableRole) {
this.immutableRole = new ImmutableRole(this.node.scope as Construct, `ImmutableRole${this.node.id}`, this);
}

return this.immutableRole;
}
}

Expand Down
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-iam/test/immutable-role.test.ts
@@ -1,5 +1,5 @@
import '@aws-cdk/assert/jest';
import { Stack } from '@aws-cdk/core';
import { Construct, Stack } from '@aws-cdk/core';
import * as iam from '../lib';

// tslint:disable:object-literal-key-quotes
Expand Down Expand Up @@ -106,4 +106,11 @@ describe('ImmutableRole', () => {
},
});
});

// this pattern is used here:
// aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts#L517
test('immutable role is a construct', () => {
new Construct(immutableRole as unknown as Construct, 'Child');
new Construct(mutableRole.withoutPolicyUpdates() as unknown as Construct, 'Child2');
});
});

0 comments on commit 56be032

Please sign in to comment.