-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-3429] Don't include the empty string "" as a defaultAclUser #2286
Conversation
QA tests have started for PR 2286 at commit
|
QA tests have finished for PR 2286 at commit
|
@@ -162,7 +162,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf) extends Logging { | |||
|
|||
// always add the current user and SPARK_USER to the viewAcls | |||
private val defaultAclUsers = Set[String](System.getProperty("user.name", ""), | |||
Option(System.getenv("SPARK_USER")).getOrElse("")) | |||
Option(System.getenv("SPARK_USER")).getOrElse("")).filter(_ != "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you change this to be !_.isEmpty
Thanks for working on this, I've been meaning to fix this for a while. Could you also please file a jira and link them. The header of the pr should include jira number like [SPARK-XXXX] |
QA tests have started for PR 2286 at commit
|
QA tests have finished for PR 2286 at commit
|
Looks like Jenkins failed for unrelated reasons |
retest this please |
LGTM |
QA tests have started for PR 2286 at commit
|
QA tests have finished for PR 2286 at commit
|
Merging this into master and 1.1, thanks. |
Changes logging from ``` 14/09/05 02:01:08 INFO SecurityManager: Changing view acls to: aash, 14/09/05 02:01:08 INFO SecurityManager: Changing modify acls to: aash, 14/09/05 02:01:08 INFO SecurityManager: SecurityManager: authentication disabled; ui acls disabled; users with view permissions: Set(aash, ); users with modify permissions: Set(aash, ) ``` to ``` 14/09/05 02:28:28 INFO SecurityManager: Changing view acls to: aash 14/09/05 02:28:28 INFO SecurityManager: Changing modify acls to: aash 14/09/05 02:28:28 INFO SecurityManager: SecurityManager: authentication disabled; ui acls disabled; users with view permissions: Set(aash); users with modify permissions: Set(aash) ``` Note that the first set of logs have a Set of size 2 containing "aash" and the empty string "" cc tgravescs Author: Andrew Ash <[email protected]> Closes #2286 from ash211/empty-default-acl and squashes the following commits: 18cc612 [Andrew Ash] Use .isEmpty instead of =="" cf973a1 [Andrew Ash] Don't include the empty string "" as a defaultAclUser (cherry picked from commit ce59725) Signed-off-by: Andrew Or <[email protected]>
Changes logging from
to
Note that the first set of logs have a Set of size 2 containing "aash" and the empty string ""
cc @tgravescs