Skip to content

[SPARK-25088][CORE][MESOS][DOCS] Update Rest Server docs & defaults. #22071

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.

Sign up for GitHub

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

Closed
squito wants to merge 2 commits into from
Closed

[SPARK-25088][CORE][MESOS][DOCS] Update Rest Server docs & defaults. #22071

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4ca224
[SPARK-25088][CORE][MESOS][DOCS] Update Rest Server docs & defaults.
squito Aug 1, 2018
897b587
feedback
squito Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private[deploy] class SparkSubmitArguments(args: Seq[String], env: Map[String, S
var driverCores: String = null
var submissionToKill: String = null
var submissionToRequestStatusFor: String = null
var useRest: Boolean = true // used internally
var useRest: Boolean = false // used internally

/** Default properties present in the currently defined defaults file. */
lazy val defaultSparkProperties: HashMap[String, String] = {
Expand Down Expand Up @@ -115,6 +115,8 @@ private[deploy] class SparkSubmitArguments(args: Seq[String], env: Map[String, S
// Use `sparkProperties` map along with env vars to fill in any missing parameters
loadEnvironmentArguments()

useRest = sparkProperties.getOrElse("spark.master.rest.enabled", "false").toBoolean

validateArguments()

/**
Expand Down
10 changes: 9 additions & 1 deletion core/src/main/scala/org/apache/spark/deploy/master/Master.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,18 @@ private[deploy] class Master(
}

// Alternative application submission gateway that is stable across Spark versions
private val restServerEnabled = conf.getBoolean("spark.master.rest.enabled", true)
private val restServerEnabled = conf.getBoolean("spark.master.rest.enabled", false)
private var restServer: Option[StandaloneRestServer] = None
private var restServerBoundPort: Option[Int] = None

{
val authKey = SecurityManager.SPARK_AUTH_SECRET_CONF
require(conf.getOption(authKey).isEmpty || !restServerEnabled,
s"The RestSubmissionServer does not support authentication via ${authKey}. Either turn " +
"off the RestSubmissionServer with spark.master.rest.enabled=false, or do not use " +
"authentication.")
}

override def onStart(): Unit = {
logInfo("Starting Spark master at " + masterUrl)
logInfo(s"Running Spark version ${org.apache.spark.SPARK_VERSION}")
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/org/apache/spark/deploy/rest/RestSubmissionServer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ private[spark] abstract class RestSubmissionServer(
val host: String,
val requestedPort: Int,
val masterConf: SparkConf) extends Logging {

protected val submitRequestServlet: SubmitRequestServlet
protected val killRequestServlet: KillRequestServlet
protected val statusRequestServlet: StatusRequestServlet
Expand Down
2 changes: 2 additions & 0 deletions docs/running-on-mesos.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ can find the results of the driver from the Mesos Web UI.

To use cluster mode, you must start the `MesosClusterDispatcher` in your cluster via the `sbin/start-mesos-dispatcher.sh` script,
passing in the Mesos master URL (e.g: mesos://host:5050). This starts the `MesosClusterDispatcher` as a daemon running on the host.
Note that the `MesosClusterDispatcher` does not support authentication. You should ensure that all network access to it is
protected (port 7077 by default).

By setting the Mesos proxy config property (requires mesos version >= 1.4), `--conf spark.mesos.proxy.baseURL=http://localhost:5050` when launching the dispatcher, the mesos sandbox URI for each driver is added to the mesos dispatcher UI.

Expand Down
7 changes: 6 additions & 1 deletion docs/security.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ secrets to be secure.

For other resource managers, `spark.authenticate.secret` must be configured on each of the nodes.
This secret will be shared by all the daemons and applications, so this deployment configuration is
not as secure as the above, especially when considering multi-tenant clusters.
not as secure as the above, especially when considering multi-tenant clusters. In this
configuration, a user with the secret can effectively impersonate any other user.

The Rest Submission Server and the MesosClusterDispatcher do not support authentication. You should
ensure that all network access to the REST API & MesosClusterDispatcher (port 6066 and 7077
respectively by default) are restricted to hosts that are trusted to submit jobs.

<table class="table">
<tr><th>Property Name</th><th>Default</th><th>Meaning</th></tr>
Expand Down
7 changes: 7 additions & 0 deletions ...-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ private[mesos] class MesosClusterDispatcher(
conf: SparkConf)
extends Logging {

{
val authKey = SecurityManager.SPARK_AUTH_SECRET_CONF
Copy link
Contributor

@tnachen tnachen Aug 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better to place this in the MesosRestServer code, since it's not really about the framework (MesosClusterDispatcher) but the RestServer receiving requests.

Copy link
Contributor Author

@squito squito Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this too, and originally wrote it that way ... but then I figured the way it works now, its really the same thing. If I put it in the MesosRestServer, it might seem like you could run the ClusterDispatcher without the RestServer somehow -- but maybe the exception itself is clear enough?

anyway, don't feel particularly strongly either way.

Copy link
Contributor

@tnachen tnachen Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, my reasoning is that it could be harder for someone looking at the code to figure out why this is not allowed, since we don't really mention about the rest server which is really the one requiring security to be turned off. Another reason it will be beneficial to have the check in the MesosRestServer is that the MesosClusterDispatcher framework could technically be decoupled from the MesosRestServer and allow another way to receive requests, so to increase flexibility and avoid someone forgetting about why we put this here, my suggestion is to move the check closer to where it's being required will help maintain this a bit better.

Copy link
Contributor

@tgravescs tgravescs Aug 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally the way I read this, I agree with @squito and it should be here. From my understanding from reading the docs on running on mesos, the user doesn't start the rest server, they start the dispatcher. The dispatcher doesn't support any sort of authentication. The fact that it uses the rest server is an implementation detail the user doesn't necessarily care about. If you change the dispatcher to have another way then it should support authentication as well or have this same error. Someone adding another way other then rest server should be made aware of this, so it being here accomplishes that.

Perhaps just adding a comment in the code about the rest server would help to clarify to developers? Or please correct me if my understanding of running the dispatcher is wrong.

Copy link
Contributor

@tgravescs tgravescs Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note if someone add another way to that supports authentication we just move it at that point.

Copy link
Contributor Author

@squito squito Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MesosClusterDispatcher framework could technically be decoupled from the MesosRestServer

I actually think that is a reason to keep it here. If somebody adds another way, then this check is still in place for the new way -- unless they consciously think about making it work, and then they'd move the check to a more appropriate spot.

I can add a comment in the code: // This doesn't support authentication because the RestSubmissionServer doesn't support it.

require(conf.getOption(authKey).isEmpty,
s"The MesosClusterDispatcher does not support authentication via ${authKey}. It is not " +
s"currently possible to run jobs in cluster mode with authentication on.")
}

private val publicAddress = Option(conf.getenv("SPARK_PUBLIC_DNS")).getOrElse(args.host)
private val recoveryMode = conf.get(RECOVERY_MODE).toUpperCase()
logInfo("Recovery mode in Mesos dispatcher set to: " + recoveryMode)
Expand Down