[SPARK-49104][CORE][DOCS] Document `JWSFilter` usage in Spark UI and REST API and rename parameter to `secretKey` by dongjoon-hyun · Pull Request #47596 · apache/spark

dongjoon-hyun · 2024-08-04T04:38:36Z

What changes were proposed in this pull request?

This PR aims the following.

Document JWSFilter and its usage in Spark UI and REST API
- Spark UI section of Configuration page
- Spark Security page
- Spark Standalone page
Rename the parameter key to secretKey to redact it in Spark Driver UI and Spark Master UI.

Why are the changes needed?

To apply recent new security features

Does this PR introduce any user-facing change?

No because this is a new feature of Apache Spark 4.0.0.

How was this patch tested?

Pass the CIs and manual review.

spark-standalone.html
security.html
configuration.html

Was this patch authored or co-authored using generative AI tooling?

No.

…REST API and rename parameter to `secretKey`

dongjoon-hyun · 2024-08-04T04:44:01Z

 <tr>
  <td><code>spark.redaction.regex</code></td>
-  <td>(?i)secret|password|token|access[.]key</td>
+  <td>(?i)secret|password|token|access[.]?key</td>


Actually, this is missed at #47392 .

dongjoon-hyun · 2024-08-04T05:03:45Z

   */
  override def init(config: FilterConfig): Unit = {
-    key = Keys.hmacShaKeyFor(Decoders.BASE64URL.decode(config.getInitParameter("key")));
+    key = Keys.hmacShaKeyFor(Decoders.BASE64URL.decode(config.getInitParameter("secretKey")));


Currently, key can be exposed if spark.ui.filters configuration is removed mistakenly.

secretKey will be redact by Spark, right?

As you make the change #47596 (comment), do we still need to change this param to secretKey?

dongjoon-hyun · 2024-08-04T17:40:57Z

Could you review this, @viirya ?

dongjoon-hyun · 2024-08-04T22:49:57Z

Could you review this PR, @yaooqinn ?

HyukjinKwon · 2024-08-04T23:48:57Z

Merged to master.

dongjoon-hyun · 2024-08-05T00:48:02Z

Thank you, @viirya and @HyukjinKwon .

To @viirya , yes. We need to rename key to secretKey to redact. Yes, the documentation change is about to match the code.

As you make the change #47596 (comment), do we still need to change this param to secretKey?

yaooqinn · 2024-08-05T02:35:20Z

Late LGTM

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - #22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - #47595 (Apache Spark 4.0.0) - #47596 - #49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - #22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - #47595 (Apache Spark 4.0.0) - #47596 - #49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit ff36aac) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - #22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - #47595 (Apache Spark 4.0.0) - #47596 - #49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit ff36aac) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 0a95243) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - #22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - #47595 (Apache Spark 4.0.0) - #47596 - #49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit ff36aac) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 0a95243) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit a6480c1) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - #22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - #47595 (Apache Spark 4.0.0) - #47596 - #49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes #56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit ff36aac) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 0a95243) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit a6480c1) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 5b84d39) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

…cret` with Master REST server ### What changes were proposed in this pull request? This PR removes the standalone `Master` check-code that rejects `spark.authenticate.secret` when the Master REST server (`spark.master.rest.enabled`) is enabled. https://github.com/apache/spark/blob/088071d869dee0cb433c5e72ba2e7851e332b391/core/src/main/scala/org/apache/spark/deploy/master/Master.scala#L138-L144 For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated. - apache#22071 ### Why are the changes needed? `spark.authenticate.secret` (the RPC authentication secret) and `spark.master.rest.enabled` (the standalone submission REST server) are independent concerns, but the removed check-code coupled them by failing Master startup whenever both were set. Since Apache Spark 4.1.0, `spark.master.rest.enabled` defaults to `true`, this check-code forced any cluster using RPC authentication to disable the REST server. This is wrong. We don't need to block like this because the REST server is protected independently like the following. - apache#47595 (Apache Spark 4.0.0) - apache#47596 - apache#49894 (Apache Spark 4.1.0) ### Does this PR introduce _any_ user-facing change? Previously, starting a standalone Master with `spark.authenticate.secret` set and `spark.master.rest.enabled=true` (the default) failed with an `IllegalArgumentException`. After this PR, the Master starts normally with both configured securely. Although this is a bug fix by enabling a previous-blocked code path. So, technically there is no loss from the user perspective. ### How was this patch tested? Added a unit test in `MasterSuite` that verifies a `Master` can be created with both `spark.master.rest.enabled=true` and `spark.authenticate.secret` set. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) Closes apache#56511 from dongjoon-hyun/SPARK-57451. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

[SPARK-49104][CORE][DOCS] Document JWSFilter usage in Spark UI and …

cd816d8

…REST API and rename parameter to `secretKey`

github-actions Bot added WEB UI DOCS CORE labels Aug 4, 2024

dongjoon-hyun commented Aug 4, 2024

View reviewed changes

Revise

50585c9

viirya approved these changes Aug 4, 2024

View reviewed changes

HyukjinKwon approved these changes Aug 4, 2024

View reviewed changes

HyukjinKwon closed this in 4e69e16 Aug 4, 2024

dongjoon-hyun deleted the SPARK-49104 branch August 5, 2024 00:48

dongjoon-hyun mentioned this pull request Jun 15, 2026

[SPARK-57451][CORE] Remove the check rejecting spark.authenticate.secret with Master REST server #56511

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-49104][CORE][DOCS] Document `JWSFilter` usage in Spark UI and REST API and rename parameter to `secretKey`#47596

[SPARK-49104][CORE][DOCS] Document `JWSFilter` usage in Spark UI and REST API and rename parameter to `secretKey`#47596
dongjoon-hyun wants to merge 2 commits into
apache:masterfrom
dongjoon-hyun:SPARK-49104

dongjoon-hyun commented Aug 4, 2024 •

edited

Loading

Uh oh!

dongjoon-hyun Aug 4, 2024

Uh oh!

dongjoon-hyun Aug 4, 2024

Uh oh!

viirya Aug 4, 2024

Uh oh!

viirya Aug 4, 2024

Uh oh!

dongjoon-hyun commented Aug 4, 2024

Uh oh!

dongjoon-hyun commented Aug 4, 2024

Uh oh!

HyukjinKwon commented Aug 4, 2024

Uh oh!

dongjoon-hyun commented Aug 5, 2024

Uh oh!

yaooqinn commented Aug 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

dongjoon-hyun commented Aug 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

dongjoon-hyun Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

viirya Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

viirya Aug 4, 2024

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Aug 4, 2024

Uh oh!

dongjoon-hyun commented Aug 4, 2024

Uh oh!

HyukjinKwon commented Aug 4, 2024

Uh oh!

dongjoon-hyun commented Aug 5, 2024

Uh oh!

yaooqinn commented Aug 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dongjoon-hyun commented Aug 4, 2024 •

edited

Loading