From 40b79323c0e1acf7c67451d4c18b5f9cd0b40f1e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Fri, 19 Feb 2021 09:08:01 +0200 Subject: [PATCH 1/2] fix: ENOTDIR invalid cwd on "cdk deploy" This commit reverts two recent changes to the asset system (#12258 and ##13076) which introduced a regression in 1.90.0. Fixes #13131 --- packages/@aws-cdk/aws-lambda-nodejs/README.md | 4 +- .../aws-lambda-nodejs/test/bundling.test.ts | 2 +- packages/@aws-cdk/aws-lambda/README.md | 11 +- packages/@aws-cdk/aws-lambda/lib/code.ts | 37 ------ .../@aws-cdk/aws-lambda/test/code.test.ts | 23 ---- .../test/docker-build-lambda/Dockerfile | 3 - .../test/docker-build-lambda/index.ts | 5 - packages/@aws-cdk/aws-s3-assets/README.md | 27 +--- packages/@aws-cdk/aws-s3-assets/lib/asset.ts | 30 ++++- packages/@aws-cdk/core/lib/asset-staging.ts | 90 +------------ packages/@aws-cdk/core/lib/bundling.ts | 74 +---------- .../@aws-cdk/core/test/archive/archive.zip | 0 packages/@aws-cdk/core/test/bundling.test.ts | 23 +--- packages/@aws-cdk/core/test/docker-stub.sh | 15 +-- packages/@aws-cdk/core/test/staging.test.ts | 118 +----------------- 15 files changed, 52 insertions(+), 410 deletions(-) delete mode 100644 packages/@aws-cdk/aws-lambda/test/docker-build-lambda/Dockerfile delete mode 100644 packages/@aws-cdk/aws-lambda/test/docker-build-lambda/index.ts delete mode 100644 packages/@aws-cdk/core/test/archive/archive.zip diff --git a/packages/@aws-cdk/aws-lambda-nodejs/README.md b/packages/@aws-cdk/aws-lambda-nodejs/README.md index 6e99c84835046..bfaa99eb0b243 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/README.md +++ b/packages/@aws-cdk/aws-lambda-nodejs/README.md @@ -148,7 +148,7 @@ new lambda.NodejsFunction(this, 'my-handler', { }, logLevel: LogLevel.SILENT, // defaults to LogLevel.WARNING keepNames: true, // defaults to false - tsconfig: 'custom-tsconfig.json' // use custom-tsconfig.json instead of default, + tsconfig: 'custom-tsconfig.json' // use custom-tsconfig.json instead of default, metafile: true, // include meta file, defaults to false banner : '/* comments */', // by default no comments are passed footer : '/* comments */', // by default no comments are passed @@ -216,7 +216,7 @@ Use `bundling.dockerImage` to use a custom Docker bundling image: ```ts new lambda.NodejsFunction(this, 'my-handler', { bundling: { - dockerImage: cdk.DockerImage.fromBuild('/path/to/Dockerfile'), + dockerImage: cdk.BundlingDockerImage.fromAsset('/path/to/Dockerfile'), }, }); ``` diff --git a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts index 51da3b8575400..bd69394ae757c 100644 --- a/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts +++ b/packages/@aws-cdk/aws-lambda-nodejs/test/bundling.test.ts @@ -20,7 +20,7 @@ beforeEach(() => { getEsBuildVersionMock.mockReturnValue('0.8.8'); fromAssetMock.mockReturnValue({ image: 'built-image', - cp: () => 'dest-path', + cp: () => {}, run: () => {}, toJSON: () => 'built-image', }); diff --git a/packages/@aws-cdk/aws-lambda/README.md b/packages/@aws-cdk/aws-lambda/README.md index 1f4ee7e5aaa46..98994962ec129 100644 --- a/packages/@aws-cdk/aws-lambda/README.md +++ b/packages/@aws-cdk/aws-lambda/README.md @@ -36,9 +36,6 @@ runtime code. * `lambda.Code.fromAsset(path)` - specify a directory or a .zip file in the local filesystem which will be zipped and uploaded to S3 before deployment. See also [bundling asset code](#bundling-asset-code). - * `lambda.Code.fromDockerBuild(path, options)` - use the result of a Docker - build as code. The runtime code is expected to be located at `/asset` in the - image and will be zipped and uploaded to S3 as an asset. The following example shows how to define a Python function and deploy the code from the local directory `my-lambda-handler` to it: @@ -453,7 +450,7 @@ new lambda.Function(this, 'Function', { bundling: { image: lambda.Runtime.PYTHON_3_6.bundlingDockerImage, command: [ - 'bash', '-c', + 'bash', '-c', 'pip install -r requirements.txt -t /asset-output && cp -au . /asset-output' ], }, @@ -465,8 +462,8 @@ new lambda.Function(this, 'Function', { Runtimes expose a `bundlingDockerImage` property that points to the [AWS SAM](https://github.com/awslabs/aws-sam-cli) build image. -Use `cdk.DockerImage.fromRegistry(image)` to use an existing image or -`cdk.DockerImage.fromBuild(path)` to build a specific image: +Use `cdk.BundlingDockerImage.fromRegistry(image)` to use an existing image or +`cdk.BundlingDockerImage.fromAsset(path)` to build a specific image: ```ts import * as cdk from '@aws-cdk/core'; @@ -474,7 +471,7 @@ import * as cdk from '@aws-cdk/core'; new lambda.Function(this, 'Function', { code: lambda.Code.fromAsset('/path/to/handler', { bundling: { - image: cdk.DockerImage.fromBuild('/path/to/dir/with/DockerFile', { + image: cdk.BundlingDockerImage.fromAsset('/path/to/dir/with/DockerFile', { buildArgs: { ARG1: 'value1', }, diff --git a/packages/@aws-cdk/aws-lambda/lib/code.ts b/packages/@aws-cdk/aws-lambda/lib/code.ts index b4f41b2804257..29cd3d02ae4de 100644 --- a/packages/@aws-cdk/aws-lambda/lib/code.ts +++ b/packages/@aws-cdk/aws-lambda/lib/code.ts @@ -57,22 +57,6 @@ export abstract class Code { return new AssetCode(path, options); } - /** - * Loads the function code from an asset created by a Docker build. - * - * By defaut, the asset is expected to be located at `/asset` in the - * image. - * - * @param path The path to the directory containing the Docker file - * @param options Docker build options - */ - public static fromDockerBuild(path: string, options: DockerBuildAssetOptions = {}): AssetCode { - const assetPath = cdk.DockerImage - .fromBuild(path, options) - .cp(options.imagePath ?? '/asset', options.outputPath); - return new AssetCode(assetPath); - } - /** * DEPRECATED * @deprecated use `fromAsset` @@ -504,24 +488,3 @@ export class AssetImageCode extends Code { }; } } - -/** - * Options when creating an asset from a Docker build. - */ -export interface DockerBuildAssetOptions extends cdk.DockerBuildOptions { - /** - * The path in the Docker image where the asset is located after the build - * operation. - * - * @default /asset - */ - readonly imagePath?: string; - - /** - * The path on the local filesystem where the asset will be copied - * using `docker cp`. - * - * @default - a unique temporary directory in the system temp directory - */ - readonly outputPath?: string; -} diff --git a/packages/@aws-cdk/aws-lambda/test/code.test.ts b/packages/@aws-cdk/aws-lambda/test/code.test.ts index 7de7998b19c85..9b99c095c2467 100644 --- a/packages/@aws-cdk/aws-lambda/test/code.test.ts +++ b/packages/@aws-cdk/aws-lambda/test/code.test.ts @@ -327,29 +327,6 @@ describe('code', () => { }); }); }); - - describe('lambda.Code.fromDockerBuild', () => { - test('can use the result of a Docker build as an asset', () => { - // given - const stack = new cdk.Stack(); - stack.node.setContext(cxapi.ASSET_RESOURCE_METADATA_ENABLED_CONTEXT, true); - - // when - new lambda.Function(stack, 'Fn', { - code: lambda.Code.fromDockerBuild(path.join(__dirname, 'docker-build-lambda')), - handler: 'index.handler', - runtime: lambda.Runtime.NODEJS_12_X, - }); - - // then - expect(stack).toHaveResource('AWS::Lambda::Function', { - Metadata: { - [cxapi.ASSET_RESOURCE_METADATA_PATH_KEY]: 'asset.38cd320fa97b348accac88e48d9cede4923f7cab270ce794c95a665be83681a8', - [cxapi.ASSET_RESOURCE_METADATA_PROPERTY_KEY]: 'Code', - }, - }, ResourcePart.CompleteDefinition); - }); - }); }); function defineFunction(code: lambda.Code, runtime: lambda.Runtime = lambda.Runtime.NODEJS_10_X) { diff --git a/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/Dockerfile b/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/Dockerfile deleted file mode 100644 index 4643fde141850..0000000000000 --- a/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/Dockerfile +++ /dev/null @@ -1,3 +0,0 @@ -FROM public.ecr.aws/amazonlinux/amazonlinux:latest - -COPY index.js /asset diff --git a/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/index.ts b/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/index.ts deleted file mode 100644 index cc867895b4efc..0000000000000 --- a/packages/@aws-cdk/aws-lambda/test/docker-build-lambda/index.ts +++ /dev/null @@ -1,5 +0,0 @@ -/* eslint-disable no-console */ -export async function handler(event: any) { - console.log('Event: %j', event); - return event; -} diff --git a/packages/@aws-cdk/aws-s3-assets/README.md b/packages/@aws-cdk/aws-s3-assets/README.md index 7a751410a2b22..aab4c46d9c44d 100644 --- a/packages/@aws-cdk/aws-s3-assets/README.md +++ b/packages/@aws-cdk/aws-s3-assets/README.md @@ -88,8 +88,8 @@ The following example uses custom asset bundling to convert a markdown file to h [Example of using asset bundling](./test/integ.assets.bundling.lit.ts). -The bundling docker image (`image`) can either come from a registry (`DockerImage.fromRegistry`) -or it can be built from a `Dockerfile` located inside your project (`DockerImage.fromBuild`). +The bundling docker image (`image`) can either come from a registry (`BundlingDockerImage.fromRegistry`) +or it can be built from a `Dockerfile` located inside your project (`BundlingDockerImage.fromAsset`). You can set the `CDK_DOCKER` environment variable in order to provide a custom docker program to execute. This may sometime be needed when building in @@ -114,7 +114,7 @@ new assets.Asset(this, 'BundledAsset', { }, }, // Docker bundling fallback - image: DockerImage.fromRegistry('alpine'), + image: BundlingDockerImage.fromRegistry('alpine'), entrypoint: ['/bin/sh', '-c'], command: ['bundle'], }, @@ -124,27 +124,6 @@ new assets.Asset(this, 'BundledAsset', { Although optional, it's recommended to provide a local bundling method which can greatly improve performance. -If the bundling output contains a single archive file (zip or jar) it will be -uploaded to S3 as-is and will not be zipped. Otherwise the contents of the -output directory will be zipped and the zip file will be uploaded to S3. This -is the default behavior for `bundling.outputType` (`BundlingOutput.AUTO_DISCOVER`). - -Use `BundlingOutput.NOT_ARCHIVED` if the bundling output must always be zipped: - -```ts -const asset = new assets.Asset(this, 'BundledAsset', { - path: '/path/to/asset', - bundling: { - image: DockerImage.fromRegistry('alpine'), - command: ['command-that-produces-an-archive.sh'], - outputType: BundlingOutput.NOT_ARCHIVED, // Bundling output will be zipped even though it produces a single archive file. - }, -}); -``` - -Use `BundlingOutput.ARCHIVED` if the bundling output contains a single archive file and -you don't want it to be zippped. - ## CloudFormation Resource Metadata > NOTE: This section is relevant for authors of AWS Resource Constructs. diff --git a/packages/@aws-cdk/aws-s3-assets/lib/asset.ts b/packages/@aws-cdk/aws-s3-assets/lib/asset.ts index 510834a61c634..938778d1381f4 100644 --- a/packages/@aws-cdk/aws-s3-assets/lib/asset.ts +++ b/packages/@aws-cdk/aws-s3-assets/lib/asset.ts @@ -1,3 +1,4 @@ +import * as fs from 'fs'; import * as path from 'path'; import * as assets from '@aws-cdk/assets'; import * as iam from '@aws-cdk/aws-iam'; @@ -12,6 +13,8 @@ import { toSymlinkFollow } from './compat'; // eslint-disable-next-line no-duplicate-imports, import/order import { Construct as CoreConstruct } from '@aws-cdk/core'; +const ARCHIVE_EXTENSIONS = ['.zip', '.jar']; + export interface AssetOptions extends assets.CopyOptions, cdk.AssetOptions { /** * A list of principals that should be able to read this asset from S3. @@ -136,12 +139,17 @@ export class Asset extends CoreConstruct implements cdk.IAsset { this.assetPath = staging.relativeStagedPath(stack); - this.isFile = staging.packaging === cdk.FileAssetPackaging.FILE; + const packaging = determinePackaging(staging.sourcePath); + + this.isFile = packaging === cdk.FileAssetPackaging.FILE; - this.isZipArchive = staging.isArchive; + // sets isZipArchive based on the type of packaging and file extension + this.isZipArchive = packaging === cdk.FileAssetPackaging.ZIP_DIRECTORY + ? true + : ARCHIVE_EXTENSIONS.some(ext => staging.sourcePath.toLowerCase().endsWith(ext)); const location = stack.synthesizer.addFileAsset({ - packaging: staging.packaging, + packaging, sourceHash: this.sourceHash, fileName: this.assetPath, }); @@ -202,3 +210,19 @@ export class Asset extends CoreConstruct implements cdk.IAsset { this.bucket.grantRead(grantee); } } + +function determinePackaging(assetPath: string): cdk.FileAssetPackaging { + if (!fs.existsSync(assetPath)) { + throw new Error(`Cannot find asset at ${assetPath}`); + } + + if (fs.statSync(assetPath).isDirectory()) { + return cdk.FileAssetPackaging.ZIP_DIRECTORY; + } + + if (fs.statSync(assetPath).isFile()) { + return cdk.FileAssetPackaging.FILE; + } + + throw new Error(`Asset ${assetPath} is expected to be either a directory or a regular file`); +} diff --git a/packages/@aws-cdk/core/lib/asset-staging.ts b/packages/@aws-cdk/core/lib/asset-staging.ts index 17fd89781004a..66c65e3d14864 100644 --- a/packages/@aws-cdk/core/lib/asset-staging.ts +++ b/packages/@aws-cdk/core/lib/asset-staging.ts @@ -5,8 +5,8 @@ import * as cxapi from '@aws-cdk/cx-api'; import { Construct } from 'constructs'; import * as fs from 'fs-extra'; import * as minimatch from 'minimatch'; -import { AssetHashType, AssetOptions, FileAssetPackaging } from './assets'; -import { BundlingOptions, BundlingOutput } from './bundling'; +import { AssetHashType, AssetOptions } from './assets'; +import { BundlingOptions } from './bundling'; import { FileSystem, FingerprintOptions } from './fs'; import { Names } from './names'; import { Cache } from './private/cache'; @@ -17,8 +17,6 @@ import { Stage } from './stage'; // eslint-disable-next-line import { Construct as CoreConstruct } from './construct-compat'; -const ARCHIVE_EXTENSIONS = ['.zip', '.jar']; - /** * A previously staged asset */ @@ -140,9 +138,6 @@ export class AssetStaging extends CoreConstruct { private readonly cacheKey: string; - private _packaging = FileAssetPackaging.ZIP_DIRECTORY; - private _isArchive = true; - constructor(scope: Construct, id: string, props: AssetStagingProps) { super(scope, id); @@ -208,20 +203,6 @@ export class AssetStaging extends CoreConstruct { return this.assetHash; } - /** - * How this asset should be packaged. - */ - public get packaging(): FileAssetPackaging { - return this._packaging; - } - - /** - * Whether this asset is an archive (zip or jar). - */ - public get isArchive(): boolean { - return this._isArchive; - } - /** * Return the path to the staged asset, relative to the Cloud Assembly (manifest) directory of the given stack * @@ -300,16 +281,11 @@ export class AssetStaging extends CoreConstruct { const bundleDir = this.determineBundleDir(this.assetOutdir, assetHash); this.bundle(bundling, bundleDir); - // Check bundling output content and determine if we will need to archive - const bundlingOutputType = bundling.outputType ?? BundlingOutput.AUTO_DISCOVER; - const bundledAsset = determineBundledAsset(bundleDir, bundlingOutputType); - this._packaging = bundledAsset.packaging; - // Calculate assetHash afterwards if we still must - assetHash = assetHash ?? this.calculateHash(this.hashType, bundling, bundledAsset.path); - const stagedPath = path.resolve(this.assetOutdir, renderAssetFilename(assetHash, bundledAsset.extension)); + assetHash = assetHash ?? this.calculateHash(this.hashType, bundling, bundleDir); + const stagedPath = path.resolve(this.assetOutdir, renderAssetFilename(assetHash)); - this.stageAsset(bundledAsset.path, stagedPath, 'move'); + this.stageAsset(bundleDir, stagedPath, 'move'); return { assetHash, stagedPath }; } @@ -347,8 +323,6 @@ export class AssetStaging extends CoreConstruct { const stat = fs.statSync(sourcePath); if (stat.isFile()) { fs.copyFileSync(sourcePath, targetPath); - this._packaging = FileAssetPackaging.FILE; - this._isArchive = ARCHIVE_EXTENSIONS.includes(path.extname(sourcePath).toLowerCase()); } else if (stat.isDirectory()) { fs.mkdirSync(targetPath); FileSystem.copyDirectory(sourcePath, targetPath, this.fingerprintOptions); @@ -528,57 +502,3 @@ function sortObject(object: { [key: string]: any }): { [key: string]: any } { } return ret; } - -/** - * Returns the single archive file of a directory or undefined - */ -function singleArchiveFile(directory: string): string | undefined { - if (!fs.existsSync(directory)) { - throw new Error(`Directory ${directory} does not exist.`); - } - - if (!fs.statSync(directory).isDirectory()) { - throw new Error(`${directory} is not a directory.`); - } - - const content = fs.readdirSync(directory); - if (content.length === 1) { - const file = path.join(directory, content[0]); - const extension = path.extname(content[0]).toLowerCase(); - if (fs.statSync(file).isFile() && ARCHIVE_EXTENSIONS.includes(extension)) { - return file; - } - } - - return undefined; -} - -interface BundledAsset { - path: string, - packaging: FileAssetPackaging, - extension?: string -} - -/** - * Returns the bundled asset to use based on the content of the bundle directory - * and the type of output. - */ -function determineBundledAsset(bundleDir: string, outputType: BundlingOutput): BundledAsset { - const archiveFile = singleArchiveFile(bundleDir); - - // auto-discover means that if there is an archive file, we take it as the - // bundle, otherwise, we will archive here. - if (outputType === BundlingOutput.AUTO_DISCOVER) { - outputType = archiveFile ? BundlingOutput.ARCHIVED : BundlingOutput.NOT_ARCHIVED; - } - - switch (outputType) { - case BundlingOutput.NOT_ARCHIVED: - return { path: bundleDir, packaging: FileAssetPackaging.ZIP_DIRECTORY }; - case BundlingOutput.ARCHIVED: - if (!archiveFile) { - throw new Error('Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`'); - } - return { path: archiveFile, packaging: FileAssetPackaging.FILE, extension: path.extname(archiveFile) }; - } -} diff --git a/packages/@aws-cdk/core/lib/bundling.ts b/packages/@aws-cdk/core/lib/bundling.ts index 059f2260c967b..b1247fd913ea0 100644 --- a/packages/@aws-cdk/core/lib/bundling.ts +++ b/packages/@aws-cdk/core/lib/bundling.ts @@ -79,41 +79,6 @@ export interface BundlingOptions { * @experimental */ readonly local?: ILocalBundling; - - /** - * The type of output that this bundling operation is producing. - * - * @default BundlingOutput.AUTO_DISCOVER - * - * @experimental - */ - readonly outputType?: BundlingOutput; -} - -/** - * The type of output that a bundling operation is producing. - * - * @experimental - */ -export enum BundlingOutput { - /** - * The bundling output directory includes a single .zip or .jar file which - * will be used as the final bundle. If the output directory does not - * include exactly a single archive, bundling will fail. - */ - ARCHIVED = 'archived', - - /** - * The bundling output directory contains one or more files which will be - * archived and uploaded as a .zip file to S3. - */ - NOT_ARCHIVED = 'not-archived', - - /** - * If the bundling output directory contains a single archive file (zip or jar) - * it will not be zipped. Otherwise the bundling output will be zipped. - */ - AUTO_DISCOVER = 'auto-discover', } /** @@ -135,8 +100,6 @@ export interface ILocalBundling { /** * A Docker image used for asset bundling - * - * @deprecated use DockerImage */ export class BundlingDockerImage { /** @@ -153,8 +116,6 @@ export class BundlingDockerImage { * * @param path The path to the directory containing the Docker file * @param options Docker build options - * - * @deprecated use DockerImage.fromBuild() */ public static fromAsset(path: string, options: DockerBuildOptions = {}) { const buildArgs = options.buildArgs || {}; @@ -185,7 +146,7 @@ export class BundlingDockerImage { } /** @param image The Docker image */ - protected constructor(public readonly image: string, private readonly _imageHash?: string) {} + private constructor(public readonly image: string, private readonly _imageHash?: string) {} /** * Provides a stable representation of this image for JSON serialization. @@ -233,16 +194,10 @@ export class BundlingDockerImage { } /** - * Copies a file or directory out of the Docker image to the local filesystem. - * - * If `outputPath` is omitted the destination path is a temporary directory. - * - * @param imagePath the path in the Docker image - * @param outputPath the destination path for the copy operation - * @returns the destination path + * Copies a file or directory out of the Docker image to the local filesystem */ - public cp(imagePath: string, outputPath?: string): string { - const { stdout } = dockerExec(['create', this.image], {}); // Empty options to avoid stdout redirect here + public cp(imagePath: string, outputPath: string) { + const { stdout } = dockerExec(['create', this.image]); const match = stdout.toString().match(/([0-9a-f]{16,})/); if (!match) { throw new Error('Failed to extract container ID from Docker create output'); @@ -250,33 +205,16 @@ export class BundlingDockerImage { const containerId = match[1]; const containerPath = `${containerId}:${imagePath}`; - const destPath = outputPath ?? FileSystem.mkdtemp('cdk-docker-cp-'); try { - dockerExec(['cp', containerPath, destPath]); - return destPath; + dockerExec(['cp', containerPath, outputPath]); } catch (err) { - throw new Error(`Failed to copy files from ${containerPath} to ${destPath}: ${err}`); + throw new Error(`Failed to copy files from ${containerPath} to ${outputPath}: ${err}`); } finally { dockerExec(['rm', '-v', containerId]); } } } -/** - * A Docker image - */ -export class DockerImage extends BundlingDockerImage { - /** - * Builds a Docker image - * - * @param path The path to the directory containing the Docker file - * @param options Docker build options - */ - public static fromBuild(path: string, options: DockerBuildOptions = {}) { - return BundlingDockerImage.fromAsset(path, options); - } -} - /** * A Docker volume */ diff --git a/packages/@aws-cdk/core/test/archive/archive.zip b/packages/@aws-cdk/core/test/archive/archive.zip deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/packages/@aws-cdk/core/test/bundling.test.ts b/packages/@aws-cdk/core/test/bundling.test.ts index 8b03dce3da0d3..258860d65585c 100644 --- a/packages/@aws-cdk/core/test/bundling.test.ts +++ b/packages/@aws-cdk/core/test/bundling.test.ts @@ -3,7 +3,7 @@ import * as crypto from 'crypto'; import * as path from 'path'; import { nodeunitShim, Test } from 'nodeunit-shim'; import * as sinon from 'sinon'; -import { BundlingDockerImage, DockerImage, FileSystem } from '../lib'; +import { BundlingDockerImage, FileSystem } from '../lib'; nodeunitShim({ 'tearDown'(callback: any) { @@ -263,25 +263,4 @@ nodeunitShim({ test.ok(spawnSyncStub.calledWith(sinon.match.any, ['rm', '-v', containerId])); test.done(); }, - - 'cp utility copies to a temp dir of outputPath is omitted'(test: Test) { - // GIVEN - const containerId = '1234567890abcdef1234567890abcdef'; - sinon.stub(child_process, 'spawnSync').returns({ - status: 0, - stderr: Buffer.from('stderr'), - stdout: Buffer.from(`${containerId}\n`), - pid: 123, - output: ['stdout', 'stderr'], - signal: null, - }); - - // WHEN - const tempPath = DockerImage.fromRegistry('alpine').cp('/foo/bar'); - - // THEN - test.ok(/cdk-docker-cp-/.test(tempPath)); - - test.done(); - }, }); diff --git a/packages/@aws-cdk/core/test/docker-stub.sh b/packages/@aws-cdk/core/test/docker-stub.sh index 94f806f69a120..fe48e93d4a207 100755 --- a/packages/@aws-cdk/core/test/docker-stub.sh +++ b/packages/@aws-cdk/core/test/docker-stub.sh @@ -24,18 +24,5 @@ if echo "$@" | grep "DOCKER_STUB_SUCCESS"; then exit 0 fi -if echo "$@" | grep "DOCKER_STUB_MULTIPLE_FILES"; then - outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1) - touch ${outdir}/test1.txt - touch ${outdir}/test2.txt - exit 0 -fi - -if echo "$@" | grep "DOCKER_STUB_SINGLE_ARCHIVE"; then - outdir=$(echo "$@" | xargs -n1 | grep "/asset-output" | head -n1 | cut -d":" -f1) - touch ${outdir}/test.zip - exit 0 -fi - -echo "Docker mock only supports one of the following commands: DOCKER_STUB_SUCCESS_NO_OUTPUT,DOCKER_STUB_FAIL,DOCKER_STUB_SUCCESS,DOCKER_STUB_MULTIPLE_FILES,DOCKER_SINGLE_ARCHIVE" +echo "Docker mock only supports one of the following commands: DOCKER_STUB_SUCCESS_NO_OUTPUT,DOCKER_STUB_FAIL,DOCKER_STUB_SUCCESS" exit 1 diff --git a/packages/@aws-cdk/core/test/staging.test.ts b/packages/@aws-cdk/core/test/staging.test.ts index 76bf74f9cffd7..347c5fcea3b63 100644 --- a/packages/@aws-cdk/core/test/staging.test.ts +++ b/packages/@aws-cdk/core/test/staging.test.ts @@ -1,11 +1,10 @@ import * as os from 'os'; import * as path from 'path'; -import { FileAssetPackaging } from '@aws-cdk/cloud-assembly-schema'; import * as cxapi from '@aws-cdk/cx-api'; import * as fs from 'fs-extra'; import { nodeunitShim, Test } from 'nodeunit-shim'; import * as sinon from 'sinon'; -import { App, AssetHashType, AssetStaging, BundlingDockerImage, BundlingOptions, BundlingOutput, FileSystem, Stack, Stage } from '../lib'; +import { App, AssetHashType, AssetStaging, BundlingDockerImage, BundlingOptions, FileSystem, Stack, Stage } from '../lib'; const STUB_INPUT_FILE = '/tmp/docker-stub.input'; const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat'; @@ -13,9 +12,7 @@ const STUB_INPUT_CONCAT_FILE = '/tmp/docker-stub.input.concat'; enum DockerStubCommand { SUCCESS = 'DOCKER_STUB_SUCCESS', FAIL = 'DOCKER_STUB_FAIL', - SUCCESS_NO_OUTPUT = 'DOCKER_STUB_SUCCESS_NO_OUTPUT', - MULTIPLE_FILES = 'DOCKER_STUB_MULTIPLE_FILES', - SINGLE_ARCHIVE = 'DOCKER_STUB_SINGLE_ARCHIVE', + SUCCESS_NO_OUTPUT = 'DOCKER_STUB_SUCCESS_NO_OUTPUT' } const FIXTURE_TEST1_DIR = path.join(__dirname, 'fs', 'fixtures', 'test1'); @@ -53,34 +50,6 @@ nodeunitShim({ test.deepEqual(staging.sourcePath, sourcePath); test.deepEqual(path.basename(staging.stagedPath), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00'); test.deepEqual(path.basename(staging.relativeStagedPath(stack)), 'asset.2f37f937c51e2c191af66acf9b09f548926008ec68c575bd2ee54b6e997c0e00'); - test.deepEqual(staging.packaging, FileAssetPackaging.ZIP_DIRECTORY); - test.deepEqual(staging.isArchive, true); - test.done(); - }, - - 'staging of an archive file correctly sets packaging and isArchive'(test: Test) { - // GIVEN - const stack = new Stack(); - const sourcePath = path.join(__dirname, 'archive', 'archive.zip'); - - // WHEN - const staging = new AssetStaging(stack, 's1', { sourcePath }); - - test.deepEqual(staging.packaging, FileAssetPackaging.FILE); - test.deepEqual(staging.isArchive, true); - test.done(); - }, - - 'staging of a non-archive file correctly sets packaging and isArchive'(test: Test) { - // GIVEN - const stack = new Stack(); - const sourcePath = __filename; - - // WHEN - const staging = new AssetStaging(stack, 's1', { sourcePath }); - - test.deepEqual(staging.packaging, FileAssetPackaging.FILE); - test.deepEqual(staging.isArchive, false); test.done(); }, @@ -816,89 +785,6 @@ nodeunitShim({ ); test.equal(asset.assetHash, '33cbf2cae5432438e0f046bc45ba8c3cef7b6afcf47b59d1c183775c1918fb1f'); // hash of MyStack/Asset - test.done(); - }, - - 'bundling that produces a single archive file is autodiscovered'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - - // WHEN - const staging = new AssetStaging(stack, 'Asset', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [DockerStubCommand.SINGLE_ARCHIVE], - }, - }); - - // THEN - const assembly = app.synth(); - test.deepEqual(fs.readdirSync(assembly.directory), [ - 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48', // this is the bundle dir but it's empty - 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48.zip', - 'cdk.out', - 'manifest.json', - 'stack.template.json', - 'tree.json', - ]); - test.equal(fs.readdirSync(path.join(assembly.directory, 'asset.f43148c61174f444925231b5849b468f21e93b5d1469cd07c53625ffd039ef48')).length, 0); // empty bundle dir - test.deepEqual(staging.packaging, FileAssetPackaging.FILE); - test.deepEqual(staging.isArchive, true); - - test.done(); - }, - - 'bundling that produces a single archive file with NOT_ARCHIVED'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - - // WHEN - const staging = new AssetStaging(stack, 'Asset', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [DockerStubCommand.SINGLE_ARCHIVE], - outputType: BundlingOutput.NOT_ARCHIVED, - }, - }); - - // THEN - const assembly = app.synth(); - test.deepEqual(fs.readdirSync(assembly.directory), [ - 'asset.86ec07746e1d859290cfd8b9c648e581555649c75f51f741f11e22cab6775abc', - 'cdk.out', - 'manifest.json', - 'stack.template.json', - 'tree.json', - ]); - test.deepEqual(staging.packaging, FileAssetPackaging.ZIP_DIRECTORY); - test.deepEqual(staging.isArchive, true); - - test.done(); - }, - - 'throws with ARCHIVED and bundling that does not produce a single archive file'(test: Test) { - // GIVEN - const app = new App(); - const stack = new Stack(app, 'stack'); - const directory = path.join(__dirname, 'fs', 'fixtures', 'test1'); - - // WHEN - test.throws(() => new AssetStaging(stack, 'Asset', { - sourcePath: directory, - bundling: { - image: BundlingDockerImage.fromRegistry('alpine'), - command: [DockerStubCommand.MULTIPLE_FILES], - outputType: BundlingOutput.ARCHIVED, - }, - }), /Bundling output directory is expected to include only a single .zip or .jar file when `output` is set to `ARCHIVED`/); - - test.done(); }, }); From 36df9066717acf63fc28b9fbcf3df7bb8acead4e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Fri, 19 Feb 2021 09:45:05 +0200 Subject: [PATCH 2/2] add breaking change exception for reverted code --- allowed-breaking-changes.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/allowed-breaking-changes.txt b/allowed-breaking-changes.txt index 2ca2ca5b6067f..964bb4d5c7712 100644 --- a/allowed-breaking-changes.txt +++ b/allowed-breaking-changes.txt @@ -56,3 +56,10 @@ incompatible-argument:@aws-cdk/aws-ecs.TaskDefinition.addVolume # We made properties optional and it's really fine but our differ doesn't think so. weakened:@aws-cdk/cloud-assembly-schema.DockerImageSource weakened:@aws-cdk/cloud-assembly-schema.FileSource + +# https://github.com/aws/aws-cdk/pull/13145 +removed:@aws-cdk/core.AssetStaging.isArchive +removed:@aws-cdk/core.AssetStaging.packaging +removed:@aws-cdk/core.BundlingOutput +removed:@aws-cdk/core.BundlingOptions.outputType +