Edigueh opened a new pull request, #58678:
URL: https://github.com/apache/spark/pull/58678
### What changes were proposed in this pull request?
Enable the Spark Connect gRPC server to terminate TLS and, when
configured, require and verify a client certificate on incoming
connections (mutual TLS / mTLS). Configuration is read from the standard
`spark.ssl.*` hierarchy under a new `connect` namespace
(`spark.ssl.connect.*`), parsed via `SSLOptions` -- the same machinery
that already backs `spark.ssl.ui`, `spark.ssl.historyServer`, and
`spark.ssl.rpc`.
The change adds a small helper on `SparkConnectService` that reads
`SecurityManager.getSSLOptions("connect")` and, when enabled:
- Builds a Netty `SslContext` from PEM server key material
(`certChain` + `privateKey` + optional `privateKeyPassword`).
- Optionally installs a JKS trust store (`trustStore` +
`trustStorePassword` + optional `trustStoreType`) via
`SslContextBuilder.trustManager(...)`.
- When `needClientAuth=true`, calls
`SslContextBuilder.clientAuth(ClientAuth.REQUIRE)` so the server
demands and verifies a client cert against the trust store.
- Wraps the builder with `GrpcSslContexts.configure(...)` (mandatory
for gRPC ALPN + cipher-suite requirements) and hands the result to
`NettyServerBuilder.sslContext(...)` inside `startGRPCService()`.
`SSLOptions.parse` is extended to exclude `spark.ssl.connect` from the
default `spark.ssl.enabled` inheritance (mirroring the existing carve-out
for `spark.ssl.rpc`), so TLS on the Connect server is strictly opt-in.
Fail-fast errors:
- `enabled=true` but no `certChain` / `privateKey` -> `SparkException`
naming the missing key.
- `needClientAuth=true` but no `trustStore` -> `SparkException` naming
the missing key.
- Trust store file unreadable / bad password / corrupt -> `SparkException`
wrapping the underlying `IOException` / `GeneralSecurityException`
and naming the trust-store path, so operators see an actionable
message instead of a raw stack trace at server startup.
- `spark.ssl.connect.openSslEnabled=true` -> `SparkException` at
startup rather than silently falling back to the JDK provider, so
operators do not think they are running on OpenSSL when they are not.
Silent-ignore footguns are made loud:
- `spark.ssl.connect.trustStoreReloadingEnabled=true` logs a WARN (the
trust store is loaded once at startup).
Deliberately deferred to follow-ups (each is a self-contained change):
- JKS server key material (`keyStore` / `keyStorePassword` /
`keyStoreType` / `keyPassword`) -- server cert stays PEM-only for now.
- OpenSSL provider (`openSslEnabled`): accepted config key but rejected
at startup for now; native OpenSSL wiring is the follow-up.
- `protocol` / `enabledAlgorithms` overrides.
- Reloading trust manager (`trustStoreReloadingEnabled`): full reload
support requires a daemon-thread lifecycle tied to
`SparkConnectService.stop(...)`.
- PEM trust-anchor path on `SSLOptions` (Connect trust store is JKS-only
for now, matching what `spark.ssl.rpc.trustStore` already accepts).
- Client-side mTLS on the Spark Connect Python and Scala clients
(presenting a client cert). The existing `use_ssl` option on both
clients continues to use system trust roots with no client key
material; a separate PR will extend that.
### Why are the changes needed?
Today, an operator wanting TLS or mTLS between a Connect client and the
Connect server must front the gRPC endpoint with an external
reverse-proxy sidecar (Envoy, nginx, haproxy) configured for HTTP/2 and,
for mTLS, terminating client certs there. This is operationally awkward
and duplicates configuration a native Spark deployment already carries
in its `spark.ssl.*` namespace.
Client-side TLS on Spark Connect has existed since SPARK-42533 (3.4.1);
the server side has been the gap for both plain TLS and mutual TLS.
### Does this PR introduce _any_ user-facing change?
Yes. New user-visible surface, opt-in and behavior-preserving by default:
1. A new `spark.ssl.connect.*` namespace, documented in
`docs/security.md`. Supported keys in this iteration:
- `enabled`, `certChain`, `privateKey`, `privateKeyPassword` (server
TLS)
- `needClientAuth`, `trustStore`, `trustStorePassword`,
`trustStoreType` (mTLS)
Rejected at startup with a `SparkException`: `openSslEnabled` (a
follow-up will wire native OpenSSL).
Documented as accepted-but-not-yet-honored (server logs a WARN):
`trustStoreReloadingEnabled`, `trustStoreReloadIntervalMs`.
Documented as ignored: `port` (Connect binds a single port,
`spark.connect.grpc.binding.port`, regardless of TLS).
2. `SSLOptions.parse` no longer inherits `spark.ssl.enabled` into
`spark.ssl.connect.enabled`. This matches the existing carve-out for
`spark.ssl.rpc.enabled` and is called out in `docs/security.md`. A
user who previously set `spark.ssl.enabled=true` does not accidentally
end up with TLS on the Connect endpoint until they explicitly opt in
with `spark.ssl.connect.enabled=true`.
With `spark.ssl.connect.enabled=false` (the default) the Connect
server's plaintext behavior is unchanged.
### How was this patch tested?
Two new suites totaling 15 tests, all passing locally.
`SparkConnectServiceTlsSuite` (14 unit tests) exercises the
`buildConnectSslContext` helper directly:
Server-side TLS:
- disabled default returns None,
- `spark.ssl.enabled=true` does NOT enable Connect TLS (regression guard
for the opt-in contract),
- enabled + valid PEM builds an `SslContext` that Netty accepts,
- the resulting context wires into `NettyServerBuilder.sslContext(...)`
without error (proves `GrpcSslContexts.configure(...)` did not leave
the builder in a shape gRPC rejects),
- enabled without `certChain` fails fast with a clear message,
- enabled without `privateKey` fails fast with a clear message,
- encrypted PKCS#8 private key + `privateKeyPassword` builds an
`SslContext`,
- `openSslEnabled=true` is rejected at startup with a `SparkException`
naming the key.
mTLS:
- `needClientAuth=true` without a `trustStore` fails fast, naming the
key,
- a corrupt trust store fails fast with a `SparkException` naming the
trust-store path,
- `trustStoreReloadingEnabled=true` logs a warning and still loads the
trust store statically (asserted via `withLogAppender`),
- valid client cert produces a successful TLS handshake (real
`NettyServerBuilder` + Netty gRPC client channel on an ephemeral port;
`ManagedChannel.getState(true)` reaches `READY`),
- absent client cert against an mTLS server produces
`TRANSIENT_FAILURE`,
- client cert signed by a CA NOT in the server's trust store produces
`TRANSIENT_FAILURE`.
`SparkConnectServiceTlsE2ESuite` (1 end-to-end test) boots the real
`SparkConnectService.start(sc)` via the `SparkConnectServerTest` base,
plumbing the same PEM/JKS fixtures through `extraServerConfs`. A
TLS-aware Netty client channel then connects with a valid client cert
and must reach `ConnectivityState.READY`. This guards against future
refactors of `startGRPCService()` that drop the `sb.sslContext(_)`
wire-up -- a regression the unit suite alone would not catch.
Static PKI fixtures live under
`sql/connect/server/src/test/resources/connect-tls/`, alongside a
`README.md` documenting the `openssl` / `keytool` regen script. This
mirrors the pre-existing precedent in
`common/network-common/src/test/resources/` where JKS and PEM material
are also committed.
Regression: `SSLOptionsSuite` still passes -- verifies the
`spark.ssl.connect` carve-out did not perturb the existing
`spark.ssl.rpc` behavior.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.7, claude-opus-4-7[1m])
--
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]
