Moved validation to the API side

pkg/apis/scheduling/types.go

@@ -23,6 +23,9 @@ const (
 	// that do not specify any priority class and there is no priority class
 	// marked as default.
 	DefaultPriorityWhenNoDefaultClassExists = 0
+	// SystemPriorityClassPrefix is the prefix reserved for system priority class names. Other priority
+	// classes are not allowed to start with this prefix.
+	SystemPriorityClassPrefix = "system-"
 )
 
 // +genclient

pkg/apis/scheduling/validation/validation.go

@@ -17,14 +17,22 @@ limitations under the License.
 package validation
 
 import (
+	"strings"
+
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
 	"k8s.io/kubernetes/pkg/apis/scheduling"
 )
 
-// ValidatePriorityClassName can be used to check whether the given priority
+// ValidatePriorityClassName checks whether the given priority class name is valid.
-// class name is valid.
+func ValidatePriorityClassName(name string, prefix bool) []string {
-var ValidatePriorityClassName = apivalidation.NameIsDNSSubdomain
+	var allErrs []string
+	if strings.HasPrefix(name, scheduling.SystemPriorityClassPrefix) {
+		allErrs = append(allErrs, "priority class names with '"+scheduling.SystemPriorityClassPrefix+"' prefix are reserved for system use only")
+	}
+	allErrs = append(allErrs, apivalidation.NameIsDNSSubdomain(name, prefix)...)
+	return allErrs
+}
 
 // ValidatePriorityClass tests whether required fields in the PriorityClass are
 // set correctly.

pkg/apis/scheduling/validation/validation_test.go

@@ -53,6 +53,10 @@ func TestValidatePriorityClass(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{Name: "tier&1", Namespace: ""},
 			Value:      100,
 		},
+		"invalid system name": {
+			ObjectMeta: metav1.ObjectMeta{Name: scheduling.SystemPriorityClassPrefix + "test"},
+			Value:      100,
+		},
 	}
 
 	for k, v := range errorCases {

plugin/pkg/admission/priority/admission.go

@@ -19,7 +19,6 @@ package admission
 import (
 	"fmt"
 	"io"
-	"strings"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
@@ -42,12 +41,11 @@ const (
 	HighestUserDefinablePriority = 1000000000
 	// SystemCriticalPriority is the beginning of the range of priority values for critical system components.
 	SystemCriticalPriority = 2 * HighestUserDefinablePriority
-	// SystemPriorityClassPrefix is the prefix reserved for system priority class names. Other priority
-	// classes are not allowed to start with this prefix.
-	SystemPriorityClassPrefix = "system-"
 )
 
 // SystemPriorityClasses defines special priority classes which are used by system critical pods that should not be preempted by workload pods.
+// NOTE: In order to avoid conflict of names with user-defined priority classes, all the names must
+// start with scheduling.SystemPriorityClassPrefix which is by default "system-".
 var SystemPriorityClasses = map[string]int32{
 	"system-cluster-critical": SystemCriticalPriority,
 	"system-node-critical":    SystemCriticalPriority + 1000,
@@ -207,9 +205,6 @@ func (p *PriorityPlugin) validatePriorityClass(a admission.Attributes) error {
 	if pc.Value > HighestUserDefinablePriority {
 		return admission.NewForbidden(a, fmt.Errorf("maximum allowed value of a user defined priority is %v", HighestUserDefinablePriority))
 	}
-	if strings.HasPrefix(pc.Name, SystemPriorityClassPrefix) {
-		return admission.NewForbidden(a, fmt.Errorf("priority class names with '%v' prefix are reserved for system use only: %v", SystemPriorityClassPrefix, pc.Name))
-	}
 	if _, ok := SystemPriorityClasses[pc.Name]; ok {
 		return admission.NewForbidden(a, fmt.Errorf("the name of the priority class is a reserved name for system use only: %v", pc.Name))
 	}

plugin/pkg/admission/priority/admission_test.go

@@ -127,21 +127,6 @@ func TestPriorityClassAdmission(t *testing.T) {
 			systemClass,
 			true,
 		},
-		{
-			"forbidden system name prefix",
-			[]*scheduling.PriorityClass{},
-			&scheduling.PriorityClass{
-				TypeMeta: metav1.TypeMeta{
-					Kind: "PriorityClass",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "system-something",
-				},
-				Value:       5,
-				Description: "Name with 'system-' prefix is reserved for system use",
-			},
-			true,
-		},
 	}
 
 	for _, test := range tests {