Skip to content

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

Closed
dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-57451
Closed

[SPARK-57451][CORE] Remove the check rejecting spark.authenticate.secret with Master REST server#56511
dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-57451

Conversation

@dongjoon-hyun

@dongjoon-hyun dongjoon-hyun commented Jun 15, 2026

Copy link
Copy Markdown
Member

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.

{
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.")
}

For the record, the check was introduced at Apache Spark 2.4.0. And, currently, it's outdated.

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.

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)

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Could you review this when you have some time, @peter-toth ?

@dongjoon-hyun

Copy link
Copy Markdown
Member Author

Thank you so much, @peter-toth !

dongjoon-hyun added a commit that referenced this pull request Jun 15, 2026
…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>
@dongjoon-hyun

dongjoon-hyun commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

Merged to master/4.x for now because we have Apache Spark 4.2.0 RC3 vote currently.

  • If RC3 fails, I will consider to cherry-pick this to branch-4.2 and older.
  • If RC3 succeeds, this will be Apache Spark 4.2.1.

cc @huaxingao

@dongjoon-hyun dongjoon-hyun deleted the SPARK-57451 branch June 15, 2026 06:14
dongjoon-hyun added a commit that referenced this pull request Jun 17, 2026
…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>
dongjoon-hyun added a commit that referenced this pull request Jun 17, 2026
…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>
dongjoon-hyun added a commit that referenced this pull request Jun 17, 2026
…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>
iemejia pushed a commit to iemejia/spark that referenced this pull request Jun 17, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants