From dcc6057e8d36f82e94175f4d28fe7b1425415cbd Mon Sep 17 00:00:00 2001 From: John Strunk Date: Thu, 31 Aug 2023 11:58:01 -0400 Subject: [PATCH 1/2] Fix shellcheck errors Signed-off-by: John Strunk --- mover-restic/entry.sh | 4 ++-- mover-rsync-tls/client.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mover-restic/entry.sh b/mover-restic/entry.sh index 663832495..4fb2e7827 100755 --- a/mover-restic/entry.sh +++ b/mover-restic/entry.sh @@ -152,8 +152,8 @@ function reverse_array() { while ((left < right)); do # triangle swap local -i temp="${_arr[$left]}" - _arr[$left]="${_arr[$right]}" - _arr[$right]="$temp" + _arr[left]="${_arr[$right]}" + _arr[right]="$temp" # increment indices ((left++)) diff --git a/mover-rsync-tls/client.sh b/mover-rsync-tls/client.sh index 5fe9acbaf..0a0648edb 100644 --- a/mover-rsync-tls/client.sh +++ b/mover-rsync-tls/client.sh @@ -18,6 +18,7 @@ SCRIPT="$(realpath "$0")" SCRIPT_DIR="$(dirname "$SCRIPT")" cd "$SCRIPT_DIR" +# shellcheck disable=SC2317 # It's reachable due to the TRAP function stop_stunnel() { ## Terminate stunnel kill -TERM "$(<"$STUNNEL_PID_FILE")" From b922dd5c89613b4381e228a1eabcc0ba214469ed Mon Sep 17 00:00:00 2001 From: John Strunk Date: Thu, 31 Aug 2023 11:58:30 -0400 Subject: [PATCH 2/2] Fix schedule validation Signed-off-by: John Strunk --- api/v1alpha1/replicationdestination_types.go | 3 +- api/v1alpha1/replicationsource_types.go | 3 +- ...lsync.backube_replicationdestinations.yaml | 4 +-- .../volsync.backube_replicationsources.yaml | 4 +-- .../volsync.clusterserviceversion.yaml | 2 +- ...lsync.backube_replicationdestinations.yaml | 4 +-- .../volsync.backube_replicationsources.yaml | 4 +-- controllers/statemachine/machine_test.go | 33 +++++++++++++++++++ ...lsync.backube_replicationdestinations.yaml | 4 +-- .../volsync.backube_replicationsources.yaml | 4 +-- 10 files changed, 50 insertions(+), 15 deletions(-) diff --git a/api/v1alpha1/replicationdestination_types.go b/api/v1alpha1/replicationdestination_types.go index a8778aabe..bd5fdb1c2 100644 --- a/api/v1alpha1/replicationdestination_types.go +++ b/api/v1alpha1/replicationdestination_types.go @@ -46,7 +46,8 @@ type ReplicationDestinationTriggerSpec struct { // schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that // can be used to schedule replication to occur at regular, time-based // intervals. - //+kubebuilder:validation:Pattern=`^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$` + // nolint:lll + //+kubebuilder:validation:Pattern=`^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$` //+optional Schedule *string `json:"schedule,omitempty"` // manual is a string value that schedules a manual trigger. diff --git a/api/v1alpha1/replicationsource_types.go b/api/v1alpha1/replicationsource_types.go index 8112e0580..a0292fb06 100644 --- a/api/v1alpha1/replicationsource_types.go +++ b/api/v1alpha1/replicationsource_types.go @@ -46,7 +46,8 @@ type ReplicationSourceTriggerSpec struct { // schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that // can be used to schedule replication to occur at regular, time-based // intervals. - //+kubebuilder:validation:Pattern=`^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$` + // nolint:lll + //+kubebuilder:validation:Pattern=`^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$` //+optional Schedule *string `json:"schedule,omitempty"` // manual is a string value that schedules a manual trigger. diff --git a/bundle/manifests/volsync.backube_replicationdestinations.yaml b/bundle/manifests/volsync.backube_replicationdestinations.yaml index 10417df32..2a1a8b01d 100644 --- a/bundle/manifests/volsync.backube_replicationdestinations.yaml +++ b/bundle/manifests/volsync.backube_replicationdestinations.yaml @@ -939,8 +939,8 @@ spec: schedule: description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, - time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object diff --git a/bundle/manifests/volsync.backube_replicationsources.yaml b/bundle/manifests/volsync.backube_replicationsources.yaml index 498bb08e8..375913e76 100644 --- a/bundle/manifests/volsync.backube_replicationsources.yaml +++ b/bundle/manifests/volsync.backube_replicationsources.yaml @@ -1184,8 +1184,8 @@ spec: schedule: description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, - time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object diff --git a/bundle/manifests/volsync.clusterserviceversion.yaml b/bundle/manifests/volsync.clusterserviceversion.yaml index 9362ddfd8..1727171d4 100644 --- a/bundle/manifests/volsync.clusterserviceversion.yaml +++ b/bundle/manifests/volsync.clusterserviceversion.yaml @@ -55,7 +55,7 @@ metadata: } ] capabilities: Basic Install - createdAt: "2023-07-13T13:28:33Z" + createdAt: "2023-08-31T16:03:08Z" olm.skipRange: '>=0.4.0 <0.8.0' operators.operatorframework.io/builder: operator-sdk-v1.26.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/config/crd/bases/volsync.backube_replicationdestinations.yaml b/config/crd/bases/volsync.backube_replicationdestinations.yaml index d91ad1a70..110c3c192 100644 --- a/config/crd/bases/volsync.backube_replicationdestinations.yaml +++ b/config/crd/bases/volsync.backube_replicationdestinations.yaml @@ -940,8 +940,8 @@ spec: schedule: description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, - time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object diff --git a/config/crd/bases/volsync.backube_replicationsources.yaml b/config/crd/bases/volsync.backube_replicationsources.yaml index a730a4bbe..e51715794 100644 --- a/config/crd/bases/volsync.backube_replicationsources.yaml +++ b/config/crd/bases/volsync.backube_replicationsources.yaml @@ -1185,8 +1185,8 @@ spec: schedule: description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, - time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object diff --git a/controllers/statemachine/machine_test.go b/controllers/statemachine/machine_test.go index 3fa2e3a26..a2262b8a6 100644 --- a/controllers/statemachine/machine_test.go +++ b/controllers/statemachine/machine_test.go @@ -21,6 +21,7 @@ import ( "context" "errors" "fmt" + "regexp" "time" . "github.com/onsi/ginkgo/v2" @@ -314,3 +315,35 @@ var _ = Context("Issue 290: Synchronizing condition error doesn't clear", func() Expect(c.Reason).To(Equal(volsyncv1alpha1.SynchronizingReasonSync)) }) }) + +var _ = DescribeTable("Crontab parsing and validation", + func(cronspec string, isValid bool) { + // cronspecValidation is the regex used to validate crontab entries it + // needs to be kept in sync with the regex in + // api/v1alpha1/replicationdestination_types.go and + // api/v1alpha1/replicationsource_types.go + // + // For interactive testing of cronspecs, see: + // https://regex101.com/r/AXEJLy/2 + // nolint:lll + var cronspecValidation = regexp.MustCompile(`^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$`) + _, err := getSchedule(cronspec) + if isValid { // needs to pass regex validation and be parsable by cron library + Expect(cronspecValidation.MatchString(cronspec)).To(BeTrue()) + Expect(err).NotTo(HaveOccurred()) + } else { // should regex validation and return an error from cron library + Expect(cronspecValidation.MatchString(cronspec)).To(BeFalse()) + Expect(err).To(HaveOccurred()) + } + }, + Entry("Midnight Jan 1", "0 0 1 1 *", true), + Entry("Every 5 minutes (slash notation)", "*/5 * * * *", true), + Entry("Hourly (@ notation)", "@hourly", true), + Entry("1st of month Mar-May (range notation)", "0 0 1 3-5 *", true), + Entry("9am, 5pm (comma notation)", "0 9,17 * * *", true), + Entry("Junk string", "something", false), + Entry("Empty string", "", false), + Entry("Every 3 hours (slash notation)", "19 */3 * * * ", true), + Entry("All numbers", "6 5 4 3 2", true), + Entry("Hour range (9am - 5pm)", "0 9-17 * * *", true), +) diff --git a/helm/volsync/templates/volsync.backube_replicationdestinations.yaml b/helm/volsync/templates/volsync.backube_replicationdestinations.yaml index 98e612a83..c6540c794 100644 --- a/helm/volsync/templates/volsync.backube_replicationdestinations.yaml +++ b/helm/volsync/templates/volsync.backube_replicationdestinations.yaml @@ -577,8 +577,8 @@ spec: description: manual is a string value that schedules a manual trigger. Once a sync completes then status.lastManualSync is set to the same string value. A consumer of a manual trigger should set spec.trigger.manual to a known value and then wait for lastManualSync to be updated by the operator to the same value, which means that the manual trigger will then pause and wait for further updates to the trigger. type: string schedule: - description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object diff --git a/helm/volsync/templates/volsync.backube_replicationsources.yaml b/helm/volsync/templates/volsync.backube_replicationsources.yaml index e2f035457..7b6719b14 100644 --- a/helm/volsync/templates/volsync.backube_replicationsources.yaml +++ b/helm/volsync/templates/volsync.backube_replicationsources.yaml @@ -729,8 +729,8 @@ spec: description: manual is a string value that schedules a manual trigger. Once a sync completes then status.lastManualSync is set to the same string value. A consumer of a manual trigger should set spec.trigger.manual to a known value and then wait for lastManualSync to be updated by the operator to the same value, which means that the manual trigger will then pause and wait for further updates to the trigger. type: string schedule: - description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, time-based intervals. - pattern: ^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$ + description: schedule is a cronspec (https://en.wikipedia.org/wiki/Cron#Overview) that can be used to schedule replication to occur at regular, time-based intervals. nolint:lll + pattern: ^(@(annually|yearly|monthly|weekly|daily|hourly))|((((\d+,)*\d+|(\d+(\/|-)\d+)|\*(\/\d+)?)\s?){5})$ type: string type: object type: object