Immutable roles cannot be used as constructs #6885

markusl · 2020-03-20T08:04:32Z

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 Report

The text was updated successfully, but these errors were encountered:

alan-cooney · 2020-03-20T09:08:25Z

Also experiencing this on 1.30.0

eladb · 2020-03-20T11:23:29Z

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

eladb · 2020-03-20T12:12:19Z

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

eladb · 2020-03-20T12:21:58Z

@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 · 2020-03-20T12:29:26Z

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

alan-cooney · 2020-03-20T12:32:24Z

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 · 2020-03-20T13:12:29Z

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 · 2020-03-20T13:59:50Z

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 · 2020-03-20T14:21:08Z

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 · 2020-03-20T14:22:50Z

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

eladb · 2020-03-20T14:42:25Z

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 · 2020-03-20T14:43:18Z

@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 · 2020-03-20T14:58:15Z

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 · 2020-03-20T18:45:59Z

@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 · 2020-03-20T19:00:50Z

@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

> aws-recreate@0.1.0 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 · 2020-03-20T19:04:23Z

@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 · 2020-03-20T19:07:39Z

@markusl can you check if commenting out this line:

        role,

makes the error disappear?

skinny85 · 2020-03-20T19:15:40Z

@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:

aws-recreate@0.1.0 /Users/adamruka/workplace/cdk/on-call/construct-doesnt-have-associated-node
├─┬ @aws-cdk/assert@1.27.0
│ ├─┬ @aws-cdk/cloudformation-diff@1.27.0
│ │ ├─┬ @aws-cdk/cfnspec@1.27.0
│ │ │ └─┬ md5@2.2.1
│ │ │   ├── charenc@0.0.2
│ │ │   ├── crypt@0.0.2
│ │ │   └── is-buffer@1.1.6
│ │ ├── colors@1.4.0 deduped
│ │ ├── diff@4.0.2 deduped
│ │ ├── fast-deep-equal@3.1.1
│ │ ├─┬ string-width@4.2.0
│ │ │ ├── emoji-regex@8.0.0
│ │ │ ├── is-fullwidth-code-point@3.0.0
│ │ │ └─┬ strip-ansi@6.0.0
│ │ │   └── ansi-regex@5.0.0
│ │ └── table@5.4.6 deduped
│ ├── UNMET PEER DEPENDENCY @aws-cdk/core@1.27.0 deduped
│ └─┬ @aws-cdk/cx-api@1.27.0
│   └── semver@7.1.3
├─┬ @aws-cdk/aws-lambda@1.30.0
│ ├─┬ @aws-cdk/aws-cloudwatch@1.30.0
...

markusl · 2020-03-20T19:48:40Z

@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 · 2020-03-20T20:04:27Z

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

martzcodes · 2020-03-20T20:08:03Z

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 · 2020-03-20T20:11:04Z

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 · 2020-03-20T20:20:14Z

@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 · 2020-03-20T20:43:08Z

@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 · 2020-03-20T20:54:01Z

@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 · 2020-03-20T21:02:58Z

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 · 2020-03-20T21:35:57Z

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 · 2020-03-21T08:40:40Z

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 · 2020-03-21T13:50:29Z

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

Investigating...

eladb · 2020-03-21T14:19:31Z

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.

aws-cdk/packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts

Line 517 in 4501b8b

super(role as unknown as cdk.Construct, SingletonPolicy.UUID);

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 · 2020-03-21T14:25:23Z

Created aws/constructs#16

eladb · 2020-03-21T14:30:42Z

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

aws-cdk/packages/@aws-cdk/aws-events-targets/lib/util.ts

Line 17 in a5a6284

const role = new iam.Role(scope as Construct, id, {

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

eladb · 2020-03-21T15:24:45Z

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 · 2020-03-21T15:31:57Z

@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 · 2020-03-21T16:28:24Z

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

eladb · 2020-03-21T16:52:28Z

Glad to hear it works.

When `Node.of()` is called with an object that does not really extend `Construct`, we can't find the associated construct node. This change improves the error message to provide some hint for why this could happen. Fixes #16 Related aws/aws-cdk#6885

* 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

rix0rrr · 2020-08-12T10:00:42Z

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

markusl added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 20, 2020

eladb self-assigned this Mar 20, 2020

eladb mentioned this issue Mar 21, 2020

In certain cases it is not possible to downcast IConstruct to Construct aws/constructs#16

Closed

eladb mentioned this issue Mar 21, 2020

fix(iam): immutable role cannot be used as a construct #6920

Merged

eladb added the p1 label Mar 21, 2020

eladb changed the title ~~Error: construct does not have an associated node after upgrading to CDK 1.29.0~~ Immutable roles cannot be used as constructs Mar 21, 2020

eladb mentioned this issue Mar 21, 2020

fix: cryptic error message in Node.of() aws/constructs#17

Merged

MrArnoldPalmer mentioned this issue Apr 7, 2020

Regression 1.27 -> 1.31: construct does not have an associated node.. #7234

Closed

SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Apr 14, 2020

eladb assigned rix0rrr and unassigned eladb Aug 4, 2020

rix0rrr closed this as completed Aug 12, 2020

dariusjs mentioned this issue Dec 22, 2020

api-gateway: Immutable Constructs cannot be used for LambdaProxyIntegration and addRoutes #12201

Closed

Immutable roles cannot be used as constructs #6885

Immutable roles cannot be used as constructs #6885

Comments

markusl commented Mar 20, 2020 • edited

Reproduction Steps

Environment

alan-cooney commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

alan-cooney commented Mar 20, 2020

eladb commented Mar 20, 2020

martzcodes commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

eladb commented Mar 20, 2020

markusl commented Mar 20, 2020 • edited

skinny85 commented Mar 20, 2020 • edited

martzcodes commented Mar 20, 2020 • edited

markusl commented Mar 20, 2020

skinny85 commented Mar 20, 2020

skinny85 commented Mar 20, 2020

markusl commented Mar 20, 2020

skinny85 commented Mar 20, 2020

martzcodes commented Mar 20, 2020

skinny85 commented Mar 20, 2020

markusl commented Mar 20, 2020 • edited

skinny85 commented Mar 20, 2020

markusl commented Mar 20, 2020

skinny85 commented Mar 20, 2020

skinny85 commented Mar 20, 2020 • edited

markusl commented Mar 21, 2020

eladb commented Mar 21, 2020 • edited

eladb commented Mar 21, 2020

eladb commented Mar 21, 2020

eladb commented Mar 21, 2020

eladb commented Mar 21, 2020

eladb commented Mar 21, 2020

markusl commented Mar 21, 2020

eladb commented Mar 21, 2020

rix0rrr commented Aug 12, 2020

markusl commented Mar 20, 2020 •

edited

markusl commented Mar 20, 2020 •

edited

skinny85 commented Mar 20, 2020 •

edited

martzcodes commented Mar 20, 2020 •

edited

markusl commented Mar 20, 2020 •

edited

skinny85 commented Mar 20, 2020 •

edited

eladb commented Mar 21, 2020 •

edited