From 9091661539b3c47e250e28f79f5a51096f92f6dc Mon Sep 17 00:00:00 2001
From: Shengwen YU <yshengwen@vmware.com>
Date: Thu, 20 Jul 2023 17:55:48 +0800
Subject: [PATCH] fix: replication policy cron setting - the 1st field must be
 0; the Minutes field cannot be * (#18923)

Signed-off-by: Shengwen Yu <yshengwen@vmware.com>
---
 src/controller/replication/model/model.go     |  7 +++
 .../replication/model/model_test.go           | 60 ++++++++++++++++++-
 src/controller/replication/policy_test.go     |  4 +-
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/src/controller/replication/model/model.go b/src/controller/replication/model/model.go
index 6821640a4..a257687a7 100644
--- a/src/controller/replication/model/model.go
+++ b/src/controller/replication/model/model.go
@@ -108,6 +108,13 @@ func (p *Policy) Validate() error {
 				return errors.New(nil).WithCode(errors.BadRequestCode).
 					WithMessage("invalid cron string for scheduled trigger: %s", p.Trigger.Settings.Cron)
 			}
+			cronParts := strings.Split(p.Trigger.Settings.Cron, " ")
+			if cronParts[0] != "0" {
+				return errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("the 1st field (indicating Seconds of time) of the cron setting must be 0")
+			}
+			if cronParts[1] == "*" {
+				return errors.New(nil).WithCode(errors.BadRequestCode).WithMessage("* is not allowed for the Minutes field of the cron setting of replication policy")
+			}
 		default:
 			return errors.New(nil).WithCode(errors.BadRequestCode).
 				WithMessage("invalid trigger type")
diff --git a/src/controller/replication/model/model_test.go b/src/controller/replication/model/model_test.go
index c396b9917..fa84ca0cc 100644
--- a/src/controller/replication/model/model_test.go
+++ b/src/controller/replication/model/model_test.go
@@ -199,7 +199,7 @@ func TestValidate(t *testing.T) {
 	err = policy.Validate()
 	assert.True(errors.IsErr(err, errors.BadRequestCode))
 
-	// pass
+	// invalid cron: the 1st field (indicating Seconds of time) of the cron setting must be 0
 	policy = &Policy{
 		Name: "policy01",
 		SrcRegistry: &model.Registry{
@@ -226,5 +226,63 @@ func TestValidate(t *testing.T) {
 		},
 	}
 	err = policy.Validate()
+	assert.True(errors.IsErr(err, errors.BadRequestCode))
+
+	// invalid cron: * is not allowed for the Minutes field of the cron setting of replication policy
+	policy = &Policy{
+		Name: "policy01",
+		SrcRegistry: &model.Registry{
+			ID: 0,
+		},
+		DestRegistry: &model.Registry{
+			ID: 1,
+		},
+		Filters: []*model.Filter{
+			{
+				Type:  model.FilterTypeResource,
+				Value: "image",
+			},
+			{
+				Type:  model.FilterTypeName,
+				Value: "library/**",
+			},
+		},
+		Trigger: &model.Trigger{
+			Type: model.TriggerTypeScheduled,
+			Settings: &model.TriggerSettings{
+				Cron: "0 * * * * *",
+			},
+		},
+	}
+	err = policy.Validate()
+	assert.True(errors.IsErr(err, errors.BadRequestCode))
+
+	// pass
+	policy = &Policy{
+		Name: "policy01",
+		SrcRegistry: &model.Registry{
+			ID: 0,
+		},
+		DestRegistry: &model.Registry{
+			ID: 1,
+		},
+		Filters: []*model.Filter{
+			{
+				Type:  model.FilterTypeResource,
+				Value: "image",
+			},
+			{
+				Type:  model.FilterTypeName,
+				Value: "library/**",
+			},
+		},
+		Trigger: &model.Trigger{
+			Type: model.TriggerTypeScheduled,
+			Settings: &model.TriggerSettings{
+				Cron: "0 0 * * * *",
+			},
+		},
+	}
+	err = policy.Validate()
 	assert.Nil(err)
 }
diff --git a/src/controller/replication/policy_test.go b/src/controller/replication/policy_test.go
index 86c8b9521..65ad9c9ae 100644
--- a/src/controller/replication/policy_test.go
+++ b/src/controller/replication/policy_test.go
@@ -78,7 +78,7 @@ func (r *replicationTestSuite) TestCreatePolicy() {
 		Trigger: &model.Trigger{
 			Type: model.TriggerTypeScheduled,
 			Settings: &model.TriggerSettings{
-				Cron: "0 * * * * *",
+				Cron: "0 0 * * * *",
 			},
 		},
 		Enabled: true,
@@ -106,7 +106,7 @@ func (r *replicationTestSuite) TestUpdatePolicy() {
 		Trigger: &model.Trigger{
 			Type: model.TriggerTypeScheduled,
 			Settings: &model.TriggerSettings{
-				Cron: "0 * * * * *",
+				Cron: "0 0 * * * *",
 			},
 		},
 		Enabled: true,