[GitHub] [geode] Bill commented on a change in pull request #6308: GEODE-9139 SSLException in starting up a Locator

[GitBox <gi...@apache.org>]

Bill commented on a change in pull request #6308:
URL: https://github.com/apache/geode/pull/6308#discussion_r612051364



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/DualServerSNIAcceptanceTest.java
##########
@@ -60,7 +61,7 @@
 public class DualServerSNIAcceptanceTest {
 
   private static final URL DOCKER_COMPOSE_PATH =
-      SingleServerSNIAcceptanceTest.class.getResource("docker-compose.yml");
+      DualServerSNIAcceptanceTest.class.getResource("dual-server-docker-compose.yml");

Review comment:
       good catch!

##########
File path: geode-assembly/src/acceptanceTest/resources/org/apache/geode/client/sni/scripts/geode-starter-2.gfsh
##########
@@ -15,7 +15,7 @@
 # limitations under the License.
 #
 
-start locator --name=locator-maeve --connect=false --redirect-output --hostname-for-clients=locator-maeve --properties-file=/geode/config/gemfire.properties --security-properties-file=/geode/config/gfsecurity.properties --J=-Dgemfire.ssl-keystore=/geode/config/locator-maeve-keystore.jks
+start locator --name=locator-maeve --connect=false --redirect-output --hostname-for-clients=locator-maeve --jmx-manager-hostname-for-clients=geode --properties-file=/geode/config/gemfire.properties --security-properties-file=/geode/config/gfsecurity.properties --J=-Dgemfire.ssl-keystore=/geode/config/locator-maeve-keystore.jks

Review comment:
       I don't think this script is called any more. Please delete it.

##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/GenerateSNIKeyAndTrustStores.java
##########
@@ -51,10 +50,8 @@ public void generateStores() throws Exception {
       CertificateMaterial certificate = new CertificateBuilder(365 * 100, "SHA256withRSA")
           .commonName(certName)
           .issuedBy(ca)
-          .sanDnsName("geode") // for inside the docker container
-          .sanDnsName("localhost") // for inside the docker container
-          .sanIpAddress(InetAddress.getByName("0.0.0.0")) // for inside the docker container
-          .sanDnsName(certName) // for client endpoint validation
+          .sanDnsName(certName)
+          .sanDnsName("geode")

Review comment:
       This looks better! Leaving off "localhost" and the address of the first interface seems much less hackey.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org