Skip to content
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

Closed
wants to merge 2 commits into from
Closed

[SPARK-3429] Don't include the empty string "" as a defaultAclUser #2286

wants to merge 2 commits into from

Conversation

ash211
Copy link
Contributor

@ash211 ash211 commented Sep 5, 2014

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

@SparkQA
Copy link

SparkQA commented Sep 5, 2014

QA tests have started for PR 2286 at commit cf973a1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 5, 2014

QA tests have finished for PR 2286 at commit cf973a1.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -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(_ != "")
Copy link
Contributor

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

@tgravescs
Copy link
Contributor

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]

@ash211 ash211 changed the title Don't include the empty string "" as a defaultAclUser [SPARK-3429] Don't include the empty string "" as a defaultAclUser Sep 7, 2014
@SparkQA
Copy link

SparkQA commented Sep 7, 2014

QA tests have started for PR 2286 at commit 18cc612.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 7, 2014

QA tests have finished for PR 2286 at commit 18cc612.

  • This patch fails unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ash211
Copy link
Contributor Author

ash211 commented Sep 7, 2014

Looks like Jenkins failed for unrelated reasons

@andrewor14
Copy link
Contributor

retest this please

@andrewor14
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have started for PR 2286 at commit 18cc612.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have finished for PR 2286 at commit 18cc612.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class CreateTableAsSelect(
    • case class CreateTableAsSelect(

@andrewor14
Copy link
Contributor

Merging this into master and 1.1, thanks.

@asfgit asfgit closed this in ce59725 Sep 12, 2014
asfgit pushed a commit that referenced this pull request Sep 12, 2014
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants