Skip to content

Commit 35d9f42

Browse files
sgress454sgress454
authored
Add shared software update schedule validation logic (#38016)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** For #33391 # Details This PR adds a shared method for validating a software auto-update configuration, and updates the datastore and API handler methods to use it. # Checklist for submitter ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually Tested in the UI (to ensure valid calls still work) and via API calls (to test validation)
1 parent 09c3c72 commit 35d9f42

File tree

6 files changed

+240
-26
lines changed

6 files changed

+240
-26
lines changed

‎ee/server/service/vpp.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,20 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID
781781
return nil, nil, err
782782
}
783783

784+
// If there's an auto-update config, validate it.
785+
// Note that applying this config is done in a separate service method.
786+
schedule := fleet.SoftwareAutoUpdateSchedule{
787+
SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{
788+
AutoUpdateEnabled: payload.AutoUpdateEnabled,
789+
AutoUpdateStartTime: payload.AutoUpdateStartTime,
790+
AutoUpdateEndTime: payload.AutoUpdateEndTime,
791+
},
792+
}
793+
794+
if err := schedule.WindowIsValid(); err != nil {
795+
return nil, nil, ctxerr.Wrap(ctx, err, "UpdateAppStoreApp: validating auto-update schedule")
796+
}
797+
784798
var teamName string
785799
if teamID != nil && *teamID != 0 {
786800
tm, err := svc.ds.TeamLite(ctx, *teamID)

‎server/datastore/mysql/software_titles.go‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -832,29 +832,22 @@ WHERE
832832
}
833833

834834
func (ds *Datastore) UpdateSoftwareTitleAutoUpdateConfig(ctx context.Context, titleID uint, teamID uint, config fleet.SoftwareAutoUpdateConfig) error {
835-
// Validate start and end time.
836-
invalidTimeErr := "invalid auto-update time format: must be in HH:MM 24-hour format"
837-
for _, t := range []*string{config.AutoUpdateStartTime, config.AutoUpdateEndTime} {
838-
if t == nil {
839-
if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled {
840-
return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr)
841-
}
842-
continue
843-
}
844-
duration, err := time.Parse("15:04", *t)
845-
if err != nil {
846-
return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr)
847-
}
848-
if duration.Hour() < 0 || duration.Hour() > 23 || duration.Minute() < 0 || duration.Minute() > 59 {
849-
return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr)
850-
}
835+
// Validate schedule if enabled.
836+
schedule := fleet.SoftwareAutoUpdateSchedule{
837+
SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{
838+
AutoUpdateEnabled: config.AutoUpdateEnabled,
839+
AutoUpdateStartTime: config.AutoUpdateStartTime,
840+
AutoUpdateEndTime: config.AutoUpdateEndTime,
841+
},
842+
}
843+
if err := schedule.WindowIsValid(); err != nil {
844+
return ctxerr.Wrap(ctx, err, "validating auto-update schedule")
851845
}
852-
853846
var startTime, endTime string
854-
if config.AutoUpdateStartTime != nil {
847+
if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled && config.AutoUpdateStartTime != nil {
855848
startTime = *config.AutoUpdateStartTime
856849
}
857-
if config.AutoUpdateEndTime != nil {
850+
if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled && config.AutoUpdateEndTime != nil {
858851
endTime = *config.AutoUpdateEndTime
859852
}
860853

‎server/datastore/mysql/software_titles_test.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2655,7 +2655,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) {
26552655
AutoUpdateEndTime: ptr.String(endTime),
26562656
})
26572657
require.Error(t, err)
2658-
require.Contains(t, err.Error(), "invalid auto-update time format")
2658+
require.Contains(t, err.Error(), "Error parsing start time")
26592659

26602660
// Attempt to enable auto-update with invalid end time.
26612661
startTime = "12:00"
@@ -2666,7 +2666,18 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) {
26662666
AutoUpdateEndTime: ptr.String(endTime),
26672667
})
26682668
require.Error(t, err)
2669-
require.Contains(t, err.Error(), "invalid auto-update time format")
2669+
require.Contains(t, err.Error(), "Error parsing end time")
2670+
2671+
// Attempt to enable auto-update with less than an hour between start and end time.
2672+
startTime = "12:00"
2673+
endTime = "12:30"
2674+
err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, titleID, *teamID, fleet.SoftwareAutoUpdateConfig{
2675+
AutoUpdateEnabled: ptr.Bool(true),
2676+
AutoUpdateStartTime: ptr.String(startTime),
2677+
AutoUpdateEndTime: ptr.String(endTime),
2678+
})
2679+
require.Error(t, err)
2680+
require.Contains(t, err.Error(), "The update window must be at least one hour long")
26702681

26712682
// Enable auto-update.
26722683
startTime = "02:00"
@@ -2687,6 +2698,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) {
26872698
require.Equal(t, endTime, *titleResult.AutoUpdateEndTime)
26882699

26892700
// Add valid, disabled auto-update schedule for the other VPP app.
2701+
// The schedule should be ignored since it's disabled, but it should still be created.
26902702
err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, title2ID, *teamID, fleet.SoftwareAutoUpdateConfig{
26912703
AutoUpdateEnabled: ptr.Bool(false),
26922704
AutoUpdateStartTime: ptr.String(startTime),
@@ -2706,8 +2718,8 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) {
27062718
require.Equal(t, title2ID, schedules[1].TitleID)
27072719
require.Equal(t, team1.ID, schedules[1].TeamID)
27082720
require.False(t, *schedules[1].AutoUpdateEnabled)
2709-
require.Equal(t, startTime, *schedules[1].AutoUpdateStartTime)
2710-
require.Equal(t, endTime, *schedules[1].AutoUpdateEndTime)
2721+
require.Equal(t, "", *schedules[1].AutoUpdateStartTime)
2722+
require.Equal(t, "", *schedules[1].AutoUpdateEndTime)
27112723

27122724
// Filter by enabled only.
27132725
schedules, err = ds.ListSoftwareAutoUpdateSchedules(ctx, *teamID, "ipados_apps", fleet.SoftwareAutoUpdateScheduleFilter{
@@ -2727,9 +2739,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) {
27272739

27282740
// Disable auto-update.
27292741
err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, titleID, *teamID, fleet.SoftwareAutoUpdateConfig{
2730-
AutoUpdateEnabled: ptr.Bool(false),
2731-
AutoUpdateStartTime: nil,
2732-
AutoUpdateEndTime: nil,
2742+
AutoUpdateEnabled: ptr.Bool(false),
27332743
})
27342744
require.NoError(t, err)
27352745

‎server/fleet/software.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,36 @@ type SoftwareAutoUpdateSchedule struct {
270270
SoftwareAutoUpdateConfig
271271
}
272272

273+
func (s SoftwareAutoUpdateSchedule) WindowIsValid() error {
274+
if s.AutoUpdateEnabled == nil || !*s.AutoUpdateEnabled {
275+
return nil
276+
}
277+
if s.AutoUpdateStartTime == nil || s.AutoUpdateEndTime == nil || *s.AutoUpdateStartTime == "" || *s.AutoUpdateEndTime == "" {
278+
return errors.New("Start and end time must both be set")
279+
}
280+
// Validate that the times are in HH:MM format.
281+
// Note that durations can be arbitrarily long, but parsing in this way
282+
// automatically validates that the hours are between 0 and 23 and the minutes are between 0 and 59.
283+
startDuration, err := time.Parse("15:04", *s.AutoUpdateStartTime)
284+
if err != nil {
285+
return fmt.Errorf("Error parsing start time: %w", err)
286+
}
287+
endDuration, err := time.Parse("15:04", *s.AutoUpdateEndTime)
288+
if err != nil {
289+
return fmt.Errorf("Error parsing end time: %w", err)
290+
}
291+
// Validate that the window is at least one hour long.
292+
// If the end time is less than the start time, the window wraps to the next day, so we need to add 24 hours to the end time in that case.
293+
if endDuration.Before(startDuration) {
294+
endDuration = endDuration.Add(24 * time.Hour)
295+
}
296+
if endDuration.Sub(startDuration) < time.Hour {
297+
return errors.New("The update window must be at least one hour long")
298+
}
299+
300+
return nil
301+
}
302+
273303
type SoftwareAutoUpdateScheduleFilter struct {
274304
Enabled *bool
275305
}

‎server/fleet/software_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,165 @@ func TestHostSoftwareEntryMarshalJSON(t *testing.T) {
262262

263263
assert.JSONEq(t, expectedJSON, string(data))
264264
}
265+
266+
func TestAutoUpdateScheduleValidation(t *testing.T) {
267+
testCases := []struct {
268+
name string
269+
schedule SoftwareAutoUpdateSchedule
270+
isValid bool
271+
}{
272+
{
273+
name: "schedule disabled",
274+
schedule: SoftwareAutoUpdateSchedule{
275+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
276+
AutoUpdateEnabled: ptr.Bool(false),
277+
AutoUpdateStartTime: nil,
278+
AutoUpdateEndTime: nil,
279+
},
280+
},
281+
isValid: true,
282+
},
283+
{
284+
name: "missing start time",
285+
schedule: SoftwareAutoUpdateSchedule{
286+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
287+
AutoUpdateEnabled: ptr.Bool(true),
288+
AutoUpdateStartTime: nil,
289+
AutoUpdateEndTime: ptr.String("15:30"),
290+
},
291+
},
292+
isValid: false,
293+
},
294+
{
295+
name: "missing end time",
296+
schedule: SoftwareAutoUpdateSchedule{
297+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
298+
AutoUpdateEnabled: ptr.Bool(true),
299+
AutoUpdateStartTime: ptr.String("14:30"),
300+
AutoUpdateEndTime: nil,
301+
},
302+
},
303+
isValid: false,
304+
},
305+
{
306+
name: "empty start time",
307+
schedule: SoftwareAutoUpdateSchedule{
308+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
309+
AutoUpdateEnabled: ptr.Bool(true),
310+
AutoUpdateStartTime: ptr.String(""),
311+
AutoUpdateEndTime: ptr.String("15:30"),
312+
},
313+
},
314+
isValid: false,
315+
},
316+
{
317+
name: "empty end time",
318+
schedule: SoftwareAutoUpdateSchedule{
319+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
320+
AutoUpdateEnabled: ptr.Bool(true),
321+
AutoUpdateStartTime: ptr.String("14:30"),
322+
AutoUpdateEndTime: ptr.String(""),
323+
},
324+
},
325+
isValid: false,
326+
},
327+
{
328+
name: "valid schedule",
329+
schedule: SoftwareAutoUpdateSchedule{
330+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
331+
AutoUpdateEnabled: ptr.Bool(true),
332+
AutoUpdateStartTime: ptr.String("14:30"),
333+
AutoUpdateEndTime: ptr.String("15:30"),
334+
},
335+
},
336+
isValid: true,
337+
},
338+
{
339+
name: "valid schedule (wrapped around midnight)",
340+
schedule: SoftwareAutoUpdateSchedule{
341+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
342+
AutoUpdateEnabled: ptr.Bool(true),
343+
AutoUpdateStartTime: ptr.String("23:30"),
344+
AutoUpdateEndTime: ptr.String("00:30"),
345+
},
346+
},
347+
isValid: true,
348+
},
349+
{
350+
name: "start time invalid",
351+
schedule: SoftwareAutoUpdateSchedule{
352+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
353+
AutoUpdateEnabled: ptr.Bool(true),
354+
AutoUpdateStartTime: ptr.String("invalid"),
355+
AutoUpdateEndTime: ptr.String("15:30"),
356+
},
357+
},
358+
isValid: false,
359+
},
360+
{
361+
name: "end time invalid",
362+
schedule: SoftwareAutoUpdateSchedule{
363+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
364+
AutoUpdateEnabled: ptr.Bool(true),
365+
AutoUpdateStartTime: ptr.String("14:30"),
366+
AutoUpdateEndTime: ptr.String("invalid"),
367+
},
368+
},
369+
isValid: false,
370+
},
371+
{
372+
name: "start time hour out of range",
373+
schedule: SoftwareAutoUpdateSchedule{
374+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
375+
AutoUpdateEnabled: ptr.Bool(true),
376+
AutoUpdateStartTime: ptr.String("24:00"),
377+
AutoUpdateEndTime: ptr.String("15:30"),
378+
},
379+
},
380+
isValid: false,
381+
},
382+
{
383+
name: "end time hour out of range",
384+
schedule: SoftwareAutoUpdateSchedule{
385+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
386+
AutoUpdateEnabled: ptr.Bool(true),
387+
AutoUpdateStartTime: ptr.String("14:30"),
388+
AutoUpdateEndTime: ptr.String("24:00"),
389+
},
390+
},
391+
isValid: false,
392+
},
393+
{
394+
name: "window is less than one hour",
395+
schedule: SoftwareAutoUpdateSchedule{
396+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
397+
AutoUpdateEnabled: ptr.Bool(true),
398+
AutoUpdateStartTime: ptr.String("14:30"),
399+
AutoUpdateEndTime: ptr.String("15:29"),
400+
},
401+
},
402+
isValid: false,
403+
},
404+
{
405+
name: "window is less than one hour (wrapped around midnight)",
406+
schedule: SoftwareAutoUpdateSchedule{
407+
SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{
408+
AutoUpdateEnabled: ptr.Bool(true),
409+
AutoUpdateStartTime: ptr.String("23:30"),
410+
AutoUpdateEndTime: ptr.String("00:29"),
411+
},
412+
},
413+
isValid: false,
414+
},
415+
}
416+
for _, tc := range testCases {
417+
t.Run(tc.name, func(t *testing.T) {
418+
err := tc.schedule.WindowIsValid()
419+
if tc.isValid {
420+
assert.NoError(t, err)
421+
} else {
422+
assert.Error(t, err)
423+
}
424+
})
425+
}
426+
}

‎server/service/vpp.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,11 @@ func updateAppStoreAppEndpoint(ctx context.Context, request interface{}, svc fle
136136
Categories: req.Categories,
137137
Configuration: req.Configuration,
138138
DisplayName: req.DisplayName,
139+
SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{
140+
AutoUpdateEnabled: req.AutoUpdateEnabled,
141+
AutoUpdateStartTime: req.AutoUpdateStartTime,
142+
AutoUpdateEndTime: req.AutoUpdateEndTime,
143+
},
139144
})
140145
if err != nil {
141146
return updateAppStoreAppResponse{Err: err}, nil

0 commit comments

Comments
 (0)