fix: refactored ValidateCronString (update function signature; valida…

…te the 1st to be 0 when there are 6 fields)

Signed-off-by: Shengwen Yu <yshengwen@vmware.com>

src/common/utils/utils.go

@@ -302,22 +302,19 @@ func CronParser() cronlib.Parser {
 }
 
 // ValidateCronString check whether it is a valid cron string and whether the 1st field (indicating Seconds of time) of the cron string is a fixed value of 0 or not
-func ValidateCronString(cron string) (bool, error) {
+func ValidateCronString(cron string) error {
 	if len(cron) == 0 {
-		return false, fmt.Errorf("empty cron string is invalid")
+		return fmt.Errorf("empty cron string is invalid")
 	}
 	_, err := CronParser().Parse(cron)
 	if err != nil {
-		return false, err
+		return err
 	}
 	cronParts := strings.Split(cron, " ")
-	if len(cronParts) == 0 {
-		return false, fmt.Errorf("invalid cron string")
-	}
-	if cronParts[0] != "0" {
-		return false, fmt.Errorf("the 1st field (indicating Seconds of time) of the cron setting must be 0")
+	if len(cronParts) == 6 && cronParts[0] != "0" {
+		return fmt.Errorf("the 1st field (indicating Seconds of time) of the cron setting must be 0")
 	}
-	return true, nil
+	return nil
 }
 
 // MostMatchSorter is a sorter for the most match, usually invoked in sort Less function

src/common/utils/utils_test.go

@@ -441,47 +441,39 @@ func TestValidateCronString(t *testing.T) {
 	testCases := []struct {
 		description string
 		input       string
-		isValid     bool
 		hasErr      bool
 	}{
 		// empty cron string
 		{
 			description: "test case 1",
 			input:       "",
-			isValid:     false,
 			hasErr:      true,
 		},
 
 		// invalid cron format
 		{
 			description: "test case 2",
-			input:       "1 2 3",
-			isValid:     false,
+			input:       "0 2 3",
 			hasErr:      true,
 		},
 
 		// the 1st field (indicating Seconds of time) of the cron setting must be 0
 		{
 			description: "test case 3",
 			input:       "1 0 0 1 1 0",
-			isValid:     false,
 			hasErr:      true,
 		},
 
 		// valid cron string
 		{
-			description: "test case 2",
-			input:       "0 0 0 1 1 0",
-			isValid:     true,
+			description: "test case 4",
+			input:       "0 1 2 1 1 *",
 			hasErr:      false,
 		},
 	}
 
 	for _, tc := range testCases {
-		gotIsValid, err := ValidateCronString(tc.input)
-		if tc.isValid != gotIsValid {
-			t.Errorf("%s, expected isValid to be %v, but the actual isValid is %v", tc.description, tc.isValid, gotIsValid)
-		}
+		err := ValidateCronString(tc.input)
 		if tc.hasErr {
 			if err == nil {
 				t.Errorf("%s, expect having error, while actual error returned is nil", tc.description)

src/pkg/p2p/preheat/models/policy/policy.go

@@ -122,8 +122,7 @@ type Trigger struct {
 func (s *Schema) ValidatePreheatPolicy() error {
 	// currently only validate cron string of preheat policy
 	if s.Trigger != nil && s.Trigger.Type == TriggerTypeScheduled && len(s.Trigger.Settings.Cron) > 0 {
-		isValid, err := utils.ValidateCronString(s.Trigger.Settings.Cron)
-		if !isValid || err != nil {
+		if err := utils.ValidateCronString(s.Trigger.Settings.Cron); err != nil {
 			return errors.New(nil).WithCode(errors.BadRequestCode).
 				WithMessage("invalid cron string for scheduled preheat: %s, error: %v", s.Trigger.Settings.Cron, err)
 		}

src/pkg/retention/policy/models.go

@@ -67,8 +67,7 @@ func (m *Metadata) ValidateRetentionPolicy() error {
 		if m.Trigger.Kind == TriggerKindSchedule && m.Trigger.Settings != nil {
 			cronItem, ok := m.Trigger.Settings[TriggerSettingsCron]
 			if ok && len(cronItem.(string)) > 0 {
-				isValid, err := utils.ValidateCronString(cronItem.(string))
-				if !isValid || err != nil {
+				if err := utils.ValidateCronString(cronItem.(string)); err != nil {
 					return errors.New(nil).WithCode(errors.BadRequestCode).
 						WithMessage("invalid cron string for scheduled tag retention: %s, error: %v", cronItem.(string), err)
 				}

src/server/v2.0/handler/gc.go

@@ -148,8 +148,7 @@ func (g *gcAPI) createSchedule(ctx context.Context, cronType, cron string, polic
 }
 
 func (g *gcAPI) updateSchedule(ctx context.Context, cronType, cron string, policy gc.Policy) error {
-	isValid, err := utils.ValidateCronString(cron)
-	if !isValid || err != nil {
+	if err := utils.ValidateCronString(cron); err != nil {
 		return errors.New(nil).WithCode(errors.BadRequestCode).
 			WithMessage("invalid cron string for scheduled gc: %s, error: %v", cron, err)
 	}

src/server/v2.0/handler/purge.go

@@ -141,8 +141,7 @@ func (p *purgeAPI) kick(ctx context.Context, vendorType string, scheType string,
 }
 
 func (p *purgeAPI) updateSchedule(ctx context.Context, vendorType, cronType, cron string, policy pg.JobPolicy, extraParams map[string]interface{}) error {
-	isValid, err := utils.ValidateCronString(cron)
-	if !isValid || err != nil {
+	if err := utils.ValidateCronString(cron); err != nil {
 		return errors.New(nil).WithCode(errors.BadRequestCode).
 			WithMessage("invalid cron string for scheduled log rotation purge: %s, error: %v", cron, err)
 	}

src/server/v2.0/handler/scan_all.go

@@ -195,8 +195,7 @@ func (s *scanAllAPI) GetLatestScheduledScanAllMetrics(ctx context.Context, param
 }
 
 func (s *scanAllAPI) createOrUpdateScanAllSchedule(ctx context.Context, cronType, cron string, previous *scheduler.Schedule) (int64, error) {
-	isValid, err := utils.ValidateCronString(cron)
-	if !isValid || err != nil {
+	if err := utils.ValidateCronString(cron); err != nil {
 		return 0, errors.New(nil).WithCode(errors.BadRequestCode).
 			WithMessage("invalid cron string for scheduled scan all: %s, error: %v", cron, err)
 	}