Skip to content

Commit

Permalink
[SPARK-4223] [CORE] Support * in acls.
Browse files Browse the repository at this point in the history
SPARK-4223.

Currently we support setting view and modify acls but you have to specify a list of users. It would be nice to support * meaning all users have access.

Manual tests to verify that: "*" works for any user in:
a. Spark ui: view and kill stage.     Done.
b. Spark history server.                  Done.
c. Yarn application killing.  Done.

Author: zhuol <zhuol@yahoo-inc.com>

Closes #8398 from zhuoliu/4223.
  • Loading branch information
zhuol authored and rxin committed Sep 1, 2015
1 parent 3f63bd6 commit ec01280
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 7 deletions.
26 changes: 22 additions & 4 deletions core/src/main/scala/org/apache/spark/SecurityManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,16 @@ private[spark] class SecurityManager(sparkConf: SparkConf)
setViewAcls(Set[String](defaultUser), allowedUsers)
}

def getViewAcls: String = viewAcls.mkString(",")
/**
* Checking the existence of "*" is necessary as YARN can't recognize the "*" in "defaultuser,*"
*/
def getViewAcls: String = {
if (viewAcls.contains("*")) {
"*"
} else {
viewAcls.mkString(",")
}
}

/**
* Admin acls should be set before the view or modify acls. If you modify the admin
Expand All @@ -321,7 +330,16 @@ private[spark] class SecurityManager(sparkConf: SparkConf)
logInfo("Changing modify acls to: " + modifyAcls.mkString(","))
}

def getModifyAcls: String = modifyAcls.mkString(",")
/**
* Checking the existence of "*" is necessary as YARN can't recognize the "*" in "defaultuser,*"
*/
def getModifyAcls: String = {
if (modifyAcls.contains("*")) {
"*"

This comment has been minimized.

Copy link
@jaceklaskowski

jaceklaskowski Sep 2, 2015

Contributor

Doh! Yet another magic "number". Could we introduce a constant and give it a helpful name? Please.

} else {
modifyAcls.mkString(",")
}
}

/**
* Admin acls should be set before the view or modify acls. If you modify the admin
Expand Down Expand Up @@ -394,7 +412,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf)
def checkUIViewPermissions(user: String): Boolean = {
logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " viewAcls=" +
viewAcls.mkString(","))
!aclsEnabled || user == null || viewAcls.contains(user)
!aclsEnabled || user == null || viewAcls.contains(user) || viewAcls.contains("*")
}

/**
Expand All @@ -409,7 +427,7 @@ private[spark] class SecurityManager(sparkConf: SparkConf)
def checkModifyPermissions(user: String): Boolean = {
logDebug("user=" + user + " aclsEnabled=" + aclsEnabled() + " modifyAcls=" +
modifyAcls.mkString(","))
!aclsEnabled || user == null || modifyAcls.contains(user)
!aclsEnabled || user == null || modifyAcls.contains(user) || modifyAcls.contains("*")
}


Expand Down
41 changes: 41 additions & 0 deletions core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,47 @@ class SecurityManagerSuite extends SparkFunSuite {

}

test("set security with * in acls") {
val conf = new SparkConf
conf.set("spark.ui.acls.enable", "true")
conf.set("spark.admin.acls", "user1,user2")
conf.set("spark.ui.view.acls", "*")
conf.set("spark.modify.acls", "user4")

val securityManager = new SecurityManager(conf)
assert(securityManager.aclsEnabled() === true)

// check for viewAcls with *
assert(securityManager.checkUIViewPermissions("user1") === true)

This comment has been minimized.

Copy link
@jaceklaskowski

jaceklaskowski Sep 2, 2015

Contributor

Is === true required here? Doesn't assert check for boolean values by default?

This comment has been minimized.

Copy link
@rxin

rxin Sep 7, 2015

Contributor

want to submit a pull request?

assert(securityManager.checkUIViewPermissions("user5") === true)
assert(securityManager.checkUIViewPermissions("user6") === true)
assert(securityManager.checkModifyPermissions("user4") === true)
assert(securityManager.checkModifyPermissions("user7") === false)
assert(securityManager.checkModifyPermissions("user8") === false)

// check for modifyAcls with *
securityManager.setModifyAcls(Set("user4"), "*")
assert(securityManager.checkModifyPermissions("user7") === true)
assert(securityManager.checkModifyPermissions("user8") === true)

securityManager.setAdminAcls("user1,user2")
securityManager.setModifyAcls(Set("user1"), "user2")
securityManager.setViewAcls(Set("user1"), "user2")
assert(securityManager.checkUIViewPermissions("user5") === false)
assert(securityManager.checkUIViewPermissions("user6") === false)
assert(securityManager.checkModifyPermissions("user7") === false)
assert(securityManager.checkModifyPermissions("user8") === false)

// check for adminAcls with *
securityManager.setAdminAcls("user1,*")
securityManager.setModifyAcls(Set("user1"), "user2")
securityManager.setViewAcls(Set("user1"), "user2")
assert(securityManager.checkUIViewPermissions("user5") === true)
assert(securityManager.checkUIViewPermissions("user6") === true)
assert(securityManager.checkModifyPermissions("user7") === true)
assert(securityManager.checkModifyPermissions("user8") === true)
}

test("ssl on setup") {
val conf = SSLSampleConfigs.sparkSSLConfig()
val expectedAlgorithms = Set(
Expand Down
9 changes: 6 additions & 3 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1286,7 +1286,8 @@ Apart from these, the following properties are also available, and may be useful
<td>
Comma separated list of users/administrators that have view and modify access to all Spark jobs.
This can be used if you run on a shared cluster and have a set of administrators or devs who
help debug when things work.
help debug when things work. Putting a "*" in the list means any user can have the priviledge
of admin.
</td>
</tr>
<tr>
Expand Down Expand Up @@ -1327,7 +1328,8 @@ Apart from these, the following properties are also available, and may be useful
<td>Empty</td>
<td>
Comma separated list of users that have modify access to the Spark job. By default only the
user that started the Spark job has access to modify it (kill it for example).
user that started the Spark job has access to modify it (kill it for example). Putting a "*" in
the list means any user can have access to modify it.
</td>
</tr>
<tr>
Expand All @@ -1349,7 +1351,8 @@ Apart from these, the following properties are also available, and may be useful
<td>Empty</td>
<td>
Comma separated list of users that have view access to the Spark web ui. By default only the
user that started the Spark job has view access.
user that started the Spark job has view access. Putting a "*" in the list means any user can
have view access to this Spark job.
</td>
</tr>
</table>
Expand Down

0 comments on commit ec01280

Please sign in to comment.