-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10190/#review18517
-----------------------------------------------------------


Hey Joey, looks great! I don't have time to do the authoritative review of 
this, so I'll let someone else do that. I just had a comment about the key 
store stuff.  Since the FC already does it might make sense to refactor that 
code to be used here as well. This could be handled in a followup jira.


flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java
<https://reviews.apache.org/r/10190/#comment38830>

    This should not block this item from being committed, but I think we should 
file a followup jira to ensure the configuration properties align with the file 
channel encryption: http://flume.apache.org/FlumeUserGuide.html#file-channel
    
    Additionally, it might make sense to refactor things like this class 
https://github.com/apache/flume/blob/trunk/flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/encryption/JCEFileKeyProvider.java
 so we can use that here.
    


- Brock Noland


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
> 
>

Reply via email to