From f7a309fcdff678f7929b90b09b2f49b33ccb64e0 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 14 Mar 2019 15:41:08 -0700 Subject: [PATCH 1/5] Make Action#addInputArtifact idempotent. --- .../aws-codepipeline-api/lib/action.ts | 5 +++++ .../aws-codepipeline/test/test.action.ts | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index 5553cf105a57a..784548a6d1de0 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts @@ -258,6 +258,11 @@ export abstract class Action { } 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; + } this._actionInputArtifacts.push(artifact); return this; } diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts index adc8a8f62c03f..12946d3a802a9 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,24 @@ export = { test.done(); }, + + 'input Artifacts': { + 'can be added multiple times to an Action safely'(test: Test) { + const artifact = new actions.Artifact('Artifact'); + + 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(); + }, + } }; function boundsValidationResult(numberOfArtifacts: number, min: number, max: number): string[] { From 164e0a1d504bff326b4cd7be30da0c561ae09a36 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 14 Mar 2019 16:27:55 -0700 Subject: [PATCH 2/5] Validate that Action input Artifacts have unique names. --- .../aws-codepipeline-api/lib/action.ts | 6 +++++ .../aws-codepipeline/test/test.action.ts | 22 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index 784548a6d1de0..04eab3d427036 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts @@ -263,6 +263,12 @@ export abstract class Action { 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/test.action.ts b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts index 12946d3a802a9..ede900ea16154 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts @@ -152,7 +152,7 @@ export = { 'input Artifacts': { 'can be added multiple times to an Action safely'(test: Test) { - const artifact = new actions.Artifact('Artifact'); + const artifact = new actions.Artifact('SomeArtifact'); const stack = new cdk.Stack(); const project = new codebuild.PipelineProject(stack, 'Project'); @@ -166,7 +166,25 @@ export = { 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(); + }, + }, }; function boundsValidationResult(numberOfArtifacts: number, min: number, max: number): string[] { From 7e9fbd4f2db588277f8a6c4d2b21f97e59858acf Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 14 Mar 2019 16:44:33 -0700 Subject: [PATCH 3/5] Remove the no longer required check for Artifact name uniqueness. --- .../@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 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..4648c603c80c5 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts @@ -293,8 +293,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 +356,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); } From 5008182881c375e16362e59f4d46c501d6811ef6 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 14 Mar 2019 17:25:10 -0700 Subject: [PATCH 4/5] Add the additionalInputArtifacts property when creating CloudFormation deploy Actions. --- .../lib/pipeline-actions.ts | 21 +++++++++++++++++++ .../test/integ.pipeline-cfn.expected.json | 7 +++++-- .../test/integ.pipeline-cfn.ts | 7 ++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index 4648c603c80c5..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); + } } /** 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], }), ], }); From 4c18d0bc68535fa2411ed2e4e4902e7ec83f7b9c Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 14 Mar 2019 17:26:43 -0700 Subject: [PATCH 5/5] Make Action#addOutputArtifact idempotent. --- .../aws-codepipeline-api/lib/action.ts | 7 ++++++ .../aws-codepipeline/test/test.action.ts | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts index 04eab3d427036..d98bbc52dd2c6 100644 --- a/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts +++ b/packages/@aws-cdk/aws-codepipeline-api/lib/action.ts @@ -252,6 +252,13 @@ 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; diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts index ede900ea16154..b12452eea55c3 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.action.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.action.ts @@ -185,6 +185,28 @@ export = { 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[] {