Skip to content

Commit

Permalink
fix(core): bundling skipped with --exclusively option and stacks unde…
Browse files Browse the repository at this point in the history
…r stage (aws#17210)

We were comparing bundling stacks of the form `Stage/Stack` with stack
names of the form `Stage-Stack`.

For stacks with `NodejsFunction`s this leads to assets containing the whole CDK project because
when bundling is skipped the asset references the source directory which is the project root.

Closes aws#12898
Closes aws#15346


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
jogold authored and Andrew Beresford committed Nov 29, 2021
1 parent 6e30c0b commit aa287e6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 1 deletion.
3 changes: 2 additions & 1 deletion packages/@aws-cdk/core/lib/asset-staging.ts
Expand Up @@ -189,7 +189,8 @@ export class AssetStaging extends CoreConstruct {
if (props.bundling) {
// Check if we actually have to bundle for this stack
const bundlingStacks: string[] = this.node.tryGetContext(cxapi.BUNDLING_STACKS) ?? ['*'];
skip = !bundlingStacks.find(pattern => minimatch(Stack.of(this).stackName, pattern));
// bundlingStacks is of the form `Stage/Stack`, convert it to `Stage-Stack` before comparing to stack name
skip = !bundlingStacks.find(pattern => minimatch(Stack.of(this).stackName, pattern.replace('/', '-')));
const bundling = props.bundling;
stageThisAsset = () => this.stageByBundling(bundling, skip);
} else {
Expand Down
34 changes: 34 additions & 0 deletions packages/@aws-cdk/core/test/staging.test.ts
Expand Up @@ -859,8 +859,42 @@ describe('staging', () => {
expect(asset.stagedPath).toEqual(directory);
expect(asset.relativeStagedPath(stack)).toEqual(directory);
expect(asset.assetHash).toEqual('f66d7421aa2d044a6c1f60ddfc76dc78571fcd8bd228eb48eb394e2dbad94a5c');
});

test('correctly skips bundling with stack under stage', () => {
// GIVEN
const app = new App();

const stage = new Stage(app, 'Stage');
stage.node.setContext(cxapi.BUNDLING_STACKS, ['Stage/Stack1']);

const stack1 = new Stack(stage, 'Stack1');
const stack2 = new Stack(stage, 'Stack2');
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

new AssetStaging(stack1, 'Asset', {
sourcePath: directory,
assetHashType: AssetHashType.OUTPUT,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.SUCCESS],
},
});

new AssetStaging(stack2, 'Asset', {
sourcePath: directory,
assetHashType: AssetHashType.OUTPUT,
bundling: {
image: DockerImage.fromRegistry('alpine'),
command: [DockerStubCommand.MULTIPLE_FILES],
},
});

const dockerStubInput = readDockerStubInputConcat();
// Docker ran for the asset in Stack1
expect(dockerStubInput).toMatch(DockerStubCommand.SUCCESS);
// DOcker did not run for the asset in Stack2
expect(dockerStubInput).not.toMatch(DockerStubCommand.MULTIPLE_FILES);
});

test('bundling still occurs with partial wildcard', () => {
Expand Down

0 comments on commit aa287e6

Please sign in to comment.