feat(scheduler): validate schedule name length #31200
Conversation
github-actions
bot
added
the
beginning-contributor
There was a problem hiding this comment.
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.
|
Exemption Request: not really sure how to cover this with an integration test - I thought a unit test might suffice for something this small? |
aws-cdk-automation
added
pr-linter/exemption-requested
| @@ -324,6 +324,9 @@ export class Schedule extends Resource implements ISchedule { | |||
| const flexibleTimeWindow = props.timeWindow ?? TimeWindow.off(); | |||
|
|
|||
| this.validateTimeFrame(props.start, props.end); | |||
| if (props.scheduleName && !Token.isUnresolved(props.scheduleName) && props.scheduleName.length > 64) { | |||
| throw new Error(`schedule name '${props.scheduleName}' exceeds 64 characters in length`); | |||
There was a problem hiding this comment.
It would be user-friendly and good to display the length.
| throw new Error(`schedule name '${props.scheduleName}' exceeds 64 characters in length`); | |
| throw new Error(`scheduleName cannot be longer than 64 characters, got: ${props.scheduleName.length}`); |
| @@ -214,5 +214,16 @@ describe('Schedule', () => { | |||
| }); | |||
| }).toThrow('The provided duration must be between 1 minute and 1440 minutes, got 0'); | |||
| }); | |||
|
|
|||
| test('throw error when schedule name exceeds 64', () => { | |||
There was a problem hiding this comment.
| test('throw error when schedule name exceeds 64', () => { | |
| test('throw error when scheduleName exceeds 64 characters', () => { |
aws-cdk-automation
removed
the
pr/needs-community-review
|
As this issue is not about a bug, the PR title may be good for |
LaurenceWarne
changed the title
|
Thanks for the review, I've updated the title and made the requested changes. |
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-maintainer-review
kaizencc
changed the title
kaizencc
added
pr-linter/exempt-readme
aws-cdk-automation
removed
the
pr/needs-maintainer-review
|
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 CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
N/A
Reason for this change
To catch name too long errors before hitting Cloudformation.
Description of changes
I've added a length assertion in the
Scheduleconstructor against the schedule name length, the documentation on schedule name already makes it clear that there is a limit though AFAICS this isn't validated.Also see the Cloudformation doc here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-scheduler-schedule.html#cfn-scheduler-schedule-name.
Description of how you validated changes
I've added a unit test.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license