----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61417/#review182181 -----------------------------------------------------------
1. The help message for the property should state what it does and whether it has a default. JKS was our assumption in the past, but with the introduction of this feature other values (such as 'pkcs12') are possible. Current wording: "For Java truststore file format, this property has the value jks (or JKS)." Suggested wording: "Specifies the type of the ssl truststore. The default is '' (an empty string) For Java truststore format, specify 'jks' or 'JKS'." The following questions come from me reading for patterns, please forgive if they're stupid ones: 2. In configureLegacyClusterSSL (and ..ServerSSL and ..JMXSSL and ..ServiceSSL), I see two occurrences of "sslConfig.setTruststoreType(getDistributionConfig().getClusterSSLKeyStoreType());". Should the second one in each case be getClusterSSLTrustStoreType() ? 3. In DistributionConfigImpl.java, there's a long list of static imports for gateway, http, jmx, etc. KEYSTORE_TYPE is included for each group, but not TRUSTSTORE_TYPE. Is this correct? - Dave Barnes On Aug. 3, 2017, 9:15 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61417/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2017, 9:15 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, > Patrick Rhomberg, and Udo Kohlmeyer. > > > Repository: geode > > > Description > ------- > > GEODE-3328: adding ssl-truststore-type to the config > > this is what's requested in the JIRA ticket. This changeset just deals with > adding the property into gemfire properties > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java > 63f6505101f6edb62167b854d3d16d76d0a893cd > > geode-core/src/main/java/org/apache/geode/distributed/internal/AbstractDistributionConfig.java > 795f6a5a4a4f42fe065c9ca6013fd5598f9311d8 > > geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java > c2a395de0bfe21ed8efeb6b25e331f7085b3bf4f > > geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java > fbe894c96447b2e30594eb2ed0652dd08e1028ce > > geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java > f86f07eb5e58b8509e909b7748795530efd65339 > geode-core/src/main/java/org/apache/geode/management/GemFireProperties.java > 08fa9b54ea066b4158478ae89d8295ed0b1a337b > > geode-core/src/main/java/org/apache/geode/management/internal/beans/BeanUtilFuncs.java > 499ef010f354ebf88765190f1b5eb945da83accc > > geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java > 525f988cd3cb4f19872168df9b7105645f5c0498 > > > Diff: https://reviews.apache.org/r/61417/diff/1/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
