search

aws / aws-pdk

The AWS PDK provides building blocks for common patterns together with development tools to manage and build your projects.
https://aws.github.io/aws-pdk/
Apache License 2.0
387 stars 76 forks source link

[BUG] Error: LogicalId defined outside of stack #684

Open kornicameister opened 10 months ago

kornicameister commented 10 months ago

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

kornicameister commented 10 months ago

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

JeremyJonas commented 10 months ago

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?

Code where error occurs: https://github.com/aws/aws-pdk/blob/43c0d6d4d6cd3b5deaf8c2c5d752ac3f99d4099c/packages/cdk-graph/src/core/graph.ts#L1170-L1179

kornicameister commented 10 months ago

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

kornicameister commented 10 months ago

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

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.

JeremyJonas commented 10 months ago

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.

kornicameister commented 10 months ago

@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)
JeremyJonas commented 9 months ago

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.

kornicameister commented 9 months ago

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 commented 9 months ago

InterestingCould you share a link to your code ? Or at least your .projenrc file ?Please note: My working hours may not be your working hours. Please do not feel obligated to reply outside of your normal work schedule!Sent on the go - please excuse mistakes On 13 Feb 2024, at 11:29 pm, Tomasz Trębski @.***> wrote: 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?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

kornicameister commented 9 months ago

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

andreaspoldi commented 9 months ago

+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

kornicameister commented 9 months ago

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.

sebastianrothbucher commented 7 months ago

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
sebastianrothbucher commented 7 months ago

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

sebastianrothbucher commented 5 months ago

btw: it only appears with PDK and automatic diagram generation

kornicameister commented 5 months ago

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

sebastianrothbucher commented 5 months ago

I'm good with the fix above, thx

Zirkonium88 commented 1 week ago

I'm facing the same issue, when creating stacks for Service Catalog. It does not matter, if Products are created from asset or from stack. In this case, the error is more meaningful, as you need to use different classes: one for the CloudFormation Stack of the portfolio and child clases for product creation.

From the Portfolio Stack, you need to run the follwoing code to add a Bucket product:

import aws_cdk as cdk

class S3BucketProduct(servicecatalog.ProductStack):
    def __init__(self, scope, id):
        super().__init__(scope, id)

        s3.Bucket(self, "BucketProduct")

product = servicecatalog.CloudFormationProduct(self, "Product",
    product_name="My Product",
    owner="Product Owner",
    product_versions=[servicecatalog.CloudFormationProductVersion(
        product_version_name="v1",
        cloud_formation_template=servicecatalog.CloudFormationTemplate.from_product_stack(S3BucketProduct(self, "S3BucketProduct"))
    )
    ]
)

Not using the diagram plugin fixes the issue directly. Thanks for this great package!

My error:

(.venv) XYZ ~/projects/pmi-service-catalog$ cdk synth --context environment=developer
jsii.errors.JavaScriptError: 
  @jsii/kernel.RuntimeError: Error: LogicalId defined outside of stack: MountPoint - Node:PARAMETER::ServiceCatalogStackmrhtservicecatalogproductec2901525B4MountPoint3F1F2267
      at Kernel._Kernel_ensureSync (/tmp/tmpqi6dbgyw/lib/program.js:9510:23)
      at Kernel.invoke (/tmp/tmpqi6dbgyw/lib/program.js:8874:102)
      at KernelHost.processRequest (/tmp/tmpqi6dbgyw/lib/program.js:10715:36)
      at KernelHost.run (/tmp/tmpqi6dbgyw/lib/program.js:10675:22)
      at Immediate._onImmediate (/tmp/tmpqi6dbgyw/lib/program.js:10676:46)
      at process.processImmediate (node:internal/timers:476:21)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/XYZ /projects/pmi-service-catalog/app.py", line 132, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/home/XYZ /projects/pmi-service-catalog/app.py", line 127, in main
    app.synth()
  File "/home/XYZ /projects/pmi-service-catalog/.venv/lib/python3.10/site-packages/aws_cdk/__init__.py", line 21371, in synth
    return typing.cast(_CloudAssembly_c693643e, jsii.invoke(self, "synth", [options]))
  File "/home/XYZ/projects/pmi-service-catalog/.venv/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 149, in wrapped
    return _recursize_dereference(kernel, fn(kernel, *args, **kwargs))
  File "/home/XYZ /projects/pmi-service-catalog/.venv/lib/python3.10/site-packages/jsii/_kernel/__init__.py", line 399, in invoke
    response = self.provider.invoke(
  File "/home/XYZ /projects/pmi-service-catalog/.venv/lib/python3.10/site-packages/jsii/_kernel/providers/process.py", line 380, in invoke
    return self._process.send(request, InvokeResponse)
  File "/home/XYZ /projects/pmi-service-catalog/.venv/lib/python3.10/site-packages/jsii/_kernel/providers/process.py", line 342, in send
    raise RuntimeError(resp.error) from JavaScriptError(resp.stack)
RuntimeError: Error: LogicalId defined outside of stack: MountPoint - Node:PARAMETER::ServiceCatalogStackmrhtservicecatalogproductec2901525B4MountPoint3F1F2267

Subprocess exited with error 1