Skip to content

Commit

Permalink
chore(core): verify stack artifacts are written in topological order (#…
Browse files Browse the repository at this point in the history
…8515)


Add a test to verify stacks are serialized to the manifest file in a topological order, based on their dependencies. By adding this test we are committing to maintain the topological order of the stacks in the manifest file, allowing users to consume it without needing to sort the stacks. 

Note that all of the CDK toolkit commands such as `cdk list` and `cdk deploy` already assume this and do not sort stacks upon execution.
> see [`cdk list`](https:/aws/aws-cdk/blob/master/packages/aws-cdk/lib/cdk-toolkit.ts#L264) and [`cdk deploy`](https:/aws/aws-cdk/blob/master/packages/aws-cdk/lib/cdk-toolkit.ts#L111) for reference  

I have initially added the test in `cx-api` module but after more consideration I think it is better suited in the `core` module since we want to verify this behavior in the context of a CDK application. I have left both tests in this PR for the sake of discussion in the event the reviewer thinks this actually fits better in `cx-api`.

helps #8436

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
NetaNir authored Jun 16, 2020
1 parent 1e6a8e9 commit 02752d3
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 1 deletion.
26 changes: 26 additions & 0 deletions packages/@aws-cdk/core/test/test.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,32 @@ export = {

test.done();
},

'stacks are written to the assembly file in a topological order'(test: Test) {
// WHEN
const assembly = withApp({}, (app) => {
const stackC = new Stack(app, 'StackC');
const stackD = new Stack(app, 'StackD');
const stackA = new Stack(app, 'StackA');
const stackB = new Stack(app, 'StackB');

// Create the following dependency order:
// A ->
// C -> D
// B ->
stackC.addDependency(stackA);
stackC.addDependency(stackB);
stackD.addDependency(stackC);
});

// THEN
const artifactsIds = assembly.artifacts.map(a => a.id);
test.ok(artifactsIds.indexOf('StackA') < artifactsIds.indexOf('StackC'));
test.ok(artifactsIds.indexOf('StackB') < artifactsIds.indexOf('StackC'));
test.ok(artifactsIds.indexOf('StackC') < artifactsIds.indexOf('StackD'));

test.done();
},
};

class MyConstruct extends Construct {
Expand Down
58 changes: 58 additions & 0 deletions packages/@aws-cdk/cx-api/test/cloud-assembly-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,62 @@ test('write and read nested cloud assembly artifact', () => {

const nested = art?.nestedAssembly;
expect(nested?.artifacts.length).toEqual(0);
});

test('artifcats are written in topological order', () => {
// GIVEN
const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'cloud-assembly-builder-tests'));
const session = new cxapi.CloudAssemblyBuilder(outdir);
const templateFile = 'foo.template.json';

const innerAsmDir = path.join(outdir, 'hello');
new cxapi.CloudAssemblyBuilder(innerAsmDir).buildAssembly();

// WHEN

// Create the following dependency order:
// A ->
// C -> D
// B ->
session.addArtifact('artifact-D', {
type: cxschema.ArtifactType.AWS_CLOUDFORMATION_STACK,
environment: 'aws://1222344/us-east-1',
dependencies: ['artifact-C'],
properties: {
templateFile,
},
});

session.addArtifact('artifact-C', {
type: cxschema.ArtifactType.AWS_CLOUDFORMATION_STACK,
environment: 'aws://1222344/us-east-1',
dependencies: ['artifact-B', 'artifact-A'],
properties: {
templateFile,
},
});

session.addArtifact('artifact-B', {
type: cxschema.ArtifactType.AWS_CLOUDFORMATION_STACK,
environment: 'aws://1222344/us-east-1',
properties: {
templateFile,
},
});

session.addArtifact('artifact-A', {
type: cxschema.ArtifactType.AWS_CLOUDFORMATION_STACK,
environment: 'aws://1222344/us-east-1',
properties: {
templateFile,
},
});

const asm = session.buildAssembly();
const artifactsIds = asm.artifacts.map(a => a.id);

// THEN
expect(artifactsIds.indexOf('artifact-A')).toBeLessThan(artifactsIds.indexOf('artifact-C'));
expect(artifactsIds.indexOf('artifact-B')).toBeLessThan(artifactsIds.indexOf('artifact-C'));
expect(artifactsIds.indexOf('artifact-C')).toBeLessThan(artifactsIds.indexOf('artifact-D'));
});
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ test('assets', () => {
test('can-read-0.36.0', () => {
// WHEN
new CloudAssembly(path.join(FIXTURES, 'single-stack-0.36'));
// THEN: no eexception
// THEN: no exception
expect(true).toBeTruthy();
});

Expand Down

0 comments on commit 02752d3

Please sign in to comment.