diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go index 2a49b4da87..eb49b10089 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go @@ -3,6 +3,7 @@ package validation import ( "context" + "github.com/robfig/cron/v3" "google.golang.org/grpc/codes" "github.com/flyteorg/flyte/flyteadmin/pkg/common" @@ -91,6 +92,19 @@ func validateSchedule(request *admin.LaunchPlanCreateRequest, expectedInputs *co "KickoffTimeInputArg must reference a datetime input. [%v] is a [%v]", schedule.GetKickoffTimeInputArg(), param.GetVar().GetType()) } } + + // validate cron expression + var cronExpression string + if schedule.GetCronExpression() != "" { + cronExpression = schedule.GetCronExpression() + } else if schedule.GetCronSchedule() != nil { + cronExpression = schedule.GetCronSchedule().GetSchedule() + } + if cronExpression != "" { + if _, err := cron.ParseStandard(cronExpression); err != nil { + return errors.NewFlyteAdminErrorf(codes.InvalidArgument, "Invalid cron expression: %v", err) + } + } } return nil } diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go index ab2832eeeb..7ceb0eae4d 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go @@ -358,7 +358,7 @@ func TestValidateSchedule_ArgNotFixed(t *testing.T) { }, } t.Run("with deprecated cron expression", func(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * *") err := validateSchedule(request, inputMap) assert.NotNil(t, err) @@ -370,7 +370,7 @@ func TestValidateSchedule_ArgNotFixed(t *testing.T) { assert.NotNil(t, err) }) t.Run("with cron schedule", func(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithCronSchedule("* * * * *") err := validateSchedule(request, inputMap) assert.NotNil(t, err) @@ -378,7 +378,7 @@ func TestValidateSchedule_ArgNotFixed(t *testing.T) { } func TestValidateSchedule_KickoffTimeArgDoesNotExist(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * *") inputMap := &core.ParameterMap{ Parameters: map[string]*core.Parameter{}, } @@ -389,7 +389,7 @@ func TestValidateSchedule_KickoffTimeArgDoesNotExist(t *testing.T) { } func TestValidateSchedule_KickoffTimeArgPointsAtWrongType(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * *") inputMap := &core.ParameterMap{ Parameters: map[string]*core.Parameter{ foo: { @@ -409,7 +409,7 @@ func TestValidateSchedule_KickoffTimeArgPointsAtWrongType(t *testing.T) { } func TestValidateSchedule_NoRequired(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * *") inputMap := &core.ParameterMap{ Parameters: map[string]*core.Parameter{ foo: { @@ -428,7 +428,7 @@ func TestValidateSchedule_NoRequired(t *testing.T) { } func TestValidateSchedule_KickoffTimeBound(t *testing.T) { - request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *") + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * *") inputMap := &core.ParameterMap{ Parameters: map[string]*core.Parameter{ foo: { @@ -446,3 +446,34 @@ func TestValidateSchedule_KickoffTimeBound(t *testing.T) { err := validateSchedule(request, inputMap) assert.Nil(t, err) } + +func TestValidateSchedule_InvalidCronExpression(t *testing.T) { + inputMap := &core.ParameterMap{ + Parameters: map[string]*core.Parameter{ + foo: { + Var: &core.Variable{ + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_DATETIME}}, + }, + Behavior: &core.Parameter_Required{ + Required: true, + }, + }, + }, + } + + t.Run("with unsupported cron special characters on deprecated cron schedule: #", func(t *testing.T) { + request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * MON#1") + request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = foo + + err := validateSchedule(request, inputMap) + assert.NotNil(t, err) + }) + + t.Run("with unsupported cron special characters: #", func(t *testing.T) { + request := testutils.GetLaunchPlanRequestWithCronSchedule("* * * * MON#1") + request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = foo + + err := validateSchedule(request, inputMap) + assert.NotNil(t, err) + }) +}