Skip to content

Commit

Permalink
feat(integ-runner): snapshots will now diff nested stack templates
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrgrain
mrgrain committed Nov 15, 2022
1 parent 4d7d85b commit 3476def
Show file tree
Hide file tree
Showing 29 changed files with 1,873 additions and 104 deletions.
34 changes: 33 additions & 1 deletion packages/@aws-cdk/integ-runner/lib/runner/private/cloud-assembly.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,25 @@ export class AssemblyManifestReader {
return stacks;
}

/**
* Get the nested stacks for a given stack
* returns a map of artifactId to CloudFormation template
*/
public getNestedStacksForStack(stackId: string): Record<string, any> {
const nestedTemplates: string[] = this.getAssetManifestsForStack(stackId).flatMap(
manifest => manifest.files
.filter(asset => asset.source.path?.endsWith('.nested.template.json'))
.map(asset => asset.source.path!),
);

const nestedStacks: Record<string, any> = Object.fromEntries(nestedTemplates.map(templateFile => ([
templateFile.split('.', 1)[0],
fs.readJSONSync(path.resolve(this.directory, templateFile)),
])));

return nestedStacks;
}

/**
* Write trace data to the assembly manifest metadata
*/
Expand Down Expand Up @@ -125,6 +144,19 @@ export class AssemblyManifestReader {
return assets;
}

/**
* Return a list of asset artifacts for a given stack
*/
public getAssetManifestsForStack(stackId: string): AssetManifest[] {
return Object.values(this.manifest.artifacts ?? {})
.filter(artifact =>
artifact.type === ArtifactType.ASSET_MANIFEST && (artifact.properties as AssetManifestProperties)?.file === `${stackId}.assets.json`)
.map(artifact => {
const fileName = (artifact.properties as AssetManifestProperties).file;
return AssetManifest.fromFile(path.join(this.directory, fileName));
});
}

/**
* Get a list of assets from the assembly manifest
*/
Expand Down Expand Up @@ -153,7 +185,7 @@ export class AssemblyManifestReader {
assetManifest.entries.forEach(entry => {
if (entry.type === 'file') {
const source = (entry as FileManifestEntry).source;
if (source.path && source.path.startsWith('asset.')) {
if (source.path && (source.path.startsWith('asset.') || source.path.endsWith('nested.template.json'))) {
assets.push(entry as FileManifestEntry);
}
} else if (entry.type === 'docker-image') {
Expand Down
222 changes: 129 additions & 93 deletions packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,25 @@ import { Diagnostic, DiagnosticReason, DestructiveChange, SnapshotVerificationOp
import { AssemblyManifestReader } from './private/cloud-assembly';
import { IntegRunnerOptions, IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base';

interface SnapshotAssembly {
/**
* Map of stacks that are part of this assembly
*/
[key: string]: {
/**
* All templates for this stack, including nested stacks
*/
templates: {
[key: string]: any
},

/**
* List of asset Ids that are used by this assembly
*/
assets: string[]
}
}

/**
* Runner for snapshot tests. This handles orchestrating
* the validation of the integration test snapshots
Expand All @@ -24,15 +43,7 @@ export class IntegSnapshotRunner extends IntegRunner {
public testSnapshot(options: SnapshotVerificationOptions = {}): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
let doClean = true;
try {
// read the existing snapshot
const expectedStacks = this.readAssembly(this.snapshotDir);
// only diff stacks that are part of the test case
const expectedStacksToDiff: Record<string, any> = {};
for (const [stackName, template] of Object.entries(expectedStacks)) {
if (this.expectedTestSuite?.stacks.includes(stackName)) {
expectedStacksToDiff[stackName] = template;
}
}
const expectedSnapshotAssembly = this.getSnapshotAssembly(this.snapshotDir, this.expectedTestSuite?.stacks);

// synth the integration test
// FIXME: ideally we should not need to run this again if
Expand All @@ -52,18 +63,10 @@ export class IntegSnapshotRunner extends IntegRunner {
});

// read the "actual" snapshot
const actualDir = this.cdkOutDir;
const actualStacks = this.readAssembly(actualDir);
// only diff stacks that are part of the test case
const actualStacksToDiff: Record<string, any> = {};
for (const [stackName, template] of Object.entries(actualStacks)) {
if (this.actualTestSuite.stacks.includes(stackName)) {
actualStacksToDiff[stackName] = template;
}
}
const actualSnapshotAssembly = this.getSnapshotAssembly(this.cdkOutDir, this.actualTestSuite.stacks);

// diff the existing snapshot (expected) with the integration test (actual)
const diagnostics = this.diffAssembly(expectedStacksToDiff, actualStacksToDiff);
const diagnostics = this.diffAssembly(expectedSnapshotAssembly, actualSnapshotAssembly);

if (diagnostics.diagnostics.length) {
// Attach additional messages to the first diagnostic
Expand All @@ -72,7 +75,7 @@ export class IntegSnapshotRunner extends IntegRunner {
if (options.retain) {
additionalMessages.push(
`(Failure retained) Expected: ${path.relative(process.cwd(), this.snapshotDir)}`,
` Actual: ${path.relative(process.cwd(), actualDir)}`,
` Actual: ${path.relative(process.cwd(), this.cdkOutDir)}`,
),
doClean = false;
}
Expand Down Expand Up @@ -107,6 +110,36 @@ export class IntegSnapshotRunner extends IntegRunner {
}
}

/**
* For a given cloud assembly return a collection of all templates
* that should be part of the snapshot and any required meta data.
*
* @param cloudAssemblyDir The directory of the cloud assembly to look for snapshots
* @param pickStacks Pick only these stacks from the cloud assembly
* @returns A SnapshotAssembly, the collection of all templates in this snapshot and required meta data
*/
private getSnapshotAssembly(cloudAssemblyDir: string, pickStacks: string[] = []): SnapshotAssembly {
const assembly = this.readAssembly(cloudAssemblyDir);
const stacks = assembly.stacks;
const snapshots: SnapshotAssembly = {};
for (const [stackName, stackTemplate] of Object.entries(stacks)) {
if (pickStacks.includes(stackName)) {
const manifest = AssemblyManifestReader.fromPath(cloudAssemblyDir);
const assets = manifest.getAssetIdsForStack(stackName);

snapshots[stackName] = {
templates: {
[stackName]: stackTemplate,
...assembly.getNestedStacksForStack(stackName),
},
assets,
};
}
}

return snapshots;
}

/**
* For a given stack return all resource types that are allowed to be destroyed
* as part of a stack update
Expand All @@ -131,86 +164,94 @@ export class IntegSnapshotRunner extends IntegRunner {
* @returns any diagnostics and any destructive changes
*/
private diffAssembly(
expected: Record<string, any>,
actual: Record<string, any>,
expected: SnapshotAssembly,
actual: SnapshotAssembly,
): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
const failures: Diagnostic[] = [];
const destructiveChanges: DestructiveChange[] = [];

// check if there is a CFN template in the current snapshot
// that does not exist in the "actual" snapshot
for (const templateId of Object.keys(expected)) {
if (!actual.hasOwnProperty(templateId)) {
failures.push({
testName: this.testName,
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: `${templateId} exists in snapshot, but not in actual`,
});
for (const [stackId, stack] of Object.entries(expected)) {
for (const templateId of Object.keys(stack.templates)) {
if (!actual[stackId]?.templates[templateId]) {
failures.push({
testName: this.testName,
stackName: templateId,
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: `${templateId} exists in snapshot, but not in actual`,
});
}
}
}

for (const templateId of Object.keys(actual)) {
for (const [stackId, stack] of Object.entries(actual)) {
for (const templateId of Object.keys(stack.templates)) {
// check if there is a CFN template in the "actual" snapshot
// that does not exist in the current snapshot
if (!expected.hasOwnProperty(templateId)) {
failures.push({
testName: this.testName,
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: `${templateId} does not exist in snapshot, but does in actual`,
});
continue;
} else {
let actualTemplate = actual[templateId];
let expectedTemplate = expected[templateId];
const allowedDestroyTypes = this.getAllowedDestroyTypesForStack(templateId) ?? [];

// if we are not verifying asset hashes then remove the specific
// asset hashes from the templates so they are not part of the diff
// comparison
if (!this.actualTestSuite.getOptionsForStack(templateId)?.diffAssets) {
actualTemplate = this.canonicalizeTemplate(actualTemplate, templateId, this.cdkOutDir);
expectedTemplate = this.canonicalizeTemplate(expectedTemplate, templateId, this.snapshotDir);
}
const templateDiff = diffTemplate(expectedTemplate, actualTemplate);
if (!templateDiff.isEmpty) {
// go through all the resource differences and check for any
// "destructive" changes
templateDiff.resources.forEachDifference((logicalId: string, change: ResourceDifference) => {
if (!expected[stackId]?.templates[templateId]) {
failures.push({
testName: this.testName,
stackName: templateId,
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: `${templateId} does not exist in snapshot, but does in actual`,
});
continue;
} else {
let actualTemplate = actual[stackId].templates[templateId];
let expectedTemplate = expected[stackId].templates[templateId];

// if we are not verifying asset hashes then remove the specific
// asset hashes from the templates so they are not part of the diff
// comparison
if (!this.actualTestSuite.getOptionsForStack(stackId)?.diffAssets) {
actualTemplate = this.canonicalizeTemplate(actualTemplate, actual[stackId].assets);
expectedTemplate = this.canonicalizeTemplate(expectedTemplate, expected[stackId].assets);
}
const templateDiff = diffTemplate(expectedTemplate, actualTemplate);
if (!templateDiff.isEmpty) {
const allowedDestroyTypes = this.getAllowedDestroyTypesForStack(stackId) ?? [];

// go through all the resource differences and check for any
// "destructive" changes
templateDiff.resources.forEachDifference((logicalId: string, change: ResourceDifference) => {
// if the change is a removal it will not show up as a 'changeImpact'
// so need to check for it separately, unless it is a resourceType that
// has been "allowed" to be destroyed
const resourceType = change.oldValue?.Type ?? change.newValue?.Type;
if (resourceType && allowedDestroyTypes.includes(resourceType)) {
return;
}
if (change.isRemoval) {
destructiveChanges.push({
impact: ResourceImpact.WILL_DESTROY,
logicalId,
stackName: templateId,
});
} else {
switch (change.changeImpact) {
case ResourceImpact.MAY_REPLACE:
case ResourceImpact.WILL_ORPHAN:
case ResourceImpact.WILL_DESTROY:
case ResourceImpact.WILL_REPLACE:
destructiveChanges.push({
impact: change.changeImpact,
logicalId,
stackName: templateId,
});
break;
const resourceType = change.oldValue?.Type ?? change.newValue?.Type;
if (resourceType && allowedDestroyTypes.includes(resourceType)) {
return;
}
}
});
const writable = new StringWritable({});
formatDifferences(writable, templateDiff);
failures.push({
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: writable.data,
testName: this.testName,
});
if (change.isRemoval) {
destructiveChanges.push({
impact: ResourceImpact.WILL_DESTROY,
logicalId,
stackName: templateId,
});
} else {
switch (change.changeImpact) {
case ResourceImpact.MAY_REPLACE:
case ResourceImpact.WILL_ORPHAN:
case ResourceImpact.WILL_DESTROY:
case ResourceImpact.WILL_REPLACE:
destructiveChanges.push({
impact: change.changeImpact,
logicalId,
stackName: templateId,
});
break;
}
}
});
const writable = new StringWritable({});
formatDifferences(writable, templateDiff);
failures.push({
reason: DiagnosticReason.SNAPSHOT_FAILED,
message: writable.data,
stackName: templateId,
testName: this.testName,
});
}
}
}
}
Expand All @@ -221,11 +262,8 @@ export class IntegSnapshotRunner extends IntegRunner {
};
}

private readAssembly(dir: string): Record<string, any> {
const assembly = AssemblyManifestReader.fromPath(dir);
const stacks = assembly.stacks;

return stacks;
private readAssembly(dir: string): AssemblyManifestReader {
return AssemblyManifestReader.fromPath(dir);
}

/**
Expand All @@ -234,7 +272,7 @@ export class IntegSnapshotRunner extends IntegRunner {
* This makes it possible to compare templates if all that's different between
* them is the hashes of the asset values.
*/
private canonicalizeTemplate(template: any, stackName: string, manifestDir: string): any {
private canonicalizeTemplate(template: any, assets: string[]): any {
const assetsSeen = new Set<string>();
const stringSubstitutions = new Array<[RegExp, string]>();

Expand Down Expand Up @@ -262,8 +300,6 @@ export class IntegSnapshotRunner extends IntegRunner {

// find assets defined in the asset manifest
try {
const manifest = AssemblyManifestReader.fromPath(manifestDir);
const assets = manifest.getAssetIdsForStack(stackName);
assets.forEach(asset => {
if (!assetsSeen.has(asset)) {
assetsSeen.add(asset);
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/integ-runner/lib/workers/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ export interface Diagnostic {
*/
readonly testName: string;

/**
* The name of the stack
*/
readonly stackName: string;

/**
* The diagnostic message
*/
Expand Down
5 changes: 5 additions & 0 deletions packages/@aws-cdk/integ-runner/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@
"license": "Apache-2.0",
"devDependencies": {
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-sns": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@types/mock-fs": "^4.13.1",
"constructs": "^10.0.0",
"mock-fs": "^4.14.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/fs-extra": "^8.1.2",
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/integ-runner/test/cli.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('CLI', () => {
[
'xxxxx.integ-test1.js',
'xxxxx.integ-test2.js',
'xxxxx.test-with-nested-stack.js',
'xxxxx.test-with-new-assets-diff.js',
'xxxxx.test-with-new-assets.js',
'xxxxx.test-with-snapshot-assets-diff.js',
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/integ-runner/test/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export class MockCdkProvider {
}),
integOutDir: compareToSnapshot ? 'test/test-data/' + compareToSnapshot : undefined,
});
integTest.actualTests();
const results = integTest.testSnapshot();

// THEN
Expand Down
Loading

0 comments on commit 3476def

Please sign in to comment.