Skip to content

Commit

Permalink
chore: make tests independent of asset hashes
Browse files Browse the repository at this point in the history
Introduce a `canonicalizeTemplate` function in `@aws-cdk/assert` which
translates templates to a common form w.r.t. asset hashes.

Use this in some tests and in `cdk-integ-assert` to make the tests
succeed if all that is different is the specific asset hash.

Currently only supports legacy assets, should be updated for new-style
assets when we switch to those.

This change is necessary to unblock other build improvements/changes
such as #8946.
  • Loading branch information
rix0rrr committed Jul 9, 2020
1 parent a6bffa1 commit cc9704a
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 24 deletions.
72 changes: 72 additions & 0 deletions packages/@aws-cdk/assert/lib/canonicalize-assets.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Reduce template to a normal form where asset references have been normalized
*
* This makes it possible to compare templates if all that's different between
* them is the hashes of the asset values.
*
* Currently only handles parameterized assets, but can (and should)
* be adapted to handle convention-mode assets as well when we start using
* more of those.
*/
export function canonicalizeTemplate(template: any): any {
// For the weird case where we have an array of templates...
if (Array.isArray(template)) {
return template.map(canonicalizeTemplate);
}

// Find assets via parameters
const stringSubstitutions = new Array<[RegExp, string]>();
const paramRe = /^AssetParameters([a-zA-Z0-9]{64})(S3Bucket|S3VersionKey|ArtifactHash)([a-zA-Z0-9]{8})$/;

const assetsSeen = new Set<string>();
for (const paramName of Object.keys(template?.Parameters || {})) {
const m = paramRe.exec(paramName);
if (!m) { continue; }
if (assetsSeen.has(m[1])) { continue; }

assetsSeen.add(m[1]);
const ix = assetsSeen.size;

// Full parameter reference
stringSubstitutions.push([
new RegExp(`AssetParameters${m[1]}(S3Bucket|S3VersionKey|ArtifactHash)([a-zA-Z0-9]{8})`),
`Asset${ix}$1`,
]);
// Substring asset hash reference
stringSubstitutions.push([
new RegExp(`${m[1]}`),
`Asset${ix}Hash`,
]);
}


// Substitute them out
return substitute(template);

function substitute(what: any): any {
if (Array.isArray(what)) {
return what.map(substitute);
}

if (typeof what === 'object' && what !== null) {
const ret: any = {};
for (const [k, v] of Object.entries(what)) {
ret[stringSub(k)] = substitute(v);
}
return ret;
}

if (typeof what === 'string') {
return stringSub(what);
}

return what;
}

function stringSub(x: string) {
for (const [re, replacement] of stringSubstitutions) {
x = x.replace(re, replacement);
}
return x;
}
}
6 changes: 4 additions & 2 deletions packages/@aws-cdk/assert/lib/expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import * as api from '@aws-cdk/cx-api';
import { StackInspector } from './inspector';
import { SynthUtils } from './synth-utils';

export function expect(stack: api.CloudFormationStackArtifact | cdk.Stack, skipValidation = false): StackInspector {
export function expect(stack: api.CloudFormationStackArtifact | cdk.Stack | Record<string, any>, skipValidation = false): StackInspector {
// if this is already a synthesized stack, then just inspect it.
const artifact = stack instanceof api.CloudFormationStackArtifact ? stack : SynthUtils._synthesizeWithNested(stack, { skipValidation });
const artifact = stack instanceof api.CloudFormationStackArtifact ? stack
: cdk.Stack.isStack(stack) ? SynthUtils._synthesizeWithNested(stack, { skipValidation })
: stack; // This is a template already
return new StackInspector(artifact);
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/assert/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from './assertion';
export * from './canonicalize-assets';
export * from './expect';
export * from './inspector';
export * from './synth-utils';
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-lambda/test/test.layers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, haveResource, ResourcePart } from '@aws-cdk/assert';
import { canonicalizeTemplate, expect, haveResource, ResourcePart, SynthUtils } from '@aws-cdk/assert';
import * as s3 from '@aws-cdk/aws-s3';
import * as cdk from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
Expand Down Expand Up @@ -85,9 +85,9 @@ export = testCase({
});

// THEN
expect(stack).to(haveResource('AWS::Lambda::LayerVersion', {
expect(canonicalizeTemplate(SynthUtils.toCloudFormation(stack))).to(haveResource('AWS::Lambda::LayerVersion', {
Metadata: {
'aws:asset:path': 'asset.8811a2632ac5564a08fd269e159298f7e497f259578b0dc5e927a1f48ab24d34',
'aws:asset:path': 'asset.Asset1Hash',
'aws:asset:property': 'Content',
},
}, ResourcePart.CompleteDefinition));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ async function main() {
]);

await write('jest.config.js', [
"const baseConfig = require('../../../tools/cdk-build-tools/config/jest.config');",
"const baseConfig = require('cdk-build-tools/config/jest.config');",
'module.exports = baseConfig;',
]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as fs from 'fs';
import { Test } from 'nodeunit';
import * as path from 'path';
import { CustomResourceProvider, CustomResourceProviderRuntime, Duration, Size, Stack } from '../../lib';
import { Test } from 'nodeunit';
import { CustomResourceProvider, CustomResourceProviderRuntime, Duration, Size, Stack, AssetStaging } from '../../lib';
import { toCloudFormation } from '../util';

const TEST_HANDLER = `${__dirname}/mock-provider`;
Expand All @@ -20,6 +20,16 @@ export = {
// THEN
test.ok(fs.existsSync(path.join(TEST_HANDLER, '__entrypoint__.js')), 'expecting entrypoint to be copied to the handler directory');
const cfn = toCloudFormation(stack);

// The asset hash constantly changes, so in order to not have to chase it, just look
// it up from the output.
const staging = stack.node.tryFindChild('Custom:MyResourceTypeCustomResourceProvider')?.node.tryFindChild('Staging') as AssetStaging;
const assetHash = staging.sourceHash;
const paramNames = Object.keys(cfn.Parameters);
const bucketParam = paramNames[0];
const keyParam = paramNames[1];
const hashParam = paramNames[2];

test.deepEqual(cfn, {
Resources: {
CustomMyResourceTypeCustomResourceProviderRoleBD5E655F: {
Expand Down Expand Up @@ -48,9 +58,7 @@ export = {
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: {
Ref: 'AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226S3Bucket8B4D0E9E',
},
S3Bucket: { Ref: bucketParam },
S3Key: {
'Fn::Join': [
'',
Expand All @@ -61,9 +69,7 @@ export = {
{
'Fn::Split': [
'||',
{
Ref: 'AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226S3VersionKeyDECB34FE',
},
{ Ref: keyParam },
],
},
],
Expand All @@ -74,9 +80,7 @@ export = {
{
'Fn::Split': [
'||',
{
Ref: 'AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226S3VersionKeyDECB34FE',
},
{ Ref: keyParam },
],
},
],
Expand All @@ -102,17 +106,17 @@ export = {
},
},
Parameters: {
AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226S3Bucket8B4D0E9E: {
[bucketParam]: {
Type: 'String',
Description: 'S3 bucket for asset "925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226"',
Description: `S3 bucket for asset "${assetHash}"`,
},
AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226S3VersionKeyDECB34FE: {
[keyParam]: {
Type: 'String',
Description: 'S3 key for asset version "925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226"',
Description: `S3 key for asset version "${assetHash}"`,
},
AssetParameters925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226ArtifactHashEEC400F2: {
[hashParam]: {
Type: 'String',
Description: 'Artifact hash for asset "925e7fbbec7bdbf0136ef5a07b8a0fbe0b1f1bb4ea50ae2154163df78aa9f226"',
Description: `Artifact hash for asset "${assetHash}"`,
},
},
});
Expand Down
3 changes: 2 additions & 1 deletion tools/cdk-integ-tools/bin/cdk-integ-assert.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env node
// Verify that all integration tests still match their expected output
import { canonicalizeTemplate } from '@aws-cdk/assert';
import { diffTemplate, formatDifferences } from '@aws-cdk/cloudformation-diff';
import { DEFAULT_SYNTH_OPTIONS, IntegrationTests } from '../lib/integ-helpers';

Expand All @@ -20,7 +21,7 @@ async function main() {

const actual = await test.cdkSynthFast(DEFAULT_SYNTH_OPTIONS);

const diff = diffTemplate(expected, actual);
const diff = diffTemplate(canonicalizeTemplate(expected), canonicalizeTemplate(actual));

if (!diff.isEmpty) {
failures.push(test.name);
Expand Down
1 change: 1 addition & 0 deletions tools/cdk-integ-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"dependencies": {
"@aws-cdk/cloudformation-diff": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/assert": "0.0.0",
"aws-cdk": "0.0.0",
"fs-extra": "^9.0.1",
"yargs": "^15.3.1"
Expand Down

0 comments on commit cc9704a

Please sign in to comment.