From a86334f465efa382bce685c9cb72673741a0676d Mon Sep 17 00:00:00 2001 From: Naman Date: Tue, 12 May 2026 00:34:43 +0530 Subject: [PATCH] fix(utils): centralize cron expression validation (closes #863) Signed-off-by: Naman --- cmd/harbor/root/scan_all/update_schedule.go | 21 +--- pkg/utils/cron.go | 78 ++++++++++++ pkg/utils/cron_test.go | 128 ++++++++++++++++++++ pkg/views/scan-all/update/view.go | 31 +---- 4 files changed, 210 insertions(+), 48 deletions(-) create mode 100644 pkg/utils/cron.go create mode 100644 pkg/utils/cron_test.go diff --git a/cmd/harbor/root/scan_all/update_schedule.go b/cmd/harbor/root/scan_all/update_schedule.go index f2e36f47c..20aa151dc 100644 --- a/cmd/harbor/root/scan_all/update_schedule.go +++ b/cmd/harbor/root/scan_all/update_schedule.go @@ -14,7 +14,6 @@ package scan_all import ( - "errors" "fmt" "strings" @@ -144,7 +143,7 @@ func updateCustomSchedule(cron string) error { update.UpdateSchedule(&cron) } - if err := validateCron(cron); err != nil { + if err := utils.ValidateCronExpression(cron); err != nil { return err } @@ -170,20 +169,4 @@ func updateCustomSchedule(cron string) error { return nil } -func validateCron(cron string) error { - if cron == "" { - return errors.New("cron expression cannot be empty") - } - fields := strings.Fields(cron) - if len(fields) < 6 { - if len(fields) == 5 { - logrus.Debugf("Converting 5-field cron to 6-field by adding '0' for seconds") - return fmt.Errorf("harbor requires 6-field cron format (including seconds). Try: '0 %s'", cron) - } - return fmt.Errorf("harbor requires 6-field cron format (seconds minute hour day month weekday)") - } - if len(fields) > 6 { - return fmt.Errorf("too many fields in cron expression, expected 6 but got %d", len(fields)) - } - return nil -} + diff --git a/pkg/utils/cron.go b/pkg/utils/cron.go new file mode 100644 index 000000000..9ce1d1a18 --- /dev/null +++ b/pkg/utils/cron.go @@ -0,0 +1,78 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package utils + +import ( + "errors" + "fmt" + "regexp" + "strings" +) + +// cronRegex validates a 6-field Harbor cron expression: +// +// second minute hour day-of-month month day-of-week +// +// Each field accepts * (any), a numeric value in the allowed range, or */n +// (step notation). This mirrors the regex previously inlined in +// pkg/views/scan-all/update/view.go. +var cronRegex = regexp.MustCompile( + `^(\*|[0-9]|[1-5][0-9]|\*/[0-9]+) ` + + `(\*|[0-9]|[1-5][0-9]|\*/[0-9]+) ` + + `(\*|[0-9]|1[0-9]|2[0-3]|\*/[0-9]+) ` + + `(\*|[1-9]|[12][0-9]|3[01]|\*/[0-9]+) ` + + `(\*|[1-9]|1[0-2]|\*/[0-9]+) ` + + `(\*|[0-6]|\*/[0-9]+)$`, +) + +// ValidateCronExpression checks that expr is a valid 6-field cron string +// as accepted by Harbor (seconds, minutes, hours, day-of-month, month, day-of-week). +// Returns nil on success, a descriptive error on failure. +// +// This function can be used directly as a huh form Validate callback because +// its signature matches func(string) error. +func ValidateCronExpression(expr string) error { + if expr == "" { + return errors.New("cron expression cannot be empty") + } + + fields := strings.Fields(expr) + switch { + case len(fields) == 5: + return fmt.Errorf( + "you entered a 5-field cron expression, but Harbor requires 6 fields (with seconds)\n"+ + "Please add a seconds field at the beginning. For example: '0 %s'", expr, + ) + case len(fields) < 6: + return fmt.Errorf( + "harbor requires exactly 6 fields in cron expressions (seconds minute hour day month weekday), got %d", + len(fields), + ) + case len(fields) > 6: + return fmt.Errorf( + "too many fields in cron expression, expected 6 but got %d", + len(fields), + ) + } + + if !cronRegex.MatchString(expr) { + return errors.New("invalid cron expression format\n" + + "Examples:\n" + + " 0 0 0 * * * - Daily at midnight\n" + + " 0 0 */6 * * * - Every 6 hours\n" + + " 0 0 0 * * 0 - Weekly on Sunday at midnight") + } + + return nil +} diff --git a/pkg/utils/cron_test.go b/pkg/utils/cron_test.go new file mode 100644 index 000000000..7445e0f88 --- /dev/null +++ b/pkg/utils/cron_test.go @@ -0,0 +1,128 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package utils_test + +import ( + "testing" + + "github.com/goharbor/harbor-cli/pkg/utils" + "github.com/stretchr/testify/assert" +) + +func TestValidateCronExpression(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + // ---------- valid expressions ---------- + { + name: "daily at midnight", + input: "0 0 0 * * *", + wantErr: false, + }, + { + name: "every 6 hours", + input: "0 0 */6 * * *", + wantErr: false, + }, + { + name: "weekly on Sunday at midnight", + input: "0 0 0 * * 0", + wantErr: false, + }, + { + name: "all wildcards", + input: "* * * * * *", + wantErr: false, + }, + { + name: "specific time with step second", + input: "*/30 0 12 * * *", + wantErr: false, + }, + { + name: "first day of month at 3am", + input: "0 0 3 1 * *", + wantErr: false, + }, + { + name: "max values", + input: "59 59 23 31 12 6", + wantErr: false, + }, + + // ---------- empty / blank ---------- + { + name: "empty string", + input: "", + wantErr: true, + }, + + // ---------- wrong field count ---------- + { + name: "5-field classic cron (no seconds)", + input: "0 0 * * *", + wantErr: true, + }, + { + name: "4 fields", + input: "0 0 * *", + wantErr: true, + }, + { + name: "7 fields (too many)", + input: "0 0 0 * * * extra", + wantErr: true, + }, + + // ---------- bad field values ---------- + { + name: "hour out of range (25)", + input: "0 0 25 * * *", + wantErr: true, + }, + { + name: "month out of range (13)", + input: "0 0 0 * 13 *", + wantErr: true, + }, + { + name: "day-of-week out of range (7)", + input: "0 0 0 * * 7", + wantErr: true, + }, + { + name: "invalid characters", + input: "abc def ghi * * *", + wantErr: true, + }, + { + name: "single field only", + input: "*", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := utils.ValidateCronExpression(tc.input) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/pkg/views/scan-all/update/view.go b/pkg/views/scan-all/update/view.go index d2a451b42..f4676e8bd 100644 --- a/pkg/views/scan-all/update/view.go +++ b/pkg/views/scan-all/update/view.go @@ -14,12 +14,8 @@ package update import ( - "errors" - "fmt" - "regexp" - "strings" - "github.com/charmbracelet/huh" + "github.com/goharbor/harbor-cli/pkg/utils" log "github.com/sirupsen/logrus" ) @@ -32,7 +28,7 @@ func UpdateSchedule(cron *string) { Description("Standard 6-field cron format: second minute hour day-of-month month day-of-week"). Placeholder("0 0 0 * * *"). // Daily at midnight with seconds Value(cron). - Validate(validateCronExpression), + Validate(utils.ValidateCronExpression), ), ).WithTheme(theme).Run() @@ -40,26 +36,3 @@ func UpdateSchedule(cron *string) { log.Fatal(err) } } - -func validateCronExpression(cron string) error { - if cron == "" { - return errors.New("cron expression cannot be empty") - } - fields := strings.Fields(cron) - if len(fields) != 6 { - if len(fields) == 5 { - return fmt.Errorf("you entered a 5-field cron expression, but Harbor requires 6 fields (with seconds)\n"+ - "Please add a seconds field at the beginning. For example: '0 %s'", cron) - } - return fmt.Errorf("harbor requires exactly 6 fields in cron expressions (seconds minute hour day month weekday), got %d", len(fields)) - } - cronRegex := regexp.MustCompile(`^(\*|[0-9]|[1-5][0-9]|\*/[0-9]+) (\*|[0-9]|[1-5][0-9]|\*/[0-9]+) (\*|[0-9]|1[0-9]|2[0-3]|\*/[0-9]+) (\*|[1-9]|[12][0-9]|3[01]|\*/[0-9]+) (\*|[1-9]|1[0-2]|\*/[0-9]+) (\*|[0-6]|\*/[0-9]+)$`) - if !cronRegex.MatchString(cron) { - return errors.New("invalid cron expression format\n" + - "Examples:\n" + - " 0 0 0 * * * - Daily at midnight\n" + - " 0 0 */6 * * * - Every 6 hours\n" + - " 0 0 0 * * 0 - Weekly on Sunday at midnight") - } - return nil -}