feat(cloudformation): allow specifying additional inputs for deploy Actions (#2020)

Additional input Artifacts are needed when using the `parameterOverrides` property of the Action (any Artifact used in that map needs to be added to the input Artifacts of the Action, otherwise the Pipeline will fail at runtime).

Also did some cleanup in the Action superclass while I was in the area.

Fixes #1247

Author: skinny85

packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts

@@ -200,6 +200,23 @@ export interface PipelineCloudFormationDeployActionProps extends PipelineCloudFo
    * @default No overrides
    */
   parameterOverrides?: { [name: string]: any };
+
+  /**
+   * The list of additional input Artifacts for this Action.
+   * This is especially useful when used in conjunction with the `parameterOverrides` property.
+   * For example, if you have:
+   *
+   *   parameterOverrides: {
+   *     'Param1': action1.outputArtifact.bucketName,
+   *     'Param2': action2.outputArtifact.objectKey,
+   *   }
+   *
+   * , if the output Artifacts of `action1` and `action2` were not used to
+   * set either the `templateConfiguration` or the `templatePath` properties,
+   * you need to make sure to include them in the `additionalInputArtifacts` -
+   * otherwise, you'll get an "unrecognized Artifact" error during your Pipeline's execution.
+   */
+  additionalInputArtifacts?: codepipeline.Artifact[];
 }
 // tslint:enable:max-line-length
 
@@ -223,6 +240,10 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo
     });
 
     this.props = props;
+
+    for (const inputArtifact of props.additionalInputArtifacts || []) {
+      this.addInputArtifact(inputArtifact);
+    }
   }
 
   /**
@@ -293,8 +314,7 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation
     });
 
     this.addInputArtifact(props.templatePath.artifact);
-    if (props.templateConfiguration &&
-        props.templateConfiguration.artifact.artifactName !== props.templatePath.artifact.artifactName) {
+    if (props.templateConfiguration) {
       this.addInputArtifact(props.templateConfiguration.artifact);
     }
 
@@ -357,8 +377,7 @@ export class PipelineCreateUpdateStackAction extends PipelineCloudFormationDeplo
     });
 
     this.addInputArtifact(props.templatePath.artifact);
-    if (props.templateConfiguration &&
-        props.templateConfiguration.artifact.artifactName !== props.templatePath.artifact.artifactName) {
+    if (props.templateConfiguration) {
       this.addInputArtifact(props.templateConfiguration.artifact);
     }
 

packages/@aws-cdk/aws-codepipeline-api/lib/action.ts

@@ -252,12 +252,30 @@ export abstract class Action {
   }
 
   protected addOutputArtifact(name: string): Artifact {
+    // adding the same name multiple times doesn't do anything -
+    // addOutputArtifact is idempotent
+    const ret = this._outputArtifacts.find(output => output.artifactName === name);
+    if (ret) {
+      return ret;
+    }
+
     const artifact = new Artifact(name);
     this._actionOutputArtifacts.push(artifact);
     return artifact;
   }
 
   protected addInputArtifact(artifact: Artifact): Action {
+    // adding the same artifact multiple times doesn't do anything -
+    // addInputArtifact is idempotent
+    if (this._actionInputArtifacts.indexOf(artifact) !== -1) {
+      return this;
+    }
+
+    // however, a _different_ input with the same name is an error
+    if (this._actionInputArtifacts.find(input => input.artifactName === artifact.artifactName)) {
+      throw new Error(`Action ${this.actionName} already has an input with the name '${artifact.artifactName}'`);
+    }
+
     this._actionInputArtifacts.push(artifact);
     return this;
   }

packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json

@@ -211,9 +211,12 @@
                       "Arn"
                     ]
                   },
-                  "ParameterOverrides": "{\"BucketName\":{\"Fn::GetArtifactAtt\":[\"SourceArtifact\",\"BucketName\"]},\"ObjectKey\":{\"Fn::GetArtifactAtt\":[\"SourceArtifact\",\"ObjectKey\"]},\"Url\":{\"Fn::GetArtifactAtt\":[\"SourceArtifact\",\"URL\"]},\"OtherParam\":{\"Fn::GetParam\":[\"SourceArtifact\",\"params.json\",\"OtherParam\"]}}"
+                  "ParameterOverrides": "{\"BucketName\":{\"Fn::GetArtifactAtt\":[\"SourceArtifact\",\"BucketName\"]},\"ObjectKey\":{\"Fn::GetArtifactAtt\":[\"SourceArtifact\",\"ObjectKey\"]},\"Url\":{\"Fn::GetArtifactAtt\":[\"AdditionalArtifact\",\"URL\"]},\"OtherParam\":{\"Fn::GetParam\":[\"SourceArtifact\",\"params.json\",\"OtherParam\"]}}"
                 },
                 "InputArtifacts": [
+                  {
+                    "Name": "AdditionalArtifact"
+                  },
                   {
                     "Name": "SourceArtifact"
                   }
@@ -274,4 +277,4 @@
       }
     }
   }
-}
+}

packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts

@@ -1,4 +1,5 @@
 import cfn = require('@aws-cdk/aws-cloudformation');
+import cpapi = require('@aws-cdk/aws-codepipeline-api');
 import { Role } from '@aws-cdk/aws-iam';
 import { ServicePrincipal } from '@aws-cdk/aws-iam';
 import s3 = require('@aws-cdk/aws-s3');
@@ -33,6 +34,9 @@ const role = new Role(stack, 'CfnChangeSetRole', {
   assumedBy: new ServicePrincipal('cloudformation.amazonaws.com'),
 });
 
+// fake Artifact, just for testing
+const additionalArtifact = new cpapi.Artifact('AdditionalArtifact');
+
 pipeline.addStage(sourceStage);
 pipeline.addStage({
   name: 'CFN',
@@ -47,9 +51,10 @@ pipeline.addStage({
       parameterOverrides: {
         BucketName: source.outputArtifact.bucketName,
         ObjectKey: source.outputArtifact.objectKey,
-        Url: source.outputArtifact.url,
+        Url: additionalArtifact.url,
         OtherParam: source.outputArtifact.getParam('params.json', 'OtherParam'),
       },
+      additionalInputArtifacts: [additionalArtifact],
     }),
   ],
 });

packages/@aws-cdk/aws-codepipeline/test/test.action.ts

@@ -1,4 +1,3 @@
-// import { validateArtifactBounds, validateSourceAction } from '../lib/validation';
 import { expect, haveResourceLike } from '@aws-cdk/assert';
 import codebuild = require('@aws-cdk/aws-codebuild');
 import codecommit = require('@aws-cdk/aws-codecommit');
@@ -150,6 +149,64 @@ export = {
 
     test.done();
   },
+
+  'input Artifacts': {
+    'can be added multiple times to an Action safely'(test: Test) {
+      const artifact = new actions.Artifact('SomeArtifact');
+
+      const stack = new cdk.Stack();
+      const project = new codebuild.PipelineProject(stack, 'Project');
+      const action = project.toCodePipelineBuildAction({
+        actionName: 'CodeBuild',
+        inputArtifact: artifact,
+        additionalInputArtifacts: [artifact],
+      });
+
+      test.equal(action._inputArtifacts.length, 1);
+
+      test.done();
+    },
+
+    'cannot have duplicate names'(test: Test) {
+      const artifact1 = new actions.Artifact('SomeArtifact');
+      const artifact2 = new actions.Artifact('SomeArtifact');
+
+      const stack = new cdk.Stack();
+      const project = new codebuild.PipelineProject(stack, 'Project');
+
+      test.throws(() =>
+        project.toCodePipelineBuildAction({
+          actionName: 'CodeBuild',
+          inputArtifact: artifact1,
+          additionalInputArtifacts: [artifact2],
+        })
+      , /SomeArtifact/);
+
+      test.done();
+    },
+  },
+
+  'output Artifact names': {
+    'accept the same name multiple times safely'(test: Test) {
+      const artifact = new actions.Artifact('SomeArtifact');
+
+      const stack = new cdk.Stack();
+      const project = new codebuild.PipelineProject(stack, 'Project');
+      const action = project.toCodePipelineBuildAction({
+        actionName: 'CodeBuild',
+        inputArtifact: artifact,
+        outputArtifactName: 'Artifact1',
+        additionalOutputArtifactNames: [
+          'Artifact1',
+          'Artifact1',
+        ],
+      });
+
+      test.equal(action._outputArtifacts.length, 1);
+
+      test.done();
+    },
+  },
 };
 
 function boundsValidationResult(numberOfArtifacts: number, min: number, max: number): string[] {