Skip to content

Commit

Permalink
fix(core): output folder checksum is computed unnecessarily (#25392)
Browse files Browse the repository at this point in the history
The output folder checksum is being at least once in every synthesis. This is only necessary if a validation plugin was registered. Also reusing the `FileSystem.fingerprint()` function while I'm at it.

Fixes #25286

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
otaviomacedo authored May 2, 2023
1 parent 53d24f1 commit f2294ba
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 18 deletions.
26 changes: 8 additions & 18 deletions packages/aws-cdk-lib/core/lib/private/synthesis.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { createHash } from 'crypto';
import * as fs from 'fs';
import * as path from 'path';
import * as cxapi from '../../../cx-api';
Expand All @@ -16,6 +15,7 @@ import { Stage, StageSynthesisOptions } from '../stage';
import { IPolicyValidationPluginBeta1 } from '../validation';
import { ConstructTree } from '../validation/private/construct-tree';
import { PolicyValidationReportFormatter, NamedValidationPluginReport } from '../validation/private/report';
import { FileSystem } from '../fs';

const POLICY_VALIDATION_FILE_PATH = 'policy-validation-report.json';
const VALIDATION_REPORT_JSON_CONTEXT = '@aws-cdk/core:validationReportJson';
Expand Down Expand Up @@ -90,7 +90,7 @@ function getAssemblies(root: App, rootAssembly: CloudAssembly): Map<string, Clou
*/
function invokeValidationPlugins(root: IConstruct, outdir: string, assembly: CloudAssembly) {
if (!App.isApp(root)) return;
const hash = computeChecksumOfFolder(outdir);
let hash: string | undefined;
const assemblies = getAssemblies(root, assembly);
const templatePathsByPlugin: Map<IPolicyValidationPluginBeta1, string[]> = new Map();
visitAssemblies(root, 'post', construct => {
Expand All @@ -111,6 +111,11 @@ function invokeValidationPlugins(root: IConstruct, outdir: string, assembly: Clo
// eslint-disable-next-line no-console
console.log('Performing Policy Validations\n');
}

if (templatePathsByPlugin.size > 0) {
hash = FileSystem.fingerprint(outdir);
}

for (const [plugin, paths] of templatePathsByPlugin.entries()) {
try {
const report = plugin.validate({ templatePaths: paths });
Expand All @@ -126,7 +131,7 @@ function invokeValidationPlugins(root: IConstruct, outdir: string, assembly: Clo
},
});
}
if (computeChecksumOfFolder(outdir) !== hash) {
if (FileSystem.fingerprint(outdir) !== hash) {
throw new Error(`Illegal operation: validation plugin '${plugin.name}' modified the cloud assembly`);
}
}
Expand Down Expand Up @@ -162,21 +167,6 @@ function invokeValidationPlugins(root: IConstruct, outdir: string, assembly: Clo
}
}

function computeChecksumOfFolder(folder: string): string {
const hash = createHash('sha256');
const files = fs.readdirSync(folder, { withFileTypes: true });

for (const file of files) {
const fullPath = path.join(folder, file.name);
if (file.isDirectory()) {
hash.update(computeChecksumOfFolder(fullPath));
} else if (file.isFile()) {
hash.update(fs.readFileSync(fullPath));
}
}
return hash.digest().toString('hex');
}

const CUSTOM_SYNTHESIS_SYM = Symbol.for('@aws-cdk/core:customSynthesis');

/**
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk-lib/core/test/synthesis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,17 @@ describe('synthesis', () => {
expect(stack.parameters).toEqual({ paramId: 'paramValue', paramId2: 'paramValue2' });
expect(stack.environment).toEqual({ region: 'us-east-1', account: 'unknown-account', name: 'aws://unknown-account/us-east-1' });
});

test('output folder checksum is not computed by default', () => {
const fingerprint = jest.spyOn(cdk.FileSystem, 'fingerprint');
const app = new cdk.App(); // <-- no validation plugins
const stack = new cdk.Stack(app, 'one-stack');
synthesize(stack);

expect(fingerprint).not.toHaveBeenCalled();

jest.restoreAllMocks();
});
});

function list(outdir: string) {
Expand Down

0 comments on commit f2294ba

Please sign in to comment.