Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Oct 26, 2020
Merged
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assets/test/test.staging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
},

Expand All @@ -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();
},

Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-ecr-assets/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ nyc.config.js
*.snk
!.eslintrc.js

junit.xml
junit.xml
!jest.config.js
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-ecr-assets/.npmignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ tsconfig.json
# exclude cdk artifacts
**/cdk.out
junit.xml
test/
test/
jest.config.js
10 changes: 10 additions & 0 deletions packages/@aws-cdk/aws-ecr-assets/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const baseConfig = require('cdk-build-tools/config/jest.config');
module.exports = {
...baseConfig,
coverageThreshold: {
global: {
branches: 80,
statements: 80,
}
}
};
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ecr-assets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -112,6 +111,7 @@
"announce": false
},
"cdk-build": {
"jest": true,
"env": {
"AWSLINT_BASE_CONSTRUCT": true
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
Expand Down Expand Up @@ -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': [
{
Expand Down Expand Up @@ -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;
}
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-s3-assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand All @@ -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;
Expand Down
63 changes: 63 additions & 0 deletions packages/@aws-cdk/aws-s3-assets/test/asset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
Loading