> 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 > >
