----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25002/#review146653 -----------------------------------------------------------
flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/encryption/JCEFileKeyProvider.java (lines 75 - 76) <https://reviews.apache.org/r/25002/#comment213781> I think, this should be keyPassword = PasswordObfuscator.readPasswordFromFile(keyPasswordFile.getAbsolutePath(), keyStorePasswordFileType).toCharArray(); flume-ng-core/pom.xml (lines 254 - 257) <https://reviews.apache.org/r/25002/#comment213217> Why is this dependency required? flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java (lines 157 - 158) <https://reviews.apache.org/r/25002/#comment213820> These could be moved to public void configure(Context context) flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java (lines 154 - 155) <https://reviews.apache.org/r/25002/#comment213821> These could be moved to public void configure(Context context) flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java (line 194) <https://reviews.apache.org/r/25002/#comment213822> Throwing an exception here would simplify the code flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java (lines 96 - 97) <https://reviews.apache.org/r/25002/#comment213823> These could be moved to public void configure(Context context) flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java (lines 123 - 130) <https://reviews.apache.org/r/25002/#comment213824> One of the checkArgument-s could be removed (I'd remove the first) flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java (lines 46 - 59) <https://reviews.apache.org/r/25002/#comment213825> These could be private flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java (line 66) <https://reviews.apache.org/r/25002/#comment213826> Does the method actually write to a file? flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java (line 72) <https://reviews.apache.org/r/25002/#comment213835> I would extract the Cipher transformation parameter to a constant flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java (line 96) <https://reviews.apache.org/r/25002/#comment213836> I would extract the Cipher transformation parameter to a constant flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java (line 156) <https://reviews.apache.org/r/25002/#comment213827> Maybe we could use "Path of the file for reading" as parameter description? flume-ng-core/src/test/java/org/apache/flume/tools/TestPasswordObfuscator.java (line 36) <https://reviews.apache.org/r/25002/#comment213828> Is new String( required? flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSSource.java (line 65) <https://reviews.apache.org/r/25002/#comment213829> This could be moved to void afterSetup() - Balázs Donát Bessenyei On Sept. 6, 2014, 1:01 a.m., Roshan Naik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25002/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2014, 1:01 a.m.) > > > Review request for Flume. > > > Bugs: FLUME-2442 > https://issues.apache.org/jira/browse/FLUME-2442 > > > Repository: flume-git > > > Description > ------- > > Components modified: > + Avro source > + Avro sink > + HTTP source > + JMS source > + File Channel > > > Summary of what is implemented... > > 1) Extended command line to create an obfuscated password file > --------------------------------------------------------------- > New "flume-ng password /path/passwordFile" command creates a file which > contains password in obfuscated form > > 2) For components which dont already have a option of external password file > (Avro source/sink, HTTP source) > ------------------------------------------------------------------------------------------------------------- > Provided an config passwordFile setting that points to external file > user can use either the existing inline clear text password or use the > external passwordFile (ensuring backward compat) > added another optional config setting passwordFileType. It defaults to 'TEXT' > which means external password file is in clear text. It can be set to "AES" > which means the password is stored in the password file in obfuscated form > (using AES-CTR with a default key). Such a file can be created using the > "flume-ng password" command. > > > 3) For components which have ability store passwords externally (JMS source, > File channel) > ------------------------------------------------------------------------------------------- > Provided the additional passwordFileType option, same as above. This retains > backward compat while allowing one to have the external password file to > store in obfuscated form > > > Diffs > ----- > > bin/flume-ng e09e26b > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/encryption/EncryptionConfiguration.java > aaea0cd > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/encryption/JCEFileKeyProvider.java > f961ef9 > > flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/TestFileChannelEncryption.java > 6ea1216 > flume-ng-core/pom.xml f100fd1 > flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java > 5146834 > flume-ng-core/src/main/java/org/apache/flume/source/AvroSource.java 3eef687 > flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSource.java > 115b34f > > flume-ng-core/src/main/java/org/apache/flume/source/http/HTTPSourceConfigurationConstants.java > ed52827 > flume-ng-core/src/main/java/org/apache/flume/tools/PasswordObfuscator.java > PRE-CREATION > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 757a536 > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java > c75d098 > > flume-ng-core/src/test/java/org/apache/flume/source/http/TestHTTPSource.java > 5b07a6e > > flume-ng-core/src/test/java/org/apache/flume/tools/TestPasswordObfuscator.java > PRE-CREATION > flume-ng-core/src/test/resources/passwordfile.aes PRE-CREATION > flume-ng-core/src/test/resources/passwordfile.txt PRE-CREATION > flume-ng-doc/sphinx/FlumeUserGuide.rst b2058f5 > > flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java > addd97a > > flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSourceConfiguration.java > 98bf8ab > > flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSSource.java > 5423f8f > > Diff: https://reviews.apache.org/r/25002/diff/ > > > Testing > ------- > > Added unit tests to all components that were modified. > > > Thanks, > > Roshan Naik > >
