007 plugin api to select tls credentials for server connection #75
Conversation
Signed-off-by: Tom Bentley <[email protected]>
Signed-off-by: Tom Bentley <[email protected]>
| */ | ||
| interface Context { | ||
| Optional<ClientTlsContext> clientTlsContext(); | ||
|
|
There was a problem hiding this comment.
Note that currently there is no method for getting the ClientSaslContext proposed in #74). That's because that SASL info would be provided by a SASL terminator, which can currently only be invoked once a filter chain is instantiated, which happens after the proxy-server connection is established. So a cyclic dependency.
There was a problem hiding this comment.
Maybe this should be visible somewhere in a proposal, it's a bit tricky since it's cumulative over the proposals. Maybe just a note somewhere like:
Plugins will not be able to use the downstream SASL client identity proposed in 006 for TLS credential selection initially, because the filter are currently installed after the upstream connection is established.
edit: maybe in a Future Work section like I've commented elsewhere.
| */ | ||
| interface Context { | ||
| Optional<ClientTlsContext> clientTlsContext(); | ||
|
|
There was a problem hiding this comment.
Maybe this should be visible somewhere in a proposal, it's a bit tricky since it's cumulative over the proposals. Maybe just a note somewhere like:
Plugins will not be able to use the downstream SASL client identity proposed in 006 for TLS credential selection initially, because the filter are currently installed after the upstream connection is established.
edit: maybe in a Future Work section like I've commented elsewhere.
proposals/007-plugin-api-to-select-tls-credentials-for-server-connection.md
Show resolved
Hide resolved
| It will use the usual Kroxylicious plugin mechanism, leveraging `java.util.Service`-based discovery. | ||
| However, this plugin is not the same thing as a `FilterFactory`. | ||
| Rather, an implementation class will be defined on the `TargetCluster` configuration object, and instantiated once for each target cluster. | ||
| The TargetCluster's `tls` object will gain a `tlsCredentialSupplier` property, supporting `type` and `config` properties (similarly to how filters are configured). |
There was a problem hiding this comment.
Sidebar: not for this proposal. Wonder where we would put this in the operator API. In operator land KafkaService has the upstream tls details, but we can reuse a KafkaService across virtual kafka clusters. Would we put it on KafkaService, so users need multiple KafkaServices if they want different selection logic per virtual cluster. Or put it on VirtualCluster, with the downside that we spread out the upstream TLS settings across two CRs.
proposals/007-plugin-api-to-select-tls-credentials-for-server-connection.md
Show resolved
Hide resolved
proposals/007-plugin-api-to-select-tls-credentials-for-server-connection.md
Show resolved
Hide resolved
proposals/007-plugin-api-to-select-tls-credentials-for-server-connection.md
Show resolved
Hide resolved
Signed-off-by: Tom Bentley <[email protected]>
| * @return executor | ||
| * @throws IllegalStateException if the factory is not bound to a channel yet. | ||
| */ | ||
| ScheduledExecutorService filterDispatchExecutor(); |
There was a problem hiding this comment.
Should we call this pluginDispatchExecutor?
| * Implemented by a {@link io.kroxylicious.proxy.filter.Filter} that provides | ||
| * the credentials for the TLS connection between the proxy and the Kafka server. |
There was a problem hiding this comment.
| * Implemented by a {@link io.kroxylicious.proxy.filter.Filter} that provides | |
| * the credentials for the TLS connection between the proxy and the Kafka server. | |
| * Implemented by a class annotated with {@link io.kroxylicious.proxy.plugin.Plugin} that provides | |
| * the credentials for the TLS connection between the proxy and the Kafka server. |
I don't think the intention is for filters to implement this, is it?
|
An update on this, @tombentley was wanting to prototype this before merging the design. He's working on other things currently, so if anyone wants to build the prototype implementation let us know. |
5 participants
It's been suggested to break #71 into separate proposals to simplify review etc. This PR adds a proposal for a plugin API for runtime selection of the TLS certificate to be used on the proxy-to-server connection.