[yunikorn-k8shim] branch master updated: [YUNIKORN-1771] Validate configured default queue name (#649)

[cc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ccondit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/yunikorn-k8shim.git


The following commit(s) were added to refs/heads/master by this push:
     new 4cfbda0a [YUNIKORN-1771] Validate configured default queue name (#649)
4cfbda0a is described below

commit 4cfbda0a9c9ea690e05d3da1aa663d330046aa87
Author: Mit Desai <mi...@gmail.com>
AuthorDate: Fri Aug 4 13:39:13 2023 -0500

    [YUNIKORN-1771] Validate configured default queue name (#649)
    
    Closes: #649
    
    Signed-off-by: Craig Condit <cc...@apache.org>
---
 pkg/admission/conf/am_conf.go      |  5 +++++
 pkg/admission/conf/am_conf_test.go | 10 ++++++++--
 pkg/admission/util_test.go         | 16 +++++++++++++++-
 pkg/common/constants/constants.go  |  1 +
 4 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/pkg/admission/conf/am_conf.go b/pkg/admission/conf/am_conf.go
index 60cba9bd..a079369f 100644
--- a/pkg/admission/conf/am_conf.go
+++ b/pkg/admission/conf/am_conf.go
@@ -328,6 +328,11 @@ func (acc *AdmissionControllerConf) updateConfigMaps(configMaps []*v1.ConfigMap,
 
 	// labeling
 	acc.defaultQueueName = parseConfigString(configs, AMFilteringDefaultQueueName, DefaultFilteringQueueName)
+	if acc.defaultQueueName != "" && !strings.HasPrefix(acc.defaultQueueName, constants.RootQueue) {
+		log.Log(log.AdmissionConf).Warn("invalid default queue. defaultQueue must be fully qualified. Resetting to "+DefaultFilteringQueueName,
+			zap.String(AMFilteringDefaultQueueName, acc.defaultQueueName))
+		acc.defaultQueueName = DefaultFilteringQueueName
+	}
 
 	// logging
 	log.UpdateLoggingConfig(configs)
diff --git a/pkg/admission/conf/am_conf_test.go b/pkg/admission/conf/am_conf_test.go
index 600656b6..0849647c 100644
--- a/pkg/admission/conf/am_conf_test.go
+++ b/pkg/admission/conf/am_conf_test.go
@@ -43,7 +43,7 @@ func TestConfigMapVars(t *testing.T) {
 		AMAccessControlExternalUsers:     "^yunikorn$",
 		AMAccessControlExternalGroups:    "^devs$",
 		AMAccessControlTrustControllers:  "false",
-		AMFilteringDefaultQueueName:      "default.queue",
+		AMFilteringDefaultQueueName:      "root.default.queue",
 	}}})
 	assert.Equal(t, conf.GetPolicyGroup(), "testPolicyGroup")
 	assert.Equal(t, conf.GetAmServiceName(), "testYunikornService")
@@ -58,7 +58,7 @@ func TestConfigMapVars(t *testing.T) {
 	assert.Equal(t, conf.GetExternalUsers()[0].String(), "^yunikorn$")
 	assert.Equal(t, conf.GetExternalGroups()[0].String(), "^devs$")
 	assert.Equal(t, conf.GetTrustControllers(), false)
-	assert.Equal(t, conf.GetDefaultQueueName(), "default.queue")
+	assert.Equal(t, conf.GetDefaultQueueName(), "root.default.queue")
 
 	// test missing settings
 	conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, nil})
@@ -93,6 +93,12 @@ func TestConfigMapVars(t *testing.T) {
 	}}})
 	assert.Equal(t, len(conf.GetProcessNamespaces()), 0)
 
+	// test invalid defaultQueue name
+	conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, {Data: map[string]string{
+		AMFilteringDefaultQueueName: "default.queue",
+	}}})
+	assert.Equal(t, conf.GetDefaultQueueName(), DefaultFilteringQueueName)
+
 	// test disable / enable of config hot refresh
 	conf = NewAdmissionControllerConf([]*v1.ConfigMap{nil, nil})
 
diff --git a/pkg/admission/util_test.go b/pkg/admission/util_test.go
index 9a92221c..c85554ed 100644
--- a/pkg/admission/util_test.go
+++ b/pkg/admission/util_test.go
@@ -209,7 +209,21 @@ func TestDefaultQueueName(t *testing.T) {
 		assert.Equal(t, result["random"], "random")
 		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
 		assert.Equal(t, result["disableStateAware"], "true")
-		assert.Equal(t, result["queue"], "yunikorn")
+		assert.Assert(t, result["queue"] != "yunikorn")
+	} else {
+		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
+	}
+
+	customValidQueueNameConf := createConfigWithOverrides(map[string]string{
+		conf.AMFilteringDefaultQueueName: "root.yunikorn",
+	})
+	if result := updatePodLabel(pod, customValidQueueNameConf.GetNamespace(),
+		customValidQueueNameConf.GetGenerateUniqueAppIds(), customValidQueueNameConf.GetDefaultQueueName()); result != nil {
+		assert.Equal(t, len(result), 4)
+		assert.Equal(t, result["random"], "random")
+		assert.Equal(t, result["applicationId"], "yunikorn-default-autogen")
+		assert.Equal(t, result["disableStateAware"], "true")
+		assert.Equal(t, result["queue"], "root.yunikorn")
 	} else {
 		t.Fatal("UpdatePodLabelForAdmissionController is not as expected")
 	}
diff --git a/pkg/common/constants/constants.go b/pkg/common/constants/constants.go
index 968e0c94..b9783b7b 100644
--- a/pkg/common/constants/constants.go
+++ b/pkg/common/constants/constants.go
@@ -33,6 +33,7 @@ const LabelApp = "app"
 const LabelApplicationID = "applicationId"
 const AnnotationApplicationID = "yunikorn.apache.org/app-id"
 const LabelQueueName = "queue"
+const RootQueue = "root"
 const AnnotationQueueName = "yunikorn.apache.org/queue"
 const AnnotationParentQueue = "yunikorn.apache.org/parentqueue"
 const LabelDisableStateAware = "disableStateAware"


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org