Skip to content

fix: allow cron aliases in schedule validation#16100

Open
PradeepSD476 wants to merge 1 commit into
argoproj:mainfrom
PradeepSD476:fix-16052-cron-alias-validation
Open

fix: allow cron aliases in schedule validation#16100
PradeepSD476 wants to merge 1 commit into
argoproj:mainfrom
PradeepSD476:fix-16052-cron-alias-validation

Conversation

@PradeepSD476
Copy link
Copy Markdown

Fixes #16052

Motivation

CronWorkflow validation rejected valid cron aliases such as Jan and Mon-Fri even though they are supported by the runtime cron parser.

Modifications

Updated the validation regex to allow alphabetic cron aliases in CronWorkflow schedules.

This keeps semantic validation delegated to the runtime cron parser while preventing valid aliases from being rejected during CRD validation.

Verification

Verified the following schedules successfully:

*/1 * * May Fri-Sat
0 0 * * MON-FRI
0 8 3 JAN *

Also verified invalid aliases such as May,Hello are rejected by the runtime cron parser with a message: cron schedule */1 * * May,Hello Fri,Sat is malformed: failed to parse int from Hello: strconv.Atoi: parsing "Hello": invalid syntax.

Documentation

No documentation changes were required.

AI

Used ChatGPT for debugging discussion and PR preparation.

CronWorkflow validation rejected valid cron aliases
such as JAN and MON-FRI even though they are supported
by the runtime cron parser.

Updated the schedule validation regex to allow
alphabetic cron aliases while semantic validation
continues to be handled by the cron parser.

Fixes argoproj#16052

Signed-off-by: Pradeep Sagitra <sagitrapradeep2006@gmail.com>
Copy link
Copy Markdown
Member

@MasonM MasonM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks right to me. Here's the list of the aliases the parser allows: https://github.com/robfig/cron/blob/bc59245fe10efaed9d51b56900192527ed733435/spec.go#L26-L48

This could be made stricter, but that doesn't seem necessary to me.

@Joibel This is related to #15028

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Months in cron expression are rejected by validation

2 participants