From 2d463beeb1eefac9476c517875cf4b1ee7733753 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Fri, 15 Mar 2019 09:34:02 -0700 Subject: [PATCH] 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 --- .../lib/pipeline-actions.ts | 27 +++++++-- .../aws-codepipeline-api/lib/action.ts | 18 ++++++ .../test/integ.pipeline-cfn.expected.json | 7 ++- .../test/integ.pipeline-cfn.ts | 7 ++- .../aws-codepipeline/test/test.action.ts | 59 ++++++++++++++++++- 5 files changed, 110 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index b886276a4339c..77ee1bd25b4b7 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/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); } diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index 5553cf105a57a..d98bbc52dd2c6 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/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; } diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json index d61ea21cfde6e..50739be109c18 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json +++ b/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 @@ } } } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts index b047a5efe333c..44589acaff0bf 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts +++ b/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], }), ], }); diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts index adc8a8f62c03f..b12452eea55c3 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts +++ b/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[] {