> On Aug. 27, 2016, 3:20 a.m., Chaoyu Tang wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java, line 72 > > <https://reviews.apache.org/r/51435/diff/2/?file=1486030#file1486030line72> > > > > hive.conf.hidden.list is usually not default specified in > > configurations (e.g. that from hdfs-site.xml) other than HiveConf, so the > > hiddenListStr should be retruned as null for these configuraitons, and the > > properties listed in HiveConf hidden list are still not able to be stripped > > from this Config, right? > > The S3 credentials I think could be defined in the hdfs configuration > > but they are specified in HiveConf hiddenList, they should be stripped and > > have we tested them? > > Peter Vary wrote: > Thanks Chaoyu! > > You are absolutely right! > I missed this, because when it was not defined in the actual > configuration xml-s, then it used the default value - so removed the S3 > credentials anyway. > > Added it to the interestingPropNames, so it will be copied from the > HiveConf, if there is any. If it is not in the following files, or the > hiveConf, then we could still use the default value. > HCatalog specific config files: "core-default.xml", "core-site.xml", > "mapred-default.xml", "mapred-site.xml", "hdfs-site.xml", > "webhcat-default.xml", "webhcat-site.xml" > > Thanks for the catch. > > Any other idea? I might have missed something more - hcatalog is new for > me. > > Thanks, > Peter
Hi Peter, Here is what I understand the code: AppConf loads the configurations from xml files specified in HADOOP_CONF_FILENAMES and TEMPLETON_CONF_FILENAMES which does not include hive-site.xml, so it might contain the proprety such as S3 credentials which are probably specified in hdfs-site.xml. Since appConf does not defined the property hive.conf.hidden.list, so the line String hiddenListStr = HiveConf.getVar(configuration, HiveConf.ConfVars.HIVE_CONF_HIDDEN_LIST); returns null when HiveConfUtil.dumpConfig(this, sb) is called in AppConf.dumpEnvironent and the passed in parameter configuration is AppConf instance in this case. Therefore the S3 credentials in this AppConf could not be stripped. The new change only adds (or appends) the hiveConf hive.conf.hidden.list key/value pair to the value of property HIVE_PROPS_NAME (templeton.hive.properties), but the HIVE_CONF_HIDDEN_LIST is still undefined in AppConf and the property like S3 credentials are still not stripped even they have been specified in hiveConf hidden list. Does it make sense? So in order to strip the properties speified in hive.conf.hidden.list, the property hive.conf.hidden.list should be added to AppConf, right? - Chaoyu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51435/#review147068 ----------------------------------------------------------- On Aug. 30, 2016, 6:34 a.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51435/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2016, 6:34 a.m.) > > > Review request for hive, Chaoyu Tang, Gabor Szadovszky, Sergio Pena, and > Barna Zsombor Klara. > > > Bugs: HIVE-14426 > https://issues.apache.org/jira/browse/HIVE-14426 > > > Repository: hive-git > > > Description > ------- > > Used HiveConf.stripHiddenConfigurations to remove sensitive information from > configuration dump. Had to refactor, so it could be applied to simple > Configuration objects too, like AppConfig. > > Used Utilities.maskIfPassword to remove sensitive information from property > dump > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 14a538b > common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java 073a978 > > hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/AppConfig.java > dd1208b > > hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/tool/TempletonUtils.java > 201e647 > > Diff: https://reviews.apache.org/r/51435/diff/ > > > Testing > ------- > > Manually - checked both > > > Thanks, > > Peter Vary > >
