apf: add validation to exempt for borrowing

Signed-off-by: Mike Spreitzer <mspreitz@us.ibm.com>

pkg/apis/flowcontrol/validation/validation.go

@@ -369,7 +369,14 @@ func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfi
 		if spec.Limited != nil {
 			allErrs = append(allErrs, field.Forbidden(fldPath.Child("limited"), "must be nil if the type is not Limited"))
 		}
+		if spec.Exempt != nil {
+			allErrs = append(allErrs, ValidateExemptPriorityLevelConfiguration(spec.Exempt, fldPath.Child("exempt"))...)
+		}
 	case flowcontrol.PriorityLevelEnablementLimited:
+		if spec.Exempt != nil {
+			allErrs = append(allErrs, field.Required(fldPath.Child("exempt"), "must be nil if the type is Limited"))
+		}
+
 		if spec.Limited == nil {
 			allErrs = append(allErrs, field.Required(fldPath.Child("limited"), "must not be empty when type is Limited"))
 		} else {
@@ -399,6 +406,17 @@ func ValidateLimitedPriorityLevelConfiguration(lplc *flowcontrol.LimitedPriority
 	return allErrs
 }
 
+func ValidateExemptPriorityLevelConfiguration(eplc *flowcontrol.ExemptPriorityLevelConfiguration, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+	if eplc.NominalConcurrencyShares != nil && *eplc.NominalConcurrencyShares < 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("nominalConcurrencyShares"), *eplc.NominalConcurrencyShares, "must be a non-negative integer"))
+	}
+	if eplc.LendablePercent != nil && !(*eplc.LendablePercent >= 0 && *eplc.LendablePercent <= 100) {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("lendablePercent"), *eplc.LendablePercent, "must be between 0 and 100, inclusive"))
+	}
+	return allErrs
+}
+
 func getVersionedFieldNameForConcurrencyShares(requestGV schema.GroupVersion) string {
 	switch {
 	case requestGV == flowcontrolv1alpha1.SchemeGroupVersion ||

pkg/apis/flowcontrol/validation/validation_test.go

@@ -743,6 +743,22 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
 				Type: flowcontrol.LimitResponseTypeReject},
 		},
 	}
+
+	badExemptSpec1 := flowcontrol.PriorityLevelConfigurationSpec{
+		Type: flowcontrol.PriorityLevelEnablementExempt,
+		Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
+			NominalConcurrencyShares: pointer.Int32(-1),
+			LendablePercent:          pointer.Int32(101),
+		},
+	}
+	badExemptSpec2 := flowcontrol.PriorityLevelConfigurationSpec{
+		Type: flowcontrol.PriorityLevelEnablementExempt,
+		Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
+			NominalConcurrencyShares: pointer.Int32(-1),
+			LendablePercent:          pointer.Int32(-1),
+		},
+	}
+
 	testCases := []struct {
 		name                       string
 		priorityLevelConfiguration *flowcontrol.PriorityLevelConfiguration
@@ -755,6 +771,10 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
 			},
 			Spec: flowcontrol.PriorityLevelConfigurationSpec{
 				Type: flowcontrol.PriorityLevelEnablementExempt,
+				Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
+					NominalConcurrencyShares: pointer.Int32(0),
+					LendablePercent:          pointer.Int32(0),
+				},
 			},
 		},
 		expectedErrors: field.ErrorList{},
@@ -770,6 +790,65 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
 			field.Invalid(field.NewPath("spec").Child("type"), flowcontrol.PriorityLevelEnablementLimited, "type must be 'Exempt' if and only if name is 'exempt'"),
 			field.Invalid(field.NewPath("spec"), badSpec, "spec of 'exempt' must equal the fixed value"),
 		},
+	}, {
+		name: "limited must not set exempt priority level configuration for borrowing",
+		priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "broken-limited",
+			},
+			Spec: flowcontrol.PriorityLevelConfigurationSpec{
+				Type: flowcontrol.PriorityLevelEnablementLimited,
+				Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
+					NominalConcurrencyShares: pointer.Int32(10),
+					LendablePercent:          pointer.Int32(20),
+				},
+			},
+		},
+		expectedErrors: field.ErrorList{
+			field.Required(field.NewPath("spec").Child("exempt"), "must be nil if the type is Limited"),
+			field.Required(field.NewPath("spec").Child("limited"), "must not be empty when type is Limited"),
+		},
+	}, {
+		name: "exempt priority level should have appropriate values for Exempt field",
+		priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+			},
+			Spec: badExemptSpec1,
+		},
+		expectedErrors: field.ErrorList{
+			field.Invalid(field.NewPath("spec").Child("exempt").Child("nominalConcurrencyShares"), int32(-1), "must be a non-negative integer"),
+			field.Invalid(field.NewPath("spec").Child("exempt").Child("lendablePercent"), int32(101), "must be between 0 and 100, inclusive"),
+			field.Invalid(field.NewPath("spec"), badExemptSpec1, "spec of 'exempt' must equal the fixed value"),
+		},
+	}, {
+		name: "exempt priority level should have appropriate values for Exempt field",
+		priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+			},
+			Spec: badExemptSpec2,
+		},
+		expectedErrors: field.ErrorList{
+			field.Invalid(field.NewPath("spec").Child("exempt").Child("nominalConcurrencyShares"), int32(-1), "must be a non-negative integer"),
+			field.Invalid(field.NewPath("spec").Child("exempt").Child("lendablePercent"), int32(-1), "must be between 0 and 100, inclusive"),
+			field.Invalid(field.NewPath("spec"), badExemptSpec2, "spec of 'exempt' must equal the fixed value"),
+		},
+	}, {
+		name: "limited must not set exempt priority level configuration for borrowing",
+		priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "broken-limited",
+			},
+			Spec: flowcontrol.PriorityLevelConfigurationSpec{
+				Type:   flowcontrol.PriorityLevelEnablementLimited,
+				Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{},
+			},
+		},
+		expectedErrors: field.ErrorList{
+			field.Required(field.NewPath("spec").Child("exempt"), "must be nil if the type is Limited"),
+			field.Required(field.NewPath("spec").Child("limited"), "must not be empty when type is Limited"),
+		},
 	}, {
 		name: "limited requires more details",
 		priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{

staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default.go

@@ -89,6 +89,10 @@ var (
 		flowcontrol.PriorityLevelConfigurationNameExempt,
 		flowcontrol.PriorityLevelConfigurationSpec{
 			Type: flowcontrol.PriorityLevelEnablementExempt,
+			Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
+				NominalConcurrencyShares: pointer.Int32(0),
+				LendablePercent:          pointer.Int32(0),
+			},
 		},
 	)
 	MandatoryPriorityLevelConfigurationCatchAll = newPriorityLevelConfiguration(

staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default_test.go

@@ -114,4 +114,11 @@ func TestBootstrapPriorityLevelConfigurationWithBorrowing(t *testing.T) {
 			t.Errorf("bootstrap PriorityLevelConfiguration objects not accounted by this test: %v", names)
 		}
 	}
+	exemptPL := MandatoryPriorityLevelConfigurationExempt
+	if exemptPL.Spec.Exempt.NominalConcurrencyShares != nil && *exemptPL.Spec.Exempt.NominalConcurrencyShares != 0 {
+		t.Errorf("Expected exempt priority level to have NominalConcurrencyShares==0 but got %d instead", *exemptPL.Spec.Exempt.NominalConcurrencyShares)
+	}
+	if exemptPL.Spec.Exempt.LendablePercent != nil && *exemptPL.Spec.Exempt.LendablePercent != 0 {
+		t.Errorf("Expected exempt priority level to have LendablePercent==0 but got %d instead", *exemptPL.Spec.Exempt.LendablePercent)
+	}
 }