Skip to content

Commit e658521

Browse files
authored
feat: add cron expression validation to manifest validator (#2149)
* feat: add cron expression validation to manifest validator Enhances validateScheduledTriggers to parse and validate inline cron expressions from scheduled:<name>:<cron-expr> triggers. Enforces a 15-minute minimum interval, 20-schedule-per-manifest cap, and warns on schedules that fire less frequently than once per 365 days. * fix: address CodeRabbit review feedback on cron validation - Rename parsScheduledTrigger -> parseScheduledTrigger (typo) - Return hasCron bool to distinguish scheduled:name from scheduled:name: so an explicit empty cron suffix is rejected with INVALID_CRON_EXPRESSION - Use fixed anchor and 10-sample minimum for interval check instead of sampling only 2 occurrences from now, avoiding time-dependent results --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent ea716a3 commit e658521

2 files changed

Lines changed: 252 additions & 2 deletions

File tree

services/control-plane/internal/validator/domain_validator.go

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,28 @@ import (
77
"regexp"
88
"sort"
99
"strings"
10+
"time"
1011

1112
controlplanev1 "github.com/meridianhub/meridian/api/proto/meridian/control_plane/v1"
1213
mappingv1 "github.com/meridianhub/meridian/api/proto/meridian/mapping/v1"
14+
"github.com/robfig/cron/v3"
1315
"github.com/shopspring/decimal"
1416
)
1517

18+
const (
19+
// maxScheduledTriggersPerTenant is the maximum number of scheduled sagas per manifest.
20+
maxScheduledTriggersPerTenant = 20
21+
22+
// minCronInterval is the minimum allowed interval between cron occurrences.
23+
minCronInterval = 15 * time.Minute
24+
25+
// warnCronInterval is the threshold above which a schedule is considered very infrequent.
26+
warnCronInterval = 365 * 24 * time.Hour
27+
)
28+
29+
// cronParser matches the scheduler's parser: standard 5-field cron (no seconds).
30+
var cronParser = cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
31+
1632
// accountIDPattern matches valid Stripe Connect account IDs (acct_ followed by 16+ alphanumeric chars).
1733
var accountIDPattern = regexp.MustCompile(`^acct_[A-Za-z0-9]{16,}$`)
1834

@@ -362,21 +378,26 @@ func (v *ManifestValidator) validateWebhookTriggers(
362378
}
363379
}
364380

365-
// validateScheduledTriggers enforces that scheduled trigger names are unique across all sagas.
381+
// validateScheduledTriggers enforces uniqueness, cron syntax, minimum interval,
382+
// maximum schedule count, and warns on very infrequent schedules.
366383
func (v *ManifestValidator) validateScheduledTriggers(
367384
manifest *controlplanev1.Manifest,
368385
result *ValidationResult,
369386
) {
370387
// Track seen schedule names → first saga index.
371388
seen := make(map[string]int)
389+
scheduledCount := 0
372390

373391
for i, saga := range manifest.GetSagas() {
374392
trigger := saga.GetTrigger()
375393
if !strings.HasPrefix(trigger, "scheduled:") {
376394
continue
377395
}
396+
scheduledCount++
397+
398+
remainder := strings.TrimPrefix(trigger, "scheduled:")
399+
name, cronExpr, hasCron := parseScheduledTrigger(remainder)
378400

379-
name := strings.TrimPrefix(trigger, "scheduled:")
380401
if firstIdx, exists := seen[name]; exists {
381402
addError(result, ValidationError{
382403
Severity: SeverityError,
@@ -387,6 +408,95 @@ func (v *ManifestValidator) validateScheduledTriggers(
387408
} else {
388409
seen[name] = i
389410
}
411+
412+
if hasCron {
413+
if cronExpr == "" {
414+
addError(result, ValidationError{
415+
Severity: SeverityError,
416+
Path: fmt.Sprintf("sagas[%d].trigger", i),
417+
Code: "INVALID_CRON_EXPRESSION",
418+
Message: "cron expression cannot be empty; omit the trailing colon or provide a valid expression",
419+
ResourceType: "saga",
420+
ResourceID: saga.GetName(),
421+
})
422+
} else {
423+
v.validateCronExpression(cronExpr, fmt.Sprintf("sagas[%d].trigger", i), saga.GetName(), result)
424+
}
425+
}
426+
}
427+
428+
if scheduledCount > maxScheduledTriggersPerTenant {
429+
addError(result, ValidationError{
430+
Severity: SeverityError,
431+
Path: "sagas",
432+
Code: "TOO_MANY_SCHEDULED_TRIGGERS",
433+
Message: fmt.Sprintf("manifest has %d scheduled triggers, maximum is %d", scheduledCount, maxScheduledTriggersPerTenant),
434+
})
435+
}
436+
}
437+
438+
// parseScheduledTrigger splits "name" or "name:cron expr" into (name, cronExpr, hasCron).
439+
// hasCron is true when a colon separator was present, even if the expression is empty.
440+
// The cron expression may contain spaces, so only the first colon is used as separator.
441+
func parseScheduledTrigger(remainder string) (name, cronExpr string, hasCron bool) {
442+
idx := strings.Index(remainder, ":")
443+
if idx < 0 {
444+
return remainder, "", false
445+
}
446+
return remainder[:idx], remainder[idx+1:], true
447+
}
448+
449+
// validateCronExpression parses a cron expression and checks interval constraints.
450+
func (v *ManifestValidator) validateCronExpression(expr, path, sagaName string, result *ValidationResult) {
451+
schedule, err := cronParser.Parse(expr)
452+
if err != nil {
453+
addError(result, ValidationError{
454+
Severity: SeverityError,
455+
Path: path,
456+
Code: "INVALID_CRON_EXPRESSION",
457+
Message: fmt.Sprintf("invalid cron expression %q: %s", expr, err.Error()),
458+
ResourceType: "saga",
459+
ResourceID: sagaName,
460+
})
461+
return
462+
}
463+
464+
// Sample 10 consecutive occurrences from a fixed anchor to find the minimum interval.
465+
// A fixed anchor avoids results that vary with the current time (e.g. daylight saving transitions).
466+
anchor := time.Date(2001, 1, 1, 0, 0, 0, 0, time.UTC)
467+
const sampleSize = 10
468+
prev := schedule.Next(anchor)
469+
minInterval := time.Duration(1<<63 - 1) // max duration
470+
for range sampleSize - 1 {
471+
next := schedule.Next(prev)
472+
if gap := next.Sub(prev); gap < minInterval {
473+
minInterval = gap
474+
}
475+
prev = next
476+
}
477+
interval := minInterval
478+
479+
if interval < minCronInterval {
480+
addError(result, ValidationError{
481+
Severity: SeverityError,
482+
Path: path,
483+
Code: "CRON_INTERVAL_TOO_SHORT",
484+
Message: fmt.Sprintf("cron expression %q fires every %s, minimum interval is %s", expr, interval.Round(time.Second), minCronInterval),
485+
ResourceType: "saga",
486+
ResourceID: sagaName,
487+
})
488+
return
489+
}
490+
491+
if interval >= warnCronInterval {
492+
addError(result, ValidationError{
493+
Severity: SeverityWarning,
494+
Path: path,
495+
Code: "CRON_VERY_INFREQUENT",
496+
Message: fmt.Sprintf("cron expression %q fires every %s, which is more than 365 days", expr, interval.Round(time.Hour)),
497+
ResourceType: "saga",
498+
ResourceID: sagaName,
499+
})
390500
}
391501
}
392502

services/control-plane/internal/validator/domain_validator_test.go

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package validator
22

33
import (
4+
"fmt"
45
"testing"
56

67
controlplanev1 "github.com/meridianhub/meridian/api/proto/meridian/control_plane/v1"
@@ -399,3 +400,142 @@ func TestValidateMappingIdempotency_ValidContentHash(t *testing.T) {
399400
v.validateMappingIdempotency(mp, "mappings[0]", result)
400401
assert.Empty(t, result.Errors)
401402
}
403+
404+
// ─── Scheduled Trigger Cron Validation ───────────────────────────────────────
405+
406+
func TestValidateScheduledTriggers_ValidCronExpression(t *testing.T) {
407+
v, err := New()
408+
require.NoError(t, err)
409+
410+
manifest := &controlplanev1.Manifest{
411+
Sagas: []*controlplanev1.SagaDefinition{
412+
{Name: "hourly_billing", Trigger: "scheduled:hourly_billing:0 * * * *"},
413+
},
414+
}
415+
416+
result := &ValidationResult{Valid: true}
417+
v.validateScheduledTriggers(manifest, result)
418+
assert.Empty(t, result.Errors)
419+
assert.Empty(t, result.Warnings)
420+
}
421+
422+
func TestValidateScheduledTriggers_EmptyCronSuffix_Errors(t *testing.T) {
423+
v, err := New()
424+
require.NoError(t, err)
425+
426+
// scheduled:<name>: with trailing colon but empty cron is invalid
427+
manifest := &controlplanev1.Manifest{
428+
Sagas: []*controlplanev1.SagaDefinition{
429+
{Name: "billing", Trigger: "scheduled:billing:"},
430+
},
431+
}
432+
433+
result := &ValidationResult{Valid: true}
434+
v.validateScheduledTriggers(manifest, result)
435+
require.Len(t, result.Errors, 1)
436+
assert.Equal(t, "INVALID_CRON_EXPRESSION", result.Errors[0].Code)
437+
}
438+
439+
func TestValidateScheduledTriggers_NoCronExpression_Valid(t *testing.T) {
440+
v, err := New()
441+
require.NoError(t, err)
442+
443+
// scheduled:<name> with no cron is valid (cron comes from DB)
444+
manifest := &controlplanev1.Manifest{
445+
Sagas: []*controlplanev1.SagaDefinition{
446+
{Name: "billing", Trigger: "scheduled:billing"},
447+
},
448+
}
449+
450+
result := &ValidationResult{Valid: true}
451+
v.validateScheduledTriggers(manifest, result)
452+
assert.Empty(t, result.Errors)
453+
assert.Empty(t, result.Warnings)
454+
}
455+
456+
func TestValidateScheduledTriggers_InvalidCronSyntax(t *testing.T) {
457+
v, err := New()
458+
require.NoError(t, err)
459+
460+
manifest := &controlplanev1.Manifest{
461+
Sagas: []*controlplanev1.SagaDefinition{
462+
{Name: "billing", Trigger: "scheduled:billing:not-a-cron"},
463+
},
464+
}
465+
466+
result := &ValidationResult{Valid: true}
467+
v.validateScheduledTriggers(manifest, result)
468+
require.Len(t, result.Errors, 1)
469+
assert.Equal(t, "INVALID_CRON_EXPRESSION", result.Errors[0].Code)
470+
assert.Equal(t, "sagas[0].trigger", result.Errors[0].Path)
471+
}
472+
473+
func TestValidateScheduledTriggers_IntervalTooShort(t *testing.T) {
474+
v, err := New()
475+
require.NoError(t, err)
476+
477+
// Every minute - interval is 1 min, below 15 min minimum
478+
manifest := &controlplanev1.Manifest{
479+
Sagas: []*controlplanev1.SagaDefinition{
480+
{Name: "too_frequent", Trigger: "scheduled:too_frequent:* * * * *"},
481+
},
482+
}
483+
484+
result := &ValidationResult{Valid: true}
485+
v.validateScheduledTriggers(manifest, result)
486+
require.Len(t, result.Errors, 1)
487+
assert.Equal(t, "CRON_INTERVAL_TOO_SHORT", result.Errors[0].Code)
488+
}
489+
490+
func TestValidateScheduledTriggers_IntervalExactly15Minutes_Valid(t *testing.T) {
491+
v, err := New()
492+
require.NoError(t, err)
493+
494+
// Every 15 minutes - exactly at minimum
495+
manifest := &controlplanev1.Manifest{
496+
Sagas: []*controlplanev1.SagaDefinition{
497+
{Name: "quarter_hourly", Trigger: "scheduled:quarter_hourly:0,15,30,45 * * * *"},
498+
},
499+
}
500+
501+
result := &ValidationResult{Valid: true}
502+
v.validateScheduledTriggers(manifest, result)
503+
assert.Empty(t, result.Errors)
504+
}
505+
506+
func TestValidateScheduledTriggers_TooManySchedules(t *testing.T) {
507+
v, err := New()
508+
require.NoError(t, err)
509+
510+
sagas := make([]*controlplanev1.SagaDefinition, maxScheduledTriggersPerTenant+1)
511+
for i := range sagas {
512+
sagas[i] = &controlplanev1.SagaDefinition{
513+
Name: fmt.Sprintf("saga_%d", i),
514+
Trigger: fmt.Sprintf("scheduled:schedule_%d", i),
515+
}
516+
}
517+
518+
manifest := &controlplanev1.Manifest{Sagas: sagas}
519+
result := &ValidationResult{Valid: true}
520+
v.validateScheduledTriggers(manifest, result)
521+
require.Len(t, result.Errors, 1)
522+
assert.Equal(t, "TOO_MANY_SCHEDULED_TRIGGERS", result.Errors[0].Code)
523+
}
524+
525+
func TestValidateScheduledTriggers_InfrequentScheduleWarns(t *testing.T) {
526+
v, err := New()
527+
require.NoError(t, err)
528+
529+
// Every year on Jan 1 at midnight
530+
manifest := &controlplanev1.Manifest{
531+
Sagas: []*controlplanev1.SagaDefinition{
532+
{Name: "annual", Trigger: "scheduled:annual:0 0 1 1 *"},
533+
},
534+
}
535+
536+
result := &ValidationResult{Valid: true}
537+
v.validateScheduledTriggers(manifest, result)
538+
assert.Empty(t, result.Errors)
539+
require.Len(t, result.Warnings, 1)
540+
assert.Equal(t, "CRON_VERY_INFREQUENT", result.Warnings[0].Code)
541+
}

0 commit comments

Comments
 (0)