Merge pull request #59382 from bsalamat/no_system_priority

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Disallow PriorityClass names with 'system-' prefix for user defined priority classes

**What this PR does / why we need it**:
This PR changes our Priority admission controller to disallow PriorityClass names with 'system-' prefix for user defined priority classes. Please refer to #59381 for reasons why we need this.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #59381

**Release note**:

```release-note
Disallow PriorityClass names with 'system-' prefix for user defined priority classes.
```

ref #57471
/sig scheduling
/assign @liggitt

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
-// class name is valid.
-var ValidatePriorityClassName = apivalidation.NameIsDNSSubdomain
+// ValidatePriorityClassName checks whether the given priority class name is valid.
+func ValidatePriorityClassName(name string, prefix bool) []string {
+	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

@@ -44,6 +44,8 @@ const (
 )
 
 // 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,