Hi Chris, > I don't think we should populate default values for SSL-related properties > before sending properties to the SSL engine factory, > since it may confuse users who have written custom SSL engine factories > to see that properties not specified in their Connect worker config are being > passed to their factory. I slightly disagree with this.. The behavior implemented in PR is consistent with SSL configuration for clients and brokers. This behavior does not prevent the implementation of custom SSL factories for the broker and client.
But you are right about reviewing PR and KIP. Let's focus on the KIP. -- With best regards, Taras Ledkov On Tue, Dec 5, 2023 at 12:18 AM Chris Egerton <chr...@aiven.io.invalid> wrote: > > Hi Taras, > > Regarding slimming down the interface: IMO, we should do this right the > first time, and that includes not requiring unnecessary methods from users. > I think BaseSslEngineFactory is good enough as a superinterface. > > > Regarding the parsing logic: I think the KIP needs to be more explicit. We > should add something like this to the proposed changes section: > > "If any properties are present in the worker config with a prefix of > "listeners.https.", then only properties with that prefix will be passed to > the SSL engine factory. Otherwise, all top-level worker properties will be > passed to the SSL engine factory. Note that this differs slightly from > existing logic in that the set of properties (prefixed or otherwise) will > not be filtered based on a predefined set of keys; this will enable custom > SSL engine factories to define and accept custom properties." > > I also took a quick look at the prototype (I usually try not to do this > since we vote on KIP documents, not PRs). I don't think we should populate > default values for SSL-related properties before sending properties to the > SSL engine factory, since it may confuse users who have written custom SSL > engine factories to see that properties not specified in their Connect > worker config are being passed to their factory. Instead, the default SSL > engine factory used by Connect can perform this logic, and we can let other > custom factories be responsible for their own default values. > > > Cheers, > > Chris > > On Wed, Nov 29, 2023 at 8:36 AM Taras Ledkov <tled...@apache.org> wrote: > > > Hi team, > > > > Ping for review / vote for KIP-967 [1]. > > Voting thread is here [2] > > > > [1]. > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-967%3A+Support+custom+SSL+configuration+for+Kafka+Connect+RestServer > > [2]. https://github.com/apache/kafka/pull/14203 > > [2]. https://lists.apache.org/thread/wc4v5v3pynl15g1q547m2croqsqgmzpw > > > > -- > > With best regards, > > Taras Ledkov > >
