Changed: adjust validation for scheduler expression #12220
Conversation
checkpoint4 dfadf
LocalStack Community integration with Pro 2 files 2 suites 21s ⏱️ Results for commit 3e1bbfb. |
| AT_REGEX = ( | ||
| r"^at[(](19|20)\d\d-(0[1-9]|1[012])-([012]\d|3[01])T([01]\d|2[0-3]):([0-5]\d):([0-5]\d)[)]$" | ||
| ) |
There was a problem hiding this comment.
question: how was the previous regex rule created, and how does the new one differ? How are we sure this is the right format, is there documentation online? Sorry, I don't know Scheduler so well and I wondered if there was any documentation or anything that helped you create this regex. Thanks! 🙏
There was a problem hiding this comment.
We didn't have any test for what a correct schedule creation should look like.
Before, anything was allowed in scheduler because moto wasn't validating it.
I don't properly remember where I copied it from, sorry (stackoverflow or else), but apparently I didn't test the condition for at, I just tested the invalid ones, but not a valid one, and that ended up blocking a user.
I should have added this test tests_create_schedule_with_valid_schedule_expression since that PR that introduced the validation for scheduler, but I struggled making a it work against aws.
This time I took the time to fix it, and turns out the issue I was getting was because the IAM role takes some time to propagate and because of that I was getting some permission issue for the scheduler. So we just have to wait some seconds for is_aws_cloud.
There was a problem hiding this comment.
This is the aws docs for this.
Motivation
Before, the eventbridge scheduler passed no validation at all since all of the logic was defaulting to moto's logic.
We added some validation few days ago, however, seems like the rule for
at()syntax for scheduled-expression was too limited, since the validation fails on localstack, but succeeds on aws.This caused a blocker for this user: #12191 (comment).
Changes
Adjusted validation rule for schedule_expression with
at()syntax.Added a test to check the syntax works.