From 50d697362068e471d82b2ad2a8fe7a80b10036ab Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Thu, 12 Dec 2024 15:22:00 -0500 Subject: [PATCH] Add autoupdate_config and autoupdate_agent_rollout validation This commit removes the restrictions of the autoupdate_agent_rollout and autoupdate_config schedules but adds groups validation. It also adds some optional server-side validation that should not be enforced at the resource level. --- api/types/autoupdate/config.go | 38 +- api/types/autoupdate/config_test.go | 248 +++++++++++++ api/types/autoupdate/rollout.go | 27 ++ api/types/autoupdate/rollout_test.go | 214 ++++++++++++ api/types/autoupdate/utils.go | 4 +- api/types/maintenance.go | 36 ++ api/types/maintenance_test.go | 80 +++++ lib/auth/autoupdate/autoupdatev1/service.go | 170 ++++++++- .../autoupdate/autoupdatev1/service_test.go | 330 ++++++++++++++++++ lib/autoupdate/rollout/reconciler.go | 1 + lib/autoupdate/rollout/reconciler_test.go | 4 +- 11 files changed, 1126 insertions(+), 26 deletions(-) diff --git a/api/types/autoupdate/config.go b/api/types/autoupdate/config.go index 32ae056195b64..ad79765895c0d 100644 --- a/api/types/autoupdate/config.go +++ b/api/types/autoupdate/config.go @@ -79,21 +79,41 @@ func ValidateAutoUpdateConfig(c *autoupdate.AutoUpdateConfig) error { return trace.BadParameter("spec.agents.maintenance_window_duration must be greater than 10 minutes when the strategy is %q", c.Spec.Agents.Strategy) } - if err := checkAgentSchedules(c.Spec.Agents.Schedules); err != nil { + if err := checkAgentSchedules(c); err != nil { return trace.Wrap(err, "validating spec.agents.schedules") } - } return nil } -func checkAgentSchedules(schedules *autoupdate.AgentAutoUpdateSchedules) error { - // TODO: change this logic when we implement group support. - // Currently we reject any non-nil schedule - // When we'll implement schedule support, we'll treat an empty schedule as the default schedule. - if schedules == nil { - return nil +func checkAgentSchedules(c *autoupdate.AutoUpdateConfig) error { + // Validate groups + groups := c.Spec.Agents.GetSchedules().GetRegular() + seenGroups := make(map[string]int, len(groups)) + for i, group := range groups { + if group.Name == "" { + return trace.BadParameter("spec.agents.schedules.regular[%d].name should not be empty", i) + } + if _, err := types.ParseWeekdays(group.Days); err != nil { + return trace.Wrap(err, "validating spec.agents.schedules.regular[%d].days", i) + } + if group.WaitHours < 0 { + return trace.BadParameter("spec.agents.schedules.regular[%d].wait_hours cannot be negative", i) + } + if group.StartHour > 23 || group.StartHour < 0 { + return trace.BadParameter("spec.agents.schedules.regular[%d].start_hour must be between 0 and 23", i) + } + if c.Spec.Agents.Strategy == AgentsStrategyTimeBased && group.WaitHours != 0 { + return trace.BadParameter("spec.agents.schedules.regular[%d].wait_hours must be zero when strategy is %s", i, AgentsStrategyTimeBased) + } + if c.Spec.Agents.Strategy == AgentsStrategyHaltOnError && i == 0 && group.WaitHours != 0 { + return trace.BadParameter("spec.agents.schedules.regular[0].wait_hours must be zero as it's the first group") + } + if conflictingGroup, ok := seenGroups[group.Name]; ok { + return trace.BadParameter("spec.agents.schedules.regular contains groups with the same name %q at indices %d and %d", group.Name, conflictingGroup, i) + } + seenGroups[group.Name] = i } - return trace.NotImplemented("agent schedules are not implemented yet") + return nil } diff --git a/api/types/autoupdate/config_test.go b/api/types/autoupdate/config_test.go index f6b6a87aa6bd8..0981dd7e681c1 100644 --- a/api/types/autoupdate/config_test.go +++ b/api/types/autoupdate/config_test.go @@ -32,6 +32,7 @@ import ( // TestNewAutoUpdateConfig verifies validation for AutoUpdateConfig resource. func TestNewAutoUpdateConfig(t *testing.T) { + t.Parallel() tests := []struct { name string spec *autoupdate.AutoUpdateConfigSpec @@ -225,3 +226,250 @@ func TestNewAutoUpdateConfig(t *testing.T) { }) } } + +func TestValidateAutoUpdateConfig(t *testing.T) { + t.Parallel() + tests := []struct { + name string + config *autoupdate.AutoUpdateConfig + assertErr require.ErrorAssertionFunc + }{ + { + name: "valid time-based rollout with groups", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyTimeBased, + MaintenanceWindowDuration: durationpb.New(time.Hour), + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", Days: []string{"*"}, WaitHours: 0}, + }, + }, + }, + }, + }, + assertErr: require.NoError, + }, + { + name: "valid halt-on-error config with groups", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", Days: []string{"*"}, WaitHours: 1}, + }, + }, + }, + }, + }, + assertErr: require.NoError, + }, + { + name: "group with negative wait days", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", Days: []string{"*"}, WaitHours: -1}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with invalid week days", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", Days: []string{"frurfday"}, WaitHours: 1}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with no week days", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", WaitHours: 1}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with empty name", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "", Days: []string{"*"}, WaitHours: 1}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "first group with non zero wait days", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 1}, + {Name: "g2", Days: []string{"*"}, WaitHours: 0}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with non zero wait days on a time-based config", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyTimeBased, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g2", Days: []string{"*"}, WaitHours: 1}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with impossible start hour", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "dark hour", Days: []string{"*"}, StartHour: 24}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "groups with same names", + config: &autoupdate.AutoUpdateConfig{ + Kind: types.KindAutoUpdateConfig, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateConfig, + }, + Spec: &autoupdate.AutoUpdateConfigSpec{ + Agents: &autoupdate.AutoUpdateConfigSpecAgents{ + Mode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + Schedules: &autoupdate.AgentAutoUpdateSchedules{ + Regular: []*autoupdate.AgentAutoUpdateGroup{ + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + {Name: "g1", Days: []string{"*"}, WaitHours: 0}, + }, + }, + }, + }, + }, + assertErr: require.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateAutoUpdateConfig(tt.config) + tt.assertErr(t, err) + }) + } +} diff --git a/api/types/autoupdate/rollout.go b/api/types/autoupdate/rollout.go index d935244af31b3..111c9a65e0095 100644 --- a/api/types/autoupdate/rollout.go +++ b/api/types/autoupdate/rollout.go @@ -72,5 +72,32 @@ func ValidateAutoUpdateAgentRollout(v *autoupdate.AutoUpdateAgentRollout) error return trace.Wrap(err, "validating spec.strategy") } + groups := v.GetStatus().GetGroups() + seenGroups := make(map[string]int, len(groups)) + for i, group := range groups { + if group.Name == "" { + return trace.BadParameter("status.groups[%d].name is empty", i) + } + if _, err := types.ParseWeekdays(group.ConfigDays); err != nil { + return trace.BadParameter("status.groups[%d].config_days is invalid", i) + } + if group.ConfigStartHour > 23 || group.ConfigStartHour < 0 { + return trace.BadParameter("spec.agents.schedules.regular[%d].start_hour must be less than or equal to 23", i) + } + if group.ConfigWaitHours < 0 { + return trace.BadParameter("status.schedules.groups[%d].config_wait_hours cannot be negative", i) + } + if v.Spec.Strategy == AgentsStrategyTimeBased && group.ConfigWaitHours != 0 { + return trace.BadParameter("status.schedules.groups[%d].config_wait_hours must be zero when strategy is %s", i, AgentsStrategyTimeBased) + } + if v.Spec.Strategy == AgentsStrategyHaltOnError && i == 0 && group.ConfigWaitHours != 0 { + return trace.BadParameter("status.schedules.groups[0].config_wait_hours must be zero as it's the first group") + } + if conflictingGroup, ok := seenGroups[group.Name]; ok { + return trace.BadParameter("spec.agents.schedules.regular contains groups with the same name %q at indices %d and %d", group.Name, conflictingGroup, i) + } + seenGroups[group.Name] = i + } + return nil } diff --git a/api/types/autoupdate/rollout_test.go b/api/types/autoupdate/rollout_test.go index 66c1b705d1568..d95ba9ef890fd 100644 --- a/api/types/autoupdate/rollout_test.go +++ b/api/types/autoupdate/rollout_test.go @@ -30,6 +30,7 @@ import ( // TestNewAutoUpdateConfig verifies validation for AutoUpdateConfig resource. func TestNewAutoUpdateAgentRollout(t *testing.T) { + t.Parallel() tests := []struct { name string spec *autoupdate.AutoUpdateAgentRolloutSpec @@ -143,3 +144,216 @@ func TestNewAutoUpdateAgentRollout(t *testing.T) { }) } } + +var ( + timeBasedRolloutSpec = autoupdate.AutoUpdateAgentRolloutSpec{ + StartVersion: "1.2.3", + TargetVersion: "2.3.4-dev", + Schedule: AgentsScheduleRegular, + AutoupdateMode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyTimeBased, + } + haltOnErrorRolloutSpec = autoupdate.AutoUpdateAgentRolloutSpec{ + StartVersion: "1.2.3", + TargetVersion: "2.3.4-dev", + Schedule: AgentsScheduleRegular, + AutoupdateMode: AgentsUpdateModeEnabled, + Strategy: AgentsStrategyHaltOnError, + } +) + +func TestValidateAutoUpdateAgentRollout(t *testing.T) { + t.Parallel() + tests := []struct { + name string + rollout *autoupdate.AutoUpdateAgentRollout + assertErr require.ErrorAssertionFunc + }{ + { + name: "valid time-based rollout with groups", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &timeBasedRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{"*"}}, + }, + }, + }, + assertErr: require.NoError, + }, + { + name: "valid halt-on-error rollout with groups", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{"*"}, ConfigWaitHours: 1}, + }, + }, + }, + assertErr: require.NoError, + }, + { + name: "group with negative wait days", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{"*"}, ConfigWaitHours: -1}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with invalid week days", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{"frurfday"}, ConfigWaitHours: 1}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with no week days", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{}, ConfigWaitHours: 1}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with empty name", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "", ConfigDays: []string{"*"}, ConfigWaitHours: 1}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "first group with non zero wait days", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}, ConfigWaitHours: 1}, + {Name: "g2", ConfigDays: []string{"*"}}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with non zero wait days on a time-based rollout", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &timeBasedRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g2", ConfigDays: []string{"*"}, ConfigWaitHours: 1}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with impossible start hour", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "dark hour", ConfigDays: []string{"*"}, ConfigStartHour: 24}, + }, + }, + }, + assertErr: require.Error, + }, + { + name: "group with same name", + rollout: &autoupdate.AutoUpdateAgentRollout{ + Kind: types.KindAutoUpdateAgentRollout, + Version: types.V1, + Metadata: &headerv1.Metadata{ + Name: types.MetaNameAutoUpdateAgentRollout, + }, + Spec: &haltOnErrorRolloutSpec, + Status: &autoupdate.AutoUpdateAgentRolloutStatus{ + Groups: []*autoupdate.AutoUpdateAgentRolloutStatusGroup{ + {Name: "g1", ConfigDays: []string{"*"}}, + {Name: "g1", ConfigDays: []string{"*"}}, + }, + }, + }, + assertErr: require.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateAutoUpdateAgentRollout(tt.rollout) + tt.assertErr(t, err) + }) + } +} diff --git a/api/types/autoupdate/utils.go b/api/types/autoupdate/utils.go index 30658c80d71ec..7fdcf3d612903 100644 --- a/api/types/autoupdate/utils.go +++ b/api/types/autoupdate/utils.go @@ -51,10 +51,8 @@ func checkToolsMode(mode string) error { func checkScheduleName(schedule string) error { switch schedule { - case AgentsScheduleImmediate: + case AgentsScheduleImmediate, AgentsScheduleRegular: return nil - case AgentsScheduleRegular: - return trace.BadParameter("regular schedule is not implemented yet") default: return trace.BadParameter("unsupported schedule type: %q", schedule) } diff --git a/api/types/maintenance.go b/api/types/maintenance.go index 65d2f7271c6fc..31a48472e6aa8 100644 --- a/api/types/maintenance.go +++ b/api/types/maintenance.go @@ -58,6 +58,42 @@ func ParseWeekday(s string) (day time.Weekday, ok bool) { return time.Sunday, false } +// ParseWeekdays attempts to parse a slice of strings representing week days. +// The slice must not be empty but can also contain a single value "*", representing the whole week. +// Day order doesn't matter but the same week day must not be present multiple times. +// In the interest of flexibility, parsing is case-insensitive and supports the common three-letter shorthand +// accepted by many common scheduling utilites (e.g. contab, systemd timers). +func ParseWeekdays(days []string) (map[time.Weekday]struct{}, error) { + if len(days) == 0 { + return nil, trace.BadParameter("empty weekdays list") + } + // Special case, we support wildcards. + if len(days) == 1 && days[0] == Wildcard { + return map[time.Weekday]struct{}{ + time.Monday: {}, + time.Tuesday: {}, + time.Wednesday: {}, + time.Thursday: {}, + time.Friday: {}, + time.Saturday: {}, + time.Sunday: {}, + }, nil + } + weekdays := make(map[time.Weekday]struct{}, 7) + for _, day := range days { + weekday, ok := ParseWeekday(day) + if !ok { + return nil, trace.BadParameter("failed to parse weekday: %v", day) + } + // Check if this is a duplicate + if _, ok := weekdays[weekday]; ok { + return nil, trace.BadParameter("duplicate weekday: %v", weekday.String()) + } + weekdays[weekday] = struct{}{} + } + return weekdays, nil +} + // generator builds a closure that iterates valid maintenance config from the current day onward. Used in // schedule export logic and tests. func (w *AgentUpgradeWindow) generator(from time.Time) func() (start time.Time, end time.Time) { diff --git a/api/types/maintenance_test.go b/api/types/maintenance_test.go index 40296dbd60f9a..5f479b53f9e80 100644 --- a/api/types/maintenance_test.go +++ b/api/types/maintenance_test.go @@ -271,3 +271,83 @@ func TestWithinUpgradeWindow(t *testing.T) { }) } } + +func TestParseWeekdays(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input []string + expect map[time.Weekday]struct{} + expectError require.ErrorAssertionFunc + }{ + { + name: "Nil slice", + input: nil, + expect: map[time.Weekday]struct{}{}, + expectError: require.Error, + }, + { + name: "Empty slice", + input: []string{}, + expect: map[time.Weekday]struct{}{}, + expectError: require.Error, + }, + { + name: "Few valid days", + input: []string{"Mon", "Tuesday", "WEDNESDAY"}, + expect: map[time.Weekday]struct{}{ + time.Monday: {}, + time.Tuesday: {}, + time.Wednesday: {}, + }, + expectError: require.NoError, + }, + { + name: "Every day", + input: []string{"Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun"}, + expect: map[time.Weekday]struct{}{ + time.Monday: {}, + time.Tuesday: {}, + time.Wednesday: {}, + time.Thursday: {}, + time.Friday: {}, + time.Saturday: {}, + time.Sunday: {}, + }, + expectError: require.NoError, + }, + { + name: "Wildcard", + input: []string{"*"}, + expect: map[time.Weekday]struct{}{ + time.Monday: {}, + time.Tuesday: {}, + time.Wednesday: {}, + time.Thursday: {}, + time.Friday: {}, + time.Saturday: {}, + time.Sunday: {}, + }, + expectError: require.NoError, + }, + { + name: "Duplicated day", + input: []string{"Mon", "Monday"}, + expect: nil, + expectError: require.Error, + }, + { + name: "Invalid days", + input: []string{"Mon", "Tuesday", "frurfday"}, + expect: nil, + expectError: require.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseWeekdays(tt.input) + tt.expectError(t, err) + require.Equal(t, tt.expect, result) + }) + } +} diff --git a/lib/auth/autoupdate/autoupdatev1/service.go b/lib/auth/autoupdate/autoupdatev1/service.go index 77baae74e4658..edc67275f6116 100644 --- a/lib/auth/autoupdate/autoupdatev1/service.go +++ b/lib/auth/autoupdate/autoupdatev1/service.go @@ -21,12 +21,14 @@ package autoupdatev1 import ( "context" "log/slog" + "maps" "github.com/gravitational/trace" "google.golang.org/protobuf/types/known/emptypb" "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types" + update "github.com/gravitational/teleport/api/types/autoupdate" apievents "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/events" @@ -122,6 +124,10 @@ func (s *Service) CreateAutoUpdateConfig(ctx context.Context, req *autoupdate.Cr return nil, trace.Wrap(err) } + if err := validateServerSideAgentConfig(req.Config); err != nil { + return nil, trace.Wrap(err) + } + config, err := s.backend.CreateAutoUpdateConfig(ctx, req.Config) var errMsg string if err != nil { @@ -162,6 +168,10 @@ func (s *Service) UpdateAutoUpdateConfig(ctx context.Context, req *autoupdate.Up return nil, trace.Wrap(err) } + if err := validateServerSideAgentConfig(req.Config); err != nil { + return nil, trace.Wrap(err) + } + config, err := s.backend.UpdateAutoUpdateConfig(ctx, req.Config) var errMsg string if err != nil { @@ -202,6 +212,10 @@ func (s *Service) UpsertAutoUpdateConfig(ctx context.Context, req *autoupdate.Up return nil, trace.Wrap(err) } + if err := validateServerSideAgentConfig(req.Config); err != nil { + return nil, trace.Wrap(err) + } + config, err := s.backend.UpsertAutoUpdateConfig(ctx, req.Config) var errMsg string if err != nil { @@ -493,10 +507,11 @@ func (s *Service) CreateAutoUpdateAgentRollout(ctx context.Context, req *autoupd // Editing the AU agent plan is restricted to cluster administrators. As of today we don't have any way of having // resources that can only be edited by Teleport Cloud (when running cloud-hosted). - // The workaround is to check if the caller has the auth system role. - // This is not ideal as it forces local tctl usage. In the future, if we expand the permission system and make cloud + // The workaround is to check if the caller has the auth/admin system role. + // This is not ideal as it forces local tctl usage and can be bypassed if the user is very creative. + // In the future, if we expand the permission system and make cloud // a first class citizen, we'll want to update this permission check. - if !authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) { + if !(authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) || authz.HasBuiltinRole(*authCtx, string(types.RoleAdmin))) { return nil, trace.AccessDenied("this request can be only executed by an auth server") } @@ -521,10 +536,11 @@ func (s *Service) UpdateAutoUpdateAgentRollout(ctx context.Context, req *autoupd // Editing the AU agent plan is restricted to cluster administrators. As of today we don't have any way of having // resources that can only be edited by Teleport Cloud (when running cloud-hosted). - // The workaround is to check if the caller has the auth system role. - // This is not ideal as it forces local tctl usage. In the future, if we expand the permission system and make cloud + // The workaround is to check if the caller has the auth/admin system role. + // This is not ideal as it forces local tctl usage and can be bypassed if the user is very creative. + // In the future, if we expand the permission system and make cloud // a first class citizen, we'll want to update this permission check. - if !authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) { + if !(authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) || authz.HasBuiltinRole(*authCtx, string(types.RoleAdmin))) { return nil, trace.AccessDenied("this request can be only executed by an auth server") } @@ -549,10 +565,11 @@ func (s *Service) UpsertAutoUpdateAgentRollout(ctx context.Context, req *autoupd // Editing the AU agent plan is restricted to cluster administrators. As of today we don't have any way of having // resources that can only be edited by Teleport Cloud (when running cloud-hosted). - // The workaround is to check if the caller has the auth system role. - // This is not ideal as it forces local tctl usage. In the future, if we expand the permission system and make cloud + // The workaround is to check if the caller has the auth/admin system role. + // This is not ideal as it forces local tctl usage and can be bypassed if the user is very creative. + // In the future, if we expand the permission system and make cloud // a first class citizen, we'll want to update this permission check. - if !authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) { + if !(authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) || authz.HasBuiltinRole(*authCtx, string(types.RoleAdmin))) { return nil, trace.AccessDenied("this request can be only executed by an auth server") } @@ -577,10 +594,11 @@ func (s *Service) DeleteAutoUpdateAgentRollout(ctx context.Context, req *autoupd // Editing the AU agent plan is restricted to cluster administrators. As of today we don't have any way of having // resources that can only be edited by Teleport Cloud (when running cloud-hosted). - // The workaround is to check if the caller has the auth system role. - // This is not ideal as it forces local tctl usage. In the future, if we expand the permission system and make cloud + // The workaround is to check if the caller has the auth/admin system role. + // This is not ideal as it forces local tctl usage and can be bypassed if the user is very creative. + // In the future, if we expand the permission system and make cloud // a first class citizen, we'll want to update this permission check. - if !authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) { + if !(authz.HasBuiltinRole(*authCtx, string(types.RoleAuth)) || authz.HasBuiltinRole(*authCtx, string(types.RoleAdmin))) { return nil, trace.AccessDenied("this request can be only executed by an auth server") } @@ -617,3 +635,131 @@ func checkAdminCloudAccess(authCtx *authz.Context) error { } return nil } + +// Those values are arbitrary, we will want to increase them as we test. We will also want to modulate them based on the +// cluster context. We don't want people to craft schedules that can't realistically finish within a week on Cloud as +// we usually do weekly updates. However, self-hosted users can craft more complex schedules, slower rollouts, and shoot +// themselves in the foot if they want. +const ( + maxGroupsTimeBasedStrategy = 20 + maxGroupsHaltOnErrorStrategy = 10 + maxGroupsHaltOnErrorStrategyCloud = 4 + maxRolloutDurationCloudHours = 72 +) + +var ( + cloudGroupUpdateDays = []string{"Mon", "Tue", "Wed", "Thu"} +) + +// validateServerSideAgentConfig validates that the autoupdate_config.agent spec meets the cluster rules. +// Rules may vary based on the cluster, and over time. +// +// This function should not be confused with api/types/autoupdate.ValidateAutoUpdateConfig which validates the integrity +// of the resource and does not enforce potentially changing rules. +func validateServerSideAgentConfig(config *autoupdate.AutoUpdateConfig) error { + agentsSpec := config.GetSpec().GetAgents() + if agentsSpec == nil { + return nil + } + // We must check resource integrity before, because it makes no sense to try to enforce rules on an invalid resource. + // The generic backend service will likely check integrity again, but it's not a large performance problem. + err := update.ValidateAutoUpdateConfig(config) + if err != nil { + return trace.Wrap(err, "validating autoupdate config") + } + + var maxGroups int + isCloud := modules.GetModules().Features().Cloud + + switch { + case isCloud && agentsSpec.GetStrategy() == update.AgentsStrategyHaltOnError: + maxGroups = maxGroupsHaltOnErrorStrategyCloud + case agentsSpec.GetStrategy() == update.AgentsStrategyHaltOnError: + maxGroups = maxGroupsHaltOnErrorStrategy + case agentsSpec.GetStrategy() == update.AgentsStrategyTimeBased: + maxGroups = maxGroupsTimeBasedStrategy + default: + return trace.BadParameter("unknown max group for strategy %v", agentsSpec.GetStrategy()) + } + + if len(agentsSpec.GetSchedules().GetRegular()) > maxGroups { + return trace.BadParameter("max groups (%d) exceeded for strategy %s, %s schedule contains %d groups", maxGroups, agentsSpec.GetStrategy(), update.AgentsScheduleRegular, len(agentsSpec.GetSchedules().GetRegular())) + } + + if !isCloud { + return nil + } + + cloudWeekdays, err := types.ParseWeekdays(cloudGroupUpdateDays) + if err != nil { + return trace.Wrap(err, "parsing cloud weekdays") + } + + for i, group := range agentsSpec.GetSchedules().GetRegular() { + weekdays, err := types.ParseWeekdays(group.Days) + if err != nil { + return trace.Wrap(err, "parsing weekdays from group %d", i) + } + + if !maps.Equal(cloudWeekdays, weekdays) { + return trace.BadParameter("weekdays must be set to %v in cloud", cloudGroupUpdateDays) + } + + } + + if duration := computeMinRolloutTime(agentsSpec.GetSchedules().GetRegular()); duration > maxRolloutDurationCloudHours { + return trace.BadParameter("rollout takes more than %d hours to complete: estimated completion time is %d hours", maxRolloutDurationCloudHours, duration) + } + + return nil +} + +func computeMinRolloutTime(groups []*autoupdate.AgentAutoUpdateGroup) int { + if len(groups) == 0 { + return 0 + } + + // We start the rollout at the first group hour, and we wait for the group to update (1 hour). + hours := groups[0].StartHour + 1 + + for _, group := range groups[1:] { + previousStartHour := (hours - 1) % 24 + previousEndHour := hours % 24 + + // compute the difference between the current hour and the group start hour + // we then check if it's less than the WaitHours, in this case we wait a day + diff := hourDifference(previousStartHour, group.StartHour) + if diff < group.WaitHours%24 { + hours += 24 + hourDifference(previousEndHour, group.StartHour) + } else { + hours += hourDifference(previousEndHour, group.StartHour) + } + + // Handle the case where WaitHours is > 24 + // This is an integer division + waitDays := group.WaitHours / 24 + // There's a special case where the difference modulo 24 is zero, the + // wait hours are non-null, but we already waited 23 hours. + // To avoid double counting we reduce the number of wait days by 1 if + // it's not zero already. + if diff == 0 { + waitDays = max(waitDays-1, 0) + } + hours += waitDays * 24 + + // We assume the group took an hour to update + hours += 1 + } + + // We remove the group start hour we added initially + return int(hours - groups[0].StartHour) +} + +// hourDifference computed the difference between two hours. +func hourDifference(a, b int32) int32 { + diff := b - a + if diff < 0 { + diff = diff + 24 + } + return diff +} diff --git a/lib/auth/autoupdate/autoupdatev1/service_test.go b/lib/auth/autoupdate/autoupdatev1/service_test.go index be71b976d698a..a8061801f36f8 100644 --- a/lib/auth/autoupdate/autoupdatev1/service_test.go +++ b/lib/auth/autoupdate/autoupdatev1/service_test.go @@ -19,8 +19,12 @@ package autoupdatev1 import ( "context" "fmt" + "github.com/gravitational/teleport/lib/modules" + "google.golang.org/protobuf/types/known/durationpb" "slices" + "strconv" "testing" + "time" "github.com/gravitational/trace" "github.com/stretchr/testify/require" @@ -450,3 +454,329 @@ func newServiceWithStorage(t *testing.T, authState authz.AdminActionAuthState, c require.NoError(t, err) return service } + +func TestComputeMinRolloutTime(t *testing.T) { + t.Parallel() + tests := []struct { + name string + groups []*autoupdatev1pb.AgentAutoUpdateGroup + expectedHours int + }{ + { + name: "nil groups", + groups: nil, + expectedHours: 0, + }, + { + name: "empty groups", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{}, + expectedHours: 0, + }, + { + name: "single group", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + }, + }, + expectedHours: 1, + }, + { + name: "two groups, same day, different start hour, no wait time", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 2, + }, + { + Name: "g2", + StartHour: 4, + }, + }, + // g1 updates from 2:00 to 3:00, g2 updates from 4:00 to 5:00, rollout updates from 2:00 to 5:00. + expectedHours: 3, + }, + { + name: "two groups, same day, same start hour, no wait time", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 2, + }, + { + Name: "g2", + StartHour: 2, + }, + }, + // g1 and g2 can't update at the same time, the g1 updates from 2:00 to 3:00 days one, + // and g2 updates from 2:00 to 3:00 the next day. Total update spans from 2:00 day 1, to 3:00 day 2 + expectedHours: 25, + }, + { + name: "two groups, cannot happen on the same day because of wait_hours", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 2, + }, + { + Name: "g2", + StartHour: 4, + WaitHours: 6, + }, + }, + // g1 updates from 2:00 to 3:00. At 4:00 g2 can't update yet, so we wait the next day. + // On day 2, g2 updates from 4:00 to 5:00. Rollout spans from 2:00 day on to 7:00 day 2. + expectedHours: 27, + }, + { + name: "two groups, wait hours is several days", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 2, + }, + { + Name: "g2", + StartHour: 4, + WaitHours: 48, + }, + }, + // g1 updates from 2:00 to 3:00. At 4:00 g2 can't update yet, so we wait 2 days. + // On day 3, g2 updates from 4:00 to 5:00. Rollout spans from 2:00 day on to 7:00 day 3. + expectedHours: 51, + }, + { + name: "two groups, one wait hour", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 2, + }, + { + Name: "g2", + StartHour: 3, + WaitHours: 1, + }, + }, + expectedHours: 2, + }, + { + name: "two groups different days", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 23, + }, + { + Name: "g2", + StartHour: 1, + }, + }, + expectedHours: 3, + }, + { + name: "two groups different days, hour diff == wait hours == 1 day", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 12, + }, + { + Name: "g2", + StartHour: 12, + WaitHours: 24, + }, + }, + expectedHours: 25, + }, + { + name: "two groups different days, hour diff == wait hours", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 12, + }, + { + Name: "g2", + StartHour: 11, + WaitHours: 23, + }, + }, + expectedHours: 24, + }, + { + name: "everything at once", + groups: []*autoupdatev1pb.AgentAutoUpdateGroup{ + { + Name: "g1", + StartHour: 23, + }, + { + Name: "g2", + StartHour: 1, + WaitHours: 4, + }, + { + Name: "g3", + StartHour: 1, + }, + { + Name: "g4", + StartHour: 10, + WaitHours: 6, + }, + }, + expectedHours: 60, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expectedHours, computeMinRolloutTime(tt.groups)) + }) + } +} + +func generateGroups(n int, days []string) []*autoupdatev1pb.AgentAutoUpdateGroup { + groups := make([]*autoupdatev1pb.AgentAutoUpdateGroup, n) + for i := range groups { + groups[i] = &autoupdatev1pb.AgentAutoUpdateGroup{ + Name: strconv.Itoa(i), + Days: days, + StartHour: int32(i % 24), + } + } + return groups +} + +func TestValidateServerSideAgentConfig(t *testing.T) { + cloudModules := &modules.TestModules{ + TestFeatures: modules.Features{ + Cloud: true, + }, + } + selfHostedModules := &modules.TestModules{ + TestFeatures: modules.Features{ + Cloud: false, + }, + } + tests := []struct { + name string + config *autoupdatev1pb.AutoUpdateConfigSpecAgents + modules modules.Modules + expectErr require.ErrorAssertionFunc + }{ + { + name: "empty agent config", + modules: selfHostedModules, + config: nil, + expectErr: require.NoError, + }, + { + name: "over max groups time-based", + modules: selfHostedModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyTimeBased, + MaintenanceWindowDuration: durationpb.New(time.Hour), + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: generateGroups(maxGroupsTimeBasedStrategy+1, cloudGroupUpdateDays), + }, + }, + expectErr: require.Error, + }, + { + name: "over max groups halt-on-error", + modules: selfHostedModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: generateGroups(maxGroupsHaltOnErrorStrategy+1, cloudGroupUpdateDays), + }, + }, + expectErr: require.Error, + }, + { + name: "over max groups halt-on-error cloud", + modules: cloudModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: generateGroups(maxGroupsHaltOnErrorStrategyCloud+1, cloudGroupUpdateDays), + }, + }, + expectErr: require.Error, + }, + { + name: "cloud should reject custom weekdays", + modules: cloudModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: generateGroups(maxGroupsHaltOnErrorStrategyCloud, []string{"Mon"}), + }, + }, + expectErr: require.Error, + }, + { + name: "self-hosted should allow custom weekdays", + modules: selfHostedModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: generateGroups(maxGroupsHaltOnErrorStrategyCloud, []string{"Mon"}), + }, + }, + expectErr: require.NoError, + }, + { + name: "cloud should reject long rollouts", + modules: cloudModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: []*autoupdatev1pb.AgentAutoUpdateGroup{ + {Name: "g1", Days: cloudGroupUpdateDays}, + {Name: "g2", Days: cloudGroupUpdateDays, WaitHours: maxRolloutDurationCloudHours}, + }, + }, + }, + expectErr: require.Error, + }, + { + name: "self-hosted should allow long rollouts", + modules: selfHostedModules, + config: &autoupdatev1pb.AutoUpdateConfigSpecAgents{ + Mode: autoupdate.AgentsUpdateModeEnabled, + Strategy: autoupdate.AgentsStrategyHaltOnError, + Schedules: &autoupdatev1pb.AgentAutoUpdateSchedules{ + Regular: []*autoupdatev1pb.AgentAutoUpdateGroup{ + {Name: "g1", Days: cloudGroupUpdateDays}, + {Name: "g2", Days: cloudGroupUpdateDays, WaitHours: maxRolloutDurationCloudHours}, + }, + }, + }, + expectErr: require.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test setup: crafing a config and setting modules + config, err := autoupdate.NewAutoUpdateConfig( + &autoupdatev1pb.AutoUpdateConfigSpec{ + Tools: nil, + Agents: tt.config, + }) + require.NoError(t, err) + modules.SetTestModules(t, tt.modules) + + // Test execution. + tt.expectErr(t, validateServerSideAgentConfig(config)) + }) + } +} diff --git a/lib/autoupdate/rollout/reconciler.go b/lib/autoupdate/rollout/reconciler.go index 29adbf732bce9..1e186156a9daa 100644 --- a/lib/autoupdate/rollout/reconciler.go +++ b/lib/autoupdate/rollout/reconciler.go @@ -47,6 +47,7 @@ const ( ) var ( + // defaultUpdateDays is the default list of days when groups can be updated. defaultUpdateDays = []string{"Mon", "Tue", "Wed", "Thu"} ) diff --git a/lib/autoupdate/rollout/reconciler_test.go b/lib/autoupdate/rollout/reconciler_test.go index 9518fe66280c2..82353213e18d7 100644 --- a/lib/autoupdate/rollout/reconciler_test.go +++ b/lib/autoupdate/rollout/reconciler_test.go @@ -594,7 +594,7 @@ func Test_makeGroupsStatus(t *testing.T) { State: autoupdate.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_UNSTARTED, LastUpdateTime: timestamppb.New(now), LastUpdateReason: updateReasonCreated, - ConfigDays: defaultUpdateDays, + ConfigDays: defaultUpdateDays(), ConfigStartHour: defaultStartHour, }, }, @@ -609,7 +609,7 @@ func Test_makeGroupsStatus(t *testing.T) { State: autoupdate.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_UNSTARTED, LastUpdateTime: timestamppb.New(now), LastUpdateReason: updateReasonCreated, - ConfigDays: defaultUpdateDays, + ConfigDays: defaultUpdateDays(), ConfigStartHour: defaultStartHour, }, },