Thanks, the KIP LGTM. On AK2: I was interested in it because Unsafe, reflection, and bytebuddy are in my general watchdog list since we’re straying away from the standard API. Some may have preference over using Method/VarHandle/MethodHandle - but as you said this one maybe left for PR discussions. I’m happy with Arnab’s reply.
-Aditya On 2026/10/19 12:41:40 Mickael Maison wrote: > Hi Aditya, > > AK1: Updated > > AK2: A KIP typically does not get into implementation details, these > are discussed in the PR. Is there a specific reason you're interested > in the actual logic? Did Arnab's reply answered it? > > AK3: That logic does not have to be in DefaultSslEngineFactory. For > example we can have it in SslFactory so it applies to both the default > and custom implementations. > > AK4: If the KIP is accepted we can do it. > > AK5: Unfortunately currently there isn't a programmatic way to > retrieve the details about the peer configuration, so we can't > retrieve the named groups (nor ciphers, etc) from the peer for a nice > error message. > > AK6: I think the KIP has already been updated to cover that. > > AK7: Yes that's an option. I've added to the rejected alternatives > > Thanks, > Mickael > > > > On Fri, Oct 16, 2026 at 5:31 PM Aditya Kousik via dev > <[email protected]> wrote: > > > > Hi Mickael, > > > > Thanks for the KIP. It’s good to see Kafka take on the PQC world; I had a > > few comments. > > > > AK1: nit: but the Status is still marked as Draft and the Discussion Link > > is still TBD > > > > AK2: The reflection mechanism could use some details. Are we storing a > > MethodHandle reference when DefaultSslEngineFactory initializes in a static > > block? > > > > AK3: If the contract is to throw ConfigException because of missing names > > group support on the jdk, there’s nothing in the SslEngineFactory to have > > other engines follow the spec. ssl.engine.factory.class for other engines > > won’t adhere to this, and may break silently after launch. > > > > AK4: Should we file a JIRA in conjunction to deprecate/cleanup the > > reflection once all clients and servers update their minimum supported > > version >= 20? > > > > AK5: > > > a TLS connection can only be established when the client and server share > > > at least one named group > > > > I see in other replies than this would this be an sslhandshakexception that > > the jdk SSLEngine throws. Would it be worth adding a custom KafkaException > > that suggests to users on the supported named groups in the current VM via > > SSLContext.getSupportedSSLParameters().getNamedGroups(). We sometimes do > > this when serialization fails, max poll interval elapses etc. > > > > AK6: On Testing, there’s a negative test that we’re missing. We can ensure > > assertThrows(ConfigException.class) on jdk versions < 20 which maybe a lot > > of the clients out there. > > > > AK7: I think for the simplicity alone, reflection is what we might settle > > on. But I wanted to call out that there’s technically another approach that > > might get us there without it. > > We could do a multi-release JAR which has been available since jdk9. We can > > create a custom java20 source set and have gradle include it in the build. > > We can define a plain class on the base layer as > > ```java > > public class NamedGroupSupport { > > public boolean supported() { return false; } > > public void apply(SSLParameters sslParams, String[] groups) { throw new > > UnsupportedOperationException(); } > > } > > ``` > > And in the java20 source set, have the right implementation. We also need > > ```groovy > > jar { > > into(‘META-INF/versions/20’) { from sourceSets.java20.output } > > manifest { attributes ‘Multi-Release’: ‘true’ } > > } > > ``` > > With the above in addition to a custom java20 source set with > > options.release = 20 could get us there for free. > > > > But the cost is the build complexity, if someone shades the kafka-clients > > JAR they lose support. Also for clients who fork and build their own JARs > > this would not be preferable. So overall, reflection is the simpler answer > > I think but an alternative nonetheless to reject imo. > > > > Thanks, > > Aditya Kousik > > > > On 2026-10-06 10:14 UTC Mickael Maison wrote: > > > Hi, > > > > > > I'd like to start a discussion about KIP-1376 which proposes adding > > > support for TLS named groups: > > > https://cwiki.apache.org/confluence/x/dJXwGg > > > > > > Thanks, > > > Mickael > > > >
