Skip to content

Commit

Permalink
fix(core): use node.path in skip bundling check for consistency with …
Browse files Browse the repository at this point in the history
…cdk deploy CLI (aws#19950)

The pattern given to aws-cdk on the CLI (`cdk deploy $pattern`) is matched against `stack.hierarchicalId` (i.e. displayName, node.path) [1] [2] [3].

The same pattern is given to the CDK app (as `bundlingStacks`) but is matching against `stackName`, causing potential for bundling to be skipped under certain scenarios (i.e. using --exclusively with custom stack names).

To make them consistent, the skip bundling check has been changed to match against the same value as `cdk deploy $pattern` (node.path/hierarchicalId/displayName) when deciding whether to bundle an asset. Making them consistent ensures necessary stacks are bundled, avoiding the issue of uploading the entire project directory since the asset wasn't bundled.

fixes aws#19927

[1] https://github.com/aws/aws-cdk/blob/1d0270446b3effa6b8518de3c7d76f0c14e626c5/packages/aws-cdk/lib/api/cxapp/cloud-assembly.ts#L138
[2] https://github.com/aws/aws-cdk/blob/6cca62db88866479f2b1d4f52b30beab3d78aeb2/packages/@aws-cdk/cx-api/lib/cloud-artifact.ts#L143-L145
[3] https://github.com/aws/aws-cdk/blob/6cca62db88866479f2b1d4f52b30beab3d78aeb2/packages/@aws-cdk/core/lib/stack-synthesizers/_shared.ts#L66


----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
stevehodgkiss committed Jul 13, 2022
1 parent 69d25a6 commit 5cff2d9
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
6 changes: 3 additions & 3 deletions packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1168,10 +1168,10 @@ export class Stack extends Construct implements ITaggable {
public get bundlingRequired() {
const bundlingStacks: string[] = this.node.tryGetContext(cxapi.BUNDLING_STACKS) ?? ['*'];

// bundlingStacks is of the form `Stage/Stack`, convert it to `Stage-Stack` before comparing to stack name
// bundlingStacks is of the form `Stage/Stack`
return bundlingStacks.some(pattern => minimatch(
this.stackName,
pattern.replace('/', '-'),
this.node.path, // the same value used for pattern matching in aws-cdk CLI (displayName / hierarchicalId)
pattern,
));
}
}
Expand Down
40 changes: 39 additions & 1 deletion packages/@aws-cdk/core/test/staging.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,45 @@ describe('staging', () => {
const dockerStubInput = readDockerStubInputConcat();
// Docker ran for the asset in Stack1
expect(dockerStubInput).toMatch(DockerStubCommand.SUCCESS);
// DOcker did not run for the asset in Stack2
// Docker did not run for the asset in Stack2
expect(dockerStubInput).not.toMatch(DockerStubCommand.MULTIPLE_FILES);
});

test('correctly skips bundling with stack under stage and custom stack name', () => {
// 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', { stackName: 'unrelated-stack1-name' });
const stack2 = new Stack(stage, 'Stack2', { stackName: 'unrelated-stack2-name' });
const directory = path.join(__dirname, 'fs', 'fixtures', 'test1');

// WHEN
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],
},
});

// THEN
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);
});

Expand Down

0 comments on commit 5cff2d9

Please sign in to comment.