Aws-cdk: Immutable roles cannot be used as constructs

[markusl]

Hi!

After upgrading to CDK 1.29.0 we are seeing the following error:

Error: construct does not have an associated node
    at Function.of (/node_modules/constructs/lib/construct.ts:31:13)
    at new Node (/node_modules/constructs/lib/construct.ts:74:12)
    at new ConstructNode (/node_modules/@aws-cdk/core/lib/construct-compat.ts:260:24)
    at Object.createNode (/node_modules/@aws-cdk/core/lib/construct-compat.ts:69:11)
    at new Construct (/node_modules/constructs/lib/construct.ts:541:26)
    at new Construct (/node_modules/@aws-cdk/core/lib/construct-compat.ts:66:5)
    at new SingletonPolicy (/node_modules/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts:517:5)
    at Function.forRole (/node_modules/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts:507:42)
    at CloudFormationCreateUpdateStackAction.bound (/node_modules/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts:300:21)
    at CloudFormationCreateUpdateStackAction.bound (/node_modules/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts:436:32)

Reproduction Steps

A full repro here https://github.com/markusl/temp-aws-issue-repro

Environment

• CLI Version : 0.30.0
• Framework Version: 0.30.0
• OS : Macbook
• Language : TypeScript

---

This is :bug: Bug Report

[alan-cooney]

Also experiencing this on 1.30.0

[eladb]

Seems like a regression. Looking into it. Thanks for reporting

[eladb]

@markusl what is role? Is it an imported role (i.e. iam.Role.fromXxx) or a role that you define in your stack?

[eladb]

@markusl @alan-cooney can I also ask you guys to completely nuke your node_modules directory and package-lock/yarn.lock file and verify that all your CDK dependencies in package.json point to 1.30.0. Then run yarn install or npm install again and check if this reoccurs.

I suspect that your IAM library is older than 1.29.0

[eladb]

I am unable to reproduce this with a clean 1.30.0 dependency closure.

[alan-cooney]

Hi eladb - yes I'll try that now. I've been playing around with it and, for me, the issue occurs only when the role property is set.

[eladb]

Hi eladb - yes I'll try that now. I've been playing around with it and, for me, the issue occurs only when the role property is set.

Makes sense because this error stems from an attempt to add a child construct to the role, which only happens if the role is defined. Since version 1.29.0, the way we associate constructs to nodes had changed and I suspect this is the reason there is an incompatibility.

[martzcodes]

Following this: https://docs.aws.amazon.com/cdk/latest/guide/serverless_example.html

I get the same error. If you downgrade @aws-cdk/aws-core and @aws-cdk/aws-lambda to 1.27.0 the error goes away. I also notice that typescript identifies the error in creating of the lambda function new lambda.Function(this,. the this in this case has the typescript error: Argument of type 'this' is not assignable to parameter of type 'Construct'.

[eladb]

Following this: https://docs.aws.amazon.com/cdk/latest/guide/serverless_example.html

I get the same error. If you downgrade @aws-cdk/aws-core and @aws-cdk/aws-lambda to 1.27.0 the error goes away. I also notice that typescript identifies the error in creating of the lambda function new lambda.Function(this,. the this in this case has the typescript error: Argument of type 'this' is not assignable to parameter of type 'Construct'.

Can you please try to nuke & recreate your node_modules and make sure package.json has a single version for all CDK deps?

[eladb]

@martzcodes @markusl can you please paste the output of npm ls here?

[eladb]

Here is the code that I am trying to use for a repro:

import * as cdk from '@aws-cdk/core';
import * as iam from '@aws-cdk/aws-iam';
import * as pipeline from '@aws-cdk/aws-codepipeline';
import * as actions from '@aws-cdk/aws-codepipeline-actions';
import * as cc from '@aws-cdk/aws-codecommit';

export class ReproV187279444Stack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);
    const role = iam.Role.fromRoleArn(this, 'myRole', 'arn:aws:iam::account-id:role/role-name-with-path');
    // also tried this: const role = new iam.Role(this, 'MyRole', {assumedBy: new iam.AnyPrincipal()});
    const buildOutput = new pipeline.Artifact();

    new pipeline.Pipeline(this, 'Pipeline', {
      stages: [
        {
          stageName: 'Source',
          actions: [
            new actions.CodeCommitSourceAction({
              actionName: 'CodeCommit',
              repository: new cc.Repository(this, 'Repo', {
                repositoryName: 'foobar'
              }),
              output: buildOutput
            })
          ]
        },
        {
          stageName: 'Deploy',
          actions: [
            new actions.CloudFormationCreateUpdateStackAction({
              runOrder: 2,
              stackName: 'cluster-stack',
              actionName: 'UpdateProdCluster',
              templatePath: new pipeline.ArtifactPath(buildOutput, 'cdk.out/cluster.template.json'),
              role: role,
              adminPermissions: false,
              deploymentRole: role,
            }),
          ]
        }
      ]
    });
  }
}

[eladb]

@markusl can you take a look and let me know if something stands out as dramatically different from your code? The above code does not throw this error.

[markusl]

Not sure which of our instances are the problematic ones, if this does not always happen.

We also have one different from the previous, using parameters for cross-account deployment of a lambda zip:

    new codepipeline_actions.CloudFormationCreateUpdateStackAction({
        stackName,
        account,
        region: 'eu-central-1',
        actionName: stageName,
        templatePath: new codepipeline.ArtifactPath(buildOutput, `cdk/cdk.out/${stackName}.template.json`),
        parameterOverrides: {
          ['BucketNameParam']: buildOutput.s3Location.bucketName,
          ['ObjectKeyParam']: buildOutput.s3Location.objectKey,
        },
        adminPermissions: true,
        capabilities: [cloudformation.CloudFormationCapabilities.NAMED_IAM],
        role: deployerRole,
        deploymentRole: deployerRole,
        runOrder: 2,
    }),

[skinny85]

@markusl can you maybe push your code (with whatever edits you need to do to keep it anonymized, of course) to a GitHub repo, so I can clone it and see the failures?

We can't reproduce the error you're getting on our side.

Thanks,
Adam

[martzcodes]

@skinny85 here's a bare bones recreation on my machine: https://github.com/martzcodes/aws-recreate-issue

$ node --version
v12.13.1
$ npm --version
6.12.1
$ cdk --version
1.27.0 (build a98c0b3)

Also note:

npm run build

> [email protected] build /Users/mattmartz/Development/aws-recreate
> tsc

lib/widget_service.ts:8:41 - error TS2345: Argument of type 'this' is not assignable to parameter of type 'Construct'.
  Type 'WidgetService' is missing the following properties from type 'Construct': onValidate, onPrepare, onSynthesize

8     const handler = new lambda.Function(this, "WidgetHandler", {
                                          ~~~~


Found 1 error.

and

$ cdk synth
construct does not have an associated node
Subprocess exited with error 1

[markusl]

@skinny85 I was trying to narrow this down to as little code as possible. Unfortunately our infrastructure stacks are quite large which means I'm unable to share a full repro.

However, commenting out the following snippet in one of our stacks makes the error disappear:

 // Stage for a cross-account deployment
  const role = iam.Role.fromRoleArn(stack, 'DeployerRole', DevAccountDeployerRoleArn);
  pipeline.addStage({
    stageName: 'DeployDev',
    actions: [
      new codepipeline_actions.CloudFormationCreateUpdateStackAction({
        stackName: 'project-dev-cluster',
        account: DevAccountNumber,
        region: 'eu-central-1',
        actionName: 'UpdateDevCluster',
        templatePath: new codepipeline.ArtifactPath(buildOutput, 'project/cdk.out/project-dev-cluster.template.json'),
        role,
        adminPermissions: false,
        deploymentRole: role,
        capabilities: [cloudformation.CloudFormationCapabilities.NAMED_IAM]
      }),
    ]
  });

[skinny85]

@markusl can you check if commenting out this line:

        role,

makes the error disappear?

[skinny85]

@skinny85 here's a bare bones recreation on my machine: https://github.com/martzcodes/aws-recreate-issue

$ node --version
v12.13.1
$ npm --version
6.12.1
$ cdk --version
1.27.0 (build a98c0b3)

Thanks for the repo @martzcodes , but yours is a different error, and a pretty obvious case of mixing multiple versions of the CDK in one app. npm ls clearly shows:

[email protected] /Users/adamruka/workplace/cdk/on-call/construct-doesnt-have-associated-node
├─┬ @aws-cdk/[email protected]
│ ├─┬ @aws-cdk/[email protected]
│ │ ├─┬ @aws-cdk/[email protected]
│ │ │ └─┬ [email protected]
│ │ │   ├── [email protected]
│ │ │   ├── [email protected]
│ │ │   └── [email protected]
│ │ ├── [email protected] deduped
│ │ ├── [email protected] deduped
│ │ ├── [email protected]
│ │ ├─┬ [email protected]
│ │ │ ├── [email protected]
│ │ │ ├── [email protected]
│ │ │ └─┬ [email protected]
│ │ │   └── [email protected]
│ │ └── [email protected] deduped
│ ├── UNMET PEER DEPENDENCY @aws-cdk/[email protected] deduped
│ └─┬ @aws-cdk/[email protected]
│   └── [email protected]
├─┬ @aws-cdk/[email protected]
│ ├─┬ @aws-cdk/[email protected]
...

[markusl]

@markusl can you check if commenting out this line:

        role,

makes the error disappear?

Yes, that does it! Is there something we should change in our code to make this work with the latest version of CDK?

[skinny85]

@markusl if you can't share your entire code, can you at least share you package.json and package-lock.json?

[martzcodes]

Thanks for the repo @martzcodes , but yours is a different error, and a pretty obvious case of mixing multiple versions of the CDK in one app. npm ls clearly shows:

Well at the crux of it @markusl reported the error of Error: construct does not have an associated node which I am clearly also getting... you should make the cdk init be more resilient against mixing incompatible versions 😜

[skinny85]

Thanks for the repo @martzcodes , but yours is a different error, and a pretty obvious case of mixing multiple versions of the CDK in one app. npm ls clearly shows:

Well at the crux of it @markusl reported the error of Error: construct does not have an associated node which I am clearly also getting... you should make the cdk init be more resilient against mixing incompatible versions 😜

Running npm run cdk synth on the code you posted in the GitHub repo, I get:

lib/widget_service.ts:8:41 - error TS2345: Argument of type 'this' is not assignable to parameter of type 'Construct'.
  Type 'WidgetService' is missing the following properties from type 'Construct': onValidate, onPrepare, onSynthesize

which is a different error. Your package.json has:

    "@aws-cdk/aws-lambda": "^1.30.0",
    "@aws-cdk/core": "1.27.0",

Can you let me know if the error goes away if you unify the version to 1.30.0?

[markusl]

@markusl if you can't share your entire code, can you at least share you package.json and package-lock.json?

Added a repro here https://github.com/markusl/temp-aws-issue-repro

[skinny85]

@markusl thanks. I see you're not using a local CDK CLI in your package.json, but instead a global one, which is on an old version (1.27.0). Can you try upgrading it to 1.30.0, and see if that changes anything?

[markusl]

@markusl thanks. I see you're not using a local CDK CLI in your package.json, but instead a global one, which is on an old version (1.27.0). Can you try upgrading it to 1.30.0, and see if that changes anything?

The error seems to stay even when I'm using

$ cdk --version
1.30.0 (build 4f54ff7)

if that's what you meant.

[skinny85]

I cannot reproduce the error, even using your package.json and package-lock.json files. It has to be something specific in your app.

[skinny85]

Nope. Here's what I get when running that code:

$ npx cdk synth
npx: installed 201 in 41.585s
npx: installed 8 in 3.013s
construct does not have an associated node

/Users/adamruka/workplace/cdk/on-call/construct-doesnt-have-associated-node2/node_modules/constructs/lib/construct.ts:407
        throw new Error(`Validation failed with the following errors:\n  ${errorList}`);
              ^
Error: Validation failed with the following errors:
  [pipeline/Pipeline] Pipeline must have at least two stages

Can you push the entire example (including cdk.json, etc.) to GitHub now? There must be something else at play.

[markusl]

Hi!

I removed some comments for clarity. A minified example of the problem is here: https://github.com/markusl/temp-aws-issue-repro

I tried it with two Macbooks and could reproduce it on both.

[eladb]

@markusl managed to repro with your example. Thanks for taking the time.

Investigating...

[eladb]

Got it. This is indeed a bug!

The IAM role you are importing is from a different account. Therefore, Role.fromArn returns an IRole that is backed by an ImmutableRole object (because there is no way for you to mutate a role in a different account). So far so good.

The culprit is that SingletonPolicy inside codepipeline-actions assumes the IRole passed to it is a Construct by performing an explicit downcast (role as unknown as cdk.Construct). This worked in the past because ImmutableRole exposes node property which allowed it to implement IConstruct.

https://github.com/aws/aws-cdk/blob/4501b8ba566ac776042fc97435d4db96fc421e0b/packages/%40aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts#L517

In 1.29.0, due to some constraints when we extracted constructs into an outside library, we had to change the way nodes are associated with constructs and therefore it is now impossible to downcast objects that implement IConstruct to Construct.

[eladb]

Created https://github.com/aws/constructs/issues/16

[eladb]

There is another place in the framework that may be susceptible to this:

https://github.com/aws/aws-cdk/blob/a5a628484f97e9c80c211f78182c12d08151d650/packages/%40aws-cdk/aws-events-targets/lib/util.ts#L17

[eladb]

Okay, after some investigation. This is the only case in our codebase that performs this "forced downcast" (scope as unknown as Construct). In the event-targets case scope is promised to actually be a Construct.

Therefore, I am opting to simply fix this case by changing ImmutableRole to be a real construct.

[eladb]

@markusl I also have a super hacky workaround for you:

const role = iam.Role.fromRoleArn(stack, 'DeployerRole', deployerRoleArn);

// associate the role to a construct node
Object.defineProperty(role, Symbol.for('constructs.Construct.node'), {
  value: (role.node as any)._actualNode
});

[markusl]

@eladb that seems to do the trick. Huge thanks for looking into this on Saturday!

[eladb]

Glad to hear it works.

[rix0rrr]

ImmutableRole should never have been a construct, but because this is now law and because of backwards compatibility, we can't undo this anymore.