From a1a1cd956a19e4e97fd67f6c0ad06da4259273ab Mon Sep 17 00:00:00 2001 From: lacroixthomas Date: Tue, 19 Mar 2024 20:26:21 -0400 Subject: [PATCH] Fix(Counter & Lists): Add validation for `priorities` (#3714) * fix(counter&list): Add validation on priorities * fix(counter&list): Add unit testss on validation of priorities * fix(counter&list): Fix error message --------- Co-authored-by: Thomas Lacroix --- .../allocation/v1/gameserverallocation.go | 22 +++++ .../v1/gameserverallocation_test.go | 89 +++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/pkg/apis/allocation/v1/gameserverallocation.go b/pkg/apis/allocation/v1/gameserverallocation.go index da95d73663..ed17f5e296 100644 --- a/pkg/apis/allocation/v1/gameserverallocation.go +++ b/pkg/apis/allocation/v1/gameserverallocation.go @@ -465,6 +465,25 @@ func validateLists(lists map[string]ListSelector, fldPath *field.Path) field.Err return allErrs } +// validatePriorities validates that the Priorities fields has valid values for Priorities +func validatePriorities(priorities []agonesv1.Priority, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + for index, priority := range priorities { + keyPath := fldPath.Index(index) + if priority.Type != agonesv1.GameServerPriorityCounter && priority.Type != agonesv1.GameServerPriorityList { + allErrs = append(allErrs, field.Invalid(keyPath, priority.Type, "type must be \"Counter\" or \"List\"")) + } + if priority.Key == "" { + allErrs = append(allErrs, field.Invalid(keyPath, priority.Type, "key must not be nil")) + } + if priority.Order != agonesv1.GameServerPriorityAscending && priority.Order != agonesv1.GameServerPriorityDescending { + allErrs = append(allErrs, field.Invalid(keyPath, priority.Order, "order must be \"Ascending\" or \"Descending\"")) + } + } + + return allErrs +} + // validateCounterActions validates that the Counters field has valid values for CounterActions func validateCounterActions(counters map[string]CounterAction, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList @@ -592,6 +611,9 @@ func (gsa *GameServerAllocation) Validate() field.ErrorList { } if runtime.FeatureEnabled(runtime.FeatureCountsAndLists) { + if gsa.Spec.Priorities != nil { + allErrs = append(allErrs, validatePriorities(gsa.Spec.Priorities, specPath.Child("priorities"))...) + } if gsa.Spec.Counters != nil { allErrs = append(allErrs, validateCounterActions(gsa.Spec.Counters, specPath.Child("counters"))...) } diff --git a/pkg/apis/allocation/v1/gameserverallocation_test.go b/pkg/apis/allocation/v1/gameserverallocation_test.go index 1ecd2c2ec6..d118813440 100644 --- a/pkg/apis/allocation/v1/gameserverallocation_test.go +++ b/pkg/apis/allocation/v1/gameserverallocation_test.go @@ -1041,6 +1041,95 @@ func TestGameServerListActions(t *testing.T) { } } +func TestValidatePriorities(t *testing.T) { + t.Parallel() + + runtime.FeatureTestMutex.Lock() + defer runtime.FeatureTestMutex.Unlock() + assert.NoError(t, runtime.ParseFeatures(fmt.Sprintf("%s=true", runtime.FeatureCountsAndLists))) + + fieldPath := field.NewPath("spec.Priorities") + + testScenarios := map[string]struct { + priorities []agonesv1.Priority + wantErr bool + }{ + "Valid priorities": { + priorities: []agonesv1.Priority{ + { + Type: agonesv1.GameServerPriorityList, + Key: "test", + Order: agonesv1.GameServerPriorityAscending, + }, + { + Type: agonesv1.GameServerPriorityCounter, + Key: "test", + Order: agonesv1.GameServerPriorityDescending, + }, + }, + wantErr: false, + }, + "No type": { + priorities: []agonesv1.Priority{ + { + Key: "test", + Order: agonesv1.GameServerPriorityDescending, + }, + }, + wantErr: true, + }, + "Invalid type": { + priorities: []agonesv1.Priority{ + { + Key: "test", + Type: "invalid", + Order: agonesv1.GameServerPriorityDescending, + }, + }, + wantErr: true, + }, + "No Key": { + priorities: []agonesv1.Priority{ + { + Type: agonesv1.GameServerPriorityCounter, + Order: agonesv1.GameServerPriorityDescending, + }, + }, + wantErr: true, + }, + "No Order": { + priorities: []agonesv1.Priority{ + { + Type: agonesv1.GameServerPriorityList, + Key: "test", + }, + }, + wantErr: true, + }, + "Invalid Order": { + priorities: []agonesv1.Priority{ + { + Type: agonesv1.GameServerPriorityList, + Key: "test", + Order: "invalid", + }, + }, + wantErr: true, + }, + } + + for test, testScenario := range testScenarios { + t.Run(test, func(t *testing.T) { + allErrs := validatePriorities(testScenario.priorities, fieldPath) + if testScenario.wantErr { + assert.NotNil(t, allErrs) + } else { + assert.Nil(t, allErrs) + } + }) + } +} + func TestValidateCounterActions(t *testing.T) { t.Parallel()