chore(cli): statically guarantee that assets aren't forgotten around …

…`makeBodyParameter`

The protocol of `makeBodyParameter` has 2 requirements:

- Stack assets must already be uploaded (because the stack template will
  be among them).
- It may produce a new stack template asset to be uploaded.

This protocol was easy to misuse, so change the API to make it more
resilient to misuse:

- Before calling, `makeBodyParameter` now requires an
  `AssetsPublishedProof` value. This is a value that can only be
  produced by calling certain functions that publish assets, and cannot
  be constructed from any other source location than the module that
  defines it. This ensures that `makeBodyParameter` cannot be called
  without the uploading of assets.
- The return value of `makeBodyParameter` no longer is just
  CloudFormation API parameters, with the implicit assumption that any
  assets added to the `AssetManifestBuilder` would be uploaded as well:
  instead, the return value is either CloudFormation parameters, or an
  object with a function that trades adding to an `AssetManifestBuilder`
  for the CloudFormation parameters.

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -17,7 +17,7 @@ import {
 import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
 import { TemplateBodyParameter, makeBodyParameter } from './util/template-body-parameter';
 import { AssetManifestBuilder } from '../util/asset-manifest-builder';
-import { publishAssets } from '../util/asset-publishing';
+import { iAmDeployStack, publishAssets } from '../util/asset-publishing';
 
 export interface DeployStackResult {
   readonly noOp: boolean;
@@ -190,7 +190,7 @@ export interface DeployStackOptions {
    *
    * @default - Use the stored template
    */
-  readonly overrideTemplate?: any;
+  readonly overrideTemplate?: unknown;
 
   /**
    * Whether to build/publish assets in parallel
@@ -280,13 +280,23 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     debug(`${deployName}: deploying...`);
   }
 
-  const bodyParameter = await makeBodyParameter(
+  let bodyParameter;
+  const bodyAction = await makeBodyParameter(
     stackArtifact,
     options.resolvedEnvironment,
-    legacyAssets,
     options.envResources,
     options.sdk,
+    iAmDeployStack(),
     options.overrideTemplate);
+  switch (bodyAction.type) {
+    case 'direct':
+      bodyParameter = bodyAction.param;
+      break;
+    case 'upload':
+      bodyParameter = bodyAction.addToManifest(legacyAssets);
+      break;
+  }
+
   await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv, {
     parallel: options.assetParallelism,
   });

packages/aws-cdk/lib/api/deployments.ts

@@ -419,27 +419,26 @@ export class Deployments {
     const { stackSdk, resolvedEnvironment, envResources } = await this.prepareSdkFor(stackArtifact, undefined, Mode.ForReading);
     const cfn = stackSdk.cloudFormation();
 
-    await uploadStackTemplateAssets(stackArtifact, this);
+    const proof = await uploadStackTemplateAssets(stackArtifact, this.sdkProvider, resolvedEnvironment);
 
     // Upload the template, if necessary, before passing it to CFN
-    const builder = new AssetManifestBuilder();
-    const cfnParam = await makeBodyParameter(
+    let cfnParam;
+    const bodyAction = await makeBodyParameter(
       stackArtifact,
       resolvedEnvironment,
-      builder,
       envResources,
-      stackSdk);
-
-    // If the `makeBodyParameter` before this added assets, make sure to publish them before
-    // calling the API.
-    const addedAssets = builder.toManifest(stackArtifact.assembly.directory);
-    for (const entry of addedAssets.entries) {
-      await this.buildSingleAsset('no-version-validation', addedAssets, entry, {
-        stack: stackArtifact,
-      });
-      await this.publishSingleAsset(addedAssets, entry, {
-        stack: stackArtifact,
-      });
+      stackSdk,
+      proof);
+    switch (bodyAction.type) {
+      case 'direct':
+        cfnParam = bodyAction.param;
+        break;
+
+      case 'upload':
+        const builder = new AssetManifestBuilder();
+        cfnParam = bodyAction.addToManifest(builder);
+        await publishAssets(builder.toManifest(stackArtifact.assembly.directory), this.sdkProvider, resolvedEnvironment);
+        break;
     }
 
     const response = await cfn.getTemplateSummary(cfnParam).promise();

packages/aws-cdk/lib/api/util/cloudformation.ts

@@ -3,12 +3,13 @@ import { DescribeChangeSetOutput } from '@aws-cdk/cloudformation-diff';
 import { SSMPARAM_NO_INVALIDATE } from '@aws-cdk/cx-api';
 import * as cxapi from '@aws-cdk/cx-api';
 import { CloudFormation } from 'aws-sdk';
-import { AssetManifest, FileManifestEntry } from 'cdk-assets';
+import { AssetManifest, DestinationPattern, FileManifestEntry } from 'cdk-assets';
 import { StackStatus } from './cloudformation/stack-status';
 import { makeBodyParameter, TemplateBodyParameter } from './template-body-parameter';
 import { debug } from '../../logging';
 import { deserializeStructure } from '../../serialize';
 import { AssetManifestBuilder } from '../../util/asset-manifest-builder';
+import { AssetsPublishedProof, multipleAssetPublishedProof, publishAssets } from '../../util/asset-publishing';
 import { SdkProvider } from '../aws-auth';
 import { Deployments } from '../deployments';
 
@@ -345,37 +346,47 @@ export async function createDiffChangeSet(options: PrepareChangeSetOptions): Pro
  *
  * This is used in the `uploadBodyParameterAndCreateChangeSet` function to find
  * all template asset files to build and publish.
- *
- * Returns a tuple of [AssetManifest, FileManifestEntry[]]
  */
-function templatesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtifact): [AssetManifest, FileManifestEntry[]] {
-  const assets: (FileManifestEntry)[] = [];
+function templatesFromAssetManifestArtifact(artifact: cxapi.AssetManifestArtifact): AssetManifest {
   const fileName = artifact.file;
   const assetManifest = AssetManifest.fromFile(fileName);
 
-  assetManifest.entries.forEach(entry => {
+  return assetManifest.select(assetManifest.entries.flatMap((entry) => {
     if (entry.type === 'file') {
       const source = (entry as FileManifestEntry).source;
       if (source.path && (source.path.endsWith('.template.json'))) {
-        assets.push(entry as FileManifestEntry);
+        return [new DestinationPattern(entry.id.assetId, entry.id.destinationId)];
       }
     }
-  });
-  return [assetManifest, assets];
+    return [];
+  }));
 }
 
 async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOptions): Promise<DescribeChangeSetOutput | undefined> {
   try {
-    await uploadStackTemplateAssets(options.stack, options.deployments);
     const preparedSdk = (await options.deployments.prepareSdkWithDeployRole(options.stack));
+    const proof = await uploadStackTemplateAssets(options.stack, options.sdkProvider, preparedSdk.resolvedEnvironment);
 
-    const bodyParameter = await makeBodyParameter(
+    let bodyParameter;
+    const bodyAction = await makeBodyParameter(
       options.stack,
       preparedSdk.resolvedEnvironment,
-      new AssetManifestBuilder(),
       preparedSdk.envResources,
       preparedSdk.stackSdk,
+      proof,
     );
+    switch (bodyAction.type) {
+      case 'direct':
+        bodyParameter = bodyAction.param;
+        break;
+
+      case 'upload':
+        const builder = new AssetManifestBuilder();
+        bodyParameter = bodyAction.addToManifest(builder);
+        await publishAssets(builder.toManifest(options.stack.assembly.directory), options.sdkProvider, preparedSdk.resolvedEnvironment);
+        break;
+    }
+
     const cfn = preparedSdk.stackSdk.cloudFormation();
     const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;
 
@@ -410,23 +421,17 @@ async function uploadBodyParameterAndCreateChangeSet(options: PrepareChangeSetOp
  * asset manifest, because technically that is the only place that knows about
  * bucket and assumed roles and such.
  */
-export async function uploadStackTemplateAssets(stack: cxapi.CloudFormationStackArtifact, deployments: Deployments) {
-  for (const artifact of stack.dependencies) {
-    // Skip artifact if it is not an Asset Manifest Artifact
-    if (!cxapi.AssetManifestArtifact.isAssetManifestArtifact(artifact)) {
-      continue;
-    }
-
-    const [assetManifest, file_entries] = templatesFromAssetManifestArtifact(artifact);
-    for (const entry of file_entries) {
-      await deployments.buildSingleAsset(artifact, assetManifest, entry, {
-        stack,
-      });
-      await deployments.publishSingleAsset(assetManifest, entry, {
-        stack,
-      });
-    }
-  }
+export async function uploadStackTemplateAssets(
+  stack: cxapi.CloudFormationStackArtifact,
+  sdkProvider: SdkProvider,
+  environment: cxapi.Environment,
+): Promise<AssetsPublishedProof> {
+  const assetDependencies = stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact);
+
+  return multipleAssetPublishedProof(assetDependencies, (artifact) => {
+    const templates = templatesFromAssetManifestArtifact(artifact);
+    return publishAssets(templates, sdkProvider, environment);
+  });
 }
 
 async function createChangeSet(options: CreateChangeSetOptions): Promise<DescribeChangeSetOutput> {

packages/aws-cdk/lib/api/util/template-body-parameter.ts

@@ -5,6 +5,7 @@ import * as fs from 'fs-extra';
 import { debug, error } from '../../logging';
 import { toYAML } from '../../serialize';
 import { AssetManifestBuilder } from '../../util/asset-manifest-builder';
+import { AssetsPublishedProof } from '../../util/asset-publishing';
 import { contentHash } from '../../util/content-hash';
 import { ISDK } from '../aws-auth';
 import { EnvironmentResources } from '../environment-resources';
@@ -16,38 +17,44 @@ export type TemplateBodyParameter = {
 
 const LARGE_TEMPLATE_SIZE_KB = 50;
 
+type MakeBodyParameterResult =
+  | { type: 'direct'; param: TemplateBodyParameter }
+  | { type: 'upload'; addToManifest: (builder: AssetManifestBuilder) => TemplateBodyParameter };
+
 /**
  * Prepares the body parameter for +CreateChangeSet+.
  *
- * If the template is small enough to be inlined into the API call, just return
- * it immediately.
- *
- * Otherwise, add it to the asset manifest to get uploaded to the staging
- * bucket and return its coordinates. If there is no staging bucket, an error
- * is thrown.
+ * If the template has already been uploaded to an asset bucket or the template
+ * is small enough to be inlined into the API call, returns a 'direct' type response
+ * with can go into the CloudFormation API call. The `_assetsPublishedProof` parameter
+ * exists to statically prove that `publishAssets` has been called already.
  *
- * @param stack     the synthesized stack that provides the CloudFormation template
- * @param toolkitInfo information about the toolkit stack
+ * Otherwise, returns an object with an `addToManifest` function; call that with an `AssetManifestBuilder`
+ * (and publish the added artifacts!) to obtain the CloudFormation API call parameters.
+ * there is no staging bucket, an error is thrown.
  */
 export async function makeBodyParameter(
   stack: cxapi.CloudFormationStackArtifact,
   resolvedEnvironment: cxapi.Environment,
-  assetManifest: AssetManifestBuilder,
   resources: EnvironmentResources,
   sdk: ISDK,
-  overrideTemplate?: any,
-): Promise<TemplateBodyParameter> {
+  _assetsPublishedProof: AssetsPublishedProof,
+  overrideTemplate?: unknown,
+): Promise<MakeBodyParameterResult> {
 
   // If the template has already been uploaded to S3, just use it from there.
   if (stack.stackTemplateAssetObjectUrl && !overrideTemplate) {
-    return { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment, sdk) };
+    return {
+      type: 'direct',
+      param: { TemplateURL: restUrlFromManifest(stack.stackTemplateAssetObjectUrl, resolvedEnvironment, sdk) },
+    };
   }
 
   // Otherwise, pass via API call (if small) or upload here (if large)
   const templateJson = toYAML(overrideTemplate ?? stack.template);
 
   if (templateJson.length <= LARGE_TEMPLATE_SIZE_KB * 1024) {
-    return { TemplateBody: templateJson };
+    return { type: 'direct', param: { TemplateBody: templateJson } };
   }
 
   const toolkitInfo = await resources.lookupToolkit();
@@ -72,16 +79,22 @@ export async function makeBodyParameter(
     await fs.writeFile(templateFilePath, templateJson, { encoding: 'utf-8' });
   }
 
-  assetManifest.addFileAsset(templateHash, {
-    path: templateFile,
-  }, {
-    bucketName: toolkitInfo.bucketName,
-    objectKey: key,
-  });
+  return {
+    type: 'upload',
+    addToManifest(builder) {
+      builder.addFileAsset(templateHash, {
+        path: templateFile,
+      }, {
+        bucketName: toolkitInfo.bucketName,
+        objectKey: key,
+      });
+
+      const templateURL = `${toolkitInfo.bucketUrl}/${key}`;
+      debug('Storing template in S3 at:', templateURL);
+      return { TemplateURL: templateURL };
+    },
+  };
 
-  const templateURL = `${toolkitInfo.bucketUrl}/${key}`;
-  debug('Storing template in S3 at:', templateURL);
-  return { TemplateURL: templateURL };
 }
 
 /**

packages/aws-cdk/lib/util/asset-publishing.ts

@@ -35,7 +35,7 @@ export async function publishAssets(
   sdk: SdkProvider,
   targetEnv: cxapi.Environment,
   options: PublishAssetsOptions = {},
-) {
+): Promise<AssetsPublishedProof> {
   // This shouldn't really happen (it's a programming error), but we don't have
   // the types here to guide us. Do an runtime validation to be super super sure.
   if (
@@ -60,6 +60,8 @@ export async function publishAssets(
   if (publisher.hasFailures) {
     throw new Error('Failed to publish one or more assets. See the error messages above for more information.');
   }
+
+  return { [PUBLISH_ASSET_PROOF_SYM]: true };
 }
 
 export interface BuildAssetsOptions {
@@ -215,4 +217,48 @@ class PublishingProgressListener implements cdk_assets.IPublishProgressListener
     const handler = this.quiet && type !== 'fail' ? debug : EVENT_TO_LOGGER[type];
     handler(`[${event.percentComplete}%] ${type}: ${event.message}`);
   }
+}
+
+const PUBLISH_ASSET_PROOF_SYM = Symbol('publish_assets_proof');
+
+/**
+ * This interface represents proof that assets have been published.
+ *
+ * Objects of this type can only be obtained by calling functions in this module.
+ */
+export interface AssetsPublishedProof {
+  [PUBLISH_ASSET_PROOF_SYM]: true;
+}
+
+/**
+ * If you promise that you will publish assets for every element of an array, this function will attest it for every size of an array.
+ *
+ * Without this function, there would be no way to get `AssetsPublishedProof` for an empty array,
+ * since you'd have no single element to call `publishAssets` on.
+ */
+export async function multipleAssetPublishedProof<A>(xs: A[], block: (x: A) => Promise<AssetsPublishedProof>): Promise<AssetsPublishedProof> {
+  for (const x of xs) {
+    // Don't even care about the return value, just the type checking is good enough.
+    await block(x);
+  }
+  return { [PUBLISH_ASSET_PROOF_SYM]: true };
+}
+
+/**
+ * Conjure an `AssetsPublishedProof` out of thin air.
+ *
+ * This is only allowed in one location: the function `deployStack` in
+ * `deploy-stack.ts`.  Do not call this function anywhere but there.
+ *
+ * The reason we have this is because the assets are published in an elaborate
+ * way in the work graph there, and it's too much work for too little benefit to
+ * do an elaborate token refactoring over there.
+ *
+ * This proof protocol exists for ancillary locations where we also need to send
+ * a template to CloudFormation like `diff` and `import`, and that template
+ * might be represented like a stack asset. Use of the proof forces a call
+ * to `uploadStackTemplateAssets` in those locations.
+ */
+export function iAmDeployStack(): AssetsPublishedProof {
+  return { [PUBLISH_ASSET_PROOF_SYM]: true };
 }