aws/aws-pdk

[BUG] Error: LogicalId defined outside of stack

Opened this issue · 17 comments

Describe the bug

I have encountered a bug that was also reported here: #271.

When trying to generate a graph for a stack that uses cdk-temp-stack, cdk synth fails with error also reported in #271

Expected Behavior

Possible to generate a graph.

Current Behavior

Error: LogicalId defined outside of stack: AutoDestructDeleteStackServiceRoleADD04D27 - Node:CFN_RESOURCE::xxxAutoDestructDeleteStackServiceRole11230D55
    at new Node (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/graph.ts:1173:15)
    at new CfnResourceNode (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/graph.ts:2051:5)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:266:18)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:296:9)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:296:9)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:296:9)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:296:9)
    at visit (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:296:9)
    at computeGraph (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/core/compute.ts:306:3)
    at CdkGraph._synthesize (/Users/tomasztrebski/dev-ay/ay5/node_modules/@aws/pdk/cdk-graph/cdk-graph.ts:284:31)

Reproduction Steps

Originally I thought the problem comes from using TimeToLive from @cloudcomponents/cdk-temp-stack.
However building a short repro with cdk.Stack does not reveal the problem.
Reason why I'm bringing up cdk.Stack is that I have built my custom stack that sets up some defaults like permissionBoundary, synethizer and TimeToLive to match regulations in my company.

Error occurs if I add MyStack to an cdk.App but does not for ordinary cdk.Stack.
I cannot share the full code of my stack however it roughly does this:

#!/usr/bin/env node
import { CdkGraph } from '@aws/pdk/cdk-graph';
import { TimeToLive } from '@cloudcomponents/cdk-temp-stack';
import * as cdk from 'aws-cdk-lib';
import 'source-map-support/register';

import { IConstruct } from 'constructs';

const app = new cdk.App();
const env = {
  account:
    process.env['CDK_DEPLOY_ACCOUNT'] || process.env['CDK_DEFAULT_ACCOUNT'],
  region: process.env['CDK_DEPLOY_REGION'] || process.env['CDK_DEFAULT_REGION'],
};

class Stack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props: cdk.StackProps) {
    super(scope, id, {
      ...props,
      terminationProtection: false,
      analyticsReporting: false,
      permissionsBoundary: cdk.PermissionsBoundary.fromName('test'),
      synthesizer: new cdk.DefaultStackSynthesizer({
        qualifier: 'xx',
        dockerTagPrefix: 'xx',
      }),
    });

    cdk.Aspects.of(this).add(new ApplyDestroyPolicyAspect());
    new TimeToLive(this, 'AutoDestruct', {
      ttl: cdk.Duration.days(1),
    });
  }
}

class ApplyDestroyPolicyAspect implements cdk.IAspect {
  visit(node: IConstruct): void {
    if (node instanceof cdk.CfnResource) {
      node.applyRemovalPolicy(cdk.RemovalPolicy.DESTROY);
    }
  }
}

new Stack(app, 'Test', {
  env,
  stackName: 'xxx',
});

new CdkGraph(app);

Error is not throw here though.
I can share a resulting cloudformation template though:

{
 "Description": "[testing] XXX",
 "Resources": {
  "AutoDestructDeleteStackServiceRoleADD04D27": {
   "Type": "AWS::IAM::Role",
   "Properties": {
    "AssumeRolePolicyDocument": {
     "Statement": [
      {
       "Action": "sts:AssumeRole",
       "Effect": "Allow",
       "Principal": {
        "Service": "lambda.amazonaws.com"
       }
      }
     ],
     "Version": "2012-10-17"
    },
    "ManagedPolicyArns": [
     {
      "Fn::Join": [
       "",
       [
        "arn:",
        {
         "Ref": "AWS::Partition"
        },
        ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
       ]
      ]
     }
    ],
    "PermissionsBoundary": "arn:aws:iam::123456789012:policy/ays-cdk-v1-permissions-boundary"
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "xxx/AutoDestruct/DeleteStack/ServiceRole/Resource"
   }
  },
  "AutoDestructDeleteStackServiceRoleDefaultPolicy42286F0B": {
   "Type": "AWS::IAM::Policy",
   "Properties": {
    "PolicyDocument": {
     "Statement": [
      {
       "Action": "*",
       "Effect": "Allow",
       "Resource": "*"
      }
     ],
     "Version": "2012-10-17"
    },
    "PolicyName": "AutoDestructDeleteStackServiceRoleDefaultPolicy42286F0B",
    "Roles": [
     {
      "Ref": "AutoDestructDeleteStackServiceRoleADD04D27"
     }
    ]
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "xxx/AutoDestruct/DeleteStack/ServiceRole/DefaultPolicy/Resource"
   }
  },
  "AutoDestructDeleteStack239CEEE3": {
   "Type": "AWS::Lambda::Function",
   "Properties": {
    "Code": {
     "S3Bucket": "cdk-cdk-v1-assets-123456789012-eu-central-1",
     "S3Key": "ece017a7d7cfba4a1602f6d267cf5a02781708db95bbf4ff8c2394796f26b7a2.zip"
    },
    "Handler": "index.handler",
    "Role": {
     "Fn::GetAtt": [
      "AutoDestructDeleteStackServiceRoleADD04D27",
      "Arn"
     ]
    },
    "Runtime": "nodejs14.x"
   },
   "DependsOn": [
    "AutoDestructDeleteStackServiceRoleDefaultPolicy42286F0B",
    "AutoDestructDeleteStackServiceRoleADD04D27"
   ],
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "xxx/AutoDestruct/DeleteStack/Resource",
    "aws:asset:path": "asset.ece017a7d7cfba4a1602f6d267cf5a02781708db95bbf4ff8c2394796f26b7a2",
    "aws:asset:is-bundled": false,
    "aws:asset:property": "Code"
   }
  },
  "AutoDestructTimeToLive6FA1EF2B": {
   "Type": "AWS::Events::Rule",
   "Properties": {
    "ScheduleExpression": "rate(3 hours)",
    "State": "ENABLED",
    "Targets": [
     {
      "Arn": {
       "Fn::GetAtt": [
        "AutoDestructDeleteStack239CEEE3",
        "Arn"
       ]
      },
      "Id": "Target0",
      "Input": {
       "Fn::Join": [
        "",
        [
         "{\"stackId\":\"",
         {
          "Ref": "AWS::StackId"
         },
         "\"}"
        ]
       ]
      }
     }
    ]
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "xxx/AutoDestruct/TimeToLive/Resource"
   }
  },
  "AutoDestructTimeToLiveAllowEventRulexxxAutoDestructDeleteStackC6581343BEFD32FE": {
   "Type": "AWS::Lambda::Permission",
   "Properties": {
    "Action": "lambda:InvokeFunction",
    "FunctionName": {
     "Fn::GetAtt": [
      "AutoDestructDeleteStack239CEEE3",
      "Arn"
     ]
    },
    "Principal": "events.amazonaws.com",
    "SourceArn": {
     "Fn::GetAtt": [
      "AutoDestructTimeToLive6FA1EF2B",
      "Arn"
     ]
    }
   },
   "UpdateReplacePolicy": "Delete",
   "DeletionPolicy": "Delete",
   "Metadata": {
    "aws:cdk:path": "xxx/AutoDestruct/TimeToLive/AllowEventRulexxxAutoDestructDeleteStackC6581343"
   }
  }
 },
 "Parameters": {
  "BootstrapVersion": {
   "Type": "AWS::SSM::Parameter::Value<String>",
   "Default": "/cdk-bootstrap/cdk-v1/version",
   "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
  }
 },
 "Rules": {
  "CheckBootstrapVersion": {
   "Assertions": [
    {
     "Assert": {
      "Fn::Not": [
       {
        "Fn::Contains": [
         [
          "1",
          "2",
          "3",
          "4",
          "5"
         ],
         {
          "Ref": "BootstrapVersion"
         }
        ]
       }
      ]
     },
     "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
    }
   ]
  }
 }
}

Possible Solution

No response

Additional Information/Context

CDK: 2.121.1

PDK version used

0.22.50

What languages are you seeing this issue on?

Typescript

Environment details (OS name and version, etc.)

MacOS Sierra

cc @JeremyJonas , I noticed you've been handling the other bug.
I figure it's worth mentioning you.

Hi @kornicameister, could you explain a bit more details about your "custom" stack vs "ordinary stack"? And from your comment the sample code provide works right, as you stated no error with "ordinary stack" and your example is an "ordinary stack" right?

Given the AutoDestructDeleteStackServiceRoleADD04D27 role resource is inside of the Stack, seems that the inference of the stack is not getting detected. We use the Stack.of logic for this.

Does your custom stack evaluate to true for Stack.isStack(yourStack) and does Stack.of(yourConstruct) evaluate to your custom stack?

  • // Infer the stack this construct belongs to
    let stack: Graph.StackNode | undefined;
    try {
    if (construct.node.scope) {
    const stackUUID = getConstructUUID(Stack.of(construct));
    stack = store.getStack(stackUUID);
    }
    } catch {
    // ignore - expected to throw if construct is not contained in a stack
    }
  • Note the catch ignore is targeting CDK managed stuff (like metadata tree) outside of a stack, not resources like in your case.

Code where error occurs:

this.logicalId = props.logicalId;
if (this.logicalId) {
if (this.stack == null) {
throw new Error(
`LogicalId defined outside of stack: ${this.logicalId} - ${String(
this
)}`
);
}
this.store.recordLogicalId(this.stack, this.logicalId, this);

@JeremyJonas yes, those tests you asked about, everything seems to be just fine.

@JeremyJonas as to what this stack of mine really is. It is simply speaking a class that extends from cdk.Stack and set things up according to my taste. The idea was that no matter what my team decides to take for a spin they always adhere to certain regulations I've laid out among which you can find:

  • auto destruction of testing stacks with some sane defaults and lookup strategies for overrides
  • synthetizing
  • match organization setup

And that's really all that is. For most of the time my stack does not really create extra resources, just sets values later passed into constructor of cdk.Stack. The only time something is created is for testing configuration which is exactly what fails.


Update: I actually made some more tests. Now I noticed that cdk synth + MyStack fails to create a graph with this error:

Error: LogicalId defined outside of stack: StackName - Node:OUTPUT::aysauthapiexampleproductionStackName97FD9A29

So we're failing now on the output which makes me think that whatever construct gets created in my stack is something...flawed.

I don't know what that can be... maybe something messed up on jsii part and packaging the construct/stack?


Update2: I removed the outputs from 1st update and now I get the failure on another resource. Error is exactly the same, just resource ID changes.

Hi @kornicameister, does Stack.isStack(yourCustomStack) equal true? If it properly extends Stack it should, and that is the way we detect the stack associated with a resource. Please ensure that your custom stack is a Stack, and that the resources within your custom stack return reference to your custom stack when you call Stack.of(failingResource).

Let me know if all these conditions are still true, which means an issue on our end, otherwise means need to provide support for detecting stacks like yours and will need to go a bit deeper into understanding how it is implemented and why it does not get reported as a stack.

@JeremyJonas I created couple of tests to verify what you wanted:

it('should return true if stack is a stack', () => {
	expect(cdk.Stack.isStack(stack)).toBe(true);
});
it('should return the same stack from cdk.Stack.of', () => {
  expect(cdk.Stack.of(stack)).toBe(stack);
});
it('should return the same stack from cdk.Stack.of with another resource', () => {
   const ttl = new TimeToLive(stack, 'TTL', {
      ttl: cdk.Duration.hours(1),
    });
  expect(cdk.Stack.of(ttl)).toBe(stack);
});

here's the result:

✓ should return true if stack is a stack (6 ms)
✓ should return the same stack from cdk.Stack.of with another resource (6 ms)
✓ should return the same stack from cdk.Stack.of (6 ms)

Hi @kornicameister, was away for a bit sorry for delay. Your test disproves my initial thoughts on this one. Without being able to repro this not sure how to resolve. Any chance you could scrap together an example repro of it? You could also run in inspect mode with breakpoint at the error, maybe can provide more details, which would be my first step if running your repro code.

If all else fails, we could try adding a flag to ignore the error. But I am definitely curious to know what is actually causing it.

I can try, but wonder if that's gonna work out.

@JeremyJonas one thing... can a fact that this error occurs when using said stack of mine via NPM package.
You see I have a library of re-usable CDK components that I ship via NPM package.
Might be I have something messed up on this end?

@mteichtahl I am not using projen. My setup is based on top of pure jsii + lerna for mono-repo support.

That's my package.json (note: redacted):

{
  "name": "xxx",
  "version": "0.0.0",
  "description": "Collection of CDK components",
  "license": "UNLICENSED",
  "main": "dist/index.js",
  "scripts": {
    "build": "jsii -vvv --compress-assembly --fix-peer-dependencies",
    "postbuild": "npm run docgen",
    "build:watch": "jsii -w",
    "docgen": "npx rimraf README.md && jsii-docgen -o README.md -r",
    "prepackage": "npm run build",
    "package": "npx rimraf dist/ && jsii-pacmak -vvv",
    "test:unit": "jest",
    "test:unit:watch": "jest --watch --watchman"
  },
  "types": "dist/index.d.ts",
  "dependencies": {
    "@cloudcomponents/cdk-temp-stack": "^2.1.0"
  },
  "peerDependencies": {
    "aws-cdk-lib": "^2.98",
    "constructs": "^10.3.0"
  },
  "devDependencies": {
    "aws-cdk-lib": "2.115.0",
    "constructs": "10.3.0",
    "jsii": "~5.3.7",
    "jsii-diff": "^1.94.0",
    "jsii-docgen": "^10.3.7",
    "jsii-pacmak": "^1.94.0",
    "jsii-reflect": "^1.94.0",
    "jsii-rosetta": "~5.3.4"
  },
  "engines": {
    "node": ">=18.16.0"
  },
  "jsii": {
    "outdir": "dist",
    "targets": {},
    "tsc": {
      "outDir": "dist",
      "rootDir": "src"
    },
    "versionFormat": "short"
  }
}

I do not have any other jsii configuration file for that.
Build seems to be running great and I can use package without any issue in other repositories.

+1, same thing happens simply with EKS L3 construct

`import { Stack, StackProps } from "aws-cdk-lib";
import { Construct } from "constructs";
import { KubectlV29Layer } from '@aws-cdk/lambda-layer-kubectl-v29';
import * as eks from 'aws-cdk-lib/aws-eks';

export class ApplicationStack extends Stack {
constructor(scope: Construct, id: string, props?: StackProps) {
super(scope, id, props);

// provisioning a cluster
const cluster = new eks.Cluster(this, 'hello-eks', {
  version: eks.KubernetesVersion.V1_29,
  kubectlLayer: new KubectlV29Layer(this, 'kubectl'),
});

// apply a kubernetes manifest to the cluster
cluster.addManifest('mypod', {
  apiVersion: 'v1',
  kind: 'Pod',
  metadata: { name: 'mypod' },
  spec: {
    containers: [
      {
        name: 'hello',
        image: 'paulbouwer/hello-kubernetes:1.5',
        ports: [ { containerPort: 8080 } ],
      },
    ],
  },
});

}
}`

running 'npm run build' throws

/Users/andrea.spoldi/architects/pdk/prova/node_modules/@aws/pdk/cdk-graph/core/graph.ts:1173 throw new Error( ^ Error: LogicalId defined outside of stack: Handler886CB40B - Node:CFN_RESOURCE::infradevawscdkawseksKubectlProviderHandler14CEB732

Not that I am happy...but I am happy that we have something to reproduce an error.

I tried reproducing that but without any luck.

I have the same situation with EKS higher-level constructs. After some debugging, I found a temp solution (i.e. a hack), don't have a lasting idea yet. And it only seems to be a problem when using PDK along with CDK.

Here's the patch that gets things going again - though not great:

--- node_modules/@aws/pdk/cdk-graph/core/compute.orig   2024-04-11 20:35:50
+++ node_modules/@aws/pdk/cdk-graph/core/compute.js     2024-04-11 19:53:19
@@ -101,6 +101,7 @@
                 node = new Graph.StackNode(nodeProps);
                 break;
             }
+            case 'aws-cdk-lib.aws_eks.KubectlProvider':
             case types_1.ConstructInfoFqnEnum.NESTED_STACK: {
                 // NB: handle NestedStack<->CfnStack as single Node with NestedStack construct as source
                 // https://github.com/aws/aws-cdk/blob/119c92f65bf26c3fdf4bb818a4a4812022a3744a/packages/%40aws-cdk/core/lib/nested-stack.ts#L119
\ No newline at end of file

Reason seems to be: classes having custom fqn are instanceof NestedStack, but the info gets lost here

btw: it only appears with PDK and automatic diagram generation

@sebastianrothbucher did you manage to get a fix? need some more info?

I'm good with the fix above, thx