Hi Noa, Thanks for the KIP. A few comments/questions:
1. If we support filenames starting with `classpath:` by requiring `file:`prefix, then we are presumably not supporting files starting `file:`. Not necessarily an issue, but we do need to document any restrictions. 2. On the broker-side, trust stores are dynamically updatable. And we use file modification time to decide whether trust store needs to be reloaded. This is less of an issue once we implement https://cwiki.apache.org/confluence/display/KAFKA/KIP-339%3A+Create+a+new+IncrementalAlterConfigs+API, but at the moment, we are relying on actual files on the file system for which we can compare modification times. 3. On the client-side, trust stores are not currently updatable. And we don't have an API to make them updatable. By using class path, we preclude the use of file modification times in future to detect key or trust store updates for clients. It will be good to get feedback from the community on whether this is a reasonable longer-term restriction. 4. It will be good to get more feedback from the community on whether loading trust stores from CLASSPATH is a feature that is likely to be widely adopted. If not, perhaps https://cwiki.apache.org/confluence/display/KAFKA/KIP-383%3A++Pluggable+interface+for+SSL+Factory will be sufficient to enable custom factories that do load trust store from the CLASSPATH. Regards, Rajini On Tue, Dec 4, 2018 at 7:17 PM Sönke Liebau <soenke.lie...@opencore.com.invalid> wrote: > Hi Neo, > > thanks for the KIP, the proposal sounds useful! > Also I agree on both assumptions that you made: > - users whose current truststore location starts with classpath: should be > very few and extremely far between (and arguably made questionable choices > when naming their files/directories), I personally think it is safe to > ignore these > - this could also be useful for loading keystores, not just truststores > > One additional idea maybe, looking at the Spring documentation they seem to > support filesystem, classpath and URL resources. Would it make sense to add > something to allow loading the truststore from a url as well when touching > this functionality? > > Best regards, > Sönke > > > On Fri, Nov 30, 2018 at 6:01 PM Noa Resare <n...@resare.com> wrote: > > > I wrote a KIP for my minimal suggested change to support reading a > > truststore from the classpath as well as from a file. > > > > The KIP is available here: > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-398%3A+Support+reading+trust+store+from+classpath > > < > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-398:+Support+reading+trust+store+from+classpath > > > > > > > Any feedback or comments would be most welcome. > > > > Cheers > > Noa > > > > -- > Sönke Liebau > Partner > Tel. +49 179 7940878 > OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany >
