Github user pnowojski commented on a diff in the pull request:
https://github.com/apache/flink/pull/6355#discussion_r204336114
--- Diff:
flink-runtime/src/main/java/org/apache/flink/runtime/net/SSLUtils.java ---
@@ -163,80 +163,188 @@ public static void
setSSLVerifyHostname(Configuration sslConfig, SSLParameters s
}
/**
- * Creates the SSL Context for the client if SSL is configured.
+ * Configuration settings and key/trustmanager instances to set up an
SSL client connection.
+ */
+ public static class SSLClientConfiguration {
--- End diff --
What's the value of introducing `SSLClientConfiguration`? As far as I can
tell, the only point is to provide accessors to `handshakeTimeoutMS` and
`closeNotifyFlushTimeoutMs` in `NettyClient#connect`, but it complicates
initialisation by introducing one more extra obligatory step.
Wouldn't it be better to wrap `SSLContext` with our class that provides
those accessors? It seems like this would also remove the need for separate
`SSLClientConfiguration` and `SSLServerConfiguration`, since all of theirs
fields except of `handshakeTimeoutMS` and `closeNotifyFlushTimeoutMs`
are/should be private.
---
