Skip to content

[SPARK-16987] [None] Add spark-default.conf property to define https port for spark history server #15652

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
chie8842 wants to merge 11 commits into from
Closed

[SPARK-16987] [None] Add spark-default.conf property to define https port for spark history server #15652

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
955a82c
added secure port configuration
Oct 26, 2016
8ac0369
added configuration
Oct 26, 2016
47108b4
change config description and fix some typos.
Oct 27, 2016
3964609
Return the unintended change
Oct 27, 2016
1e2c985
deleted unnecessary brank spaces
Nov 4, 2016
935415c
fixed documentation about particular protocol
Nov 5, 2016
62aeb6c
added comment about particular protocols
Nov 6, 2016
bd945aa
reflected some comments, and improved for trivial issues in ml library.
Nov 9, 2016
9d69f02
reflected comment
chie8842 Nov 10, 2016
30a2927
rebased unrelated changes
Nov 11, 2016
83df9bc
solve conflict between global and specific configuration
chie8842 Nov 18, 2016
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
5 changes: 5 additions & 0 deletions core/src/main/scala/org/apache/spark/SSLOptions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ private[spark] case class SSLOptions(
trustStorePassword: Option[String] = None,
trustStoreType: Option[String] = None,
protocol: Option[String] = None,
port: Int = 0,
enabledAlgorithms: Set[String] = Set.empty)
extends Logging {

Expand Down Expand Up @@ -147,6 +148,7 @@ private[spark] object SSLOptions extends Logging {
* $ - `[ns].trustStorePassword` - a password to the trust-store file
* $ - `[ns].trustStoreType` - the type of trust-store
* $ - `[ns].protocol` - a protocol name supported by a particular Java version
* $ - `[ns].port` - a port number
* $ - `[ns].enabledAlgorithms` - a comma separated list of ciphers
*
* For a list of protocols and ciphers supported by particular Java versions, you may go to
Expand Down Expand Up @@ -191,6 +193,8 @@ private[spark] object SSLOptions extends Logging {
val protocol = conf.getOption(s"$ns.protocol")
.orElse(defaults.flatMap(_.protocol))

val port = conf.getInt(s"$ns.port", defaultValue = defaults.map(_.port).getOrElse(0))

val enabledAlgorithms = conf.getOption(s"$ns.enabledAlgorithms")
.map(_.split(",").map(_.trim).filter(_.nonEmpty).toSet)
.orElse(defaults.map(_.enabledAlgorithms))
Expand All @@ -207,6 +211,7 @@ private[spark] object SSLOptions extends Logging {
trustStorePassword,
trustStoreType,
protocol,
port,
enabledAlgorithms)
}

Expand Down
12 changes: 10 additions & 2 deletions core/src/main/scala/org/apache/spark/ui/JettyUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,22 @@ private[spark] object JettyUtils extends Logging {

sslOptions.createJettySslContextFactory().foreach { factory =>
// If the new port wraps around, do not try a privileged port.

require(sslOptions.port == 0 || (1024 <= sslOptions.port && sslOptions.port < 65536))

val securePort =
if (currentPort != 0) {
(currentPort + 400 - 1024) % (65536 - 1024) + 1024
if (1024 < sslOptions.port && sslOptions.port < 65536) {
Copy link
Member

@srowen srowen Nov 6, 2016

Choose a reason for hiding this comment

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

This condition should only happen when the port is 0 right? given the require above. It would be clearer to check against 0 only.

Copy link
Contributor

@tejasapatil tejasapatil Nov 6, 2016

Choose a reason for hiding this comment

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

In the require() above you have 1024 <= sslOptions.port but in this if() you have 1024 < sslOptions.port. Just wanted to check if thats intentional.

sslOptions.port
} else {
(currentPort + 400 - 1024) % (65536 - 1024) + 1024
Copy link
Contributor

@tejasapatil tejasapatil Nov 6, 2016

Choose a reason for hiding this comment

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

Can you add a comment in the code explaining the math done here ? It will help readability of the code.

}
} else {
0
}
val scheme = "https"
// Create a connector on port securePort to listen for HTTPS requests
// Create a connector on port securePort to listen for HTTPS requests.

val connector = new ServerConnector(server, factory)
connector.setPort(securePort)

Expand Down
2 changes: 2 additions & 0 deletions core/src/test/scala/org/apache/spark/SSLOptionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class SSLOptionsSuite extends SparkFunSuite with BeforeAndAfterAll {
"TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_CBC_SHA")
conf.set("spark.ui.ssl.enabledAlgorithms", "ABC, DEF")
conf.set("spark.ssl.protocol", "SSLv3")
conf.set("spark.ssl.port", "18999")

val defaultOpts = SSLOptions.parse(conf, "spark.ssl", defaults = None)
val opts = SSLOptions.parse(conf, "spark.ui.ssl", defaults = Some(defaultOpts))
Expand All @@ -128,6 +129,7 @@ class SSLOptionsSuite extends SparkFunSuite with BeforeAndAfterAll {
assert(opts.keyStorePassword === Some("12345"))
assert(opts.keyPassword === Some("password"))
assert(opts.protocol === Some("SSLv3"))
assert(opts.port === 18999)
assert(opts.enabledAlgorithms === Set("ABC", "DEF"))
}

Expand Down
9 changes: 9 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1663,6 +1663,15 @@ Apart from these, the following properties are also available, and may be useful
page.
</td>
</tr>
<tr>
<td><code>spark.ssl.port</code></td>
<td>0</td>
<td>
Port number to listen on for SSL connections.
Default value of 0 means the port will be determined automatically.
Attention that the port should be separated for each particular protocols.
Copy link
Member

@srowen srowen Nov 6, 2016

Choose a reason for hiding this comment

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

I think it would be clearer to write something like "Note that the SSL port for individual services can be overridden by setting values like spark.ssl.YYY.port as described above."

Copy link
Contributor

@tejasapatil tejasapatil Nov 6, 2016

Choose a reason for hiding this comment

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

In the code above you are ensuring that the SSL port is always between 1024 and 65536. You could document that here as "allowed range of values".

Copy link
Member

@srowen srowen Nov 9, 2016

Choose a reason for hiding this comment

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

I don't think it's clear what the last sentence means. I'd either remove it or say something like "the port can be specified for services individually, with properties like spark.ssl.YYYY.port, as described above."

</td>
</tr>
<tr>
<td><code>spark.ssl.needClientAuth</code></td>
<td>false</td>
Expand Down