> On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1322 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1322> > > > > Confusingly, SSL refers to TLS not SSL. So even though the parameter is > > named SSL, the actual protocol is TLS
fixed everywhere > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1331 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1331> > > > > TLS based encryption with no authentication. fixed everywhere with TLS based encryption with optional authentication. client side and server side cert based authentication is available: http://kafka.apache.org/documentation#security_configclients > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1342 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1342> > > > > s/SSL/TLS fixed everywhere > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1376 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1376> > > > > s/certification/certificate fixed everywhere > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1377 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1377> > > > > s/certification/certificate fixed everywhere > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, lines 1439-1453 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1439> > > > > Do we need to specify useTicketCache=false ? That's what I've used when > > configuring this so far. Although the default is false. > > > > Also, I've not set storeKey=true before. useTicketCache is false by default that is why I left it out (ie it has no effect). storeKey=true was recommended by kafka documentation for long running processes and this is what Kerb5 docs also recommended. http://kafka.apache.org/documentation#security_kerberos_sasl_clientconfig > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, lines 1458-1459 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1458> > > > > Are we not documenting this? flume ships with 0.9.0.1 so removed > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 2887 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line2887> > > > > It's an oxymoron for the Sink to need to do offset migration. Therefore > > let's rephrase to: > > > > Unlike the Kafka Source / Kafka Channel a "client" section is not > > required, unless it is needed by other connecting components. fixed > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1323 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1323> > > > > Presumably we can't use this as we only ship the 0.9 client API in > > Flume 1.7? removed 0.10 and SASL/Plaintext everywhere > On Oct. 7, 2016, 7:44 p.m., Tristan Stevens wrote: > > flume-ng-doc/sphinx/FlumeUserGuide.rst, line 1434 > > <https://reviews.apache.org/r/52627/diff/3/?file=1526446#file1526446line1434> > > > > Since the Kafka Source may also connect to Zookeeper for offset > > migration, the "Client" section was also added to this example. This won't > > be needed unless you require offset migration, or you require this section > > for other secure components. fixed for source and channel. On Oct. 7, 2016, 7:44 p.m., Attila Simon wrote: > > Great job Simon - thanks for the time you've put into this. > > > > I've got a feeling however that we're unnecessarily duplicating some of the > > wordage here. Could we take the whole "Security and Kafka *" section and > > place under the "Security" section - and just make it clear which bits > > apply to consumers (Source and Channel) and which bits apply to Producers > > (Sink and Channel)? I think this would make it tidier and also aid > > maintainability. > > > > Some of the comments apply to both Source and Sink, but I've only raised > > them once. I also take the point that they are probably also defects in the > > Channel bit, that you didn't write. Sorry about that! I gave a lot of thought to this. From user experience point of view it looked really terrible to read the common parts in a single place then jump forth and back to have the specifics keeping in mind the base information instead of having the related information as a whole in a single place. I know it came with the price of docs duplication but the gain was better reader experience. - Attila ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52627/#review151849 ----------------------------------------------------------- On Oct. 10, 2016, 5:35 p.m., Attila Simon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52627/ > ----------------------------------------------------------- > > (Updated Oct. 10, 2016, 5:35 p.m.) > > > Review request for Flume. > > > Bugs: FLUME-2971 > https://issues.apache.org/jira/browse/FLUME-2971 > > > Repository: flume-git > > > Description > ------- > > The patch aims to extend the existing documentation of secure Kafka channel > with describing SSL+Plaintext setup as well as providing the whole package > (SSL+Kerberos+Plain) for KafkaSource and KafkaSink. > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst ab71d38 > > Diff: https://reviews.apache.org/r/52627/diff/ > > > Testing > ------- > > "mvn site" generated the user guide without an error message in the html. > Embedded links are checked not to be broken. > > Known to require attention: Content of the jaas file has to be checked > focusing on the requirement of the Client section in every setup. > > > Thanks, > > Attila Simon > >
