steveniemitz commented on code in PR #21457:
URL: https://github.com/apache/flink/pull/21457#discussion_r1041123103


##########
flink-core/src/main/java/org/apache/flink/configuration/SecurityOptions.java:
##########
@@ -217,6 +217,16 @@ public class SecurityOptions {
                     .withDescription(
                             "Turns on SSL for external communication via the 
REST endpoints.");
 
+    @Documentation.Section(Documentation.Sections.EXPERT_SECURITY_SSL)
+    public static final ConfigOption<String> SSL_REST_SSL_CONTEXT_SUPPLIER =
+            key("security.ssl.rest.ssl-context-supplier")
+                    .stringType()
+                    .noDefaultValue()
+                    .withDescription(
+                            "A fully qualified class name that implements the 
Supplier<SslContext> interface."
+                                    + " The implementation must have a public 
constructor with the signature"
+                                    + " (Configuration config, boolean 
isClient, SslProvider sslProvider)");

Review Comment:
   I'd prefer to keep the `Supplier<SslContext>` interface at the end (or 
something similar) since it makes passing it around much simpler.  In your 
interface for example you'll need the Configuration, clientMode, and 
SslProvider at the time when you need to get the SslContext.
   
   For example, if the interface looked like this, `SSLHandlerFactory` would 
also need to then have access to the configuration and SslProvider.
   
   I ended up with making `SslContextSupplier.Factory`, which at least gets rid 
of the required constructor.



-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to