feat(codepipeline): stage level condition feature L2 construct #33809

hwum · 2025-03-18T00:08:13Z

Issue # (if applicable)

Reason for this change

Codepipeline launched this feature last years and still missed the update in cdk library.

Description of changes

Support this feature in L2 Construct:https://docs.aws.amazon.com/codepipeline/latest/userguide/concepts-how-it-works-conditions.html

Describe any new or updated permissions being added

No

Description of how you validated changes

Unit test, integ test, and local cdk library deployment(linked to local cdkApp and successfully deploy it)

Checklist

[Y ] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

codecov · 2025-03-18T00:17:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.38%. Comparing base (6a5638a) to head (46745bb).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #33809   +/-   ##
=======================================
  Coverage   82.38%   82.38%           
=======================================
  Files         120      120           
  Lines        6938     6938           
  Branches     1170     1170           
=======================================
  Hits         5716     5716           
  Misses       1119     1119           
  Partials      103      103

Flag	Coverage Δ
suite.unit	`82.38% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`∅ <ø> (∅)`
packages/aws-cdk-lib/core	`82.38% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aws-cdk-automation

(This review is outdated)

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

GavinZZ

Thank you, in general looks good to me. Left some minor feedbacks.

GavinZZ · 2025-03-20T22:37:52Z

packages/aws-cdk-lib/aws-codepipeline/lib/private/stage.ts

+  public readonly beforeEntry?: Conditions;
+  public readonly onSuccess?: Conditions;
+  public readonly onFailure?: FailureConditions;


Do they need to be public? Do you expect CDK users to be able to access these fields like

const pipeline = new CodePipeline(.....); pipeline.beforeEntry

Sure. Switch these to private

GavinZZ · 2025-03-20T22:43:12Z

packages/aws-cdk-lib/aws-codepipeline/lib/rule.ts

+   *
+   * @default - No configuration
+   */
+  readonly configuration?: any;


Is there a way to make the type more strict? I see that it's an json object, is there certain interface we can draft for the object?

That Ideal one is a Map<any,any>, but it's kind of same as any. So I choose to match the CFN type

IMO I think we should at least make this a Map/object since the CFN type is JSON.

GavinZZ · 2025-03-20T22:44:36Z

packages/aws-cdk-lib/aws-codepipeline/lib/rule.ts

+   *
+   * @default 'Rule'
+   */
+  readonly category?: string;


Seems that category only supports Rule for now. Do you expect that this will support more values in the future? I think we should make it a ENUM-like class type. https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#enums

No. This one is fixed value applied for Rule

GavinZZ · 2025-03-20T22:49:50Z

packages/aws-cdk-lib/aws-codepipeline/lib/rule.ts

+      ruleTypeId: {
+        provider: this._props.provider,
+        version: this._props.version,
+        category: 'Rule',
+        owner: 'AWS',
+      },


Nice, I like how you flatten these in the props. Thanks!

GavinZZ · 2025-03-20T22:50:58Z

packages/aws-cdk-lib/aws-codepipeline/lib/rule.ts

+      ruleTypeId: {
+        provider: this._props.provider,
+        version: this._props.version,
+        category: 'Rule',


I see you're hardcoding Rule instead of using this.props.category. Should we not expose the property isntead then?

The category in RuleProps should be deleted.

GavinZZ · 2025-03-20T22:52:27Z

packages/aws-cdk-lib/aws-codepipeline/test/rules.test.ts

+    }));
+    // --
+
+    // eslint-disable-next-line no-console


remove this plz

GavinZZ · 2025-03-20T22:52:33Z

packages/aws-cdk-lib/aws-codepipeline/test/rules.test.ts

+
+    // eslint-disable-next-line no-console
+    console.log(Template.fromStack(stack).findResources('AWS::CodePipeline::Pipeline'));
+    // eslint-disable-next-line no-console


GavinZZ · 2025-03-20T22:53:11Z

packages/aws-cdk-lib/aws-codepipeline/test/stages.test.ts

+      }));
+      // --
+
+      // eslint-disable-next-line no-console


GavinZZ · 2025-03-20T22:53:15Z

packages/aws-cdk-lib/aws-codepipeline/test/stages.test.ts

+
+      // eslint-disable-next-line no-console
+      console.log(Template.fromStack(stack).findResources('AWS::CodePipeline::Pipeline'));
+      // eslint-disable-next-line no-console


GavinZZ · 2025-03-20T22:54:10Z

...g/framework-integ/test/aws-codepipeline-actions/test/integ.pipeline-with-stage-conditions.ts

+const awsApiCall1 = integrationTest.assertions.awsApiCall('CodePipeline', 'getPipeline', { name: 'my-pipeline-stage-test' });
+awsApiCall1.assertAtPath('pipeline.name', ExpectedResult.stringLikeRegexp('my-pipeline-stage-test'));
+awsApiCall1.assertAtPath('pipeline.stages.0.name', ExpectedResult.stringLikeRegexp('Source'));
+awsApiCall1.assertAtPath('pipeline.stages.1.name', ExpectedResult.stringLikeRegexp('Lambda'));
+awsApiCall1.assertAtPath('pipeline.stages.1.actions.0.name', ExpectedResult.stringLikeRegexp('Lambda'));
+awsApiCall1.assertAtPath('pipeline.stages.1.onSuccess.conditions.0.result', ExpectedResult.stringLikeRegexp('FAIL'));
+awsApiCall1.assertAtPath('pipeline.stages.1.onFailure.conditions.0.result', ExpectedResult.stringLikeRegexp('ROLLBACK'));


Nice, thanks for adding assertions to integ tests!

Pull request has been modified.

GavinZZ

LGTM, just one more comment

GavinZZ · 2025-03-21T17:25:31Z

packages/aws-cdk-lib/aws-codepipeline/lib/rule.ts

+   *
+   * @default - No configuration
+   */
+  readonly configuration?: any;


IMO I think we should at least make this a Map/object since the CFN type is JSON.

L2 Construct for stage level condition feature

hwum · 2025-03-21T18:03:54Z

LGTM, just one more comment
I'm following the same pattern here:https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-codepipeline/lib/action.ts#L143-L145
Since action config and rule config is same data structure in Codepipeline backend

Pull request has been modified.

GavinZZ

Thank you for the contribution and addressing the comments!

aws-cdk-automation · 2025-03-21T19:20:26Z

AWS CodeBuild CI Report

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

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

mergify · 2025-03-21T19:20:56Z

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).

github-actions · 2025-03-21T19:21:13Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

aws-cdk-automation requested a review from a team March 18, 2025 00:08

github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Mar 18, 2025

aws-cdk-automation previously requested changes Mar 18, 2025

View reviewed changes

hwum changed the title ~~feat(codepipeline): L2 Construct for stage level condition feature~~ feat(codepipeline): stage level condition feature L2 construct Mar 18, 2025

aws-cdk-automation previously requested changes Mar 18, 2025

View reviewed changes

hwum force-pushed the dev branch from 65c9d52 to 38b2f64 Compare March 18, 2025 21:09

hwum force-pushed the dev branch from 38b2f64 to dc9055d Compare March 19, 2025 03:05

SimonCMoore added p1 and removed p2 labels Mar 19, 2025

hwum force-pushed the dev branch 6 times, most recently from 5790787 to f85ce3b Compare March 19, 2025 21:52

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 19, 2025

GavinZZ previously requested changes Mar 20, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 20, 2025

hwum force-pushed the dev branch from f85ce3b to 557eebc Compare March 20, 2025 23:08

hwum force-pushed the dev branch from 557eebc to 5ae669d Compare March 20, 2025 23:27

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 20, 2025

GavinZZ previously requested changes Mar 21, 2025

View reviewed changes

feat(codepipeline): L2 Construct for stage level condition feature

ab961a6

L2 Construct for stage level condition feature

hwum force-pushed the dev branch from 5ae669d to 371a242 Compare March 21, 2025 18:04

hwum requested a review from GavinZZ March 21, 2025 18:04

Fix: Update index.js

bfc7b1b

hwum force-pushed the dev branch from 371a242 to bfc7b1b Compare March 21, 2025 18:24

Merge branch 'main' into dev

46745bb

GavinZZ approved these changes Mar 21, 2025

View reviewed changes

mergify bot merged commit 8e4374f into aws:main Mar 21, 2025
21 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Mar 21, 2025

hwum deleted the dev branch March 21, 2025 20:01

feat(codepipeline): stage level condition feature L2 construct #33809

feat(codepipeline): stage level condition feature L2 construct #33809

Uh oh!

Conversation

hwum commented Mar 18, 2025 • edited by GavinZZ Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue # (if applicable)

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hwum commented Mar 21, 2025

Uh oh!

GavinZZ left a comment

Choose a reason for hiding this comment

Uh oh!

aws-cdk-automation commented Mar 21, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Mar 21, 2025

Uh oh!

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

hwum commented Mar 18, 2025 •

edited by GavinZZ

Loading

aws-cdk-automation left a comment •

edited

Loading

codecov bot commented Mar 18, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading