There has been no discussion since my last update week ago. Unless someone has some further comments in the next 48 hours, I will start the voting for this KIP.
Thanks & Regards Jakub On Tue, Oct 17, 2017 at 5:54 PM, Jakub Scholz <ja...@scholz.cz> wrote: > Ok, so I updated the KIP according to what we discussed. Please have a > look at the updates. Two points I'm not 100% sure about: > > 1) Should we mark the rest.host.name and rest.port options as deprecated? > > 2) I needed to also address the advertised hostname / port. With multiple > listeners it is not clear anymore which one should be used. I saw as one > option to add advertised.listeners option and some modified version of > inter.broker.listener.name option to follow what is done in Kafka > brokers. But for the Connect REST interface, we do not advertise the > address to the clients like in Kafka broker. So we only need to tell other > workers how to connect - and for that we need only one advertised address. > So I decided to reuse the existing rest.advertised.host.name and > rest.advertised.port options and add additional option > rest.advertised.security.protocol to specify whether HTTP or HTTPS should > be used. Does this make sense to you? DO you think this is the right > approach? > > Thanks & Regards > Jakub > > On Mon, Oct 16, 2017 at 6:34 PM, Randall Hauch <rha...@gmail.com> wrote: > >> The broker's configuration options are "listeners" (plural) and >> "listeners.security.protocol.map". I agree that following the pattern set >> by the broker is better, so these are really good ideas. However, at this >> point I don't see a need for the "listeners.security.procotol.map", which >> for the broker must be set if the listener name is not a security >> protocol. >> Can we not simply just allow "HTTP" and "HTTPS" as the names of the >> listeners (rather than the broker's "PLAINTEXT", "SSL", etc.)? If so, then >> for example "listeners" might be set to "http://myhost:8081, >> https://myhost:80";, which seems to work out nicely without needing >> listener >> names other than security protocols. >> >> I also like using the worker's SSL and SASL security configs by default if >> "https" is included in the listener, but allowing the overriding of this >> via other additional properties. Here, I'm not a big fan of >> "listeners.name.https.*" prefix, which I think is pretty verbose, but I >> could see "listener.https.*" as a prefix. This allows us to add other >> security protocols at some point, if that ever becomes necessary. >> >> +1 for continuing down this road. Nice work. >> >> On Mon, Oct 16, 2017 at 9:51 AM, Ted Yu <yuzhih...@gmail.com> wrote: >> >> > +1 to this proposal. >> > >> > On Mon, Oct 16, 2017 at 7:49 AM, Jakub Scholz <ja...@scholz.cz> wrote: >> > >> > > I was having some more thoughts about it. We can simply take over what >> > > Kafka broker implements for the listeners: >> > > - We can take over the "listener" and "listener.security.protocol.ma >> p" >> > > options to define multiple REST listeners and the security protocol >> they >> > > should use >> > > - The HTTPS interface will by default use the default configuration >> > options >> > > ("ssl.keystore.localtion" etc.). But if desired, the values can be >> > > overridden for given listener (again, as in Kafka broker " >> listener.name >> > > .<LISTENER_NAME>.ssl.keystore.location") >> > > >> > > This should address both issues raised. But before I incorporate it >> into >> > > the KIP, I would love to get some feedback if this sounds OK. Please >> let >> > me >> > > know what do you think ... >> > > >> > > Jakub >> > > >> > > On Sun, Oct 15, 2017 at 12:23 AM, Jakub Scholz <ja...@scholz.cz> >> wrote: >> > > >> > > > I agree, adding both HTTP and HTTPS is not complicated. I just >> didn't >> > saw >> > > > the use case for it. But I can add it. Would you add just support >> for a >> > > > single HTTP and single HTTPS interface? Or do you see some value >> even >> > in >> > > > allowing more than 2 interfaces (for example one HTTP and two HTTPS >> > with >> > > > different configuration)? It could be done similarly to how the >> Kafka >> > > > broker does it through the "listener" configuration parameter with >> > comma >> > > > separated list. What do you think? >> > > > >> > > > As for the "rest" prefix - if we remove it, some of the same >> > > configuration >> > > > options are already used today as the option for connecting from >> Kafka >> > > > Connect to Kafka broker. So I'm not sure we should mix them. I can >> > > > definitely imagine some cases where the client SSL configuration >> will >> > not >> > > > be the same as the REST HTTPS configuration. That is why I added the >> > > > prefix. If we remove the prefix, how would you deal with this? >> > > > >> > > > On Fri, Oct 13, 2017 at 6:25 PM, Randall Hauch <rha...@gmail.com> >> > wrote: >> > > > >> > > >> Also, do we need these properties to be preceded with `rest`? I'd >> > argue >> > > >> that we're just configuring the worker's SSL information, and that >> the >> > > >> REST >> > > >> API would just use that. If we added another non-REST API, we'd >> want >> > to >> > > >> use >> > > >> the same security configuration. >> > > >> >> > > >> It's not that complicated in Jetty to support both "http" and >> "https" >> > > >> simultaneously, so IMO we should add that from the beginning. >> > > >> >> > > >> On Fri, Oct 13, 2017 at 9:34 AM, Randall Hauch <rha...@gmail.com> >> > > wrote: >> > > >> >> > > >> > It'd be useful to specify the default values for the >> configuration >> > > >> > properties. >> > > >> > >> > > >> > On Tue, Oct 10, 2017 at 2:53 AM, Jakub Scholz <ja...@scholz.cz> >> > > wrote: >> > > >> > >> > > >> >> FYI: Based on Ewen's suggestion from the related JIRA, I added a >> > > >> >> clarification to the KIP that it doesn't do anything around >> > > >> authorization >> > > >> >> / >> > > >> >> ACLs. While authorization / ACLs would be for sure valuable >> > feature I >> > > >> >> would >> > > >> >> prefer to leave it for different KIP. >> > > >> >> >> > > >> >> Jakub >> > > >> >> >> > > >> >> On Mon, Oct 9, 2017 at 5:25 PM, Jakub Scholz <ja...@scholz.cz> >> > > wrote: >> > > >> >> >> > > >> >> > Hi, >> > > >> >> > >> > > >> >> > I would like to start a discussion about KIP-208: Add SSL >> support >> > > to >> > > >> >> Kafka >> > > >> >> > Connect REST interface (https://cwiki.apache.org/ >> > > >> >> > confluence/display/KAFKA/KIP-208%3A+Add+SSL+support+to+ >> > > >> >> > Kafka+Connect+REST+interface). >> > > >> >> > >> > > >> >> > I think this would be useful feature to improve the security >> of >> > > Kafka >> > > >> >> > Connect. >> > > >> >> > >> > > >> >> > Thanks & Regards >> > > >> >> > Jakub >> > > >> >> > >> > > >> >> >> > > >> > >> > > >> > >> > > >> >> > > > >> > > > >> > > >> > >> > >
