aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.85k forks source link

(pipelines): Complete support for manual approval step #21363

Open leantorres73 opened 2 years ago

leantorres73 commented 2 years ago

Describe the feature

Right now, as the CodePipeline is backend agnostic, there is a ManualApprovalStep but it's incomplete for CodePipeline, we need the extra props like additionalInformation , externalLink , etc.

I understand the point of making it generic but it sounds weird that we do have a CodeBuildStep because we add support to AWS, but we don't fully support AWS features at the same time.

Use Case

Provide a complete support for CodePipeline manual approval step.

Proposed Solution

Create a step like CodePipelineManualApprovalStep

Other Information

No response

Acknowledgements

CDK version used

2.22

Environment details (OS name and version, etc.)

Any environment

peterwoodworth commented 2 years ago

Thanks for the suggestion!

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

Hi-Fi commented 2 years ago

Currently _codepipelineactions has ManualApprovalAction that support those things, so utilizing that if engine is codepipeline might be easiest way? Just printing warning or something like that if trying to use those props on other engine.

ManualApprovalStep itself also already mentions that it doesn't work with all possible engines, only those that support pausing. So maybe it's not bad to have other things that are engine specific? It also seems to be using ManualApprovalAction already at https://github.com/aws/aws-cdk/blob/ff3c01a85d1bd32c149e83fda5bf44ec3253e99d/packages/%40aws-cdk/pipelines/lib/codepipeline/codepipeline.ts#L564-L568.

leantorres73 commented 2 years ago

Currently _codepipelineactions has ManualApprovalAction that support those things, so utilizing that if engine is codepipeline might be easiest way? Just printing warning or something like that if trying to use those props on other engine.

ManualApprovalStep itself also already mentions that it doesn't work with all possible engines, only those that support pausing. So maybe it's not bad to have other things that are engine specific? It also seems to be using ManualApprovalAction already at

https://github.com/aws/aws-cdk/blob/ff3c01a85d1bd32c149e83fda5bf44ec3253e99d/packages/%40aws-cdk/pipelines/lib/codepipeline/codepipeline.ts#L564-L568

.

I haven't had time to work on this ticket but I also think this could be an easy way to implement it, adding another if in there is pretty straighforward

if (step instanceof CodePipelineManualApprovalStep) {
      return {
        produceAction: (stage, options) => {
          stage.addAction(new cpa.ManualApprovalAction({
            actionName: options.actionName,
            runOrder: options.runOrder,
            additionalInformation: step.additionalInformation,
            externalLink: step.externalLink,
            ...
          }));
          return { runOrdersConsumed: 1 };
        },
      };
    }
jk2l commented 1 year ago

It will be good to have complete support. in most case I can just write a L3 construct for it, but the design of this atm prevent any customization