SPARK-1890 and SPARK-1891- add admin and modify acls #1196
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
| * Split a comma separated String, filter out any empty items, and return a Set of strings | ||
| */ | ||
| private def stringToSet(list: String): Set[String] = { | ||
| (list.split(',')).map(_.trim()).filter(!_.isEmpty).toSet |
There was a problem hiding this comment.
removed a couple.
|
Looks ok, aside from the minor semantic issue above. The comment below scares me a little bit: But I don't know enough about this code at the moment to be able to see if that's an actual problem. |
|
You can't determine the user unless some sort of authentication filter is in place. the UI returns null in that case. You can't check acls against a null user so all you can do is assume its either on or off. Since an authentication filter could choose to not filter all web UI pages, some may come back with a user and some may not. That is why we assume if there is no user everyone has access. The only way I would see around that would be to build in some sort config with a real list. We could also change this behavior for say CLI interfaces, if we want them to do something different then the web ui interfaces. |
|
QA tests have started for PR 1196. This patch merges cleanly. |
|
QA results for PR 1196: |
|
Jenkins, test this please. @tgravescs are you waiting for more feedback on this? It seems pretty good to me! |
|
QA tests have started for PR 1196. This patch merges cleanly. |
|
QA results for PR 1196: |
|
Yep, I was just waiting on a review. If you are good with it then I'll commit. |
|
merged into master and branch-1.1 |
It was easier to combine these 2 jira since they touch many of the same places. This pr adds the following: - adds modify acls - adds admin acls (list of admins/users that get added to both view and modify acls) - modify Kill button on UI to take modify acls into account - changes config name of spark.ui.acls.enable to spark.acls.enable since I choose poorly in original name. We keep backwards compatibility so people can still use spark.ui.acls.enable. The acls should apply to any web ui as well as any CLI interfaces. - send view and modify acls information on to YARN so that YARN interfaces can use (yarn cli for killing applications for example). Author: Thomas Graves <[email protected]> Closes #1196 from tgravescs/SPARK-1890 and squashes the following commits: 8292eb1 [Thomas Graves] review comments b92ec89 [Thomas Graves] remove unneeded variable from applistener 4c765f4 [Thomas Graves] Add in admin acls 72eb0ac [Thomas Graves] Add modify acls (cherry picked from commit 1c5555a) Signed-off-by: Thomas Graves <[email protected]>
It was easier to combine these 2 jira since they touch many of the same places. This pr adds the following: - adds modify acls - adds admin acls (list of admins/users that get added to both view and modify acls) - modify Kill button on UI to take modify acls into account - changes config name of spark.ui.acls.enable to spark.acls.enable since I choose poorly in original name. We keep backwards compatibility so people can still use spark.ui.acls.enable. The acls should apply to any web ui as well as any CLI interfaces. - send view and modify acls information on to YARN so that YARN interfaces can use (yarn cli for killing applications for example). Author: Thomas Graves <[email protected]> Closes apache#1196 from tgravescs/SPARK-1890 and squashes the following commits: 8292eb1 [Thomas Graves] review comments b92ec89 [Thomas Graves] remove unneeded variable from applistener 4c765f4 [Thomas Graves] Add in admin acls 72eb0ac [Thomas Graves] Add modify acls
It was easier to combine these 2 jira since they touch many of the same places. This pr adds the following: