----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69534/#review211367 -----------------------------------------------------------
Fix it, then Ship it! Thanks for the changes. Couple of minor comments. Rest looks good to me. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 133-135 (patched) <https://reviews.apache.org/r/69534/#comment296374> Nit, constants are recommended to be in upper-case. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 379 (patched) <https://reviews.apache.org/r/69534/#comment296372> you are catching IOException and changing it to IllegalArgumentException which may not be the right thing to do all the times. I would suggest changing this to RuntimeException since setConf() doesn't throw checked exceptions. Also, would be good to have e thrown as a cause as well, so would suggest using throw new RuntimeException("Failed to set .., e); constructor instead. standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java Lines 1082 (patched) <https://reviews.apache.org/r/69534/#comment296373> If you decide to change the thrown exception to RuntimeException, this might need a change too. - Vihang Karajgaonkar On Dec. 16, 2018, 3:11 a.m., Morio Ramdenbourg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69534/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2018, 3:11 a.m.) > > > Review request for hive, Adam Holley, Karthik Manamcheri, Peter Vary, and > Vihang Karajgaonkar. > > > Bugs: HIVE-20992 > https://issues.apache.org/jira/browse/HIVE-20992 > > > Repository: hive-git > > > Description > ------- > > The following new properties were added: > > 1. metastore.dbaccess.use.SSL (hive.metastore.dbaccess.use.SSL) > 2. javax.net.ssl.trustStore > 3. javax.net.ssl.trustStorePassword > 4. javax.net.ssl.trustStoreType > > This was in an effort to guide the user towards an easier SSL > configuration experience. This is the minimum requirement to set up SSL > encryption to the HMS backend store. > > This also solves the issue of the truststore password being stored in > plain text. It can now be encrypted by default and loaded through the > MetastoreConf.getPassword() method which handles secure password access > > The property "hive.metastore.dbaccess.ssl.properties" is now > deprecated, but it will still be kept for backwards-compatibility purposes. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > e25a8cf9a19d78c0cc00bb2e5e0abee4d851ad98 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 3fa21b768cd120cd89343c9ccd142d5e2ccdef2e > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java > 0cf113c927f2274d085e07cd72921fb35227e1f3 > > > Diff: https://reviews.apache.org/r/69534/diff/5/ > > > Testing > ------- > > Tests: > 1. Unit tests were added to cover the functionality of configuring the Java > system properties. > 2. Performed some manual and sanity tests to ensure that SSL was still > configurable to a remote DB. I performed these on MySQL, PostgreSQL, Oracle, > and Derby DB by creating generic DB hosts and setting them up with SSL. Once > SSL was set up, I triggered the metastore to perform database calls, and > captured packets using tcpdump. I then uploaded my packet captures to > Wireshark, and ensured that none of the data was human-readable. > > I plan to upload a document to our Wiki explaining the process of enabling > TLS to these databases. > > > Thanks, > > Morio Ramdenbourg > >
