Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

aws.events: Remove / change default CRON Schedule behavior #17881

Closed
3sztof opened this issue Dec 7, 2021 · 3 comments · Fixed by #19197
Closed

aws.events: Remove / change default CRON Schedule behavior #17881

3sztof opened this issue Dec 7, 2021 · 3 comments · Fixed by #19197
Labels
@aws-cdk/aws-events Related to CloudWatch Events breaking-change This issue requires a breaking change to remediate. effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p1

Comments

@3sztof
Copy link

3sztof commented Dec 7, 2021 •
edited
Loading

General Issue

Default parameter values of CRON Schedule can lead to event spamm

The Question

The default parameters for CRON Schedule configuration are "*" - run every minute / hour / day etc. - reference.

This can potentially lead to misconfigurations and, if not tested, to spamm of events that trigger further actions.

Here is an example:

schedule=aws_events.Schedule.cron(
    hour='8',
    week_day='monday')

In the example above, the user might assume that the event will run at 8:00 every Monday. The fact that the default value for minute leads to "*" in generated CRON, will cause unexpected spamm of events.

Honestly speaking, I don't see any good option for defaults, I believe the user should be forced to supply all parameter values explicitly to avoid misconfigurations (just like he would be required to think over the CRON expression syntax).

CDK CLI Version

1.133.0

OS

Windows 10

Language

Python

Language Version

Python 3.9.2
@3sztof 3sztof added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Dec 7, 2021
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Dec 7, 2021
@peterwoodworth peterwoodworth added feature-request A feature should be added or improved. feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. breaking-change This issue requires a breaking change to remediate. effort/small Small work item – less than a day of effort and removed guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Dec 9, 2021
@peterwoodworth
Copy link
Contributor

Thanks for the suggestion @3sztof

I believe this would be a breaking change for customers who are currently using this with the default settings - so I don't think this is something we'll be able to change

@rix0rrr do you have any comment on why the wildcard is used as default here?

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 8, 2022

@rix0rrr do you have any comment on why the wildcard is used as default here?

Honestly just slipped my mind 😅. It's a good callout. I'll see what I can do about it.

@rix0rrr rix0rrr removed their assignment Feb 9, 2022
TheRealAmazonKendra added a commit that referenced this issue Mar 1, 2022
@TheRealAmazonKendra
chore(aws-events): add warning when minute is not defined
a0c279b 
Per issue #17881, the default value for CRON schedule configuration parameters are  which runs every minute/hour/day.

This change adds a warning to be emitted upon synthesis and deployment, if minute is empty, that the CRON job will run every minute.

Closes #17881.
@TheRealAmazonKendra TheRealAmazonKendra mentioned this issue Mar 1, 2022
Merged
TheRealAmazonKendra added a commit that referenced this issue Mar 11, 2022
@TheRealAmazonKendra
chore(aws-events): add warning when minute is not defined
0f7cbcd 
Per issue #17881, the default value for CRON schedule configuration parameters are  which runs every minute/hour/day.

This change adds a warning to be emitted upon synthesis and deployment, if minute is empty, that the CRON job will run every minute.

Closes #17881.
@mergify mergify bot closed this as completed in #19197 Mar 14, 2022
mergify bot pushed a commit that referenced this issue Mar 14, 2022
@TheRealAmazonKendra
chore(aws-events): add warning when minute is not defined (#19197)
885d0c2 
Per issue #17881, the default value for CRON schedule configuration parameters are  which runs every minute/hour/day.

This change adds a warning to be emitted upon synthesis and deployment, if minute is empty, that the CRON job will run every minute.

Closes #17881.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
@aws-cdk/aws-events Related to CloudWatch Events breaking-change This issue requires a breaking change to remediate. effort/small Small work item – less than a day of effort feature/enhancement A new API to make things easier or more intuitive. A catch-all for general feature requests. feature-request A feature should be added or improved. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore(aws-events): add warning when minute is not defined aws/aws-cdk
3 participants
@rix0rrr @3sztof @peterwoodworth