chore(eks): relax cdk8s dependency to constructs + runtime validation (…

…aws#10933) To avoid forcing a `cdk8s` dependency on the monolithic packages, we don't directly use `cdk8s.Chart` in the EKS integration. Instead, we use a generic `Construct` and assert at runtime that it indeed a cdk8s chart. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
skilbjo · Oct 18, 2020 · 2640d9a · 2640d9a
1 parent d943d26
commit 2640d9a
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 16 deletions.
diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts
@@ -7,7 +7,6 @@ import * as kms from '@aws-cdk/aws-kms';
 import * as lambda from '@aws-cdk/aws-lambda';
 import * as ssm from '@aws-cdk/aws-ssm';
 import { Annotations, CfnOutput, CfnResource, IResource, Resource, Stack, Tags, Token, Duration } from '@aws-cdk/core';
-import * as cdk8s from 'cdk8s';
 import { Construct, Node } from 'constructs';
 import * as YAML from 'yaml';
 import { AwsAuth } from './aws-auth';
@@ -141,7 +140,7 @@ export interface ICluster extends IResource, ec2.IConnectable {
    * @param chart the cdk8s chart.
    * @returns a `KubernetesManifest` construct representing the chart.
    */
-  addCdk8sChart(id: string, chart: cdk8s.Chart): KubernetesManifest;
+  addCdk8sChart(id: string, chart: Construct): KubernetesManifest;
 
 }
 
@@ -641,8 +640,16 @@ abstract class ClusterBase extends Resource implements ICluster {
    * @param chart the cdk8s chart.
    * @returns a `KubernetesManifest` construct representing the chart.
    */
-  public addCdk8sChart(id: string, chart: cdk8s.Chart): KubernetesManifest {
-    return this.addManifest(id, ...chart.toJson());
+  public addCdk8sChart(id: string, chart: Construct): KubernetesManifest {
+
+    const cdk8sChart = chart as any;
+
+    // see https://github.com/awslabs/cdk8s/blob/master/packages/cdk8s/src/chart.ts#L84
+    if (typeof cdk8sChart.toJson !== 'function') {
+      throw new Error(`Invalid cdk8s chart. Must contain a 'toJson' method, but found ${typeof cdk8sChart.toJson}`);
+    }
+
+    return this.addManifest(id, ...cdk8sChart.toJson());
   }
 }
 

diff --git a/packages/@aws-cdk/aws-eks/lib/legacy-cluster.ts b/packages/@aws-cdk/aws-eks/lib/legacy-cluster.ts
@@ -3,7 +3,6 @@ import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import * as ssm from '@aws-cdk/aws-ssm';
-import * as cdk8s from 'cdk8s';
 import { Annotations, CfnOutput, Resource, Stack, Token, Tags } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { ICluster, ClusterAttributes, KubernetesVersion, NodeType, DefaultCapacityType, EksOptimizedImage, AutoScalingGroupCapacityOptions, MachineImageType, AutoScalingGroupOptions, CommonClusterOptions } from './cluster';
@@ -372,7 +371,7 @@ export class LegacyCluster extends Resource implements ICluster {
     throw new Error('legacy cluster does not support adding helm charts');
   }
 
-  public addCdk8sChart(_id: string, _chart: cdk8s.Chart): KubernetesManifest {
+  public addCdk8sChart(_id: string, _chart: Construct): KubernetesManifest {
     throw new Error('legacy cluster does not support adding cdk8s charts');
   }
 
@@ -434,7 +433,7 @@ class ImportedCluster extends Resource implements ICluster {
     throw new Error('legacy cluster does not support adding helm charts');
   }
 
-  public addCdk8sChart(_id: string, _chart: cdk8s.Chart): KubernetesManifest {
+  public addCdk8sChart(_id: string, _chart: Construct): KubernetesManifest {
     throw new Error('legacy cluster does not support adding cdk8s charts');
   }
 

diff --git a/packages/@aws-cdk/aws-eks/package.json b/packages/@aws-cdk/aws-eks/package.json
@@ -81,7 +81,8 @@
     "nodeunit": "^0.11.3",
     "pkglint": "0.0.0",
     "sinon": "^9.1.0",
-    "cdk8s-plus": "^0.29.0"
+    "cdk8s-plus": "^0.29.0",
+    "cdk8s": "^0.30.0"
   },
   "dependencies": {
     "@aws-cdk/aws-autoscaling": "0.0.0",
@@ -92,7 +93,6 @@
     "@aws-cdk/aws-ssm": "0.0.0",
     "@aws-cdk/core": "0.0.0",
     "@aws-cdk/custom-resources": "0.0.0",
-    "cdk8s": "^0.30.0",
     "constructs": "^3.0.4",
     "yaml": "1.10.0"
   },
@@ -109,7 +109,6 @@
     "@aws-cdk/aws-ssm": "0.0.0",
     "@aws-cdk/core": "0.0.0",
     "@aws-cdk/custom-resources": "0.0.0",
-    "cdk8s": "^0.30.0",
     "constructs": "^3.0.4"
   },
   "engines": {

diff --git a/packages/@aws-cdk/aws-eks/test/test.cluster.ts b/packages/@aws-cdk/aws-eks/test/test.cluster.ts
@@ -22,6 +22,38 @@ const CLUSTER_VERSION = eks.KubernetesVersion.V1_18;
 
 export = {
 
+  'throws when a non cdk8s chart construct is added as cdk8s chart'(test: Test) {
+
+    const { stack } = testFixture();
+
+    const cluster = new eks.Cluster(stack, 'Cluster', {
+      version: CLUSTER_VERSION,
+    });
+
+    // create a plain construct, not a cdk8s chart
+    const someConstruct = new constructs.Construct(stack, 'SomeConstruct');
+
+    test.throws(() => cluster.addCdk8sChart('chart', someConstruct), /Invalid cdk8s chart. Must contain a \'toJson\' method, but found undefined/);
+    test.done();
+
+  },
+
+  'throws when a core construct is added as cdk8s chart'(test: Test) {
+
+    const { stack } = testFixture();
+
+    const cluster = new eks.Cluster(stack, 'Cluster', {
+      version: CLUSTER_VERSION,
+    });
+
+    // create a plain construct, not a cdk8s chart
+    const someConstruct = new cdk.Construct(stack, 'SomeConstruct');
+
+    test.throws(() => cluster.addCdk8sChart('chart', someConstruct), /Invalid cdk8s chart. Must contain a \'toJson\' method, but found undefined/);
+    test.done();
+
+  },
+
   'cdk8s chart can be added to cluster'(test: Test) {
 
     const { stack } = testFixture();

diff --git a/packages/aws-cdk-lib/package.json b/packages/aws-cdk-lib/package.json
@@ -258,16 +258,14 @@
     "@types/node": "^10.17.35",
     "cdk-build-tools": "0.0.0",
     "constructs": "^3.0.4",
-    "cdk8s": "^0.30.0",
     "fs-extra": "^9.0.1",
     "pkglint": "0.0.0",
     "ts-node": "^9.0.0",
     "typescript": "~3.8.3",
     "ubergen": "0.0.0"
   },
   "peerDependencies": {
-    "constructs": "^3.0.4",
-    "cdk8s": "^0.30.0"
+    "constructs": "^3.0.4"
   },
   "homepage": "https://github.com/aws/aws-cdk",
   "engines": {

diff --git a/packages/monocdk/package.json b/packages/monocdk/package.json
@@ -257,16 +257,14 @@
     "@types/node": "^10.17.35",
     "cdk-build-tools": "0.0.0",
     "constructs": "^3.0.4",
-    "cdk8s": "^0.30.0",
     "fs-extra": "^9.0.1",
     "pkglint": "0.0.0",
     "ts-node": "^9.0.0",
     "typescript": "~3.8.3",
     "ubergen": "0.0.0"
   },
   "peerDependencies": {
-    "constructs": "^3.0.4",
-    "cdk8s": "^0.30.0"
+    "constructs": "^3.0.4"
   },
   "homepage": "https://github.com/aws/aws-cdk",
   "engines": {