diff --git a/CHANGELOG.md b/CHANGELOG.md index 726a3009330..50b7ad5b4de 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pkg/scalers/cron_scaler.go b/pkg/scalers/cron_scaler.go index 71fbf2a122d..4f6fac823bd 100644 --- a/pkg/scalers/cron_scaler.go +++ b/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 { diff --git a/pkg/scalers/cron_scaler_test.go b/pkg/scalers/cron_scaler_test.go index 0a0c2975dba..5b6eccdcd75 100644 --- a/pkg/scalers/cron_scaler_test.go +++ b/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})