----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69834/#review212345 -----------------------------------------------------------
Fix it, then Ship it! Some minor comments related to logs. Rest looks good. standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Lines 472 (patched) <https://reviews.apache.org/r/69834/#comment298112> Nit, It think it would be useful to say "Defaults to jssecacerts, if it exists, otherwise uses cacerts" standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 362 (patched) <https://reviews.apache.org/r/69834/#comment298114> May be its useful to specify what is the default. So a message like ".. has not been set. Defaulting to jssecacerts, if it exists. Otherwise, cacerts." standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Line 369 (original), 371 (patched) <https://reviews.apache.org/r/69834/#comment298115> nit, instead of defaulting to default .. may be just say Using default Java truststore password. - Vihang Karajgaonkar On Jan. 25, 2019, 7:22 p.m., Morio Ramdenbourg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69834/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2019, 7:22 p.m.) > > > Review request for hive, Adam Holley, Karthik Manamcheri, Na Li, and Vihang > Karajgaonkar. > > > Bugs: HIVE-21083 > https://issues.apache.org/jira/browse/HIVE-21083 > > > Repository: hive-git > > > Description > ------- > > It was identified that a valid way of configuring TLS is by using the Java > default truststore and directly adding the trusted certificates to it. The > previous HMS implementation did not support this. > > Modified the TLS properties in the following ways: > - Removed the requirement for metastore.dbaccess.ssl.truststore.path. If the > user does not specify a custom one, then it will default to the Java > truststore. > - Removed the logs / warnings on metastore.dbaccess.ssl.truststore.password. > This used to generate a lot of noise if the user did not provide one. Also, > the contents of the truststore is certificates, which is public information > and doesn't require strict security. > - Removed the unit test that checks for an empty truststore path. > > > Diffs > ----- > > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java > 75f0c0a356f3b894408aa54b9cce5220d47d7f26 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 9f721243c94d48eef35acdcbd0c2e143ab6d23ec > > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java > 29738ba19b0d5ed9ec224d2288c0c1c922d0674c > > > Diff: https://reviews.apache.org/r/69834/diff/3/ > > > Testing > ------- > > - Existing unit test coverage > - Manual testing by verifying that these properties can configure TLS to a > MySQL DB > > > Thanks, > > Morio Ramdenbourg > >
