dongjoon-hyun commented on code in PR #57285:
URL: https://github.com/apache/spark/pull/57285#discussion_r3676966929
##########
core/src/main/scala/org/apache/spark/internal/config/package.scala:
##########
@@ -1730,6 +1730,18 @@ package object config {
.checkValue(_ > 0, "The minimum renewal interval must be a positive time
value.")
.createWithDefaultString("30s")
+ private[spark] val CREDENTIALS_DIRECT_PROVIDERS_ENABLED =
+ ConfigBuilder("spark.security.credentials.directProviders.enabled")
Review Comment:
Unfortunately, the config namespace design looks improper to me.
`spark.security.credentials.directProviders.enabled` shares the exact shape
of the per-service toggle pattern
`spark.security.credentials.<service>.enabled`. If a provider ever registers
with `serviceName = "directProviders"`, `isServiceEnabled` would read this
config, and users may also confuse the two meanings of the same pattern.
To be safe, I guess we should not use `spark.security.credentials`
namespace. OIDC PR changes to `spark.security.oidc` namespace due to the
similar reason. I'd like to recommend the following.
```suggestion
private[spark] val DIRECT_CREDENTIAL_PROVIDERS_ENABLED =
ConfigBuilder("spark.security.directCredentialProviders.enabled")
```
(If renamed, the constant references in `HadoopDelegationTokenManager`,
`CoarseGrainedSchedulerBackend`, and `LocalSchedulerBackend`, the docs in
`docs/security.md`, the warning messages, and the raw string reads in the test
providers of `NonKerberosCredentialsSuite` need to be updated together.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
