Update cron scaler to parse the cron with parser instead of searching for '-'

CHANGELOG.md

@@ -27,6 +27,7 @@
 ### Improvements
 
 - Improve validation in Cron scaler in case start & end input is same.([#2032](https://github.com/kedacore/keda/pull/2032))
+- Improve the cron validation in Cron Scaler ([#2038](https://github.com/kedacore/keda/pull/2038))
 - Add Bearer auth for Metrics API scaler ([#2028](https://github.com/kedacore/keda/pull/2028))
 
 ### Breaking Changes

pkg/scalers/cron_scaler.go

@@ -73,17 +73,20 @@ func parseCronMetadata(config *ScalerConfig) (*cronMetadata, error) {
 	} else {
 		return nil, fmt.Errorf("no timezone specified. %s", config.TriggerMetadata)
 	}
+	parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
 	if val, ok := config.TriggerMetadata["start"]; ok && val != "" {
-		if strings.Contains(val, "-") {
-			return nil, fmt.Errorf("error parsing start schedule. %s: range or hyphenated inputs are not allowed", config.TriggerMetadata)
+		_, err := parser.Parse(val)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing start schedule: %s", err)
 		}
 		meta.start = val
 	} else {
 		return nil, fmt.Errorf("no start schedule specified. %s", config.TriggerMetadata)
 	}
 	if val, ok := config.TriggerMetadata["end"]; ok && val != "" {
-		if strings.Contains(val, "-") {
-			return nil, fmt.Errorf("error parsing end schedule. %s: range or hyphenated inputs are not allowed", config.TriggerMetadata)
+		_, err := parser.Parse(val)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing end schedule: %s", err)
 		}
 		meta.end = val
 	} else {

pkg/scalers/cron_scaler_test.go

@@ -26,13 +26,22 @@ var validCronMetadata = map[string]string{
 	"desiredReplicas": "10",
 }
 
+// A complete valid metadata example which is enabled every even hours
+var validCronMetadata2 = map[string]string{
+	"timezone":        "Etc/UTC",
+	"start":           "0 */2 * * *",    // Every 2 hours (even)
+	"end":             "0 1-23/2 * * *", // Every 2 hours starting at 1 (odd)
+	"desiredReplicas": "10",
+}
+
 var testCronMetadata = []parseCronMetadataTestData{
 	{map[string]string{}, true},
 	{validCronMetadata, false},
+	{validCronMetadata2, false},
 	{map[string]string{"timezone": "Asia/Kolkata", "start": "30 * * * *", "end": "45 * * * *"}, true},
 	{map[string]string{"start": "30 * * * *", "end": "45 * * * *", "desiredReplicas": "10"}, true},
-	{map[string]string{"timezone": "Asia/Kolkata", "start": "30-33 * * * *", "end": "45 * * * *", "desiredReplicas": "10"}, true},
-	{map[string]string{"timezone": "Asia/Kolkata", "start": "30 * * * *", "end": "45-50 * * * *", "desiredReplicas": "10"}, true},
+	{map[string]string{"timezone": "Asia/Kolkata", "start": "30-33 * * * *", "end": "45 * * * *", "desiredReplicas": "10"}, false},
+	{map[string]string{"timezone": "Asia/Kolkata", "start": "30 * * * *", "end": "45-50 * * * *", "desiredReplicas": "10"}, false},
 	{map[string]string{"timezone": "Asia/Kolkata", "start": "-30 * * * *", "end": "45 * * * *", "desiredReplicas": "10"}, true},
 	{map[string]string{"timezone": "Asia/Kolkata", "start": "30 * * * *", "end": "-50 * * * *", "desiredReplicas": "10"}, true},
 	{map[string]string{"timezone": "Asia/Kolkata", "start": "30 * * * *", "end": "50 * * -3 *", "desiredReplicas": "10"}, true},
@@ -41,10 +50,12 @@ var testCronMetadata = []parseCronMetadataTestData{
 
 var cronMetricIdentifiers = []cronMetricIdentifier{
 	{&testCronMetadata[1], "cron-Etc-UTC-00xxThu-5923xxThu"},
+	{&testCronMetadata[2], "cron-Etc-UTC-0xSl2xxx-01-23Sl2xxx"},
 }
 
-var tz, _ = time.LoadLocation(validCronMetadata["timezone"])
+var tz, _ = time.LoadLocation(validCronMetadata2["timezone"])
 var currentDay = time.Now().In(tz).Weekday().String()
+var currentHour = time.Now().In(tz).Hour()
 
 func TestCronParseMetadata(t *testing.T) {
 	for _, testData := range testCronMetadata {
@@ -68,6 +79,16 @@ func TestIsActive(t *testing.T) {
 	}
 }
 
+func TestIsActiveRange(t *testing.T) {
+	scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata2})
+	isActive, _ := scaler.IsActive(context.TODO())
+	if currentHour%2 == 0 {
+		assert.Equal(t, isActive, true)
+	} else {
+		assert.Equal(t, isActive, false)
+	}
+}
+
 func TestGetMetrics(t *testing.T) {
 	scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata})
 	metrics, _ := scaler.GetMetrics(context.TODO(), "ReplicaCount", nil)
@@ -79,6 +100,17 @@ func TestGetMetrics(t *testing.T) {
 	}
 }
 
+func TestGetMetricsRange(t *testing.T) {
+	scaler, _ := NewCronScaler(&ScalerConfig{TriggerMetadata: validCronMetadata2})
+	metrics, _ := scaler.GetMetrics(context.TODO(), "ReplicaCount", nil)
+	assert.Equal(t, metrics[0].MetricName, "ReplicaCount")
+	if currentHour%2 == 0 {
+		assert.Equal(t, metrics[0].Value.Value(), int64(10))
+	} else {
+		assert.Equal(t, metrics[0].Value.Value(), int64(1))
+	}
+}
+
 func TestCronGetMetricSpecForScaling(t *testing.T) {
 	for _, testData := range cronMetricIdentifiers {
 		meta, err := parseCronMetadata(&ScalerConfig{TriggerMetadata: testData.metadataTestData.metadata})