diff --git a/handler/scheduleHandler/handler.go b/handler/scheduleHandler/handler.go index 0c6f915..3c48a9c 100644 --- a/handler/scheduleHandler/handler.go +++ b/handler/scheduleHandler/handler.go @@ -12,7 +12,7 @@ import ( ) type IScheduleHandler interface { - getScheduleByName(schedule string, r client.Reader, ctx context.Context) (*workloadschedulerv1.Schedule, error) + GetScheduleByName(schedule string, r client.Reader, ctx context.Context) (*workloadschedulerv1.Schedule, error) FetchWorkloadSchedules(schedules []workloadschedulerv1.WorkloadScheduleUnit, r client.Reader, ctx context.Context) ([]workloadschedulerv1.Schedule, error) IsThisDayIncluded(days []string, now time.Time) bool ValidateSchedule(schedule *workloadschedulerv1.Schedule) error @@ -74,7 +74,7 @@ func (s *ScheduleHandler) IsThisDayIncluded(days []string, now time.Time) bool { func (s *ScheduleHandler) FetchWorkloadSchedules(_schedules []workloadschedulerv1.WorkloadScheduleUnit, r client.Reader, ctx context.Context) ([]workloadschedulerv1.Schedule, error) { var schedules []workloadschedulerv1.Schedule for _, _schedule := range _schedules { - schedule, err := s.IScheduleHandler.getScheduleByName(_schedule.Schedule, r, ctx) + schedule, err := s.IScheduleHandler.GetScheduleByName(_schedule.Schedule, r, ctx) if err != nil { return nil, fmt.Errorf(fmt.Sprintf("error when fetching: %v schedule", _schedule), err) } else { @@ -84,7 +84,7 @@ func (s *ScheduleHandler) FetchWorkloadSchedules(_schedules []workloadschedulerv return schedules, nil } -func (s *ScheduleHandler) getScheduleByName(_schedule string, r client.Reader, ctx context.Context) (*workloadschedulerv1.Schedule, error) { +func (s *ScheduleHandler) GetScheduleByName(_schedule string, r client.Reader, ctx context.Context) (*workloadschedulerv1.Schedule, error) { schedule := &workloadschedulerv1.Schedule{} err := r.Get(ctx, client.ObjectKey{Name: _schedule}, schedule) if err != nil { diff --git a/handler/scheduleHandler/handler_test.go b/handler/scheduleHandler/handler_test.go index d25dac5..01f0b60 100644 --- a/handler/scheduleHandler/handler_test.go +++ b/handler/scheduleHandler/handler_test.go @@ -84,13 +84,13 @@ func TestScheduleHandler_getSchedulesByName(t *testing.T) { s := &ScheduleHandler{} tt.setupMocks() defer tt.verifyMocks() - got, err := s.getScheduleByName(tt.args.schedule, testreader, tt.args.ctx) + got, err := s.GetScheduleByName(tt.args.schedule, testreader, tt.args.ctx) if (err != nil) != tt.wantErr { - t.Errorf("getScheduleByName() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetScheduleByName() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("getScheduleByName() got = %v, want %v", got, tt.want) + t.Errorf("GetScheduleByName() got = %v, want %v", got, tt.want) } }) } @@ -101,7 +101,7 @@ type testScheduleHandler struct { IScheduleHandler } -func (s *testScheduleHandler) getScheduleByName(schedule string, r client.Reader, ctx context.Context) (*v1.Schedule, error) { +func (s *testScheduleHandler) GetScheduleByName(schedule string, r client.Reader, ctx context.Context) (*v1.Schedule, error) { args := s.Called(schedule, r, ctx) if args.Get(0) == nil { return nil, args.Error(1) @@ -128,7 +128,7 @@ func TestScheduleHandler_CheckIfSchedulesExist(t *testing.T) { }{ {name: "should return nil schedules when error occurs when getting schedule.", setupMocks: func() { testschedulehandler = &testScheduleHandler{} - testschedulehandler.On("getScheduleByName", mock.IsType("weekday"), mock.IsType(&testReader{}), mock.IsType(context.TODO())).Return(nil, fmt.Errorf("some error")) + testschedulehandler.On("GetScheduleByName", mock.IsType("weekday"), mock.IsType(&testReader{}), mock.IsType(context.TODO())).Return(nil, fmt.Errorf("some error")) s.IScheduleHandler = testschedulehandler }, verifyMocks: func() { testschedulehandler.AssertExpectations(t) @@ -141,7 +141,7 @@ func TestScheduleHandler_CheckIfSchedulesExist(t *testing.T) { want: nil, wantErr: true}, {name: "should return schedules when schedule retrieved successfully.", setupMocks: func() { testschedulehandler = &testScheduleHandler{} - testschedulehandler.On("getScheduleByName", mock.IsType("weekday"), mock.IsType(&testReader{}), mock.IsType(context.TODO())).Return(&v1.Schedule{}, nil) + testschedulehandler.On("GetScheduleByName", mock.IsType("weekday"), mock.IsType(&testReader{}), mock.IsType(context.TODO())).Return(&v1.Schedule{}, nil) s.IScheduleHandler = testschedulehandler }, verifyMocks: func() { testschedulehandler.AssertExpectations(t) diff --git a/handler/workloadScheduleHandler/handler.go b/handler/workloadScheduleHandler/handler.go index 03adf35..b0e3464 100644 --- a/handler/workloadScheduleHandler/handler.go +++ b/handler/workloadScheduleHandler/handler.go @@ -9,6 +9,7 @@ import ( "fmt" "github.com/alistanis/cartesian" apps "k8s.io/api/apps/v1" + "k8s.io/apimachinery/pkg/util/validation" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sort" @@ -28,8 +29,9 @@ type WorkloadHandler interface { type IWorkloadScheduleHandler interface { EvaluateWorkloadSchedulers(schedulers *workloadschedulerv1.WorkloadScheduleList, r client.Reader, ctx context.Context) (map[string][]workloadschedulerv1.Schedule, map[string]workloadschedulerv1.WorkloadSchedule) ProcessWorkloadSchedules(schedules map[string][]workloadschedulerv1.Schedule, schedulerMap map[string]workloadschedulerv1.WorkloadSchedule, c client.Client, ctx context.Context) error - ValidateWorkloadSchedule(schedule *workloadschedulerv1.WorkloadSchedule) error + ValidateWorkloadSchedule(schedule *workloadschedulerv1.WorkloadSchedule, r client.Reader) error } + type DeploymentHandler struct { config.Config apps.Deployment @@ -52,9 +54,21 @@ func New() *WorkloadScheduleHandler { return &WorkloadScheduleHandler{ScheduleHandler: scheduleHandler.New(), Config: *config.New()} } -func (w *WorkloadScheduleHandler) ValidateWorkloadSchedule(workloadSchedule *workloadschedulerv1.WorkloadSchedule) error { +func (w *WorkloadScheduleHandler) ValidateWorkloadSchedule(workloadSchedule *workloadschedulerv1.WorkloadSchedule, r client.Reader) error { if workloadSchedule.Spec.Schedules == nil || len(workloadSchedule.Spec.Schedules) == 0 { return fmt.Errorf("schedules need to be defined") + + } else { + for _, schedule := range workloadSchedule.Spec.Schedules { + if errs := validation.IsDNS1123Label(schedule.Schedule); errs != nil { + return fmt.Errorf("schedule: %s is not valid. %v", schedule.Schedule, errs) + } + + _, err := w.ScheduleHandler.GetScheduleByName(schedule.Schedule, r, context.Background()) + if err != nil { + return err + } + } } return nil } diff --git a/handler/workloadScheduleHandler/handler_test.go b/handler/workloadScheduleHandler/handler_test.go index 6f72519..e27afb7 100644 --- a/handler/workloadScheduleHandler/handler_test.go +++ b/handler/workloadScheduleHandler/handler_test.go @@ -3,6 +3,7 @@ package workloadScheduleHandler import ( v1 "bennsimon.github.io/workload-scheduler-operator/api/v1" "bennsimon.github.io/workload-scheduler-operator/handler/scheduleHandler" + "bennsimon.github.io/workload-scheduler-operator/util/config" "context" "fmt" "github.com/stretchr/testify/mock" @@ -258,3 +259,47 @@ func TestWorkloadScheduleHandler_BuildSpecMap(t *testing.T) { }) } } + +type NopScheduleHandler struct { + scheduleHandler.IScheduleHandler +} + +func (s *NopScheduleHandler) GetScheduleByName(_schedule string, r client.Reader, ctx context.Context) (*v1.Schedule, error) { + return nil, nil +} + +func TestWorkloadScheduleHandler_ValidateWorkloadSchedule(t *testing.T) { + type fields struct { + ScheduleHandler scheduleHandler.IScheduleHandler + Config config.Config + } + type args struct { + workloadSchedule *v1.WorkloadSchedule + r client.Reader + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + {name: "should return error if schedules is empty", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{}}}}, wantErr: true}, + {name: "should return error if schedules is nil", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: nil}}}, wantErr: true}, + {name: "should return error if schedule name is invalid", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{{Schedule: "week day"}}}}}, wantErr: true}, + {name: "should return error if schedule name is invalid", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{{Schedule: "Weekday"}}}}}, wantErr: true}, + {name: "should return error if schedule name is invalid", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{{Schedule: ".weekday"}}}}}, wantErr: true}, + {name: "should return error if schedule name is invalid", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{{Schedule: ""}}}}}, wantErr: true}, + {name: "should not return error if schedule name is valid", fields: fields{ScheduleHandler: &NopScheduleHandler{}}, args: args{workloadSchedule: &v1.WorkloadSchedule{Spec: v1.WorkloadScheduleSpec{Schedules: []v1.WorkloadScheduleUnit{{Schedule: "weekday"}}}}}, wantErr: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &WorkloadScheduleHandler{ + ScheduleHandler: tt.fields.ScheduleHandler, + Config: tt.fields.Config, + } + if err := w.ValidateWorkloadSchedule(tt.args.workloadSchedule, tt.args.r); (err != nil) != tt.wantErr { + t.Errorf("ValidateWorkloadSchedule() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/controller/workloadschedule_controller.go b/internal/controller/workloadschedule_controller.go index fd9143a..4a16c7e 100644 --- a/internal/controller/workloadschedule_controller.go +++ b/internal/controller/workloadschedule_controller.go @@ -46,7 +46,7 @@ func (r *WorkloadScheduleReconciler) Reconcile(ctx context.Context, req ctrl.Req if err != nil { return ctrl.Result{}, err } - err = r.IWorkloadScheduleHandler.ValidateWorkloadSchedule(workloadSchedule) + err = r.IWorkloadScheduleHandler.ValidateWorkloadSchedule(workloadSchedule, r) if err != nil { return ctrl.Result{}, err }