Skip to content

Conversation

hornwind
Copy link

This change will allow to immediately discard invalid values when parsing a logicalBackupSchedule string.

Previously used regular expression ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ may pass invalid values to cron, like "00 03 * * 7"
Although "00 03 * * 7" may be valid on some systems, but this value will throw an error when applied to kubernetes.
https://en.wikipedia.org/wiki/Cron

FYI: kubernetes uses cron implementation from go module "github.com/robfig/cron/v3"
https://github.com/robfig/cron/blob/v3/spec.go
https://github.com/robfig/cron/blob/v3/parser.go

This change will allow to immediately discard invalid values when parsing a logicalBackupSchedule string.

Previously used regular expression '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$' may pass invalid values to cron, like "00 03 * * 7"
Although "00 03 * * 7" may be valid on some systems, but this value will throw an error when applied to kubernetes.
https://en.wikipedia.org/wiki/Cron

FYI: kubernetes uses cron implementation from go module "github.com/robfig/cron/v3"
https://github.com/robfig/cron/blob/v3/spec.go
https://github.com/robfig/cron/blob/v3/parser.go
@FxKu FxKu changed the title Fix postgresql CRD for more safely validation relax validation pattern for logicalBackupSchedule Dec 14, 2022
@FxKu FxKu added this to the 2.0 milestone Jan 30, 2023
Copy link

@notoriousR-O-B notoriousR-O-B left a comment

Choose a reason for hiding this comment

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

Could this be simplified by replacing (0?[0-9]|[1-5]?[0-9]) with ([0-5]?[0-9])?

@notoriousR-O-B
Copy link

In my view this validation should just be removed, as it will fail many valid schedule strings using intervals, named days, ranges, etc. and it would be nearly impossible to properly validate them. See #2323.

@FxKu FxKu modified the milestones: 1.11.0, 2.0 Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting for review

Development

Successfully merging this pull request may close these issues.

3 participants