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 <[email protected]>
googleforgames · Mar 20, 2024 · a1a1cd9 · a1a1cd9
1 parent 8685515
commit a1a1cd9
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 0 deletions.
diff --git 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
@@ -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()