From 5c01da8d82f77e0241890101258aace2dac1902d Mon Sep 17 00:00:00 2001 From: barticus Date: Mon, 10 Aug 2020 22:53:17 +1000 Subject: [PATCH] fix(pipelines): manual approval of changeset uses wrong ordering (#9508) From `addApplication` or `addStackArtifactDeployment` method. Currently, when calling `addStackArtifactDeployment`, you can pass in `AddStackOptions`, but the `executeRunOrder` property is not respected later in the process where `DeployCdkStackAction.fromStackArtifact` is called. From addApplication, this property is used when manualApprovals has been opted in to, but as it is not respected, it results in bug #9101 Fixes #9101 (I think!) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/pipelines/lib/stage.ts | 7 +++--- .../pipelines/test/stack-ordering.test.ts | 25 ++++++++++++++++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/pipelines/lib/stage.ts b/packages/@aws-cdk/pipelines/lib/stage.ts index 6b379f686fe2e..dd5aa28c5e50e 100644 --- a/packages/@aws-cdk/pipelines/lib/stage.ts +++ b/packages/@aws-cdk/pipelines/lib/stage.ts @@ -184,7 +184,7 @@ export class CdkStage extends Construct { if (this._prepared) { return; } this._prepared = true; - for (const { prepareRunOrder: runOrder, stackArtifact } of this.stacksToDeploy) { + for (const { prepareRunOrder, stackArtifact, executeRunOrder } of this.stacksToDeploy) { const artifact = this.host.stackOutputArtifact(stackArtifact.id); this.pipelineStage.addAction(DeployCdkStackAction.fromStackArtifact(this, stackArtifact, { @@ -192,7 +192,8 @@ export class CdkStage extends Construct { cloudAssemblyInput: this.cloudAssemblyArtifact, output: artifact, outputFileName: artifact ? 'outputs.json' : undefined, - prepareRunOrder: runOrder, + prepareRunOrder, + executeRunOrder, })); } } @@ -387,4 +388,4 @@ interface DeployStackCommand { prepareRunOrder: number; executeRunOrder: number; stackArtifact: cxapi.CloudFormationStackArtifact; -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/pipelines/test/stack-ordering.test.ts b/packages/@aws-cdk/pipelines/test/stack-ordering.test.ts index e755572c78544..45d0cb76bf6b8 100644 --- a/packages/@aws-cdk/pipelines/test/stack-ordering.test.ts +++ b/packages/@aws-cdk/pipelines/test/stack-ordering.test.ts @@ -55,6 +55,29 @@ test('multiple independent stacks go in parallel', () => { }); }); +test('manual approval is inserted in correct location', () => { + // WHEN + pipeline.addApplicationStage(new TwoStackApp(app, 'MyApp'), { + manualApprovals: true, + }); + + // THEN + expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', { + Stages: arrayWith({ + Name: 'MyApp', + Actions: sortedByRunOrder([ + objectLike({ Name: 'Stack1.Prepare' }), + objectLike({ Name: 'ManualApproval' }), + objectLike({ Name: 'Stack1.Deploy' }), + objectLike({ Name: 'Stack2.Prepare' }), + objectLike({ Name: 'ManualApproval2' }), + objectLike({ Name: 'Stack2.Deploy' }), + ]), + }), + }); +}); + + class TwoStackApp extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { super(scope, id, props); @@ -80,4 +103,4 @@ class ThreeStackApp extends Stage { stack3.addDependency(stack1); stack3.addDependency(stack2); } -} \ No newline at end of file +}