> On Aug. 3, 2017, 11:36 p.m., Dave Barnes wrote: > > 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?
1. I am simply copying from the ssl-keystore-type, should this suggested description also apply to the keystore as well? 2. the truststore type does not exist for legacy ssl specifications, so there is no getClusterSSLTrustStoreType() method. Here I am just assuming the keystore and truststore type is the same. 3. IDEA did that according to the latest import style. - Jinmei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61417/#review182181 ----------------------------------------------------------- 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 > >