feat(ecr-assets): fail if tokens are used in buildArgs (#3989)

* feat(ecr-assets): fail if tokens are used in buildArgs

Since `buildArgs` are used before deployment, if tokens are used,
we will fail quickly with a nice message.

Fixes #3981

* do not import Token, since its not used

Author: Elad Ben-Israel

packages/@aws-cdk/aws-ecr-assets/lib/image-asset.ts

@@ -1,6 +1,7 @@
 import assets = require('@aws-cdk/assets');
 import ecr = require('@aws-cdk/aws-ecr');
 import cdk = require('@aws-cdk/core');
+import { Token } from '@aws-cdk/core';
 import cxapi = require('@aws-cdk/cx-api');
 import fs = require('fs');
 import path = require('path');
@@ -19,14 +20,18 @@ export interface DockerImageAssetProps extends assets.CopyOptions {
    * from a Kubernetes Pod. Note, this is only the repository name, without the
    * registry and the tag parts.
    *
-   * @default automatically derived from the asset's ID.
+   * @default - automatically derived from the asset's ID.
    */
   readonly repositoryName?: string;
 
   /**
-   * Build args to pass to the `docker build` command
+   * Build args to pass to the `docker build` command.
    *
-   * @default no build args are passed
+   * Since Docker build arguments are resolved before deployment, keys and
+   * values cannot refer to unresolved tokens (such as `lambda.functionArn` or
+   * `queue.queueUrl`).
+   *
+   * @default - no build args are passed
    */
   readonly buildArgs?: { [key: string]: string };
 }
@@ -59,6 +64,9 @@ export class DockerImageAsset extends cdk.Construct implements assets.IAsset {
   constructor(scope: cdk.Construct, id: string, props: DockerImageAssetProps) {
     super(scope, id);
 
+    // verify buildArgs do not use tokens in neither keys nor values
+    validateBuildArgs(props.buildArgs);
+
     // resolve full path
     const dir = path.resolve(props.directory);
     if (!fs.existsSync(dir)) {
@@ -110,3 +118,11 @@ export class DockerImageAsset extends cdk.Construct implements assets.IAsset {
     this.artifactHash = imageSha;
   }
 }
+
+function validateBuildArgs(buildArgs?: { [key: string]: string }) {
+  for (const [ key, value ] of Object.entries(buildArgs || {})) {
+    if (Token.isUnresolved(key) || Token.isUnresolved(value)) {
+      throw new Error(`Cannot use tokens in keys or values of "buildArgs" since they are needed before deployment`);
+    }
+  }
+}

packages/@aws-cdk/aws-ecr-assets/test/test.image-asset.ts

@@ -1,6 +1,6 @@
 import { expect, haveResource, SynthUtils } from '@aws-cdk/assert';
 import iam = require('@aws-cdk/aws-iam');
-import cdk = require('@aws-cdk/core');
+import { App, Lazy, Stack } from '@aws-cdk/core';
 import fs = require('fs');
 import { Test } from 'nodeunit';
 import path = require('path');
@@ -11,7 +11,7 @@ import { DockerImageAsset } from '../lib';
 export = {
   'test instantiating Asset Image'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
 
     // WHEN
     new DockerImageAsset(stack, 'Image', {
@@ -31,7 +31,7 @@ export = {
 
   'with build args'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
 
     // WHEN
     const asset = new DockerImageAsset(stack, 'Image', {
@@ -49,7 +49,7 @@ export = {
 
   'asset.repository.grantPull can be used to grant a principal permissions to use the image'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
     const user = new iam.User(stack, 'MyUser');
     const asset = new DockerImageAsset(stack, 'Image', {
       directory: path.join(__dirname, 'demo-image')
@@ -106,7 +106,7 @@ export = {
 
   'asset.repository.addToResourcePolicy can be used to modify the ECR resource policy via the adoption custom resource'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
     const asset = new DockerImageAsset(stack, 'Image', {
       directory: path.join(__dirname, 'demo-image')
     });
@@ -141,7 +141,7 @@ export = {
 
   'fails if the directory does not exist'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
 
     // THEN
     test.throws(() => {
@@ -154,7 +154,7 @@ export = {
 
   'fails if the directory does not contain a Dockerfile'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const stack = new Stack();
 
     // THEN
     test.throws(() => {
@@ -166,8 +166,8 @@ export = {
   },
 
   'docker directory is staged if asset staging is enabled'(test: Test) {
-    const app = new cdk.App();
-    const stack = new cdk.Stack(app, 'stack');
+    const app = new App();
+    const stack = new Stack(app, 'stack');
 
     new DockerImageAsset(stack, 'MyAsset', {
       directory: path.join(__dirname, 'demo-image')
@@ -177,6 +177,26 @@ export = {
 
     test.ok(fs.existsSync(path.join(session.directory, 'asset.1a17a141505ac69144931fe263d130f4612251caa4bbbdaf68a44ed0f405439c/Dockerfile')));
     test.ok(fs.existsSync(path.join(session.directory, 'asset.1a17a141505ac69144931fe263d130f4612251caa4bbbdaf68a44ed0f405439c/index.py')));
+    test.done();
+  },
+
+  'fails if using tokens in build args keys or values'(test: Test) {
+    // GIVEN
+    const stack = new Stack();
+    const token = Lazy.stringValue({ produce: () => 'foo' });
+    const expected = /Cannot use tokens in keys or values of "buildArgs" since they are needed before deployment/;
+
+    // THEN
+    test.throws(() => new DockerImageAsset(stack, 'MyAsset1', {
+      directory: path.join(__dirname, 'demo-image'),
+      buildArgs: { [token]: 'value' }
+    }), expected);
+
+    test.throws(() => new DockerImageAsset(stack, 'MyAsset2', {
+      directory: path.join(__dirname, 'demo-image'),
+      buildArgs: { key: token }
+    }), expected);
+
     test.done();
   }
 };