fix(core): assets are duplicated between nested Cloud Assemblies (#11008

)

We stage assets into the Cloud Assembly directory. If there are multiple
nested Cloud Assemblies, the same asset will be staged multiple times.
This leads to an N-fold increase in size of the Cloud Assembly when used
in combination with CDK Pipelines (where N is the number of stages
deployed), and may even lead the Cloud Assembly to exceed CodePipeline's
maximum artifact size of 250MB.

Add the concept of an `assetOutdir` next to a regular Cloud Assembly
`outDir`, so that multiple Cloud Assemblies can share an asset
directory. As an initial implementation, the `assetOutdir` of nested
Cloud Assemblies is just the regular `outdir` of the root Assembly.

We are playing a bit fast and loose with the semantics of file paths
across our code base; many properties just say "the path of X" without
making clear whether it's absolute or relative, and if it's relative
what it's relative to (`cwd()`? Or the Cloud Assembly directory?).

Turns out that especially in dealing with assets, the answer is
"can be anything" and things just happen to work out based on who is
providing the path and who is consuming it. In order to limit the
scope of the changes I needed to make I kept modifications to the
`AssetStaging` class:

* `stagedPath` now consistently returns an absolute path.
* `relativeStagedPath()` a path relative to the Cloud Assembly or an
  absolute path, as appropriate.

Related changes in this PR:

- Refactor the *copying* vs. *bundling* logic in `AssetStaging`. I found
  the current maze of `if`s and member variable changes too hard to
  follow to convince myself the new code would be doing the right thing,
  so I refactored it to reduce the branching factor.

- Switch the tests of `aws-ecr-assets` over to Jest using
  `nodeunitShim`.

Fixes #10877, fixes #9627, fixes #9917.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/@aws-cdk/assets/test/test.staging.ts

@@ -16,7 +16,7 @@ export = {
 
     test.deepEqual(staging.sourceHash, '2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
     test.deepEqual(staging.sourcePath, sourcePath);
-    test.deepEqual(stack.resolve(staging.stagedPath), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
+    test.deepEqual(staging.relativeStagedPath(stack), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
     test.done();
   },
 
@@ -31,7 +31,7 @@ export = {
 
     test.deepEqual(staging.sourceHash, '2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00');
     test.deepEqual(staging.sourcePath, sourcePath);
-    test.deepEqual(stack.resolve(staging.stagedPath), sourcePath);
+    test.deepEqual(staging.stagedPath, sourcePath);
     test.done();
   },
 

packages/@aws-cdk/aws-ecr-assets/.gitignore

@@ -16,4 +16,5 @@ nyc.config.js
 *.snk
 !.eslintrc.js
 
-junit.xml
+junit.xml
+!jest.config.js

packages/@aws-cdk/aws-ecr-assets/.npmignore

@@ -23,4 +23,5 @@ tsconfig.json
 # exclude cdk artifacts
 **/cdk.out
 junit.xml
-test/
+test/
+jest.config.js

packages/@aws-cdk/aws-ecr-assets/jest.config.js

@@ -0,0 +1,10 @@
+const baseConfig = require('cdk-build-tools/config/jest.config');
+module.exports = {
+  ...baseConfig,
+  coverageThreshold: {
+    global: {
+      branches: 80,
+      statements: 80,
+    }
+  }
+};

packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts

@@ -142,7 +142,7 @@ export class DockerImageAsset extends CoreConstruct implements assets.IAsset {
 
     const stack = Stack.of(this);
     const location = stack.synthesizer.addDockerImageAsset({
-      directoryName: staging.stagedPath,
+      directoryName: staging.relativeStagedPath(stack),
       dockerBuildArgs: props.buildArgs,
       dockerBuildTarget: props.target,
       dockerFile: props.file,

packages/@aws-cdk/aws-ecr-assets/package.json

@@ -65,12 +65,11 @@
   "license": "Apache-2.0",
   "devDependencies": {
     "@aws-cdk/assert": "0.0.0",
-    "@types/nodeunit": "^0.0.31",
     "@types/proxyquire": "^1.3.28",
     "aws-cdk": "0.0.0",
     "cdk-build-tools": "0.0.0",
     "cdk-integ-tools": "0.0.0",
-    "nodeunit": "^0.11.3",
+    "nodeunit-shim": "0.0.0",
     "pkglint": "0.0.0",
     "proxyquire": "^2.1.3",
     "@aws-cdk/cloud-assembly-schema": "0.0.0"
@@ -112,6 +111,7 @@
     "announce": false
   },
   "cdk-build": {
+    "jest": true,
     "env": {
       "AWSLINT_BASE_CONSTRUCT": true
     }

packages/@aws-cdk/aws-ecr-assets/test/test.image-asset.ts → packages/@aws-cdk/aws-ecr-assets/test/image-asset.test.ts

@@ -1,15 +1,18 @@
 import * as fs from 'fs';
 import * as path from 'path';
-import { expect, haveResource } from '@aws-cdk/assert';
+import { expect as ourExpect, haveResource } from '@aws-cdk/assert';
 import * as iam from '@aws-cdk/aws-iam';
 import * as cxschema from '@aws-cdk/cloud-assembly-schema';
-import { App, Lazy, Stack } from '@aws-cdk/core';
-import { Test } from 'nodeunit';
+import { App, DefaultStackSynthesizer, Lazy, LegacyStackSynthesizer, Stack, Stage } from '@aws-cdk/core';
+import * as cxapi from '@aws-cdk/cx-api';
+import { nodeunitShim, Test } from 'nodeunit-shim';
 import { DockerImageAsset } from '../lib';
 
 /* eslint-disable quote-props */
 
-export = {
+const DEMO_IMAGE_ASSET_HASH = 'baa2d6eb2a17c75424df631c8c70ff39f2d5f3bee8b9e1a109ee24ca17300540';
+
+nodeunitShim({
   'test instantiating Asset Image'(test: Test) {
     // GIVEN
     const app = new App();
@@ -103,7 +106,7 @@ export = {
     asset.repository.grantPull(user);
 
     // THEN
-    expect(stack).to(haveResource('AWS::IAM::Policy', {
+    ourExpect(stack).to(haveResource('AWS::IAM::Policy', {
       PolicyDocument: {
         'Statement': [
           {
@@ -306,4 +309,63 @@ export = {
     test.deepEqual(asset7.sourceHash, 'bc007f81fe1dd0f0bbb24af898eba3f4f15edbff19b7abb3fac928439486d667');
     test.done();
   },
-};
+});
+
+test('nested assemblies share assets: legacy synth edition', () => {
+  // GIVEN
+  const app = new App();
+  const stack1 = new Stack(new Stage(app, 'Stage1'), 'Stack', { synthesizer: new LegacyStackSynthesizer() });
+  const stack2 = new Stack(new Stage(app, 'Stage2'), 'Stack', { synthesizer: new LegacyStackSynthesizer() });
+
+  // WHEN
+  new DockerImageAsset(stack1, 'Image', { directory: path.join(__dirname, 'demo-image') });
+  new DockerImageAsset(stack2, 'Image', { directory: path.join(__dirname, 'demo-image') });
+
+  // THEN
+  const assembly = app.synth();
+
+  // Read the assets from the stack metadata
+  for (const stageName of ['Stage1', 'Stage2']) {
+    const stackArtifact = assembly.getNestedAssembly(`assembly-${stageName}`).artifacts.filter(isStackArtifact)[0];
+    const assetMeta = stackArtifact.findMetadataByType(cxschema.ArtifactMetadataEntryType.ASSET);
+    expect(assetMeta[0]).toEqual(
+      expect.objectContaining({
+        data: expect.objectContaining({
+          path: `../asset.${DEMO_IMAGE_ASSET_HASH}`,
+        }),
+      }),
+    );
+  }
+});
+
+test('nested assemblies share assets: default synth edition', () => {
+  // GIVEN
+  const app = new App();
+  const stack1 = new Stack(new Stage(app, 'Stage1'), 'Stack', { synthesizer: new DefaultStackSynthesizer() });
+  const stack2 = new Stack(new Stage(app, 'Stage2'), 'Stack', { synthesizer: new DefaultStackSynthesizer() });
+
+  // WHEN
+  new DockerImageAsset(stack1, 'Image', { directory: path.join(__dirname, 'demo-image') });
+  new DockerImageAsset(stack2, 'Image', { directory: path.join(__dirname, 'demo-image') });
+
+  // THEN
+  const assembly = app.synth();
+
+  // Read the asset manifests to verify the file paths
+  for (const stageName of ['Stage1', 'Stage2']) {
+    const manifestArtifact = assembly.getNestedAssembly(`assembly-${stageName}`).artifacts.filter(isAssetManifestArtifact)[0];
+    const manifest = JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
+
+    expect(manifest.dockerImages[DEMO_IMAGE_ASSET_HASH].source).toEqual({
+      directory: `../asset.${DEMO_IMAGE_ASSET_HASH}`,
+    });
+  }
+});
+
+function isStackArtifact(x: any): x is cxapi.CloudFormationStackArtifact {
+  return x instanceof cxapi.CloudFormationStackArtifact;
+}
+
+function isAssetManifestArtifact(x: any): x is cxapi.AssetManifestArtifact {
+  return x instanceof cxapi.AssetManifestArtifact;
+}

packages/@aws-cdk/aws-s3-assets/lib/asset.ts

@@ -84,7 +84,7 @@ export class Asset extends cdk.Construct implements cdk.IAsset {
   public readonly s3ObjectUrl: string;
 
   /**
-   * The path to the asset (stringinfied token).
+   * The path to the asset, relative to the current Cloud Assembly
    *
    * If asset staging is disabled, this will just be the original path.
    * If asset staging is enabled it will be the staged path.
@@ -125,7 +125,9 @@ export class Asset extends cdk.Construct implements cdk.IAsset {
     this.assetHash = staging.assetHash;
     this.sourceHash = this.assetHash;
 
-    this.assetPath = staging.stagedPath;
+    const stack = cdk.Stack.of(this);
+
+    this.assetPath = staging.relativeStagedPath(stack);
 
     const packaging = determinePackaging(staging.sourcePath);
 
@@ -134,12 +136,10 @@ export class Asset extends cdk.Construct implements cdk.IAsset {
       ? true
       : ARCHIVE_EXTENSIONS.some(ext => staging.sourcePath.toLowerCase().endsWith(ext));
 
-    const stack = cdk.Stack.of(this);
-
     const location = stack.synthesizer.addFileAsset({
       packaging,
       sourceHash: this.sourceHash,
-      fileName: staging.stagedPath,
+      fileName: this.assetPath,
     });
 
     this.s3BucketName = location.bucketName;

packages/@aws-cdk/aws-s3-assets/test/asset.test.ts

@@ -10,6 +10,7 @@ import * as path from 'path';
 import { Asset } from '../lib/asset';
 
 const SAMPLE_ASSET_DIR = path.join(__dirname, 'sample-asset-directory');
+const SAMPLE_ASSET_HASH = '6b84b87243a4a01c592d78e1fd3855c4bfef39328cd0a450cc97e81717fea2a2';
 
 test('simple use case', () => {
   const app = new cdk.App({
@@ -208,6 +209,60 @@ test('asset metadata is only emitted if ASSET_RESOURCE_METADATA_ENABLED_CONTEXT
   }, ResourcePart.CompleteDefinition);
 });
 
+test('nested assemblies share assets: legacy synth edition', () => {
+  // GIVEN
+  const app = new cdk.App();
+  const stack1 = new cdk.Stack(new cdk.Stage(app, 'Stage1'), 'Stack', { synthesizer: new cdk.LegacyStackSynthesizer() });
+  const stack2 = new cdk.Stack(new cdk.Stage(app, 'Stage2'), 'Stack', { synthesizer: new cdk.LegacyStackSynthesizer() });
+
+  // WHEN
+  new Asset(stack1, 'MyAsset', { path: SAMPLE_ASSET_DIR });
+  new Asset(stack2, 'MyAsset', { path: SAMPLE_ASSET_DIR });
+
+  // THEN
+  const assembly = app.synth();
+
+  // Read the assets from the stack metadata
+  for (const stageName of ['Stage1', 'Stage2']) {
+    const stackArtifact = assembly.getNestedAssembly(`assembly-${stageName}`).artifacts.filter(isStackArtifact)[0];
+    const assetMeta = stackArtifact.findMetadataByType(cxschema.ArtifactMetadataEntryType.ASSET);
+    expect(assetMeta[0]).toEqual(
+      expect.objectContaining({
+        data: expect.objectContaining({
+          packaging: 'zip',
+          path: `../asset.${SAMPLE_ASSET_HASH}`,
+        }),
+      }),
+    );
+  }
+});
+
+test('nested assemblies share assets: default synth edition', () => {
+  // GIVEN
+  const app = new cdk.App();
+  const stack1 = new cdk.Stack(new cdk.Stage(app, 'Stage1'), 'Stack', { synthesizer: new cdk.DefaultStackSynthesizer() });
+  const stack2 = new cdk.Stack(new cdk.Stage(app, 'Stage2'), 'Stack', { synthesizer: new cdk.DefaultStackSynthesizer() });
+
+  // WHEN
+  new Asset(stack1, 'MyAsset', { path: SAMPLE_ASSET_DIR });
+  new Asset(stack2, 'MyAsset', { path: SAMPLE_ASSET_DIR });
+
+  // THEN
+  const assembly = app.synth();
+
+  // Read the asset manifests to verify the file paths
+  for (const stageName of ['Stage1', 'Stage2']) {
+    const manifestArtifact = assembly.getNestedAssembly(`assembly-${stageName}`).artifacts.filter(isAssetManifestArtifact)[0];
+    const manifest = JSON.parse(fs.readFileSync(manifestArtifact.file, { encoding: 'utf-8' }));
+
+    expect(manifest.files[SAMPLE_ASSET_HASH].source).toEqual({
+      packaging: 'zip',
+      path: `../asset.${SAMPLE_ASSET_HASH}`,
+    });
+  }
+});
+
+
 describe('staging', () => {
   test('copy file assets under <outdir>/${fingerprint}.ext', () => {
     const tempdir = mkdtempSync();
@@ -326,3 +381,11 @@ describe('staging', () => {
 function mkdtempSync() {
   return fs.mkdtempSync(path.join(os.tmpdir(), 'assets.test'));
 }
+
+function isStackArtifact(x: any): x is cxapi.CloudFormationStackArtifact {
+  return x instanceof cxapi.CloudFormationStackArtifact;
+}
+
+function isAssetManifestArtifact(x: any): x is cxapi.AssetManifestArtifact {
+  return x instanceof cxapi.AssetManifestArtifact;
+}