fix(dam): immutable role cannot be used as a construct

Elad Ben-Israel · Elad Ben-Israel · commit 7c9cb6dcd50c · 2020-03-21T17:21:38.000+02:00
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
diff --git a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts b/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';
@@ -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 ]
diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts
@@ -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 {
@@ -401,7 +401,7 @@ export class Role extends Resource implements IRole {
    * Role's policies yourself.
    */
   public withoutPolicyUpdates(): IRole {
-    return new ImmutableRole(this);
+    return new ImmutableRole(this.node.scope as Construct, `ImmutableRole${this.node.id}`, this);
   }
 }
 
diff --git a/packages/@aws-cdk/aws-iam/test/immutable-role.test.ts b/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
@@ -106,4 +106,10 @@ 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');
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -235,7 +235,7 @@ export class Role extends Resource implements IRole {`
`235`	`235`
`236`	`236`	`return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount)`
`237`	`237`	`? new Import(scope, id)`
`238`		`- : new ImmutableRole(new Import(scope, id));`
	`238`	+ : new ImmutableRole(scope, `ImmutableRole${id}`, new Import(scope, id));
`239`	`239`
`240`	`240`	`function accountsAreEqualOrOneIsUnresolved(account1: string \| undefined,`
`241`	`241`	`account2: string \| undefined): boolean {`
`@@ -401,7 +401,7 @@ export class Role extends Resource implements IRole {`
`401`	`401`	`* Role's policies yourself.`
`402`	`402`	`*/`
`403`	`403`	`public withoutPolicyUpdates(): IRole {`
`404`		`- return new ImmutableRole(this);`
	`404`	+ return new ImmutableRole(this.node.scope as Construct, `ImmutableRole${this.node.id}`, this);
`405`	`405`	`}`
`406`	`406`	`}`
`407`	`407`