----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69834/#review212295 -----------------------------------------------------------
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Line 465 (original), 466 (patched) <https://reviews.apache.org/r/69834/#comment298046> The statement about exposing it in a way to be discovered by others is confusing. Consider rephrasing. Instead of providing an inadvisable statement, provide an advice instead on what the user should be doing. standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java Line 470 (original), 472 (patched) <https://reviews.apache.org/r/69834/#comment298047> Can you just say, "Defaults to the default Java truststore file..." similar to the pattern of other properties. standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Line 360 (original), 360 (patched) <https://reviews.apache.org/r/69834/#comment298048> Can we do a LOG.info here stating that the truststore path has not been set and we will default to the Java truststore file? Same for the truststore password. - Karthik Manamcheri On Jan. 24, 2019, 11:16 p.m., Morio Ramdenbourg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69834/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2019, 11:16 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/1/ > > > Testing > ------- > > - Existing unit test coverage > - Manual testing by verifying that these properties can configure TLS to a > MySQL DB > > > Thanks, > > Morio Ramdenbourg > >
