hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Vary <pv...@cloudera.com>
Subject Re: Review Request 51435: HIVE-14426 Extensive logging on info level in WebHCat
Date Wed, 31 Aug 2016 15:06:40 GMT


> 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
> 
> Chaoyu Tang wrote:
>     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?
> 
> Peter Vary wrote:
>     Hi Chaoyu,
>     
>     When starting the Main.java from Intellij in debug mode, I found the following:
>     - AppConf loads, as you wrote the HADOOP_CONF_FILENAMES and TEMPLETON_CONF_FILENAMES
- no hive-site.xml here
>     - If the hive.conf.hidden.list is in any of the files above, then it is used (not
in the default case - probably not a good idea to have this value there anyway)
>     - If my understanding is correct, if HCatalog uses Hive Metastore, then it expects
to have hive-site.xml on classpath - see the comments of the AppConfig.handleHiveProperties
method - if this is true, then we are ok, and the hive.conf.hidden.list is set now, and the
new patch (with changes after your previous comment) will copy the value to the configuration,
and the credentials are removed
>     - If hive-site.xml is not on the classpath, then I am not sure what will happen when
we try to create a HiveConf object in the first line of the handleHiveProperties method, but
if no exception is thrown, then the HiveConf.getVar(configuration, HiveConf.ConfVars.HIVE_CONF_HIDDEN_LIST)
will return the default value defined in HiveConf object - at least this is what I saw during
my tests.
>     
>     Most probably you know the HCatalog better than me, do you think it is a valid usecase
not to have a hive-conf.xml on the classpath? If so, what should be our solution here? Adding
a hive.conf. variables to the webhcat-site.xml files? Add a new templeton specific configuration
variable?
>     
>     Thanks for your time, and review,
>     Peter
> 
> Chaoyu Tang wrote:
>     "HiveConf hiveConf = new HiveConf()" in handleHiveProperties loads hive-site.xml
(if any) to its method variable hiveConf, but not the "this" AppConf, right? so the "this"
(AppConf) passed to HiveConfUtil.dumpConfig(this, sb) does still not have the HIVE_CONF_HIDDEN_LIST
defined unless you call something like
>     set(HiveConf.ConfVars.HIVE_CONF_HIDDEN_LIST.varname, HiveConf.getVar(hiveConf, HiveConf.ConfVars.HIVE_CONF_HIDDEN_LIST))
to copy the property from hiveConf to this AppConf. handleHiveProperties only appends the
HIVE_CONF_HIDDEN_LIST (key=value) from hiveConf to the value of AppConf property HIVE_PROPS_NAME,
right?

Thanks Chaoyu,

Finally I got it. I have thought I understand the method comment, but I did not :( It was
setting a new config variable, and not copying the original ones.


- Peter


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message