fix(core): ENOTDIR invalid cwd on "cdk deploy" (#13145)

Elad Ben-Israel · NetaNir · commit a735b52e4a33 · 2021-02-19T00:49:18.000-08:00
This commit reverts two recent changes to the asset system (#12258 and ##13076) which introduced a regression in 1.90.0. Fixes #13131 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
diff --git 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
+
diff --git a/packages/@aws-cdk/aws-s3-assets/README.md b/packages/@aws-cdk/aws-s3-assets/README.md
@@ -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: BundlingDockerImage.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
@@ -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
@@ -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
@@ -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',
 }
 
 /**
diff --git a/packages/@aws-cdk/core/test/archive/archive.zip b/packages/@aws-cdk/core/test/archive/archive.zip
diff --git 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
