fix: duplicated objects when charts are nested

src/app.ts

@@ -46,7 +46,7 @@ export class App extends Construct {
     // but thats ok too since we no longer treat constructs as a self-contained synthesis unit.
     validate(app);
 
-    return chartToKube(chart);
+    return chartToKube(chart).map(obj => obj.toJson());
   }
 
   private static of(c: IConstruct): App {
@@ -90,16 +90,21 @@ export class App extends Construct {
     // the necessary operations. We do however want to preserve the distributed validation.
     validate(this);
 
-    const simpleManifestNamer = (chart: Chart) => `${Names.toDnsLabel(chart)}.k8s.yaml`;
-    const manifestNamer = hasDependantCharts ? (chart: Chart) => `${index.toString().padStart(4, '0')}-${simpleManifestNamer(chart)}` : simpleManifestNamer;
+    const charts = new DependencyGraph(Node.of(this)).topology()
+      .filter(x => x instanceof Chart);
 
-    const charts: IConstruct[] = new DependencyGraph(Node.of(this)).topology().filter(x => x instanceof Chart);
-
-    let index = 0;
+    const found = new Set<IConstruct>();
+    const namer: ChartNamer = hasDependantCharts ? new IndexedChartNamer() : new SimpleChartNamer();
     for (const node of charts) {
       const chart: Chart = Chart.of(node);
-      Yaml.save(path.join(this.outdir, manifestNamer(chart)), chartToKube(chart));
-      index++;
+      const chartName = namer.name(chart);
+      const objects = chartToKube(chart);
+
+      Yaml.save(path.join(this.outdir, chartName), objects.map(obj => obj.toJson()));
+
+      for (const obj of objects) {
+        found.add(obj);
+      }
     }
 
   }
@@ -128,7 +133,9 @@ function resolveDependencies(app: App) {
 
     for (const target of targetApiObjects) {
       for (const source of sourceApiObjects) {
-        Node.of(source).addDependency(target);
+        if (target !== source) {
+          Node.of(source).addDependency(target);
+        }
       }
     }
 
@@ -143,12 +150,50 @@ function resolveDependencies(app: App) {
 
   }
 
+  const charts = new DependencyGraph(Node.of(app)).topology()
+    .filter(x => x instanceof Chart);
+
+  for (const parentChart of charts) {
+    for (const childChart of Node.of(parentChart).children.filter(x => x instanceof Chart)) {
+      // create an explicit chart dependency from nested chart relationships
+      Node.of(parentChart).addDependency(childChart);
+      hasDependantCharts = true;
+    }
+  }
+
   return hasDependantCharts;
 
 }
 
 function chartToKube(chart: Chart) {
   return new DependencyGraph(Node.of(chart)).topology()
     .filter(x => x instanceof ApiObject)
-    .map(x => (x as ApiObject).toJson());
+    .filter(x => Chart.of(x) === chart) // include an object only in its closest parent chart
+    .map(x => (x as ApiObject));
+}
+
+interface ChartNamer {
+  name(chart: Chart): string;
+}
+
+class SimpleChartNamer implements ChartNamer {
+  constructor() {
+  }
+
+  public name(chart: Chart) {
+    return `${Names.toDnsLabel(chart)}.k8s.yaml`;
+  }
+}
+
+class IndexedChartNamer extends SimpleChartNamer implements ChartNamer {
+  private index: number = 0;
+  constructor() {
+    super();
+  }
+
+  public name(chart: Chart) {
+    const name = `${this.index.toString().padStart(4, '0')}-${super.name(chart)}`;
+    this.index++;
+    return name;
+  }
 }

test/app.test.ts

@@ -163,6 +163,77 @@ test('app with chart dependencies via custom constructs', () => {
 
 });
 
+test('app with nested charts will deduplicate api objects', () => {
+
+  class CustomConstruct extends Construct {
+
+    public obj: ApiObject;
+
+    constructor(scope: Construct, id: string) {
+      super(scope, id);
+
+      this.obj = new ApiObject(this, `${id}obj`, { apiVersion: 'v1', kind: 'CustomConstruct' });
+    }
+  }
+
+  const app = Testing.app();
+  const chart = new Chart(app, 'chart1');
+  const childChart = new Chart(chart, 'chart2');
+  new CustomConstruct(chart, 'child1');
+  new CustomConstruct(childChart, 'child2');
+
+  app.synth();
+
+  expect(fs.readdirSync(app.outdir)).toEqual([
+    '0000-chart1-chart2-c883b207.k8s.yaml',
+    '0001-chart1.k8s.yaml',
+  ]);
+  expect(fs.readFileSync(path.join(app.outdir, '0000-chart1-chart2-c883b207.k8s.yaml'), 'utf8')).toMatchSnapshot();
+  expect(fs.readFileSync(path.join(app.outdir, '0001-chart1.k8s.yaml'), 'utf8')).toMatchSnapshot();
+
+});
+
+test('app with nested charts will deduplicate api objects (using custom classes)', () => {
+
+  class ChildChart1 extends Chart {
+    constructor(scope: Construct, name: string) {
+      super(scope, name);
+      new ApiObject(this, 'namespace1', { apiVersion: 'v1', kind: 'Namespace' });
+    }
+  }
+
+  class ChildChart2 extends Chart {
+    constructor(scope: Construct, name: string) {
+      super(scope, name);
+      new ApiObject(this, 'namespace2', { apiVersion: 'v1', kind: 'Namespace' });
+    }
+  }
+
+  class ParentChart extends Chart {
+    constructor(scope: Construct, name: string) {
+      super(scope, name);
+      new ChildChart1(this, 'child1');
+      new ChildChart2(this, 'child2');
+      new ApiObject(this, 'namespace3', { apiVersion: 'v1', kind: 'Namespace' });
+    }
+  }
+
+  const app = Testing.app();
+  new ParentChart(app, 'parent');
+
+  app.synth();
+
+  expect(fs.readdirSync(app.outdir)).toEqual([
+    '0000-parent-child1-c8e38b2d.k8s.yaml',
+    '0001-parent-child2-c882caae.k8s.yaml',
+    '0002-parent.k8s.yaml',
+  ]);
+  expect(fs.readFileSync(path.join(app.outdir, '0000-parent-child1-c8e38b2d.k8s.yaml'), 'utf8')).toMatchSnapshot();
+  expect(fs.readFileSync(path.join(app.outdir, '0001-parent-child2-c882caae.k8s.yaml'), 'utf8')).toMatchSnapshot();
+  expect(fs.readFileSync(path.join(app.outdir, '0002-parent.k8s.yaml'), 'utf8')).toMatchSnapshot();
+
+});
+
 test('synth calls validate', () => {
 
   class ValidatingConstruct extends Construct {

test/chart.test.ts

@@ -269,6 +269,23 @@ describe('toJson', () => {
 
   });
 
+  test('parent chart does not include objects of children charts', () => {
+
+    const app = Testing.app();
+    const chart = new Chart(app, 'chart1');
+    const childChart = new Chart(chart, 'chart2');
+    new CustomConstruct(chart, 'child1');
+    new CustomConstruct(childChart, 'child2');
+
+    expect(chart.toJson()).toMatchObject([
+      { apiVersion: 'v1', kind: 'CustomConstruct', metadata: { name: 'chart1-child1-child1obj-c868628e' } },
+    ]);
+    expect(childChart.toJson()).toMatchObject([
+      { apiVersion: 'v1', kind: 'CustomConstruct', metadata: { name: 'chart1-chart2-child2-child2obj-c828dca6' } },
+    ]);
+
+  });
+
 });
 
 function createImplictToken(value: any) {