Skip to content

Commit

Permalink
fix(pipelines): manual approval of changeset uses wrong ordering (#9508)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
barticus authored Aug 10, 2020
1 parent a311428 commit 5c01da8
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 4 deletions.
7 changes: 4 additions & 3 deletions packages/@aws-cdk/pipelines/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,15 +184,16 @@ 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, {
baseActionName: this.simplifyStackName(stackArtifact.stackName),
cloudAssemblyInput: this.cloudAssemblyArtifact,
output: artifact,
outputFileName: artifact ? 'outputs.json' : undefined,
prepareRunOrder: runOrder,
prepareRunOrder,
executeRunOrder,
}));
}
}
Expand Down Expand Up @@ -387,4 +388,4 @@ interface DeployStackCommand {
prepareRunOrder: number;
executeRunOrder: number;
stackArtifact: cxapi.CloudFormationStackArtifact;
}
}
25 changes: 24 additions & 1 deletion packages/@aws-cdk/pipelines/test/stack-ordering.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -80,4 +103,4 @@ class ThreeStackApp extends Stage {
stack3.addDependency(stack1);
stack3.addDependency(stack2);
}
}
}

0 comments on commit 5c01da8

Please sign in to comment.