You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Balázs Donát Bessenyei <be...@cloudera.com> on 2016/08/26 13:54:20 UTC
Re: Review Request 25002: Provide the optional ability to store
password externally either in clear text or in obfuscated form
-----------------------------------------------------------
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
>
>