fixes #3759

Signed-off-by: Piotr Ignatowski <piotr.ignatowski@ing.com>
volcano-sh · Oct 2, 2024 · 8b0edac · 8b0edac
1 parent 639a4ba
commit 8b0edac
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 1 deletion.
diff --git a/pkg/webhooks/admission/queues/validate/validate_queue.go b/pkg/webhooks/admission/queues/validate/validate_queue.go
@@ -153,8 +153,10 @@ func validateHierarchicalAttributes(queue *schedulingv1beta1.Queue, fldPath *fie
 		}
 		for _, queueInTree := range queueList.Items {
 			hierarchyInTree := queueInTree.Annotations[schedulingv1beta1.KubeHierarchyAnnotationKey]
+			// Add a "/" char to be sure, that we only compare parts that are full nodes.
+			// For example if we have in the cluster queue /root/scidev and wants to create a /root/sci
 			if hierarchyInTree != "" && queue.Name != queueInTree.Name &&
-				strings.HasPrefix(hierarchyInTree, hierarchy) {
+				strings.HasPrefix(hierarchyInTree, hierarchy+"/") {
 				return append(errs, field.Invalid(fldPath, hierarchy,
 					fmt.Sprintf("%s is not allowed to be in the sub path of %s of queue %s",
 						hierarchy, hierarchyInTree, queueInTree.Name)))

diff --git a/pkg/webhooks/admission/queues/validate/validate_queue_test.go b/pkg/webhooks/admission/queues/validate/validate_queue_test.go
@@ -271,6 +271,22 @@ func TestAdmitQueues(t *testing.T) {
 		t.Errorf("Marshal hierarchicalQueueInSubPathOfAnotherQueue failed for %v.", err)
 	}
 
+	hierarchicalQueueWithNameThatIsSubstringOfOtherQueue := schedulingv1beta1.Queue{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "hierarchical-queue-with-name-that-is-substring-of-other-queue",
+			Annotations: map[string]string{
+				schedulingv1beta1.KubeHierarchyAnnotationKey:       "root/node",
+				schedulingv1beta1.KubeHierarchyWeightAnnotationKey: "1/4",
+			},
+		},
+		Spec: schedulingv1beta1.QueueSpec{
+			Weight: 1,
+		},
+	}
+	hierarchicalQueueWithNameThatIsSubstringOfOtherQueueJSON, err := json.Marshal(hierarchicalQueueWithNameThatIsSubstringOfOtherQueue)
+	if err != nil {
+		t.Errorf("Marshal hierarchicalQueueInSubPathOfAnotherQueue failed for %v.", err)
+	}
 	config.VolcanoClient = fakeclient.NewSimpleClientset()
 	_, err = config.VolcanoClient.SchedulingV1beta1().Queues().Create(context.TODO(), &openStateForDelete, metav1.CreateOptions{})
 	if err != nil {
@@ -592,6 +608,35 @@ func TestAdmitQueues(t *testing.T) {
 				Allowed: true,
 			},
 		},
+		{
+			Name: "Normal Case Hierarchy Is A Substring of Another Queue",
+			AR: admissionv1.AdmissionReview{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "AdmissionReview",
+					APIVersion: "admission.k8s.io/v1beta1",
+				},
+				Request: &admissionv1.AdmissionRequest{
+					Kind: metav1.GroupVersionKind{
+						Group:   "scheduling.volcano.sh",
+						Version: "v1beta1",
+						Kind:    "Queue",
+					},
+					Resource: metav1.GroupVersionResource{
+						Group:    "scheduling.volcano.sh",
+						Version:  "v1beta1",
+						Resource: "queues",
+					},
+					Name:      "default",
+					Operation: "CREATE",
+					Object: runtime.RawExtension{
+						Raw: hierarchicalQueueWithNameThatIsSubstringOfOtherQueueJSON,
+					},
+				},
+			},
+			reviewResponse: &admissionv1.AdmissionResponse{
+				Allowed: true,
+			},
+		},
 		{
 			Name: "Abnormal Case default Queue Can Not Be Deleted",
 			AR: admissionv1.AdmissionReview{