From 5056d4c49f7ce32f93c0a3ce199fb2195ceb1201 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 19 Jun 2019 16:50:55 +0200 Subject: [PATCH 1/6] feat(toolkit): ensure consistent zip files Zip files were not consistent across deploys resulting in unnecessary S3 uploads and stack updates. Ensure consistency by appending files in series (guarantees file ordering in the zip) and reseting dates (guarantees same hash for same content). Closes #1997, Closes #2759 --- packages/aws-cdk/lib/archive.ts | 26 +++++++++++++++++++++----- packages/aws-cdk/package.json | 1 + 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/archive.ts b/packages/aws-cdk/lib/archive.ts index c924035bca369..9b7272ed668f2 100644 --- a/packages/aws-cdk/lib/archive.ts +++ b/packages/aws-cdk/lib/archive.ts @@ -1,23 +1,39 @@ import archiver = require('archiver'); import crypto = require('crypto'); import fs = require('fs-extra'); +import glob = require('glob'); +import path = require('path'); export function zipDirectory(directory: string, outputFile: string): Promise { return new Promise((ok, fail) => { const output = fs.createWriteStream(outputFile); const archive = archiver('zip'); + // The below options are needed to support following symlinks when building zip files: - // - nodir: This will prevent symlinks themselves from being copied into the zip. + // - nodir: This will prevent symlinks themselves from being copied into the zip. // - follow: This will follow symlinks and copy the files within. const globOptions = { dot: true, nodir: true, follow: true, - cwd: directory + cwd: directory, }; - archive.glob('**', globOptions); - archive.pipe(output); - archive.finalize(); + const files = glob.sync('**', globOptions); // The output here is already sorted + + output.on('open', async () => { + archive.pipe(output); + + await files.reduce(async (acc, file) => { // Append files serially to ensure file order + await acc; + const data = await fs.readFile(path.join(directory, file)); + archive.append(data, { + name: file, + date: new Date(0), // reset dates to get the same hash for the same content + }); + }, Promise.resolve()); + + archive.finalize(); + }); archive.on('warning', fail); archive.on('error', fail); diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index 07ef895d4d465..589eb6ca98399 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -60,6 +60,7 @@ "colors": "^1.3.3", "decamelize": "^3.2.0", "fs-extra": "^8.0.1", + "glob": "^7.1.4", "json-diff": "^0.5.4", "minimatch": ">=3.0", "promptly": "^3.0.3", From cfaa40da28a1b169d1de2261dabe1a23d1272101 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Wed, 19 Jun 2019 17:13:15 +0200 Subject: [PATCH 2/6] simplify --- packages/aws-cdk/lib/archive.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/lib/archive.ts b/packages/aws-cdk/lib/archive.ts index 9b7272ed668f2..83010158fbf16 100644 --- a/packages/aws-cdk/lib/archive.ts +++ b/packages/aws-cdk/lib/archive.ts @@ -23,14 +23,20 @@ export function zipDirectory(directory: string, outputFile: string): Promise { archive.pipe(output); - await files.reduce(async (acc, file) => { // Append files serially to ensure file order - await acc; + const contents = await Promise.all(files.map(async (file) => { const data = await fs.readFile(path.join(directory, file)); - archive.append(data, { - name: file, + return { + data, + name: file + }; + })); + + contents.forEach(content => { // Append files serially to ensure file order + archive.append(content.data, { + name: content.name, date: new Date(0), // reset dates to get the same hash for the same content }); - }, Promise.resolve()); + }); archive.finalize(); }); From 8e6ce583b002fc826897e97f3123210b7f5bcdbf Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Thu, 20 Jun 2019 10:41:11 +0200 Subject: [PATCH 3/6] test that dates are reset --- packages/aws-cdk/lib/archive.ts | 4 +-- packages/aws-cdk/package-lock.json | 48 +++++++++++++++++++++++++++ packages/aws-cdk/package.json | 2 ++ packages/aws-cdk/test/test.archive.ts | 8 +++++ packages/aws-cdk/tsconfig.json | 2 +- 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk/lib/archive.ts b/packages/aws-cdk/lib/archive.ts index 83010158fbf16..4702053aae214 100644 --- a/packages/aws-cdk/lib/archive.ts +++ b/packages/aws-cdk/lib/archive.ts @@ -31,10 +31,10 @@ export function zipDirectory(directory: string, outputFile: string): Promise { // Append files serially to ensure file order + contents.forEach((content) => { // Append files serially to ensure file order archive.append(content.data, { name: content.name, - date: new Date(0), // reset dates to get the same hash for the same content + date: new Date('1980-01-01T00:00:00.000Z'), // reset dates to get the same hash for the same content }); }); diff --git a/packages/aws-cdk/package-lock.json b/packages/aws-cdk/package-lock.json index d1c412cf0f97c..612cb52733666 100644 --- a/packages/aws-cdk/package-lock.json +++ b/packages/aws-cdk/package-lock.json @@ -107,6 +107,15 @@ "@types/node": "*" } }, + "@types/jszip": { + "version": "3.1.6", + "resolved": "https://registry.npmjs.org/@types/jszip/-/jszip-3.1.6.tgz", + "integrity": "sha512-m8uFcI+O2EupCfbEVQWsBM/4nhbegjOHL7cQgBpM95FeF98kdFJXzy9/8yhx4b3lCRl/gMBhcvyh30Qt3X+XPQ==", + "dev": true, + "requires": { + "@types/node": "*" + } + }, "@types/minimatch": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/@types/minimatch/-/minimatch-3.0.3.tgz", @@ -990,6 +999,12 @@ "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.1.13.tgz", "integrity": "sha512-4vf7I2LYV/HaWerSo3XmlMkp5eZ83i+/CDluXi/IGTs/O1sejBNhTtnxzmRZfvOUqj7lZjqHkeTvpgSFDlWZTg==" }, + "immediate": { + "version": "3.0.6", + "resolved": "https://registry.npmjs.org/immediate/-/immediate-3.0.6.tgz", + "integrity": "sha1-nbHb0Pr43m++D13V5Wu2BigN5ps=", + "dev": true + }, "inflight": { "version": "1.0.6", "resolved": "https://registry.npmjs.org/inflight/-/inflight-1.0.6.tgz", @@ -1098,6 +1113,18 @@ "verror": "1.10.0" } }, + "jszip": { + "version": "3.2.1", + "resolved": "https://registry.npmjs.org/jszip/-/jszip-3.2.1.tgz", + "integrity": "sha512-iCMBbo4eE5rb1VCpm5qXOAaUiRKRUKiItn8ah2YQQx9qymmSAY98eyQfioChEYcVQLh0zxJ3wS4A0mh90AVPvw==", + "dev": true, + "requires": { + "lie": "~3.3.0", + "pako": "~1.0.2", + "readable-stream": "~2.3.6", + "set-immediate-shim": "~1.0.1" + } + }, "just-extend": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-4.0.2.tgz", @@ -1129,6 +1156,15 @@ "type-check": "~0.3.2" } }, + "lie": { + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/lie/-/lie-3.3.0.tgz", + "integrity": "sha512-UaiMJzeWRlEujzAuw5LokY1L5ecNQYZKfmyZ9L7wDHb/p5etKaxXhohBcrw0EYby+G/NA52vRSN4N39dxHAIwQ==", + "dev": true, + "requires": { + "immediate": "~3.0.5" + } + }, "locate-path": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz", @@ -1388,6 +1424,12 @@ "thunkify": "^2.1.2" } }, + "pako": { + "version": "1.0.10", + "resolved": "https://registry.npmjs.org/pako/-/pako-1.0.10.tgz", + "integrity": "sha512-0DTvPVU3ed8+HNXOu5Bs+o//Mbdj9VNQMUOe9oKCwh8l0GNwpTDMKCWbRjgtD291AWnkAgkqA/LOnQS8AmS1tw==", + "dev": true + }, "path-exists": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-3.0.0.tgz", @@ -1608,6 +1650,12 @@ "resolved": "https://registry.npmjs.org/set-blocking/-/set-blocking-2.0.0.tgz", "integrity": "sha1-BF+XgtARrppoA93TgrJDkrPYkPc=" }, + "set-immediate-shim": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/set-immediate-shim/-/set-immediate-shim-1.0.1.tgz", + "integrity": "sha1-SysbJ+uAip+NzEgaWOXlb1mfP2E=", + "dev": true + }, "setprototypeof": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/setprototypeof/-/setprototypeof-1.1.1.tgz", diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index 589eb6ca98399..74b6da092b6c1 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -36,6 +36,7 @@ "devDependencies": { "@types/archiver": "^3.0.0", "@types/fs-extra": "^7.0.0", + "@types/jszip": "^3.1.6", "@types/minimatch": "^3.0.3", "@types/mockery": "^1.4.29", "@types/request": "^2.48.1", @@ -46,6 +47,7 @@ "@types/yaml": "^1.0.2", "@types/yargs": "^13.0.0", "cdk-build-tools": "^0.34.0", + "jszip": "^3.2.1", "mockery": "^2.1.0", "pkglint": "^0.34.0", "sinon": "^7.3.2" diff --git a/packages/aws-cdk/test/test.archive.ts b/packages/aws-cdk/test/test.archive.ts index f26b662e79ca2..e1afe3460dc3b 100644 --- a/packages/aws-cdk/test/test.archive.ts +++ b/packages/aws-cdk/test/test.archive.ts @@ -1,5 +1,6 @@ import { exec as _exec } from 'child_process'; import fs = require('fs-extra'); +import jszip = require('jszip'); import { Test } from 'nodeunit'; import os = require('os'); import path = require('path'); @@ -24,6 +25,13 @@ export = { test.ok(false, `extracted directory ${extractDir} differs from original ${originalDir}`); } + // inspect the zile file to check that dates are reset + const zip = await fs.readFile(zipFile); + const zipData = await jszip.loadAsync(zip); + const dates = Object.values(zipData.files).map(file => file.date.toISOString()); + test.equals(dates[0], '1980-01-01T00:00:00.000Z', 'Dates are not reset'); + test.equal(new Set(dates).size, 1, 'Dates are not equal'); + await fs.remove(stagingDir); await fs.remove(extractDir); test.done(); diff --git a/packages/aws-cdk/tsconfig.json b/packages/aws-cdk/tsconfig.json index 0c02dab2c6a45..0eac39a6083af 100644 --- a/packages/aws-cdk/tsconfig.json +++ b/packages/aws-cdk/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "target":"ES2018", "module": "commonjs", - "lib": ["es2016", "es2017.object", "es2017.string"], + "lib": ["es2016", "es2017.object", "es2017.string", "dom"], "declaration": true, "strict": true, "noImplicitAny": true, From 379d24657e01cd8cd41883e21f418759f7411cc3 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Thu, 20 Jun 2019 10:42:25 +0200 Subject: [PATCH 4/6] equal --- packages/aws-cdk/test/test.archive.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/test.archive.ts b/packages/aws-cdk/test/test.archive.ts index e1afe3460dc3b..c709cce749765 100644 --- a/packages/aws-cdk/test/test.archive.ts +++ b/packages/aws-cdk/test/test.archive.ts @@ -29,7 +29,7 @@ export = { const zip = await fs.readFile(zipFile); const zipData = await jszip.loadAsync(zip); const dates = Object.values(zipData.files).map(file => file.date.toISOString()); - test.equals(dates[0], '1980-01-01T00:00:00.000Z', 'Dates are not reset'); + test.equal(dates[0], '1980-01-01T00:00:00.000Z', 'Dates are not reset'); test.equal(new Set(dates).size, 1, 'Dates are not equal'); await fs.remove(stagingDir); From a8c12f6d8934df915f6aac697f21adc9c64d4013 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Thu, 20 Jun 2019 15:26:04 +0200 Subject: [PATCH 5/6] better with streams? --- packages/aws-cdk/lib/archive.ts | 33 +++++++++++------------------- packages/aws-cdk/package-lock.json | 2 +- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/packages/aws-cdk/lib/archive.ts b/packages/aws-cdk/lib/archive.ts index 4702053aae214..d515314006f8a 100644 --- a/packages/aws-cdk/lib/archive.ts +++ b/packages/aws-cdk/lib/archive.ts @@ -6,9 +6,6 @@ import path = require('path'); export function zipDirectory(directory: string, outputFile: string): Promise { return new Promise((ok, fail) => { - const output = fs.createWriteStream(outputFile); - const archive = archiver('zip'); - // The below options are needed to support following symlinks when building zip files: // - nodir: This will prevent symlinks themselves from being copied into the zip. // - follow: This will follow symlinks and copy the files within. @@ -20,29 +17,23 @@ export function zipDirectory(directory: string, outputFile: string): Promise { - archive.pipe(output); + const output = fs.createWriteStream(outputFile); - const contents = await Promise.all(files.map(async (file) => { - const data = await fs.readFile(path.join(directory, file)); - return { - data, - name: file - }; - })); + const archive = archiver('zip'); + archive.on('warning', fail); + archive.on('error', fail); + archive.pipe(output); - contents.forEach((content) => { // Append files serially to ensure file order - archive.append(content.data, { - name: content.name, - date: new Date('1980-01-01T00:00:00.000Z'), // reset dates to get the same hash for the same content - }); + files.forEach(file => { // Append files serially to ensure file order + const stream = fs.createReadStream(path.join(directory, file)); + archive.append(stream, { + name: file, + date: new Date('1980-01-01T00:00:00.000Z'), // reset dates to get the same hash for the same content }); - - archive.finalize(); }); - archive.on('warning', fail); - archive.on('error', fail); + archive.finalize(); + output.once('close', () => ok()); }); } diff --git a/packages/aws-cdk/package-lock.json b/packages/aws-cdk/package-lock.json index 85c41efc57ce2..5cf170ac1a8ce 100644 --- a/packages/aws-cdk/package-lock.json +++ b/packages/aws-cdk/package-lock.json @@ -1,6 +1,6 @@ { "name": "aws-cdk", - "version": "0.34.0", + "version": "0.35.0", "lockfileVersion": 1, "requires": true, "dependencies": { From 491b4c8ab34db014e20a71dd95f976f3ef606720 Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Fri, 21 Jun 2019 10:20:00 +0200 Subject: [PATCH 6/6] add comment --- packages/aws-cdk/lib/archive.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/archive.ts b/packages/aws-cdk/lib/archive.ts index d515314006f8a..a5994ea035369 100644 --- a/packages/aws-cdk/lib/archive.ts +++ b/packages/aws-cdk/lib/archive.ts @@ -25,8 +25,7 @@ export function zipDirectory(directory: string, outputFile: string): Promise { // Append files serially to ensure file order - const stream = fs.createReadStream(path.join(directory, file)); - archive.append(stream, { + archive.append(fs.createReadStream(path.join(directory, file)), { name: file, date: new Date('1980-01-01T00:00:00.000Z'), // reset dates to get the same hash for the same content }); @@ -34,6 +33,7 @@ export function zipDirectory(directory: string, outputFile: string): Promise ok()); }); }