Remove instanceof use when generating JSON manifest from chart

[dancmeyers]

This is an issue on at least version 2.3.44 and earlier of cdk8s. The actual issue experienced was that, when using aws-cdk-lib/aws-eks.Cluster.addCdk8sChart, the resulting CDK template had an empty Manifest in the custom resource. This occurred after we abstracted out some common low-level CDK8s classes to a library in our monorepo, to make it easier to use the patterns across multiple CDK apps that needed to deploy CDK8s charts to EKS clusters.

I eventually tracked the issue down to the chartToKube function's use of instanceof:

cdk8s-core/src/app.ts

Lines 289 to 294 in 379953a

	function chartToKube(chart: Chart) {
	return new DependencyGraph(Node.of(chart)).topology()
	.filter(x => x instanceof ApiObject)
	.filter(x => Chart.of(x) === chart) // include an object only in its closest parent chart
	.map(x => (x as ApiObject));
	}

When classes in a monorepo have been abstracted out to a lib like this, such that both the lib and the app using it have cdk8s as a dependency, instanceof fails to correctly identify the class hierarcy as matching when objects are passed between the two. This is the same issue as experienced in constructs in aws/constructs#955 As a result, instanceof returns false, and the function thinks there are no resources that need jsonifying.

[iliapolo]

Thanks for reporting this @dancmeyers. Looking into it 👍

[dancmeyers]

As per the request on Slack, some more information about our directory structure:

Within our repo we have a cdk directory in which all CDK(8s) code is located, including the following -

cdk/banksy
cdk/common
cdk/common/griffin-eks
cdk/common/griffin-stack
cdk/core-infra

common is where we keep our internal libs which other apps might want to use. These can be described as follows:

• griffin-eks: Our EKS library which uses cdk8s to provide abstract classes. It has aws-cdk-lib, cdk8s, cdk8s-plus-21, and constructs as dependencies.
• griffin-stack: Our CDK base stack library, which uses CDK/constructs to provide abstract classes. It has aws-cdk-lib and constructs as dependencies.

core-infra is our infrastructure CDK app which we deploy to every account. Among other things, it creates an EKS cluster using CDK and deploys standard apps to it (monitoring, external-dns, load-balancer-controller, etc) using CDK8s. It has aws-cdk-lib, cdk8s, cdk8s-plus-21, and constructs as dependencies (among others).

banksy is our 'main' application, which deploys onto the EKS cluster created in core-infra using CDK8s. It has aws-cdk-lib, cdk8s, cdk8s-plus-21, and constructs as dependencies (among others). As this is a separate CDK app, the exports for cluster name etc which are needed to define a local aws-cdk-lib/aws-eks.ICluster are specified within core-infra and imported here, rather than using CDK's automatic export/import functionality.

Because we are developing locally, in a monorepo, each library within cdk/common has a local package-lock.json (checked into the repo) and node_modules (not), as well as package.json. Each CDK app loads the common modules as dependencies using (for example) "@griffin/eks": "file:../common/griffin-eks".

[dancmeyers]

For what it's worth, I managed to get this working by doing the following:

• Create a package.json in the root cdk directory, that has as dependencies aws-cdk-lib, cdk8s, cdk8s-plus-21, and constructs. Run npm install to create the package-lock.json and node_modules directories.
• Remove those libs from the dependencies of every Node app, be that a CDK lib in cdk/common or a CDK app in cdk/<something_else>, and set them as peerDependencies (with the expected >= instead of a specific version) instead.
• Delete node_modules and package-lock.json, then re-run npm install, in all of the cdk/common subdirectories, then do the same in each CDK app directory.

There is then only one copy of cdk8s etc in the package hierarchy, the one at the root cdk directory. Nothing else pulls in an alternative version (I had to remove a few other dev-only dependencies like cdk8s-cli to make that work), so everything plays nice and I don't get empty manifests. It is a bit of a faff though, and it would be great if it could Just Work (tm) in the same way that constructs does as long as the same version is installed in the imported lib and the main app.

[RealityAnomaly]

This is also an issue with the instanceof call in app.ts on line 106. https://github.com/cdk8s-team/cdk8s-core/blob/2.x/src/app.ts#L106

[spion]

This also makes cdk8s-core impossible to use with pnpm (if you decide to split out code to any package libraries).

[dancmeyers]

That's useful to know, given that a load of our other systems are being moved to pnpm (so we can build them using Bazel), and I was going to try and migrate our CDK too so that we had consistency...