Merge pull request #87313 from MikeSpreitzer/apf-validation

Update validation for API Priority and Fairness

pkg/apis/flowcontrol/BUILD

@@ -29,6 +29,7 @@ filegroup(
     srcs = [
         ":package-srcs",
         "//pkg/apis/flowcontrol/install:all-srcs",
+        "//pkg/apis/flowcontrol/internalbootstrap:all-srcs",
         "//pkg/apis/flowcontrol/util:all-srcs",
         "//pkg/apis/flowcontrol/v1alpha1:all-srcs",
         "//pkg/apis/flowcontrol/validation:all-srcs",

pkg/apis/flowcontrol/internalbootstrap/BUILD

@@ -0,0 +1,41 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["default-internal.go"],
+    importpath = "k8s.io/kubernetes/pkg/apis/flowcontrol/internalbootstrap",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/apis/flowcontrol:go_default_library",
+        "//pkg/apis/flowcontrol/install:go_default_library",
+        "//staging/src/k8s.io/api/flowcontrol/v1alpha1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["defaults_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//staging/src/k8s.io/api/flowcontrol/v1alpha1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap:go_default_library",
+    ],
+)

pkg/apis/flowcontrol/internalbootstrap/default-internal.go

@@ -0,0 +1,76 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package internalbootstrap
+
+import (
+	fcv1a1 "k8s.io/api/flowcontrol/v1alpha1"
+	"k8s.io/apimachinery/pkg/conversion"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
+	"k8s.io/kubernetes/pkg/apis/flowcontrol"
+	"k8s.io/kubernetes/pkg/apis/flowcontrol/install"
+)
+
+// MandatoryFlowSchemas holds the untyped renditions of the mandatory
+// flow schemas.  In this map the key is the schema's name and the
+// value is the `*FlowSchema`.  Nobody should mutate anything
+// reachable from this map.
+var MandatoryFlowSchemas = internalizeFSes(bootstrap.MandatoryFlowSchemas)
+
+// MandatoryPriorityLevelConfigurations holds the untyped renditions of the
+// mandatory priority level configuration objects.  In this map the
+// key is the object's name and the value is the
+// `*PriorityLevelConfiguration`.  Nobody should mutate anything
+// reachable from this map.
+var MandatoryPriorityLevelConfigurations = internalizePLs(bootstrap.MandatoryPriorityLevelConfigurations)
+
+// NewAPFScheme constructs and returns a Scheme configured to handle
+// the API object types that are used to configure API Priority and
+// Fairness
+func NewAPFScheme() *runtime.Scheme {
+	scheme := runtime.NewScheme()
+	install.Install(scheme)
+	return scheme
+}
+
+func internalizeFSes(exts []*fcv1a1.FlowSchema) map[string]*flowcontrol.FlowSchema {
+	ans := make(map[string]*flowcontrol.FlowSchema, len(exts))
+	converter := NewAPFScheme().Converter()
+	for _, ext := range exts {
+		var untyped flowcontrol.FlowSchema
+		err := converter.Convert(ext, &untyped, 0, &conversion.Meta{})
+		if err != nil {
+			panic(err)
+		}
+		ans[ext.Name] = &untyped
+	}
+	return ans
+}
+
+func internalizePLs(exts []*fcv1a1.PriorityLevelConfiguration) map[string]*flowcontrol.PriorityLevelConfiguration {
+	ans := make(map[string]*flowcontrol.PriorityLevelConfiguration, len(exts))
+	converter := NewAPFScheme().Converter()
+	for _, ext := range exts {
+		var untyped flowcontrol.PriorityLevelConfiguration
+		err := converter.Convert(ext, &untyped, 0, &conversion.Meta{})
+		if err != nil {
+			panic(err)
+		}
+		ans[ext.Name] = &untyped
+	}
+	return ans
+}

pkg/apis/flowcontrol/internalbootstrap/defaults_test.go

@@ -0,0 +1,47 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package internalbootstrap
+
+import (
+	"testing"
+
+	fcv1a1 "k8s.io/api/flowcontrol/v1alpha1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
+)
+
+func TestMandatoryAlreadyDefaulted(t *testing.T) {
+	scheme := NewAPFScheme()
+	for _, obj := range bootstrap.MandatoryFlowSchemas {
+		obj2 := obj.DeepCopyObject().(*fcv1a1.FlowSchema)
+		scheme.Default(obj2)
+		if apiequality.Semantic.DeepEqual(obj, obj2) {
+			t.Logf("Defaulting makes no change to %#+v", *obj)
+		} else {
+			t.Errorf("Defaulting changed %#+v to %#+v", *obj, *obj2)
+		}
+	}
+	for _, obj := range bootstrap.MandatoryPriorityLevelConfigurations {
+		obj2 := obj.DeepCopyObject().(*fcv1a1.PriorityLevelConfiguration)
+		scheme.Default(obj2)
+		if apiequality.Semantic.DeepEqual(obj, obj2) {
+			t.Logf("Defaulting makes no change to %#+v", *obj)
+		} else {
+			t.Errorf("Defaulting changed %#+v to %#+v", *obj, *obj2)
+		}
+	}
+}

pkg/apis/flowcontrol/types.go

@@ -33,7 +33,10 @@ const (
 
 // System preset priority level names
 const (
-	PriorityLevelConfigurationNameExempt = "exempt"
+	PriorityLevelConfigurationNameExempt   = "exempt"
+	PriorityLevelConfigurationNameCatchAll = "catch-all"
+	FlowSchemaNameExempt                   = "exempt"
+	FlowSchemaNameCatchAll                 = "catch-all"
 )
 
 // Conditions

pkg/apis/flowcontrol/validation/BUILD

@@ -8,6 +8,8 @@ go_library(
     deps = [
         "//pkg/apis/core/validation:go_default_library",
         "//pkg/apis/flowcontrol:go_default_library",
+        "//pkg/apis/flowcontrol/internalbootstrap:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/validation:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
@@ -23,6 +25,7 @@ go_test(
         "//pkg/apis/flowcontrol:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",
         "//vendor/github.com/google/go-cmp/cmp:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],

pkg/apis/flowcontrol/validation/validation.go

@@ -20,13 +20,14 @@ import (
 	"fmt"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/api/validation"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/util/shufflesharding"
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/apis/flowcontrol"
+	"k8s.io/kubernetes/pkg/apis/flowcontrol/internalbootstrap"
 )
 
 // ValidateFlowSchemaName validates name for flow-schema.
@@ -73,7 +74,17 @@ var supportedLimitResponseType = sets.NewString(
 // ValidateFlowSchema validates the content of flow-schema
 func ValidateFlowSchema(fs *flowcontrol.FlowSchema) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMeta(&fs.ObjectMeta, false, ValidateFlowSchemaName, field.NewPath("metadata"))
-	allErrs = append(allErrs, ValidateFlowSchemaSpec(&fs.Spec, field.NewPath("spec"))...)
+	specPath := field.NewPath("spec")
+	allErrs = append(allErrs, ValidateFlowSchemaSpec(fs.Name, &fs.Spec, specPath)...)
+	if mand, ok := internalbootstrap.MandatoryFlowSchemas[fs.Name]; ok {
+		// Check for almost exact equality.  This is a pretty
+		// strict test, and it is OK in this context because both
+		// sides of this comparison are intended to ultimately
+		// come from the same code.
+		if !apiequality.Semantic.DeepEqual(fs.Spec, mand.Spec) {
+			allErrs = append(allErrs, field.Invalid(specPath, fs.Spec, fmt.Sprintf("spec of '%s' must equal the fixed value", fs.Name)))
+		}
+	}
 	allErrs = append(allErrs, ValidateFlowSchemaStatus(&fs.Status, field.NewPath("status"))...)
 	return allErrs
 }
@@ -84,7 +95,7 @@ func ValidateFlowSchemaUpdate(old, fs *flowcontrol.FlowSchema) field.ErrorList {
 }
 
 // ValidateFlowSchemaSpec validates the content of flow-schema's spec
-func ValidateFlowSchemaSpec(spec *flowcontrol.FlowSchemaSpec, fldPath *field.Path) field.ErrorList {
+func ValidateFlowSchemaSpec(fsName string, spec *flowcontrol.FlowSchemaSpec, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
 	if spec.MatchingPrecedence <= 0 {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("matchingPrecedence"), spec.MatchingPrecedence, "must be a positive value"))
@@ -92,6 +103,9 @@ func ValidateFlowSchemaSpec(spec *flowcontrol.FlowSchemaSpec, fldPath *field.Pat
 	if spec.MatchingPrecedence > flowcontrol.FlowSchemaMaxMatchingPrecedence {
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("matchingPrecedence"), spec.MatchingPrecedence, fmt.Sprintf("must not be greater than %v", flowcontrol.FlowSchemaMaxMatchingPrecedence)))
 	}
+	if (spec.MatchingPrecedence == 1) && (fsName != flowcontrol.FlowSchemaNameExempt) {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("matchingPrecedence"), spec.MatchingPrecedence, "only the schema named 'exempt' may have matchingPrecedence 1"))
+	}
 	if spec.DistinguisherMethod != nil {
 		if !supportedDistinguisherMethods.Has(string(spec.DistinguisherMethod.Type)) {
 			allErrs = append(allErrs, field.NotSupported(fldPath.Child("distinguisherMethod").Child("type"), spec.DistinguisherMethod, supportedDistinguisherMethods.List()))
@@ -139,10 +153,28 @@ func ValidateFlowSchemaSubject(subject *flowcontrol.Subject, fldPath *field.Path
 	switch subject.Kind {
 	case flowcontrol.SubjectKindServiceAccount:
 		allErrs = append(allErrs, ValidateServiceAccountSubject(subject.ServiceAccount, fldPath.Child("serviceAccount"))...)
+		if subject.User != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("user"), "user is forbidden when subject kind is not 'User'"))
+		}
+		if subject.Group != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("group"), "group is forbidden when subject kind is not 'Group'"))
+		}
 	case flowcontrol.SubjectKindUser:
 		allErrs = append(allErrs, ValidateUserSubject(subject.User, fldPath.Child("user"))...)
+		if subject.ServiceAccount != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceAccount"), "serviceAccount is forbidden when subject kind is not 'ServiceAccount'"))
+		}
+		if subject.Group != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("group"), "group is forbidden when subject kind is not 'Group'"))
+		}
 	case flowcontrol.SubjectKindGroup:
 		allErrs = append(allErrs, ValidateGroupSubject(subject.Group, fldPath.Child("group"))...)
+		if subject.ServiceAccount != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("serviceAccount"), "serviceAccount is forbidden when subject kind is not 'ServiceAccount'"))
+		}
+		if subject.User != nil {
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("user"), "user is forbidden when subject kind is not 'User'"))
+		}
 	default:
 		allErrs = append(allErrs, field.NotSupported(fldPath.Child("kind"), subject.Kind, supportedSubjectKinds.List()))
 	}
@@ -152,10 +184,13 @@ func ValidateFlowSchemaSubject(subject *flowcontrol.Subject, fldPath *field.Path
 // ValidateServiceAccountSubject validates subject of "ServiceAccount" kind
 func ValidateServiceAccountSubject(subject *flowcontrol.ServiceAccountSubject, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
+	if subject == nil {
+		return append(allErrs, field.Required(fldPath, "serviceAccount is required when subject kind is 'ServiceAccount'"))
+	}
 	if len(subject.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
 	} else if subject.Name != flowcontrol.NameAll {
-		for _, msg := range validation.ValidateServiceAccountName(subject.Name, false) {
+		for _, msg := range apimachineryvalidation.ValidateServiceAccountName(subject.Name, false) {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, msg))
 		}
 	}
@@ -174,6 +209,9 @@ func ValidateServiceAccountSubject(subject *flowcontrol.ServiceAccountSubject, f
 // ValidateUserSubject validates subject of "User" kind
 func ValidateUserSubject(subject *flowcontrol.UserSubject, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
+	if subject == nil {
+		return append(allErrs, field.Required(fldPath, "user is required when subject kind is 'User'"))
+	}
 	if len(subject.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
 	}
@@ -183,6 +221,9 @@ func ValidateUserSubject(subject *flowcontrol.UserSubject, fldPath *field.Path)
 // ValidateGroupSubject validates subject of "Group" kind
 func ValidateGroupSubject(subject *flowcontrol.GroupSubject, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
+	if subject == nil {
+		return append(allErrs, field.Required(fldPath, "group is required when subject kind is 'Group'"))
+	}
 	if len(subject.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
 	}
@@ -298,7 +339,17 @@ func ValidateFlowSchemaCondition(condition *flowcontrol.FlowSchemaCondition, fld
 // ValidatePriorityLevelConfiguration validates priority-level-configuration.
 func ValidatePriorityLevelConfiguration(pl *flowcontrol.PriorityLevelConfiguration) field.ErrorList {
 	allErrs := apivalidation.ValidateObjectMeta(&pl.ObjectMeta, false, ValidatePriorityLevelConfigurationName, field.NewPath("metadata"))
-	allErrs = append(allErrs, ValidatePriorityLevelConfigurationSpec(&pl.Spec, pl.Name, field.NewPath("spec"))...)
+	specPath := field.NewPath("spec")
+	allErrs = append(allErrs, ValidatePriorityLevelConfigurationSpec(&pl.Spec, pl.Name, specPath)...)
+	if mand, ok := internalbootstrap.MandatoryPriorityLevelConfigurations[pl.Name]; ok {
+		// Check for almost exact equality.  This is a pretty
+		// strict test, and it is OK in this context because both
+		// sides of this comparison are intended to ultimately
+		// come from the same code.
+		if !apiequality.Semantic.DeepEqual(pl.Spec, mand.Spec) {
+			allErrs = append(allErrs, field.Invalid(specPath, pl.Spec, fmt.Sprintf("spec of '%s' must equal the fixed value", pl.Name)))
+		}
+	}
 	allErrs = append(allErrs, ValidatePriorityLevelConfigurationStatus(&pl.Status, field.NewPath("status"))...)
 	return allErrs
 }
@@ -311,6 +362,9 @@ func ValidatePriorityLevelConfigurationUpdate(old, pl *flowcontrol.PriorityLevel
 // ValidatePriorityLevelConfigurationSpec validates priority-level-configuration's spec.
 func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfigurationSpec, name string, fldPath *field.Path) field.ErrorList {
 	var allErrs field.ErrorList
+	if (name == flowcontrol.PriorityLevelConfigurationNameExempt) != (spec.Type == flowcontrol.PriorityLevelEnablementExempt) {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), spec.Type, "type must be 'Exempt' if and only if name is 'exempt'"))
+	}
 	switch spec.Type {
 	case flowcontrol.PriorityLevelEnablementExempt:
 		if spec.Limited != nil {
@@ -318,7 +372,7 @@ func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfi
 		}
 	case flowcontrol.PriorityLevelEnablementLimited:
 		if spec.Limited == nil {
-			allErrs = append(allErrs, field.Required(fldPath.Child("limited"), "must not be empty"))
+			allErrs = append(allErrs, field.Required(fldPath.Child("limited"), "must not be empty when type is Limited"))
 		} else {
 			allErrs = append(allErrs, ValidateLimitedPriorityLevelConfiguration(spec.Limited, fldPath.Child("limited"))...)
 		}
@@ -344,11 +398,11 @@ func ValidateLimitResponse(lr flowcontrol.LimitResponse, fldPath *field.Path) fi
 	switch lr.Type {
 	case flowcontrol.LimitResponseTypeReject:
 		if lr.Queuing != nil {
-			allErrs = append(allErrs, field.Forbidden(fldPath.Child("queuing"), "must be nil if the type is not Limited"))
+			allErrs = append(allErrs, field.Forbidden(fldPath.Child("queuing"), "must be nil if limited.limitResponse.type is not Limited"))
 		}
 	case flowcontrol.LimitResponseTypeQueue:
 		if lr.Queuing == nil {
-			allErrs = append(allErrs, field.Required(fldPath.Child("queuing"), "must not be empty"))
+			allErrs = append(allErrs, field.Required(fldPath.Child("queuing"), "must not be empty if limited.limitResponse.type is Limited"))
 		} else {
 			allErrs = append(allErrs, ValidatePriorityLevelQueuingConfiguration(lr.Queuing, fldPath.Child("queuing"))...)
 		}

pkg/apis/flowcontrol/validation/validation_test.go

@@ -24,10 +24,78 @@ import (
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/kubernetes/pkg/apis/flowcontrol"
 )
 
 func TestFlowSchemaValidation(t *testing.T) {
+	badExempt := flowcontrol.FlowSchemaSpec{
+		MatchingPrecedence: 1,
+		PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+			Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+		},
+		Rules: []flowcontrol.PolicyRulesWithSubjects{
+			{
+				Subjects: []flowcontrol.Subject{
+					{
+						Kind:  flowcontrol.SubjectKindGroup,
+						Group: &flowcontrol.GroupSubject{Name: "system:masters"},
+					},
+				},
+				ResourceRules: []flowcontrol.ResourcePolicyRule{
+					{
+						Verbs:        []string{flowcontrol.VerbAll},
+						APIGroups:    []string{flowcontrol.APIGroupAll},
+						Resources:    []string{flowcontrol.ResourceAll},
+						ClusterScope: true,
+						Namespaces:   []string{flowcontrol.NamespaceEvery},
+					},
+				},
+				NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+					{
+						Verbs:           []string{flowcontrol.VerbAll},
+						NonResourceURLs: []string{"/"},
+					},
+				},
+			},
+		},
+	}
+	badCatchAll := flowcontrol.FlowSchemaSpec{
+		MatchingPrecedence: flowcontrol.FlowSchemaMaxMatchingPrecedence,
+		PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+			Name: flowcontrol.PriorityLevelConfigurationNameCatchAll,
+		},
+		DistinguisherMethod: &flowcontrol.FlowDistinguisherMethod{Type: flowcontrol.FlowDistinguisherMethodByUserType},
+		Rules: []flowcontrol.PolicyRulesWithSubjects{
+			{
+				Subjects: []flowcontrol.Subject{
+					{
+						Kind:  flowcontrol.SubjectKindGroup,
+						Group: &flowcontrol.GroupSubject{Name: user.AllUnauthenticated},
+					},
+					{
+						Kind:  flowcontrol.SubjectKindGroup,
+						Group: &flowcontrol.GroupSubject{Name: user.AllAuthenticated},
+					},
+				},
+				ResourceRules: []flowcontrol.ResourcePolicyRule{
+					{
+						Verbs:        []string{flowcontrol.VerbAll},
+						APIGroups:    []string{flowcontrol.APIGroupAll},
+						Resources:    []string{flowcontrol.ResourceAll},
+						ClusterScope: true,
+						Namespaces:   []string{flowcontrol.NamespaceEvery},
+					},
+				},
+				NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+					{
+						Verbs:           []string{flowcontrol.VerbAll},
+						NonResourceURLs: []string{"/"},
+					},
+				},
+			},
+		},
+	}
 	testCases := []struct {
 		name           string
 		flowSchema     *flowcontrol.FlowSchema
@@ -93,6 +161,251 @@ func TestFlowSchemaValidation(t *testing.T) {
 			},
 			expectedErrors: field.ErrorList{},
 		},
+		{
+			name: "malformed Subject union in ServiceAccount case",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "system-foo",
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 50,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: "system-bar",
+					},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:  flowcontrol.SubjectKindServiceAccount,
+									User:  &flowcontrol.UserSubject{Name: "fred"},
+									Group: &flowcontrol.GroupSubject{Name: "fred"},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{
+				field.Required(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("serviceAccount"), "serviceAccount is required when subject kind is 'ServiceAccount'"),
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("user"), "user is forbidden when subject kind is not 'User'"),
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("group"), "group is forbidden when subject kind is not 'Group'"),
+			},
+		},
+		{
+			name: "Subject union malformed in User case",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "system-foo",
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 50,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: "system-bar",
+					},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:           flowcontrol.SubjectKindUser,
+									Group:          &flowcontrol.GroupSubject{Name: "fred"},
+									ServiceAccount: &flowcontrol.ServiceAccountSubject{Namespace: "s", Name: "n"},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("serviceAccount"), "serviceAccount is forbidden when subject kind is not 'ServiceAccount'"),
+				field.Required(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("user"), "user is required when subject kind is 'User'"),
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("group"), "group is forbidden when subject kind is not 'Group'"),
+			},
+		},
+		{
+			name: "malformed Subject union in Group case",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "system-foo",
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 50,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: "system-bar",
+					},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:           flowcontrol.SubjectKindGroup,
+									User:           &flowcontrol.UserSubject{Name: "fred"},
+									ServiceAccount: &flowcontrol.ServiceAccountSubject{Namespace: "s", Name: "n"},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("serviceAccount"), "serviceAccount is forbidden when subject kind is not 'ServiceAccount'"),
+				field.Forbidden(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("user"), "user is forbidden when subject kind is not 'User'"),
+				field.Required(field.NewPath("spec").Child("rules").Index(0).Child("subjects").Index(0).Child("group"), "group is required when subject kind is 'Group'"),
+			},
+		},
+		{
+			name: "exempt flow-schema should work",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.FlowSchemaNameExempt,
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 1,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+					},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:  flowcontrol.SubjectKindGroup,
+									Group: &flowcontrol.GroupSubject{Name: "system:masters"},
+								},
+							},
+							ResourceRules: []flowcontrol.ResourcePolicyRule{
+								{
+									Verbs:        []string{flowcontrol.VerbAll},
+									APIGroups:    []string{flowcontrol.APIGroupAll},
+									Resources:    []string{flowcontrol.ResourceAll},
+									ClusterScope: true,
+									Namespaces:   []string{flowcontrol.NamespaceEvery},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{},
+		},
+		{
+			name: "bad exempt flow-schema should fail",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.FlowSchemaNameExempt,
+				},
+				Spec: badExempt,
+			},
+			expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badExempt, "spec of 'exempt' must equal the fixed value")},
+		},
+		{
+			name: "bad catch-all flow-schema should fail",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.FlowSchemaNameCatchAll,
+				},
+				Spec: badCatchAll,
+			},
+			expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badCatchAll, "spec of 'catch-all' must equal the fixed value")},
+		},
+		{
+			name: "catch-all flow-schema should work",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.FlowSchemaNameCatchAll,
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 10000,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: flowcontrol.PriorityLevelConfigurationNameCatchAll,
+					},
+					DistinguisherMethod: &flowcontrol.FlowDistinguisherMethod{Type: flowcontrol.FlowDistinguisherMethodByUserType},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:  flowcontrol.SubjectKindGroup,
+									Group: &flowcontrol.GroupSubject{Name: user.AllUnauthenticated},
+								},
+								{
+									Kind:  flowcontrol.SubjectKindGroup,
+									Group: &flowcontrol.GroupSubject{Name: user.AllAuthenticated},
+								},
+							},
+							ResourceRules: []flowcontrol.ResourcePolicyRule{
+								{
+									Verbs:        []string{flowcontrol.VerbAll},
+									APIGroups:    []string{flowcontrol.APIGroupAll},
+									Resources:    []string{flowcontrol.ResourceAll},
+									ClusterScope: true,
+									Namespaces:   []string{flowcontrol.NamespaceEvery},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{},
+		},
+		{
+			name: "non-exempt flow-schema with matchingPrecedence==1 should fail",
+			flowSchema: &flowcontrol.FlowSchema{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "fred",
+				},
+				Spec: flowcontrol.FlowSchemaSpec{
+					MatchingPrecedence: 1,
+					PriorityLevelConfiguration: flowcontrol.PriorityLevelConfigurationReference{
+						Name: "exempt",
+					},
+					Rules: []flowcontrol.PolicyRulesWithSubjects{
+						{
+							Subjects: []flowcontrol.Subject{
+								{
+									Kind:  flowcontrol.SubjectKindGroup,
+									Group: &flowcontrol.GroupSubject{Name: "gorp"},
+								},
+							},
+							NonResourceRules: []flowcontrol.NonResourcePolicyRule{
+								{
+									Verbs:           []string{flowcontrol.VerbAll},
+									NonResourceURLs: []string{"*"},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{
+				field.Invalid(field.NewPath("spec").Child("matchingPrecedence"), int32(1), "only the schema named 'exempt' may have matchingPrecedence 1")},
+		},
 		{
 			name: "flow-schema mixes * verbs/apiGroups/resources should fail",
 			flowSchema: &flowcontrol.FlowSchema{
@@ -594,11 +907,139 @@ func TestFlowSchemaValidation(t *testing.T) {
 }
 
 func TestPriorityLevelConfigurationValidation(t *testing.T) {
+	badSpec := flowcontrol.PriorityLevelConfigurationSpec{
+		Type: flowcontrol.PriorityLevelEnablementLimited,
+		Limited: &flowcontrol.LimitedPriorityLevelConfiguration{
+			AssuredConcurrencyShares: 42,
+			LimitResponse: flowcontrol.LimitResponse{
+				Type: flowcontrol.LimitResponseTypeReject},
+		},
+	}
 	testCases := []struct {
 		name                       string
 		priorityLevelConfiguration *flowcontrol.PriorityLevelConfiguration
 		expectedErrors             field.ErrorList
 	}{
+		{
+			name: "exempt should work",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementExempt,
+				},
+			},
+			expectedErrors: field.ErrorList{},
+		},
+		{
+			name: "wrong exempt spec should fail",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.PriorityLevelConfigurationNameExempt,
+				},
+				Spec: badSpec,
+			},
+			expectedErrors: field.ErrorList{
+				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 requires more details",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "broken-limited",
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementLimited,
+				},
+			},
+			expectedErrors: field.ErrorList{field.Required(field.NewPath("spec").Child("limited"), "must not be empty when type is Limited")},
+		},
+		{
+			name: "max-in-flight should work",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "max-in-flight",
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementLimited,
+					Limited: &flowcontrol.LimitedPriorityLevelConfiguration{
+						AssuredConcurrencyShares: 42,
+						LimitResponse: flowcontrol.LimitResponse{
+							Type: flowcontrol.LimitResponseTypeReject},
+					},
+				},
+			},
+			expectedErrors: field.ErrorList{},
+		},
+		{
+			name: "forbid queuing details when not queuing",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "system-foo",
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementLimited,
+					Limited: &flowcontrol.LimitedPriorityLevelConfiguration{
+						AssuredConcurrencyShares: 100,
+						LimitResponse: flowcontrol.LimitResponse{
+							Type: flowcontrol.LimitResponseTypeReject,
+							Queuing: &flowcontrol.QueuingConfiguration{
+								Queues:           512,
+								HandSize:         4,
+								QueueLengthLimit: 100,
+							}}}},
+			},
+			expectedErrors: field.ErrorList{field.Forbidden(field.NewPath("spec").Child("limited").Child("limitResponse").Child("queuing"), "must be nil if limited.limitResponse.type is not Limited")},
+		},
+		{
+			name: "wrong backstop spec should fail",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.PriorityLevelConfigurationNameCatchAll,
+				},
+				Spec: badSpec,
+			},
+			expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badSpec, "spec of 'catch-all' must equal the fixed value")},
+		},
+		{
+			name: "backstop should work",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: flowcontrol.PriorityLevelConfigurationNameCatchAll,
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementLimited,
+					Limited: &flowcontrol.LimitedPriorityLevelConfiguration{
+						AssuredConcurrencyShares: 100,
+						LimitResponse: flowcontrol.LimitResponse{
+							Type: flowcontrol.LimitResponseTypeQueue,
+							Queuing: &flowcontrol.QueuingConfiguration{
+								Queues:           128,
+								HandSize:         6,
+								QueueLengthLimit: 100,
+							}}}},
+			},
+			expectedErrors: field.ErrorList{},
+		},
+		{
+			name: "broken queuing level should fail",
+			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "system-foo",
+				},
+				Spec: flowcontrol.PriorityLevelConfigurationSpec{
+					Type: flowcontrol.PriorityLevelEnablementLimited,
+					Limited: &flowcontrol.LimitedPriorityLevelConfiguration{
+						AssuredConcurrencyShares: 100,
+						LimitResponse: flowcontrol.LimitResponse{
+							Type: flowcontrol.LimitResponseTypeQueue,
+						}}},
+			},
+			expectedErrors: field.ErrorList{field.Required(field.NewPath("spec").Child("limited").Child("limitResponse").Child("queuing"), "must not be empty if limited.limitResponse.type is Limited")},
+		},
 		{
 			name: "normal customized priority level should work",
 			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
@@ -619,18 +1060,6 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
 			},
 			expectedErrors: field.ErrorList{},
 		},
-		{
-			name: "system low priority level w/ exempt should work",
-			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: flowcontrol.PriorityLevelConfigurationNameExempt,
-				},
-				Spec: flowcontrol.PriorityLevelConfigurationSpec{
-					Type: flowcontrol.PriorityLevelEnablementExempt,
-				},
-			},
-			expectedErrors: field.ErrorList{},
-		},
 		{
 			name: "customized priority level w/ overflowing handSize/queues should fail 1",
 			priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{

staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go

@@ -33,7 +33,10 @@ const (
 
 // System preset priority level names
 const (
-	PriorityLevelConfigurationNameExempt = "exempt"
+	PriorityLevelConfigurationNameExempt   = "exempt"
+	PriorityLevelConfigurationNameCatchAll = "catch-all"
+	FlowSchemaNameExempt                   = "exempt"
+	FlowSchemaNameCatchAll                 = "catch-all"
 )
 
 // Conditions

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

@@ -25,7 +25,11 @@ import (
 	"k8s.io/apiserver/pkg/authentication/user"
 )
 
-// The objects that define an apiserver's initial behavior
+// The objects that define an apiserver's initial behavior.  The
+// registered defaulting procedures make no changes to these
+// particular objects (this is verified in the unit tests of the
+// internalbootstrap package; it can not be verified in this package
+// because that would require importing k8s.io/kubernetes).
 var (
 	MandatoryPriorityLevelConfigurations = []*flowcontrol.PriorityLevelConfiguration{
 		MandatoryPriorityLevelConfigurationExempt,
@@ -37,7 +41,7 @@ var (
 	}
 )
 
-// The objects that define an apiserver's initial behavior
+// The objects that define the current suggested additional configuration
 var (
 	SuggestedPriorityLevelConfigurations = []*flowcontrol.PriorityLevelConfiguration{
 		// "system" priority-level is for the system components that affects self-maintenance of the