----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69534/#review211129 -----------------------------------------------------------
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 460 (patched) <https://reviews.apache.org/r/69534/#comment296050> In each property state that this is for the database connection from HMS to the backend database. More redundant documentation is better since we have the client <-> HMS ssl configurations as well. standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 461 (patched) <https://reviews.apache.org/r/69534/#comment296049> I think we should rename the property to indicate that this for the dbaccess and for client access. Consider renaming to "hive.metastore.dbaccess.ssl.trustStore" etc. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 343 (patched) <https://reviews.apache.org/r/69534/#comment296051> haha I like the "dangerously". Could you file a ticket to remove this in the future and link the new ticket here. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 347 (patched) <https://reviews.apache.org/r/69534/#comment296053> Should we validate the SSL configs here and print info if something is missing. We can then fall back to not use SSL if configs are missing. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 353 (patched) <https://reviews.apache.org/r/69534/#comment296052> I prefer using exceptions which tell the user what to do in addition to what is wrong. In this case, rephrase as, "SSL has been enabled but trust store path is empty. Set the xxx property to enable SSL. Disabling SSL and continuing" - Karthik Manamcheri On Dec. 8, 2018, 6:31 a.m., Morio Ramdenbourg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69534/ > ----------------------------------------------------------- > > (Updated Dec. 8, 2018, 6:31 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 > e598a43e4dc2d2a2c25886ae7cbafd29b47c1f24 > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java > 0cf113c927f2274d085e07cd72921fb35227e1f3 > > > Diff: https://reviews.apache.org/r/69534/diff/1/ > > > 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. > > > Thanks, > > Morio Ramdenbourg > >
