fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #27310

go-to-k · 2023-09-27T07:21:33Z

This PR fixes the case that StateMachine generated for CompleteHandler in Provider cannot set logging.

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

… cannot set logging

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.

This reverts commit 5fddc2b.

…stom-resource-provider-statemachine

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.

kaizencc

Thanks for the lengthy review @lpizzinidev and for waiting patiently @go-to-k. Sorry for the delay. I just have two documentation comments and then I think we're good to go!

kaizencc · 2023-12-19T19:47:05Z

packages/aws-cdk-lib/custom-resources/lib/provider-framework/waiter-state-machine.ts

-        DefinitionString: definition,
-        RoleArn: role.roleArn,
-      },
+    const resource = new CfnStateMachine(this, 'Resource', {


We previously used CfnResource here because we did not want to depend on aws-stepfunctions module (back in CDK v1 where individual modules were released. Now that they are both part of aws-cdk-lib, this change is ok. But there are comments on line 80 that need to be updated to reflect this. I would document that we previously used CfnResource to avoid depending on aws-stepfunctions, but now it is okay.

kaizencc · 2023-12-19T19:48:47Z

packages/aws-cdk-lib/custom-resources/lib/provider-framework/waiter-state-machine.ts

+  ): CfnStateMachine.LoggingConfigurationProperty | undefined {
+    if (disableLogging) return undefined;
+
+    // https://docs.aws.amazon.com/step-functions/latest/dg/cw-logs.html#cloudwatch-iam-policy


Thanks for this link. Let's also copy in the important piece of information from this link:

// You need to specify `*` in the Resource field because CloudWatch API actions, such as CreateLogDelivery and DescribeLogGroups, don't support Resource types defined by Amazon CloudWatch Logs.

Pull request has been modified.

go-to-k · 2023-12-20T06:11:26Z

@kaizencc

Thanks for your review! I fixed.

kaizencc

Thanks @go-to-k!

mergify · 2023-12-20T17:06:30Z

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 · 2023-12-20T17:10:39Z

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 · 2023-12-20T18:16:59Z

AWS CodeBuild CI Report

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

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

mergify · 2023-12-20T18:17:23Z

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 CompleteHandler (aws#27310) This PR fixes the case that `StateMachine` generated for `CompleteHandler` in `Provider` cannot set logging. Closes aws#27283. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Reverts #27310 which allows setting up LogGroups for the state machine configured by the waiter custom-resource. Refs: bfeef00 This reverts commit bfeef00.

Reverts #27310 which allows setting up LogGroups for the state machine configured by the waiter custom-resource. Refs: bfeef00 This reverts commit bfeef00. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

… in CompleteHandler (#28706) This PR fixes the case that `StateMachine` generated for `CompleteHandler` in `Provider` cannot set logging. The default log group name of the state machine automatically created by the `Provider` with `isCompleteHandler` should start with `/aws/vendedlogs/states`. Otherwise, each time the state machine is created, the log resource policy size increases. https://docs.aws.amazon.com/step-functions/latest/dg/bp-cwl.html Closes #27283 Fixes #28577 #28744 Related PR #27310(reverted by #28699), #28587 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

fix(custom-resources): StateMachine for isCompleteHandler in provider…

c7d5edf

… cannot set logging

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p2 labels Sep 27, 2023

aws-cdk-automation requested a review from a team September 27, 2023 07:21

github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Sep 27, 2023

go-to-k changed the title ~~fix(custom-resources): StateMachine in Provider cannot set logging~~ fix(custom-resources): StateMachine for isCompleteHandler in Provider cannot set logging Sep 27, 2023

go-to-k changed the title ~~fix(custom-resources): StateMachine for isCompleteHandler in Provider cannot set logging~~ fix(custom-resources): StateMachine for CompleteHandler in Provider cannot set logging Sep 27, 2023

aws-cdk-automation previously requested changes Sep 27, 2023

View reviewed changes

change codes and integ

5fddc2b

go-to-k added 3 commits September 27, 2023 19:22

Revert "change codes and integ"

f20e343

This reverts commit 5fddc2b.

Merge branch 'main' of https://github.com/go-to-k/aws-cdk into fix/cu…

d798bf7

…stom-resource-provider-statemachine

fix: waiter-state-machine

256b91b

aws-cdk-automation previously requested changes Sep 27, 2023

View reviewed changes

change integ tests

1fe40cc

go-to-k added 13 commits September 27, 2023 21:26

change property name and fix unit tests

ce58bdc

change name to disableWaiterStateMachineLogging

1b4ec5f

change index for export

7da8364

change import order

6c941e9

add docstring

a14775f

change props for logs and integ tests

9f7c0f8

add default for destination

e8dcfd7

change doc

1755f07

change other doc

c60d5f4

change aws-eks/test/integ.alb-controller

3b6da20

aws-dynamodb/test/integ.global.js

bbe0fe4

integ for aws-dynamodb/test/integ.global-replicas-provisioned.js

b9c5fda

integ.eks-cluster-private-endpoint.js.snapshot

bd9f44b

go-to-k and others added 2 commits November 30, 2023 19:37

update snapshot for integ.provider-with-waiter-state-machine

cf76b87

Merge branch 'main' into fix/custom-resource-provider-statemachine

dfa7771

kaizencc changed the title ~~fix(custom-resources): StateMachine for CompleteHandler in Provider cannot set logging~~ fix(custom-resources): cannot set logging for state machine generated in CompleteHandler Dec 19, 2023

kaizencc previously requested changes Dec 19, 2023

View reviewed changes

modify docs by review

7163c86

change snapshots

ca92b3c

go-to-k requested a review from kaizencc December 20, 2023 06:11

kaizencc approved these changes Dec 20, 2023

View reviewed changes

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

Merge branch 'main' into fix/custom-resource-provider-statemachine

0e7aeb9

mergify bot merged commit bfeef00 into aws:main Dec 20, 2023
9 checks passed

pahud mentioned this pull request Jan 4, 2024

(custom-resources): Default logging configuration hits Cloudwatch Logs Resource Policy size limit #28577

Closed

MrArnoldPalmer added a commit that referenced this pull request Jan 12, 2024

revert: allowing log group config for SFN CR

699f2fb

Reverts #27310 which allows setting up LogGroups for the state machine configured by the waiter custom-resource. Refs: bfeef00 This reverts commit bfeef00.

MrArnoldPalmer mentioned this pull request Jan 12, 2024

revert: allowing log group config for SFN CR #28699

Merged

MrArnoldPalmer added a commit that referenced this pull request Jan 12, 2024

revert: allowing log group config for SFN CR

455f6cc

Reverts #27310 which allows setting up LogGroups for the state machine configured by the waiter custom-resource. Refs: bfeef00 This reverts commit bfeef00.

This was referenced Jan 14, 2024

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #28706

Merged

aws-eks: CloudWatch Logs Resource Policy Size Exceeded for Cluster Resource Provider #28744

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #27310

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #27310

go-to-k commented Sep 27, 2023 •

edited

aws-cdk-automation left a comment

aws-cdk-automation left a comment

kaizencc left a comment

kaizencc Dec 19, 2023

kaizencc Dec 19, 2023

go-to-k commented Dec 20, 2023

kaizencc left a comment

mergify bot commented Dec 20, 2023

mergify bot commented Dec 20, 2023

aws-cdk-automation commented Dec 20, 2023

mergify bot commented Dec 20, 2023

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #27310

fix(custom-resources): cannot set logging for state machine generated in CompleteHandler #27310

Conversation

go-to-k commented Sep 27, 2023 • edited

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

kaizencc left a comment

Choose a reason for hiding this comment

kaizencc Dec 19, 2023

Choose a reason for hiding this comment

kaizencc Dec 19, 2023

Choose a reason for hiding this comment

go-to-k commented Dec 20, 2023

kaizencc left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 20, 2023

mergify bot commented Dec 20, 2023

aws-cdk-automation commented Dec 20, 2023

AWS CodeBuild CI Report

mergify bot commented Dec 20, 2023

go-to-k commented Sep 27, 2023 •

edited