From cfe46f697b1837bf61bec5b950494c26044282ff Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 16 May 2019 03:03:23 -0700 Subject: [PATCH] fix(codepipeline): correctly validate Artifacts used by Actions in the same Stage. (#2558) The bug was: we were incorrectly iterating though the Actions of a given Stage. First we were iterating through all of the Actions looking at their inputs, and then we looped through the Actions again to look at the outputs. But that won't work if the input of an Action is the output of a different Action in the same Stage with a lower runOrder. The fix is to iterate through all of the inputs of a given Action, and then the outputs, and only then move on to the next Action in increasing runOrder sequence in the same Stage. Fixes #2549 --- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 6 +-- .../aws-codepipeline/test/test.artifacts.ts | 51 ++++++++++++++++++- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 7f9af258c0dda..b608020f8590a 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -498,8 +498,8 @@ export class Pipeline extends PipelineBase { for (const stage of this.stages) { const sortedActions = stage.actions.sort((a1, a2) => a1.runOrder - a2.runOrder); - // start with inputs for (const action of sortedActions) { + // start with inputs const inputArtifacts = action.inputs; for (const inputArtifact of inputArtifacts) { if (!inputArtifact.artifactName) { @@ -508,10 +508,8 @@ export class Pipeline extends PipelineBase { ret.push(`Artifact '${inputArtifact.artifactName}' was used as input before being used as output`); } } - } - // then process outputs by adding them to the Set - for (const action of sortedActions) { + // then process outputs by adding them to the Set const outputArtifacts = action.outputs; for (const outputArtifact of outputArtifacts) { // output Artifacts always have a name set diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.artifacts.ts b/packages/@aws-cdk/aws-codepipeline/test/test.artifacts.ts index ff09920e237df..bb7e144aa5c4e 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.artifacts.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.artifacts.ts @@ -1,4 +1,4 @@ -// import { expect, haveResourceLike } from '@aws-cdk/assert'; +import { expect, haveResourceLike } from '@aws-cdk/assert'; import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; import codepipeline = require('../lib'); @@ -123,6 +123,55 @@ export = { test.done(); }, + + "an Action's output can be used as input for an Action in the same Stage with a higher runOrder"(test: Test) { + const stack = new cdk.Stack(); + + const sourceOutput1 = new codepipeline.Artifact('sourceOutput1'); + const buildOutput1 = new codepipeline.Artifact('buildOutput1'); + const sourceOutput2 = new codepipeline.Artifact('sourceOutput2'); + + new codepipeline.Pipeline(stack, 'Pipeline', { + stages: [ + { + name: 'Source', + actions: [ + new FakeSourceAction({ + actionName: 'source1', + output: sourceOutput1, + }), + new FakeSourceAction({ + actionName: 'source2', + output: sourceOutput2, + }), + ], + }, + { + name: 'Build', + actions: [ + new FakeBuildAction({ + actionName: 'build1', + input: sourceOutput1, + output: buildOutput1, + }), + new FakeBuildAction({ + actionName: 'build2', + input: sourceOutput2, + extraInputs: [buildOutput1], + output: new codepipeline.Artifact('buildOutput2'), + runOrder: 2, + }), + ], + }, + ], + }); + + expect(stack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + // + })); + + test.done(); + }, }, };