feat(synthetics): cleanup provisioned lambda and layers for canary #32738

badmintoncryer · 2025-01-04T14:19:14Z

Issue # (if applicable)

None

Reason for this change

For the Lambda and Layer used in Canary, the deletion of related resources is handled by a custom resource, but this functionality is now supported natively by CloudFormation.

https://docs.aws.amazon.com/ja_jp/AWSCloudFormation/latest/UserGuide/aws-resource-synthetics-canary.html#cfn-synthetics-canary-provisionedresourcecleanup

Description of changes

Add provisionedResourceCleanup prop to CanaryProps
deprecate cleanup prop which uses custom resource

Describe any new or updated permissions being added

None

Description of how you validated changes

Add both unit and integ test

Checklist

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

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

codecov · 2025-01-04T14:39:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.57%. Comparing base (7329068) to head (ae988df).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32738   +/-   ##
=======================================
  Coverage   81.57%   81.57%           
=======================================
  Files         227      227           
  Lines       13793    13793           
  Branches     2419     2419           
=======================================
  Hits        11251    11251           
  Misses       2270     2270           
  Partials      272      272

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

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

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

lpizzinidev

👍 Left comments for minor adjustments

lpizzinidev · 2025-01-06T16:32:44Z

packages/aws-cdk-lib/aws-synthetics/README.md

Can you please update the Deleting underlying resources on canary deletion paragraph with a description of the new attribute?

Oh.. I've forgotten to update the readme. Thanks!

lpizzinidev · 2025-01-06T16:34:08Z

packages/aws-cdk-lib/aws-synthetics/lib/canary.ts

@@ -70,6 +70,8 @@ export enum Cleanup {
   * Clean up the underlying Lambda function only. The user is
   * responsible for cleaning up all other resources left behind
   * by the Canary.
+   *
+   * @deprecated use provisionedResourceCleanup


I guess we should deprecate the cleanup property instead 🤔

I've made cleanup deprecated.

badmintoncryer · 2025-01-07T13:13:32Z

@lpizzinidev Thank you for your review! I've addressed your comments.

lpizzinidev

Nice 👍

gracelu0 · 2025-01-17T02:38:16Z

...esting/framework-integ/test/aws-synthetics/test/integ.canary-provisioned-resource-cleanup.ts

+
+const app = new App();
+
+new IntegTest(app, 'SyntheticsCanaryProvisionedResourceCleanup', {


Could we add assertions to this test to GetCanary to get the Lambda layer, DeleteCanary and then verify the lambda layer does not exist?

@gracelu0 I've updated integ test!

aws-cdk-automation · 2025-01-18T00:11:44Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

…cleanup

aws-cdk-automation · 2025-01-19T00:13:22Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

gracelu0

Thank you for contributing!

mergify · 2025-01-23T01:18:00Z

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

mergify · 2025-01-23T01:23:22Z

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

badmintoncryer · 2025-01-23T09:22:23Z

@mergify update

mergify · 2025-01-23T09:22:57Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/README.md without workflows permission

badmintoncryer · 2025-01-23T22:49:43Z

@mergify update

mergify · 2025-01-23T22:49:53Z

update

✅ Branch has been successfully updated

gracelu0 · 2025-01-24T02:39:58Z

packages/aws-cdk-lib/aws-synthetics/lib/canary.ts

@@ -345,6 +354,10 @@ export class Canary extends cdk.Resource implements ec2.IConnectable {
      validateName(props.canaryName);
    }

+    if (props.cleanup === Cleanup.LAMBDA && props.provisionedResourceCleanup) {
+      throw new Error('Cannot specify `provisionedResourceCleanup` when `cleanup` is set to `Cleanup.LAMBDA`. Use only `provisionedResourceCleanup`.');


@badmintoncryer I think the build is failing due to this error: aws-cdk-lib: /codebuild/output/src1859401526/src/github.com/aws/aws-cdk/packages/aws-cdk-lib/aws-synthetics/lib/canary.ts aws-cdk-lib: 358:13 error Expected a non-default error object to be thrown @cdklabs/no-throw-default-error

I think the error needs to be updated from Error to ValidationError due to this PR where we are now enforcing specific error types: #33079

Pull request has been modified.

aws-cdk-automation

(This review is outdated)

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

mergify · 2025-01-24T18:56:49Z

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 · 2025-01-24T19:27:38Z

AWS CodeBuild CI Report

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

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

mergify · 2025-01-24T19:28:04Z

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-01-24T19:28:27Z

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.

badmintoncryer added 3 commits January 4, 2025 22:34

provisioned resource cleanup

6264434

add integ test

4d9e09a

add unit test

2120ab3

aws-cdk-automation requested a review from a team January 4, 2025 14:19

github-actions bot added p2 distinguished-contributor [Pilot] contributed 50+ PRs to the CDK labels Jan 4, 2025

aws-cdk-automation previously requested changes Jan 4, 2025

View reviewed changes

update validation

d3903ed

badmintoncryer marked this pull request as draft January 4, 2025 14:26

badmintoncryer marked this pull request as ready for review January 6, 2025 12:44

Merge branch 'main' into provisioned-resource-cleanup

0bbedca

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 6, 2025

lpizzinidev suggested changes Jan 6, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 6, 2025

badmintoncryer added 2 commits January 7, 2025 21:54

update

3631646

Merge branch 'main' into provisioned-resource-cleanup

5d96e01

lpizzinidev approved these changes Jan 7, 2025

View reviewed changes

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

badmintoncryer added 3 commits January 8, 2025 12:20

Merge branch 'main' into provisioned-resource-cleanup

91dd6b0

Merge branch 'main' into provisioned-resource-cleanup

beef9f6

Merge branch 'main' into provisioned-resource-cleanup

3577152

gracelu0 previously requested changes Jan 17, 2025

View reviewed changes

gracelu0 removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 17, 2025

Merge remote-tracking branch 'origin/main' into provisioned-resource-…

e217d30

…cleanup

update integ

a10d486

badmintoncryer added 5 commits January 19, 2025 13:59

refactor

2a62b6d

remove unnecessary line

4c87209

fix: remove unused import

eecbe30

add comments

71c68e7

Merge branch 'main' into provisioned-resource-cleanup

a8145ad

gracelu0 previously approved these changes Jan 23, 2025

View reviewed changes

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

Merge branch 'main' into provisioned-resource-cleanup

74306bd

gracelu0 reviewed Jan 24, 2025

View reviewed changes

github-actions bot and others added 2 commits January 24, 2025 21:59

use ValidationError

11291e7

Merge branch 'main' into provisioned-resource-cleanup

17933d1

aws-cdk-automation previously requested changes Jan 24, 2025

View reviewed changes

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

gracelu0 self-assigned this Jan 24, 2025

gracelu0 approved these changes Jan 24, 2025

View reviewed changes

Merge branch 'main' into provisioned-resource-cleanup

ae988df

mergify bot merged commit bdb4a59 into aws:main Jan 24, 2025
20 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(synthetics): cleanup provisioned lambda and layers for canary #32738

feat(synthetics): cleanup provisioned lambda and layers for canary #32738

badmintoncryer commented Jan 4, 2025

aws-cdk-automation left a comment

codecov bot commented Jan 4, 2025 •

edited

Loading

lpizzinidev left a comment

lpizzinidev Jan 6, 2025

badmintoncryer Jan 7, 2025 •

edited

Loading

lpizzinidev Jan 6, 2025

badmintoncryer Jan 7, 2025

badmintoncryer commented Jan 7, 2025

lpizzinidev left a comment

gracelu0 Jan 17, 2025

badmintoncryer Jan 21, 2025

aws-cdk-automation commented Jan 18, 2025

aws-cdk-automation commented Jan 19, 2025

gracelu0 left a comment

mergify bot commented Jan 23, 2025

mergify bot commented Jan 23, 2025

badmintoncryer commented Jan 23, 2025

mergify bot commented Jan 23, 2025

badmintoncryer commented Jan 23, 2025

mergify bot commented Jan 23, 2025

gracelu0 Jan 24, 2025

aws-cdk-automation left a comment •

edited

Loading

mergify bot commented Jan 24, 2025

aws-cdk-automation commented Jan 24, 2025

mergify bot commented Jan 24, 2025

github-actions bot commented Jan 24, 2025


		const app = new App();

		new IntegTest(app, 'SyntheticsCanaryProvisionedResourceCleanup', {

feat(synthetics): cleanup provisioned lambda and layers for canary #32738

feat(synthetics): cleanup provisioned lambda and layers for canary #32738

Conversation

badmintoncryer commented Jan 4, 2025

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

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 4, 2025 • edited Loading

Codecov Report

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Jan 6, 2025

Choose a reason for hiding this comment

badmintoncryer Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

lpizzinidev Jan 6, 2025

Choose a reason for hiding this comment

badmintoncryer Jan 7, 2025

Choose a reason for hiding this comment

badmintoncryer commented Jan 7, 2025

lpizzinidev left a comment

Choose a reason for hiding this comment

gracelu0 Jan 17, 2025

Choose a reason for hiding this comment

badmintoncryer Jan 21, 2025

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 18, 2025

aws-cdk-automation commented Jan 19, 2025

gracelu0 left a comment

Choose a reason for hiding this comment

mergify bot commented Jan 23, 2025

mergify bot commented Jan 23, 2025

badmintoncryer commented Jan 23, 2025

mergify bot commented Jan 23, 2025

❌ Mergify doesn't have permission to update

badmintoncryer commented Jan 23, 2025

mergify bot commented Jan 23, 2025

✅ Branch has been successfully updated

gracelu0 Jan 24, 2025

Choose a reason for hiding this comment

aws-cdk-automation left a comment • edited Loading

Choose a reason for hiding this comment

mergify bot commented Jan 24, 2025

aws-cdk-automation commented Jan 24, 2025

AWS CodeBuild CI Report

mergify bot commented Jan 24, 2025

github-actions bot commented Jan 24, 2025

codecov bot commented Jan 4, 2025 •

edited

Loading

badmintoncryer Jan 7, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading