Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.
Comment: use default constructor for pluggable implementation

...

This KIP will introduce a new common configuration option ssl.sslfactory.class to be added to SslConfigs

A new interface PluggableSslFactory will be created. An implementation of PluggableSslFactory will use the new config keys ssl.mode and ssl.verify.keystore.using.truststore internally so they must be documented but they  will not appear as ConfigDefs in SslConfigs.

The non-default constructors of SslFactory will be deprecated and a new default constructor will be added. and a new common configuration option ssl.sslfactory

Proposed Changes

A new public interface will be defined:

public interface PluggableSslFactory extends Reconfigurable {
public SSLEngine createSslEngine(String peerHost, int peerPort);
public SSLContext sslContext();
}
A new configuration option will be added to SslConfigs:
public static final String SSL_SSLFACTORY_CLASS_CONFIG = "ssl.sslfactory.class";
public static final String SSL_SSLFACTORY_CLASS_DOC = "...";
public static final String DEFAULT_SSL_SSLFACTORY_DOCCLASS = "org.apache.kafka.common.security.ssl.SslFactory";
config.define(SslConfigs.SSL_SSLFACTORY_CLASS_CONFIG, ConfigDef.Type.CLASS, SslConfigs.DEFAULT_SSL_SSLFACTORY_CLASS, ConfigDef.Importance.LOW, SslConfigs.SSL_SSLFACTORY_CLASS_DOC)

In SaslChannelBuilder and SslChannelBuilder, the type of the member sslFactory will be changed to PluggableSslFactory. Instead of calling the SslFactory constructor directly, the sslFactory will be created we will call a new method createSslFactory(). This method will be duplicated in both channel builders.

createSslFactory() will alter (a copy of?) the configs to contain the values for what used to be passed as arguments in the SslFactory constructor. The SSL mode will be stored with the key ssl.mode; if not null clientAuthConfigOverride will be stored with the key ssl.client.auth overwriting the value already there; keystoreVerifiableUsingTruststore will be stored with the key ssl.verify.keystore.using.truststore; The new keys ssl.mode and ssl.verify.keystore.using.truststore are private. We will define them as constants in SslConfigs but they will not have associated ConfigDefs. createSslFactory() will call the default constructor of the pluggable SSL Interface by calling Class.newInstance(...) with the same constructor arguments as before) followed by a call to configure() to pass the configs.

The class comment of the PluggableSslFactory interface must describe the required constructor taking 3 arguments: (Mode mode, String clientAuthConfigOverride, boolean keystoreVerifiableUsingTruststore)The PluggableSslFactory implementation receives its configuration options in the call to configure(Map<String, ?> configs) as before.will document the necessary default constructor and the private keys ssl.mode and ssl.verify.keystore.using.truststore

Compatibility, Deprecation, and Migration Plan

This KIP is backwards compatible because the PluggableSslFactory defaults to the existing SslFactory and the API is the same.. Most applications will create the SslFactory through the SslChannelBuilder or the SaslChannelBuilder which will hide the changes.

The default value for the config ssl.sslfactory.class is org.apache.kafka.common.security.ssl.SslFactory which selects the existing implementation.

The non-default constructors of SslFactory will be preserved and marked as deprecated. The constructor arguments will override the configs, just like clientAuthConfigOverride did, but now for all three arguments.

The new config keys ssl.mode and ssl.verify.keystore.using.truststore are private. They should not be configurable by the user.

The EchoServer used in the tests could be updated to use PluggableSslFactory. It is suggested to keep it the way it is to test backwards compatibility. 

Rejected Alternatives

Kafka could define a new configuration option to hold an instance of SSLSocketFactory. This is similar to many Java libraries that accept an instance of SSLSocketFactory. This was rejected because Kafka tries to be language neutral. It was thought it would make it more difficult to support C and Python.

...

Kafka's naming convention does not use a special tag for interfaces. Accordingly, these names were rejected ISslFactory, SslFactoryIFace, SslFactoryInterface.

The EchoServer could be updated to use PluggableSslFactory. It is suggested to keep it the way it is to test backwards compatibility.

The constructor SslFactory(Mode) is not part of the interface contract. It is only used by the EchoServer.

It would be nice if the PluggableSslFactory implementation used a default constructor. For example, we could call config.getConfiguredInstance(SslConfigs.SSL_SSLFACTORY_CONFIG, PluggableSslFactory.class) to create the instance. Unfortunately, we need to set properties through the 3 argument constructor before we can call configure(Map).

We could still have a default constructor if we add a new method to pass the 3 arguments:
preconfigure(Mode mode, String clientAuthConfigOverride, boolean keystoreVerifiableUsingTruststore);
It would not be possible to call config.getConfiguredInstance() but at least the interface members would document the whole contract. This is backwards compatible if we keep the 3-argument constructor for SslFactory and make it call preconfigure().

Another name for preconfigure() could be initialize().

only call to SslFactory.sslContext() is in EchoServer which is part of the client tests. We felt this was not a strong enough motivation to add sslContext() to the PluggableSslFactory interface, especially if EchoServer continues to call SslFactory directly.

Kafka is not consistent to name configs that hold class names. Compare [partitioner.class, interceptor.classes, principal.builder.class, sasl.server.callback.handler.class, sasl.client.callback.handler.class, sasl.login.class] versus [key.deserializer, value.deserializer, key.serializer, value.serializer]. It appears the serializer/deserializer configs are a special case. Therefore the name ssl.sslfactory.class was selected instead of ssl.sslfactory

We could require the Pluggable SSL Factory implementation to have the same 3-argument constructor as SslFactory. We find the arguments clientAuthConfigOverride and keystoreVerifiableUsingTruststore are The arguments clientAuthConfigOverride and keystoreVerifiableUsingTruststore seem somewhat arbitrary for a general pluggable interface. They were kept to minimize the changes and maximize backwards compatibilityWe prefer to use a default constructor for the pluggable SSL Factory implementation. This does not alter backwards compatibility because it is easy to keep the non-default constructors in SslFactory and have those values override whatever is in the configs.

We need the ability to send custom configuration options to the PluggableSslFactory implementation. Should this be another KIP?

...