----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31339/#review75402 -----------------------------------------------------------
Ship it! Looks great, Johny! I noted a couple of minor things but otherwise I think this is ready. I haven't checked the SASL or thrift code; I'll leave it to Hari to review that since I'm not familiar with it. flume-ng-auth/src/main/java/org/apache/flume/auth/KerberosAuthenticator.java <https://reviews.apache.org/r/31339/#comment122440> Minor: I can see how the ugi must be equal to curUser (or at least equivalent) but it seems like this should print the ugi instead of curUser because we don't actually replace the ugi. flume-ng-auth/src/main/java/org/apache/flume/auth/KerberosAuthenticator.java <https://reviews.apache.org/r/31339/#comment122439> Nit: indentation is a little off here. flume-ng-auth/src/main/java/org/apache/flume/auth/KerberosAuthenticator.java <https://reviews.apache.org/r/31339/#comment122442> I thought we decided this wasn't needed? It don't think it is major enough to block getting this in, but it seems odd. flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSink.java <https://reviews.apache.org/r/31339/#comment122441> Minor: Since this doesn't throw any Exceptions, I think it can be changed to just PrivilegedAction. Then we wouldn't need the extra try/catch. - Ryan Blue On March 5, 2015, 2:10 p.m., Johny Rufus John wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31339/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 2:10 p.m.) > > > Review request for Flume. > > > Bugs: FLUME-2631 > https://issues.apache.org/jira/browse/FLUME-2631 > > > Repository: flume-git > > > Description > ------- > > End to End authentication in Flume > > > Diffs > ----- > > flume-ng-auth/pom.xml PRE-CREATION > > flume-ng-auth/src/main/java/org/apache/flume/api/SecureRpcClientFactory.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/api/SecureThriftRpcClient.java > PRE-CREATION > > flume-ng-auth/src/main/java/org/apache/flume/auth/FlumeAuthenticationUtil.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/auth/FlumeAuthenticator.java > PRE-CREATION > > flume-ng-auth/src/main/java/org/apache/flume/auth/KerberosAuthenticator.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/auth/PrivilegedExecutor.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/auth/SecurityException.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/auth/SimpleAuthenticator.java > PRE-CREATION > flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java > PRE-CREATION > > flume-ng-auth/src/test/java/org/apache/flume/auth/TestFlumeAuthenticator.java > PRE-CREATION > flume-ng-core/pom.xml 8992414 > flume-ng-core/src/main/java/org/apache/flume/sink/ThriftSink.java baa60d0 > flume-ng-core/src/main/java/org/apache/flume/source/ThriftSource.java > 06bb604 > flume-ng-dist/pom.xml a083fe2 > flume-ng-dist/src/main/assembly/bin.xml 5aa7cc6 > flume-ng-dist/src/main/assembly/src.xml b1e79a2 > > flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java > 33a2330 > flume-ng-sdk/src/main/java/org/apache/flume/api/ThriftRpcClient.java > 4f75a2b > flume-ng-sinks/flume-dataset-sink/pom.xml e929d60 > > flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/DatasetSink.java > 3e66532 > > flume-ng-sinks/flume-dataset-sink/src/main/java/org/apache/flume/sink/kite/KerberosUtil.java > c0dbffb > > flume-ng-sinks/flume-dataset-sink/src/test/java/org/apache/flume/sink/kite/TestKerberosUtil.java > f53ef73 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java > 62f4eee > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > 33f73a9 > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestBucketWriter.java > 7c74b16 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSink.java > 5de0bd5 > > flume-ng-sinks/flume-ng-hbase-sink/src/main/java/org/apache/flume/sink/hbase/HBaseSinkSecurityManager.java > 762fce9 > pom.xml ea7ffe3 > > Diff: https://reviews.apache.org/r/31339/diff/ > > > Testing > ------- > > Tested in kerberos cluster with combinations of Thrift Src, Sink and Hdfs Sink > > > Thanks, > > Johny Rufus John > >
