You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Roshan Naik <ro...@hortonworks.com> on 2014/09/06 03:01:52 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/
-----------------------------------------------------------
(Updated Sept. 6, 2014, 1:01 a.m.)
Review request for Flume.
Changes
-------
bug fixes in unit tests
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 (updated)
-----
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
Re: Review Request 25002: Provide the optional ability to store
password externally either in clear text or in obfuscated form
Posted by Balázs Donát Bessenyei <be...@cloudera.com>.
-----------------------------------------------------------
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
>
>