Merge pull request #848 from JohnStrunk/cronspec

Fix cronspec validation

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.

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.

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

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

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

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

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

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),
+)

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

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

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++))

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")"