> On April 15, 2013, 3:39 a.m., Mike Percy wrote: > > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java, > > line 346 > > <https://reviews.apache.org/r/10190/diff/1/?file=276332#file276332line346> > > > > This will add it first on the decode and last on the encode, right?
Yes, I found this documentation to be the most helpful on the encode/decode order, but I had to read all of it to fully grok what was going on: https://docs.jboss.org/netty/3.2/api/org/jboss/netty/channel/ChannelPipeline.html Should I add a comment to make it more clear? > On April 15, 2013, 3:39 a.m., Mike Percy wrote: > > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java, > > line 357 > > <https://reviews.apache.org/r/10190/diff/1/?file=276332#file276332line357> > > > > How is this different than the Permissive Trust Manager? :) It's not, but the only reason to make PermissiveTrustManager public is for testing. I'm happy to do that if you think it will be cleaner. > On April 15, 2013, 3:39 a.m., Mike Percy wrote: > > flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java, > > line 653 > > <https://reviews.apache.org/r/10190/diff/1/?file=276336#file276336line653> > > > > I believe this means we do not attempt to verify trust based on a CA or > > anything else. Why not? What are your thoughts on deploying this in a > > production environment? That's a good point. I think we want an option to not require two-way SSL when you care about encryption but aren't worried about trust. Perhaps a better default is to use the standard Java truststore if one isn't specified and add an explicit config for trusting all certs. - Joey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10190/#review19169 ----------------------------------------------------------- On March 29, 2013, 12:44 p.m., Joey Echeverria wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10190/ > ----------------------------------------------------------- > > (Updated March 29, 2013, 12:44 p.m.) > > > Review request for Flume and Mike Percy. > > > Description > ------- > > The patch adds support for SSL to AvroSource and AvroSink. The implementation > compliments the recent addition of compression in FLUME-1915. > > > This addresses bug FLUME-997. > https://issues.apache.org/jira/browse/FLUME-997 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java 517d545 > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java ac47ee9 > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java > c699241 > flume-ng-core/src/test/resources/server.p12 PRE-CREATION > flume-ng-core/src/test/resources/truststore.jks PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst 600a360 > flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java > 8285129 > > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java > 34d73a3 > > Diff: https://reviews.apache.org/r/10190/diff/ > > > Testing > ------- > > There are tests for having SSL enabled on both the client and server with > specific tests using a truststore to verify the server certificate. There's > also a test to make sure you can enable both SSL and compression. > > I probably need to add some negative tests: > > 1) SSL server, non-SSL client > 2) SSL server, SSL client with a truststore that doesn't include the server > certificate > > > Thanks, > > Joey Echeverria > >
