Skip to content

Commit

Permalink
Add autoupdate_config and autoupdate_agent_rollout validation (#50181)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@hugoShaka
hugoShaka authored Dec 17, 2024
1 parent 04d8ca1 commit 89a3d2a
Show file tree
Hide file tree
Showing 10 changed files with 1,124 additions and 24 deletions.
38 changes: 29 additions & 9 deletions api/types/autoupdate/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
248 changes: 248 additions & 0 deletions api/types/autoupdate/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
}
}
27 changes: 27 additions & 0 deletions api/types/autoupdate/rollout.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 89a3d2a

Please sign in to comment.