Hi Greg, > Taras, are you interested in dynamic SSL reconfiguration in Connect? > Would you be willing to investigate the details of that for the KIP? I would prefer to separate this functionality into the next KIP. But if you / (the community) consider it necessary to combine this in one KIP/patch, then I’m ready to do it.
The entry point for dynamic reconfiguration for Connect is not clear for me. For a broker it is the DynamicConfig functionality. Should we add a REST endpoint to reconfigure Connect or use another way? -- With best regards, Taras Ledkov On Tue, Dec 5, 2023 at 2:42 AM Greg Harris <greg.har...@aiven.io.invalid> wrote: > > Hi Chris, > > Thank you for your comments above. I disagree with your recommendation > for a new SslEngineFactory variant/hierarchy. > > 1. A superinterface could be more confusing to users. Since this is an > interface in `clients`, the connect-specific interface would also need > to be in clients, despite being superfluous for clients users and > broker developers. > 2. A user could implement the reduced interface, and then have issues > loading their implementation in a broker, and need to find > documentation/javadocs to explain to them why. > 3. This interface has been in use since 2020, so I don't believe that > the burden of implementing these methods has been excessive. I assume > there's at least one "static" implementation out there that would have > benefitted from the proposed super-interface, but they managed to > adapt to the standardized interface. > 4. Implementations that don't want to provide basic implementations > can throw UnsupportedOperationException from the extra methods as a > last resort. > > On the other hand, how much would it take to support the full suite of > SslEngineFactory operations in Connect? Could part of this KIP be > improving Connect to make use of these methods? This would help unify > the experience for users that implement the interface specifically for > the dynamic reconfiguration support, and rely on it on the broker > side. > > Taras, are you interested in dynamic SSL reconfiguration in Connect? > Would you be willing to investigate the details of that for the KIP? > > Thanks, > Greg > > On Mon, Dec 4, 2023 at 1:17 PM 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 > > >
