feat(pipelines): allow disabling use of change sets #21619

Hi-Fi · 2022-08-16T07:28:47Z

Possibility to change deployment way for whole pipeline, for single stage or for single stack in stage.

Fixes #20827

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2022-08-16T07:28:54Z

Hi-Fi · 2022-08-18T03:58:40Z

@rix0rrr Does this looks something you thought at the comment at the issue?

rix0rrr

Thanks! I have a request for a couple of changes, see below.

rix0rrr · 2022-08-19T11:42:45Z

packages/@aws-cdk/pipelines/README.md

@@ -144,6 +144,21 @@ new MyPipelineStack(this, 'PipelineStack', {
 });
 ```

+Deployment is done by default with `CodePipeline` engine by first generating


Is this the most logical place in this README to add this content? I feel a separate section might be better.

rix0rrr · 2022-08-19T11:46:13Z

packages/@aws-cdk/pipelines/README.md

@@ -144,6 +144,21 @@ new MyPipelineStack(this, 'PipelineStack', {
 });
 ```

+Deployment is done by default with `CodePipeline` engine by first generating
+change set, and then executing it. This allow to add e.g. approval before 


Please proof-read your README section once more before submitting.

Suggested change

change set, and then executing it. This allow to add e.g. approval before

a change set, and then executing it. This allows you to add e.g. approval before

The reason we have changeset + execute is not (just) for approval steps. Like I said in a comment to the original issue, certain CloudFormation features require a ChangeSet. Hence, it's only fair to list here what features you will be unable to use if you disable the prepare step.

Is there somewhere list of those or documentation about features that require change set? Dynamic referring is only one mentioned in issue.

The list has not been added to documentation.
Below in this review, @rix0rrr, you say:

I'm told I'm wrong and direct deployments should just work, so the only reason to have changesets might just be to add an approval step.

Is this confirmed that this is the only difference?
(we are using a 30 stacks pipeline, without need for approval step, and saving 1 minute per stack when they are not updated is enormous - it seems to work without any side-effect for now)

rix0rrr · 2022-08-19T11:48:26Z

packages/@aws-cdk/pipelines/README.md

+- [whole pipeline](./test/integ.pipeline-without-prepare.ts)
+- [single stage](./test/integ.pipeline-stage-without-prepare.ts)
+- [single stack in stage](./test/integ.pipeline-stage-stack-without-prepare.ts)


These relative hyperlinks will only work on GitHub.

They do not work here: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.pipelines-readme.html

They do not work here: https://www.npmjs.com/package/@aws-cdk/pipelines

Instead, put in a code example directly that showcases how to use this feature in a simple way. It's okay to not cover every permutation.

rix0rrr · 2022-08-19T11:48:48Z

packages/@aws-cdk/pipelines/README.md

-* If you are using Docker image assets in your application stages: Docker will
+- If you are using Docker image assets in your application stages: Docker will


Why this change?

This came automatically with markdownlint.

rix0rrr · 2022-08-19T11:52:37Z

packages/@aws-cdk/pipelines/lib/helpers-internal/pipeline-graph.ts

@@ -333,7 +346,7 @@ type GraphAnnotation =
  | { readonly type: 'step'; readonly step: Step; isBuildStep?: boolean }
  | { readonly type: 'self-update' }
  | { readonly type: 'prepare'; readonly stack: StackDeployment }
-  | { readonly type: 'execute'; readonly stack: StackDeployment; readonly captureOutputs: boolean }
+  | { readonly type: 'execute'; readonly stack: StackDeployment; readonly captureOutputs: boolean; readonly withoutChangeset?: boolean }


These types constitute a contract, that is consumed for example by https://github.com/cdklabs/cdk-pipelines-github

Changing what the execute node means will break other implementors. Instead, add a new node type (for example, deploy) that has a new meaning.

Would it break, of there is new optional field? I first checked separate one, but at least thst point it seemed to generate more changes.

Of course now with working tests it would be easier to refactor.

I think that adding new node here is easier breaking change, as node is created at https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/pipelines/lib/helpers-internal/pipeline-graph.ts#L138-L142.

Here if we determine the new deploy node if no prepare step is needed, existing functionality would fail as e.g. GitHub engine would get different node what it can't handle at https://github.com/cdklabs/cdk-pipelines-github/blob/3fa9bb2df734c4eb6b5ce05946e728af65881037/src/pipeline.ts#L350-L351.

Or did I misunderstood? Optional prop should be invisible to all existing implementors, and allows those to continue as is.

Of course that execute node in general is a bit confusing, as with that existing logic to leave out prepare step single step is not really execute but deploy https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/pipelines/lib/helpers-internal/pipeline-graph.ts#L137.

Pull request has been modified.

Possibility to change deployment way for whole pipeline, for single stage or for single stack in stage.

packages/@aws-cdk/pipelines/README.md

rix0rrr

I see what you're saying on the 'execute' step. It's a bit weird, but apparently we're already doing this for the GitHub provider.

I've simplified the PR a little: allow disabling use of changesets altogether for the CodePipeline provider (and not customize on stage or stack level). I'm told I'm wrong and direct deployments should just work, so the only reason to have changesets might just be to add an approval step.

If we want to go the "customize per stack" route, we'll have to do it slightly differently anyway. Currently prepareStep: false as passed by the GitHub engine doesn't mean "I don't want change sets", it means "I don't support change sets".

Since the per-stage and per-stack API is shared between all Pipelines implementations, it would conceivably be possible to turn it on on a per-stage basis, which should lead to an error for the GitHub pipeline for which that is not possible.

packages/@aws-cdk/pipelines/README.md

mergify · 2022-09-26T10:40:52Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

In aws/aws-cdk#21619, we are making the exhaustiveness check for `GraphAnnotation` impossible on purpose, so that all CDK Pipelines implementations will be forced to add a `default` case to the `switch` (in preparation of future extensions of the graph model). Add a `default` case here already to make sure we don't fail compilation.

In aws/aws-cdk#21619, we are making the exhaustiveness check for `GraphAnnotation` impossible on purpose, so that all CDK Pipelines implementations will be forced to add a `default` case to the `switch` (in preparation of future extensions of the graph model). Add a `default` case here already to make sure we don't fail compilation. Fixes #

mergify · 2022-09-26T15:19:34Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2022-09-26T15:19:45Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 3f94a8d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Possibility to change deployment way for whole pipeline, for single stage or for single stack in stage. Fixes aws#20827 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

github-actions bot added effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2 labels Aug 16, 2022

aws-cdk-automation requested a review from a team August 16, 2022 07:29

rix0rrr previously requested changes Aug 19, 2022

View reviewed changes

Hi-Fi requested a review from rix0rrr August 31, 2022 18:50

Hi-Fi force-pushed the feat/pipeline_without_prepare branch from 10b8339 to 789dce5 Compare August 31, 2022 18:50

Hi-Fi force-pushed the feat/pipeline_without_prepare branch 3 times, most recently from 5d479c5 to 2e24e77 Compare September 2, 2022 03:43

feat(pipelines): allow making deployment in single step

62bf35f

Possibility to change deployment way for whole pipeline, for single stage or for single stack in stage.

Hi-Fi force-pushed the feat/pipeline_without_prepare branch from 2e24e77 to 62bf35f Compare September 7, 2022 16:43

polothy reviewed Sep 7, 2022

View reviewed changes

packages/@aws-cdk/pipelines/README.md Outdated Show resolved Hide resolved

rix0rrr added 2 commits September 26, 2022 11:51

Remove the per-stack and per-stage customization

42f74e2

Update docs an error message a bit

1940440

rix0rrr changed the title ~~feat(pipelines): allow making deployment in single step~~ feat(pipelines): allow disabling use of change sets Sep 26, 2022

rix0rrr approved these changes Sep 26, 2022

View reviewed changes

packages/@aws-cdk/pipelines/README.md Outdated Show resolved Hide resolved

rix0rrr mentioned this pull request Sep 26, 2022

chore: add a 'default' case to the pipeline type switch cdklabs/cdk-pipelines-github#344

Merged

Merge branch 'main' into feat/pipeline_without_prepare

3f94a8d

mergify bot merged commit 05723e7 into aws:main Sep 26, 2022

gshpychka mentioned this pull request Mar 31, 2023

(pipelines): CloudFormation deploy is not retryable #21677

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(pipelines): allow disabling use of change sets #21619

feat(pipelines): allow disabling use of change sets #21619

Hi-Fi commented Aug 16, 2022

gitpod-io bot commented Aug 16, 2022

Hi-Fi commented Aug 18, 2022

rix0rrr left a comment

rix0rrr Aug 19, 2022

rix0rrr Aug 19, 2022

Hi-Fi Aug 19, 2022

fab-mindflow Dec 27, 2022

rix0rrr Aug 19, 2022

rix0rrr Aug 19, 2022

Hi-Fi Aug 19, 2022

rix0rrr Aug 19, 2022

Hi-Fi Aug 19, 2022

Hi-Fi Aug 19, 2022

rix0rrr left a comment

mergify bot commented Sep 26, 2022

mergify bot commented Sep 26, 2022

aws-cdk-automation commented Sep 26, 2022

	change set, and then executing it. This allow to add e.g. approval before
	a change set, and then executing it. This allows you to add e.g. approval before

		* If you are using Docker image assets in your application stages: Docker will
		- If you are using Docker image assets in your application stages: Docker will

feat(pipelines): allow disabling use of change sets #21619

feat(pipelines): allow disabling use of change sets #21619

Conversation

Hi-Fi commented Aug 16, 2022

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Aug 16, 2022

Hi-Fi commented Aug 18, 2022

rix0rrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

mergify bot commented Sep 26, 2022

mergify bot commented Sep 26, 2022

aws-cdk-automation commented Sep 26, 2022

AWS CodeBuild CI Report