[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #412: [YUNIKORN-966] Retrieve the username from the SparkApp CRD

[GitBox <gi...@apache.org>]

craigcondit commented on code in PR #412:
URL: https://github.com/apache/yunikorn-k8shim/pull/412#discussion_r881771990


##########
pkg/appmgmt/sparkoperator/spark.go:
##########
@@ -124,3 +128,21 @@ func (os *Manager) deleteApplication(obj interface{}) {
 	log.Logger().Info("spark app deleted", zap.Any("SparkApplication", app))
 	os.amProtocol.NotifyApplicationComplete(app.Status.SparkApplicationID)
 }
+

Review Comment:
   We need to ensure that the YuniKorn label takes precedence if specified. Otherwise we create a security hole if we have an environment where the username label is forced by policy (such as an admission controller that checks security credentials). Allowing Spark to override this is wrong.



##########
pkg/common/constants/constants.go:
##########
@@ -50,6 +50,9 @@ const CPU = "vcore"
 const SparkLabelAppID = "spark-app-selector"
 const SparkLabelRole = "spark-role"
 const SparkLabelRoleDriver = "driver"
+const SparkDefaultUser = "root"

Review Comment:
   Spark should not have a different user than YuniKorn. The default YuniKorn user is "nobody" - see constants.go.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org