You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Brock Noland <br...@cloudera.com> on 2013/10/02 20:04:59 UTC
Review Request 14447: HIVE-5400: Allow admins to disable compile and other
commands
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14447/
-----------------------------------------------------------
Review request for hive and Edward Capriolo.
Bugs: HIVE-5400
https://issues.apache.org/jira/browse/HIVE-5400
Repository: hive-git
Description
-------
Allows admins to restrict the commands, non-sql, available to users.
Diffs
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java ed71196
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5dfcff4
hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java a10c3a8
ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java 0d0bf47
ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java PRE-CREATION
service/src/java/org/apache/hive/service/cli/operation/AddResourceOperation.java fe0c6db
service/src/java/org/apache/hive/service/cli/operation/DeleteResourceOperation.java 496bba9
service/src/java/org/apache/hive/service/cli/operation/DfsOperation.java a8b8ed4
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java 6b5a5c3
service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java 0a8825e
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 1f78a1d
service/src/java/org/apache/hive/service/cli/operation/SetOperation.java bf6969a
Diff: https://reviews.apache.org/r/14447/diff/
Testing
-------
New unit tests are added and pass.
Thanks,
Brock Noland
Re: Review Request 14447: HIVE-5400: Allow admins to disable compile and
other commands
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14447/
-----------------------------------------------------------
(Updated Oct. 2, 2013, 9:58 p.m.)
Review request for hive and Edward Capriolo.
Changes
-------
Updates patch based on review.
Bugs: HIVE-5400
https://issues.apache.org/jira/browse/HIVE-5400
Repository: hive-git
Description
-------
Allows admins to restrict the commands, non-sql, available to users.
Diffs (updated)
-----
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java ed71196
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5dfcff4
conf/hive-default.xml.template 878e157
hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java a10c3a8
ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java 0d0bf47
ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java PRE-CREATION
ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java PRE-CREATION
service/src/java/org/apache/hive/service/cli/operation/AddResourceOperation.java fe0c6db
service/src/java/org/apache/hive/service/cli/operation/DeleteResourceOperation.java 496bba9
service/src/java/org/apache/hive/service/cli/operation/DfsOperation.java a8b8ed4
service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java 6b5a5c3
service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java 0a8825e
service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 1f78a1d
service/src/java/org/apache/hive/service/cli/operation/SetOperation.java bf6969a
Diff: https://reviews.apache.org/r/14447/diff/
Testing
-------
New unit tests are added and pass.
Thanks,
Brock Noland
Re: Review Request 14447: HIVE-5400: Allow admins to disable compile and
other commands
Posted by Brock Noland <br...@cloudera.com>.
> On Oct. 2, 2013, 9:09 p.m., Carl Steinbach wrote:
> > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java, line 777
> > <https://reviews.apache.org/r/14447/diff/2/?file=360535#file360535line777>
> >
> > I think we should try to make security-related configuration properties easier to find by using an appropriate prefix, e.g. hive.security.*
> >
> > Also, "whitelist" and "blacklist" are the standard terms used by security engineers, so I think this property should be named hive.security.command.whitelist.
> >
> > It seems like this property should be a default member of hive.conf.restricted.list.
> >
> > conf/hive-default.xml.template needs to be update.
Good call on all accounts. The only issue is that hive.conf.restricted.list is empty by default at present. I am fine adding some default entries but I feel that should be done in a separate follow-on JIRA to analyze what items we'd like to add and come up with a good way other than just listing them out in a big blob in the configuration file.
> On Oct. 2, 2013, 9:09 p.m., Carl Steinbach wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java, line 53
> > <https://reviews.apache.org/r/14447/diff/2/?file=360537#file360537line53>
> >
> > There should be a space between 'if' and '('. The same rule also applies to for statements. This same formatting problem is also found in the other files included in this patch.
> >
> > Refs:
> > * https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConvention
> > * http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#430
Sorry about that, I created the patch with vim and forgot that it doesn't do this automatically like eclipse.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14447/#review26631
-----------------------------------------------------------
On Oct. 2, 2013, 9:58 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14447/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2013, 9:58 p.m.)
>
>
> Review request for hive and Edward Capriolo.
>
>
> Bugs: HIVE-5400
> https://issues.apache.org/jira/browse/HIVE-5400
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Allows admins to restrict the commands, non-sql, available to users.
>
>
> Diffs
> -----
>
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java ed71196
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5dfcff4
> conf/hive-default.xml.template 878e157
> hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java a10c3a8
> ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java 0d0bf47
> ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/operation/AddResourceOperation.java fe0c6db
> service/src/java/org/apache/hive/service/cli/operation/DeleteResourceOperation.java 496bba9
> service/src/java/org/apache/hive/service/cli/operation/DfsOperation.java a8b8ed4
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java 6b5a5c3
> service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java 0a8825e
> service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 1f78a1d
> service/src/java/org/apache/hive/service/cli/operation/SetOperation.java bf6969a
>
> Diff: https://reviews.apache.org/r/14447/diff/
>
>
> Testing
> -------
>
> New unit tests are added and pass.
>
>
> Thanks,
>
> Brock Noland
>
>
Re: Review Request 14447: HIVE-5400: Allow admins to disable compile and
other commands
Posted by Carl Steinbach <ca...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14447/#review26631
-----------------------------------------------------------
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
<https://reviews.apache.org/r/14447/#comment51874>
Can you static import this instead of using the package name? There are a couple other occurrences of this in the same file that would be nice to clean up.
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/14447/#comment51873>
I think we should try to make security-related configuration properties easier to find by using an appropriate prefix, e.g. hive.security.*
Also, "whitelist" and "blacklist" are the standard terms used by security engineers, so I think this property should be named hive.security.command.whitelist.
It seems like this property should be a default member of hive.conf.restricted.list.
conf/hive-default.xml.template needs to be update.
ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java
<https://reviews.apache.org/r/14447/#comment51875>
There should be a space between 'if' and '('. The same rule also applies to for statements. This same formatting problem is also found in the other files included in this patch.
Refs:
* https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConvention
* http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#430
- Carl Steinbach
On Oct. 2, 2013, 6:04 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14447/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2013, 6:04 p.m.)
>
>
> Review request for hive and Edward Capriolo.
>
>
> Bugs: HIVE-5400
> https://issues.apache.org/jira/browse/HIVE-5400
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Allows admins to restrict the commands, non-sql, available to users.
>
>
> Diffs
> -----
>
> cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java ed71196
> common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 5dfcff4
> hwi/src/java/org/apache/hadoop/hive/hwi/HWISessionItem.java a10c3a8
> ql/src/java/org/apache/hadoop/hive/ql/processors/CommandProcessorFactory.java 0d0bf47
> ql/src/java/org/apache/hadoop/hive/ql/processors/HiveCommand.java PRE-CREATION
> ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java PRE-CREATION
> service/src/java/org/apache/hive/service/cli/operation/AddResourceOperation.java fe0c6db
> service/src/java/org/apache/hive/service/cli/operation/DeleteResourceOperation.java 496bba9
> service/src/java/org/apache/hive/service/cli/operation/DfsOperation.java a8b8ed4
> service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java 6b5a5c3
> service/src/java/org/apache/hive/service/cli/operation/HiveCommandOperation.java 0a8825e
> service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 1f78a1d
> service/src/java/org/apache/hive/service/cli/operation/SetOperation.java bf6969a
>
> Diff: https://reviews.apache.org/r/14447/diff/
>
>
> Testing
> -------
>
> New unit tests are added and pass.
>
>
> Thanks,
>
> Brock Noland
>
>