Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cdk-pipelines] Manual approval steps are not getting the right RunOrder #9101

Closed
JFox opened this issue Jul 16, 2020 · 1 comment · Fixed by #9508
Closed

[cdk-pipelines] Manual approval steps are not getting the right RunOrder #9101

JFox opened this issue Jul 16, 2020 · 1 comment · Fixed by #9508
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. in-progress This issue is being actively worked on. p1

Comments

@JFox
Copy link

JFox commented Jul 16, 2020

When using pipeline.addAdpplicationStage({manualApprovals: true}) the resulting RunOrder for the actions is causing the deploys to be performed without waiting for the manual action to be done

Reproduction Steps

    pipeline.addApplicationStage(
      new WorkloadAccountApplication(this, 'WorkloadAccount', {
        env: { account: '1111111111', region: 'eu-central-1' },
      }),
      {
        manualApprovals: true,
      },
    );

Error Log

image

Environment

  • CLI Version : 1.51.0 (build 8c2d53c)
  • Framework Version: 1.51.0 (build 8c2d53c)
  • Node.js Version: v12.18.0
  • OS : aws/codebuild/standard:4.0
  • Language (Version): TypeScript (3.9.5)

Other


This is 🐛 Bug Report

@JFox JFox added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 16, 2020
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jul 16, 2020
@ericzbeard ericzbeard added p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 16, 2020
@ericzbeard ericzbeard assigned rix0rrr and unassigned ericzbeard Jul 16, 2020
@ericzbeard ericzbeard removed their assignment Jul 20, 2020
@roskelleycj
Copy link

When I tried this:

    const prodStage = pipeline.addApplicationStage(prod);
    prodStage.addActions(new ManualApprovalAction({
        actionName: 'ApproveChanges'
    }));

This at least put the manualApproval BEFORE the execution of Deploy. However, it also put it with the same runOrder as the Prepare, which is also undesirable, IMHO. It would be ideal to have this:

Myservice.Prepare -> ManualApproval -> Myservice.Deploy

Thus allowing for the human reviewer to inspect the ChangeSet after it is completed AND before making the approval to do the Deploy.

@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Aug 8, 2020
@mergify mergify bot closed this as completed in #9508 Aug 10, 2020
mergify bot pushed a commit that referenced this issue Aug 10, 2020
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*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants