HyukjinKwon commented on code in PR #59185:
URL: https://github.com/apache/spark/pull/59185#discussion_r4163468763
##########
core/src/main/scala/org/apache/spark/deploy/security/UserCredentialManager.scala:
##########
@@ -519,6 +526,34 @@ private[spark] object UserCredentialManager extends
Logging {
}
}
+ /**
+ * Logs a warning if OIDC credential propagation is enabled but RPC channel
encryption is not
+ * configured, since credentials may then be transmitted over the RPC
channel in cleartext.
+ * Unlike [[HadoopDelegationTokenManager]]'s direct-credential-provider path
(which fails fast),
+ * this only warns, per the SPIP's default behavior (a stricter fail-fast
mode may be introduced
+ * later based on community feedback).
+ *
+ * The encryption predicate and the remediation hint are shared with
+ * [[HadoopDelegationTokenManager]]'s `require()` via [[SecurityManager]],
so the warning and the
+ * fail-fast message cannot drift apart.
+ */
+ private[security] def warnIfRpcEncryptionDisabled(sparkConf: SparkConf):
Unit = {
+ if (!SecurityManager.isRpcEncryptionEnabled(sparkConf)) {
+ // The remediation hint is emitted as plain log text (not an MDC(REASON,
...)) so it does not
Review Comment:
**Nit (P3):** This says the hint is emitted as plain log text, but the
statement below still wraps it in `MDC(LogKeys.MESSAGE,
SecurityManager.rpcEncryptionRemediation)`. With structured logging it
therefore lands in a `message` context field, not only in the plain text.
Moving it off `LogKeys.REASON` is fine; could the comment say the hint is
carried under `LogKeys.MESSAGE` (instead of `REASON`) so it matches the code?
**Verification:**
- **Inspection:** The comment's description of how the remediation hint is
emitted matches the MDC key used in the log statement it annotates.
--
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]
