diff --git a/pkg/apis/flowcontrol/BUILD b/pkg/apis/flowcontrol/BUILD index 7670b1cf2c9..8e4248bd19f 100644 --- a/pkg/apis/flowcontrol/BUILD +++ b/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", diff --git a/pkg/apis/flowcontrol/internalbootstrap/BUILD b/pkg/apis/flowcontrol/internalbootstrap/BUILD new file mode 100644 index 00000000000..565f63683d7 --- /dev/null +++ b/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", + ], +) diff --git a/pkg/apis/flowcontrol/internalbootstrap/default-internal.go b/pkg/apis/flowcontrol/internalbootstrap/default-internal.go new file mode 100644 index 00000000000..dd7d6753b7a --- /dev/null +++ b/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 +} diff --git a/pkg/apis/flowcontrol/internalbootstrap/defaults_test.go b/pkg/apis/flowcontrol/internalbootstrap/defaults_test.go new file mode 100644 index 00000000000..51cb9e52c5e --- /dev/null +++ b/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) + } + } +} diff --git a/pkg/apis/flowcontrol/types.go b/pkg/apis/flowcontrol/types.go index 21d3233378e..ccb145b4a91 100644 --- a/pkg/apis/flowcontrol/types.go +++ b/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 diff --git a/pkg/apis/flowcontrol/validation/BUILD b/pkg/apis/flowcontrol/validation/BUILD index 8e2aa88803a..6746b991d61 100644 --- a/pkg/apis/flowcontrol/validation/BUILD +++ b/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", ], diff --git a/pkg/apis/flowcontrol/validation/validation.go b/pkg/apis/flowcontrol/validation/validation.go index 669db97808c..d0aa1e311f9 100644 --- a/pkg/apis/flowcontrol/validation/validation.go +++ b/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"))...) } diff --git a/pkg/apis/flowcontrol/validation/validation_test.go b/pkg/apis/flowcontrol/validation/validation_test.go index 187dc4da488..be77e0e1cc2 100644 --- a/pkg/apis/flowcontrol/validation/validation_test.go +++ b/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{ diff --git a/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go b/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go index 137f480735f..b53abd0e919 100644 --- a/staging/src/k8s.io/api/flowcontrol/v1alpha1/types.go +++ b/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 diff --git a/staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default.go b/staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default.go index 9b16b70b20a..7e8962b9978 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap/default.go +++ b/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