From 276582533c885092d8e1d076d426a4c9f2e0b908 Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Tue, 7 Apr 2020 14:27:38 -0700 Subject: [PATCH 1/5] GEODE-7852: SNI extension support Modified SNISocketFactory so it can be used in cache.xml Added a test for the new cache.xml element. Updated docs for cache.xml and updated client configuration instructions. --- .../client/sni/ClientSNIAcceptanceTest.java | 1 + .../client/ClientCacheFactoryJUnitTest.java | 10 ++++++- ...lientCacheFactoryJUnitTest_single_pool.xml | 9 +++++++ .../cache/client/proxy/SniSocketFactory.java | 20 +++++++++++--- .../client-cache-elements-list.html.md.erb | 1 + .../reference/topics/client-cache.html.md.erb | 26 +++++++++++++++++++ ...ting_up_a_client_server_system.html.md.erb | 17 ++++++++++-- .../test/dunit/internal/ProcessManager.java | 6 +++-- 8 files changed, 82 insertions(+), 8 deletions(-) rename geode-core/src/{test => integrationTest}/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml (82%) diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java b/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java index 69aa5d8e9e21..4f47fd05a340 100644 --- a/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java +++ b/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java @@ -97,5 +97,6 @@ public void connectToSNIProxyDocker() { region.destroy("hello"); region.put("hello", "world"); assertThat(region.get("hello")).isEqualTo("world"); + assertThat(region.get("foo")).isEqualTo("bar"); } } diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java index 90744734ff1f..e5147df54947 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java @@ -32,6 +32,7 @@ import java.io.PrintWriter; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.Socket; import java.net.URL; import java.nio.charset.Charset; import java.util.Collections; @@ -52,6 +53,7 @@ import org.apache.geode.cache.RegionService; import org.apache.geode.cache.client.internal.ProxyCache; import org.apache.geode.cache.client.internal.UserAttributes; +import org.apache.geode.cache.client.proxy.SniSocketFactory; import org.apache.geode.cache.server.CacheServer; import org.apache.geode.distributed.DistributedSystem; import org.apache.geode.distributed.internal.InternalDistributedSystem; @@ -62,6 +64,7 @@ import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID; import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator; import org.apache.geode.internal.cache.xmlcache.ClientCacheCreation; +import org.apache.geode.internal.inet.LocalHostUtil; import org.apache.geode.internal.serialization.Version; import org.apache.geode.internal.serialization.VersionedDataInputStream; import org.apache.geode.pdx.ReflectionBasedAutoSerializer; @@ -127,7 +130,7 @@ public void test000Defaults() throws Exception { } @Test - public void test001FindDefaultFromXML() throws Exception { + public void test001FindDefaultPoolFromXML() throws Exception { File cacheXmlFile = temporaryFolder.newFile("ClientCacheFactoryJUnitTest.xml"); URL url = ClientCacheFactoryJUnitTest.class .getResource("ClientCacheFactoryJUnitTest_single_pool.xml"); @@ -149,6 +152,11 @@ public void test001FindDefaultFromXML() throws Exception { .isEqualTo(PoolFactory.DEFAULT_SOCKET_CONNECT_TIMEOUT); assertThat(defPool.getServers()).isEqualTo( Collections.singletonList(new InetSocketAddress("localhost", CacheServer.DEFAULT_PORT))); + + assertThat(defPool.getSocketFactory()).isInstanceOf(SniSocketFactory.class); + Socket socket = defPool.getSocketFactory().createSocket(); + assertThat(socket.getPort()).isEqualTo(12345); + assertThat(socket.getInetAddress()).isEqualTo(LocalHostUtil.getLocalHost()); } /** diff --git a/geode-core/src/test/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml b/geode-core/src/integrationTest/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml similarity index 82% rename from geode-core/src/test/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml rename to geode-core/src/integrationTest/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml index f139459626d6..82666d93727e 100644 --- a/geode-core/src/test/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml +++ b/geode-core/src/integrationTest/resources/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest_single_pool.xml @@ -27,5 +27,14 @@ version="1.0"> + + org.apache.geode.cache.client.proxy.SniSocketFactory + + localhost + + + 40404 + + diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java b/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java index 584a4055e795..f7ee5c3ab1c3 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java @@ -18,24 +18,38 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.Socket; +import java.util.Properties; +import org.apache.geode.cache.Cache; +import org.apache.geode.cache.Declarable; import org.apache.geode.cache.client.SocketFactory; +import org.apache.geode.internal.DistributionLocator; /** * A {@link SocketFactory} that connects a client to locators and servers * through a SNI proxy. */ -public class SniSocketFactory implements SocketFactory { +public class SniSocketFactory implements SocketFactory, Declarable { - private final String hostname; - private final int port; + private String hostname; + private int port; + + public SniSocketFactory() {} // required by Declarable public SniSocketFactory(String hostname, int port) { this.hostname = hostname; this.port = port; } + @Override // Declarable + public void initialize(Cache cache, Properties properties) { + this.hostname = properties.getProperty("hostname"); + String portString = + properties.getProperty("port", "" + DistributionLocator.DEFAULT_LOCATOR_PORT); + this.port = Integer.parseInt(portString); + } + @Override public Socket createSocket() throws IOException { return new SniProxySocket(new InetSocketAddress(hostname, port)); diff --git a/geode-docs/reference/topics/client-cache-elements-list.html.md.erb b/geode-docs/reference/topics/client-cache-elements-list.html.md.erb index 0d26303c427b..edb6f6d4d8de 100644 --- a/geode-docs/reference/topics/client-cache-elements-list.html.md.erb +++ b/geode-docs/reference/topics/client-cache-elements-list.html.md.erb @@ -31,6 +31,7 @@ For details, see [<client-cache> Element Reference.](client-cache.html) + diff --git a/geode-docs/reference/topics/client-cache.html.md.erb b/geode-docs/reference/topics/client-cache.html.md.erb index 0db531bc8689..a221e3e99d34 100644 --- a/geode-docs/reference/topics/client-cache.html.md.erb +++ b/geode-docs/reference/topics/client-cache.html.md.erb @@ -338,6 +338,32 @@ Provide a server list or `locator` list, but not both. port="123456"/> ``` +## <socket-factory> + +Defines a factory to create socket connections to locators and servers. A typical use of this element is to redirect connections to an ingress gateway such as Istio or HAProxy in a cluster where the TLS (SSL) Server Name Extension field is set to indicate the actual locator or server the client is trying to reach. This allows you to expose only the gateway hostname:port without the client needing to be able to resolve the names of the locator and server machines. + +**Note:** +This setting may be used with either a Server list or a Locator list. It will be used to form connections to either. + +**Default:** + +**API:** `org.apache.geode.cache.client.proxy.ProxySocketFactories` + +**Example:** + +``` pre + + + org.apache.geode.cache.client.proxy.SniSocketFactory + + my-haproxy-address + + + 12345 + + + +``` ## <disk-store> diff --git a/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb b/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb index 7bdeaa55349f..c032cac82d96 100644 --- a/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb +++ b/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb @@ -53,8 +53,8 @@ Configure your server and client processes and data regions to run your client/s - - + + ... + + org.apache.geode.cache.client.proxy.SniSocketFactory + + my-gateway-address + + + my-gateway-port-number + + + + 3. Configure the server data regions for client/server work, following these guidelines. These do not need to be performed in this order. diff --git a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java index bee2551a70e0..73af9ddabe22 100755 --- a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java +++ b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java @@ -258,8 +258,10 @@ private String[] buildJavaCommand(int vmNum, int namingPort, String version) { // remove current-version product classes and resources from the classpath dunitClasspath = removeModulesFromPath(dunitClasspath, "geode-common", "geode-core", "geode-cq", - "geode-http-service", "geode-json", "geode-log4j", "geode-lucene", - "geode-serialization", "geode-wan", "geode-gfsh"); + "geode-http-service", "geode-json", "geode-log4j", "geode-lucene", "geode-tcp-server", + "geode-membership", "geode-management", "geode-logging", "geode-web", + "geode-rebalancer", + "geode-serialization", "geode-wan", "geode-gfsh", "geode-lucene"); classPath = versionManager.getClasspath(version) + File.pathSeparator + dunitClasspath; } From 1260318f02dfb5e84b8e85d744a133217b1abe9f Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Wed, 8 Apr 2020 07:37:54 -0700 Subject: [PATCH 2/5] addressing reviews --- .../org/apache/geode/client/sni/ClientSNIAcceptanceTest.java | 5 +++++ geode-docs/reference/topics/client-cache.html.md.erb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java b/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java index 4f47fd05a340..8c8f39792e53 100644 --- a/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java +++ b/geode-assembly/src/acceptanceTest/java/org/apache/geode/client/sni/ClientSNIAcceptanceTest.java @@ -91,12 +91,17 @@ public void connectToSNIProxyDocker() { .setPoolSocketFactory(ProxySocketFactories.sni("localhost", proxyPort)) .create(); + // the geode-starter.gfsh script has created a Region named "jellyfish" on the + // server sitting behind the haproxy gateway. Show that an empty client cache can + // put something in that region and then retrieve it. Region region = cache.createClientRegionFactory(ClientRegionShortcut.PROXY) .create("jellyfish"); region.destroy("hello"); region.put("hello", "world"); assertThat(region.get("hello")).isEqualTo("world"); + // the geode-starter.gfsh script put an entry named "foo" into the region with the + // value "bar" assertThat(region.get("foo")).isEqualTo("bar"); } } diff --git a/geode-docs/reference/topics/client-cache.html.md.erb b/geode-docs/reference/topics/client-cache.html.md.erb index a221e3e99d34..0992c1a35b2d 100644 --- a/geode-docs/reference/topics/client-cache.html.md.erb +++ b/geode-docs/reference/topics/client-cache.html.md.erb @@ -340,7 +340,7 @@ Provide a server list or `locator` list, but not both. ``` ## <socket-factory> -Defines a factory to create socket connections to locators and servers. A typical use of this element is to redirect connections to an ingress gateway such as Istio or HAProxy in a cluster where the TLS (SSL) Server Name Extension field is set to indicate the actual locator or server the client is trying to reach. This allows you to expose only the gateway hostname:port without the client needing to be able to resolve the names of the locator and server machines. +Defines a factory to create socket connections to locators and servers. A typical use of this element is to redirect connections to an ingress gateway such as Istio or HAProxy in a cluster where the TLS (SSL) Server Name Indication (SNI)field is set to indicate the actual locator or server the client is trying to reach. This allows you to expose only the gateway hostname:port without the client needing to be able to resolve the names of the locator and server machines. **Note:** This setting may be used with either a Server list or a Locator list. It will be used to form connections to either. From d301de9c6bd1d145285a4eb1435ece9e9d254820 Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Wed, 8 Apr 2020 10:38:19 -0700 Subject: [PATCH 3/5] addressing reviews --- geode-docs/reference/topics/client-cache.html.md.erb | 4 ++-- .../setting_up_a_client_server_system.html.md.erb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/geode-docs/reference/topics/client-cache.html.md.erb b/geode-docs/reference/topics/client-cache.html.md.erb index 0992c1a35b2d..ab3c0e8634b1 100644 --- a/geode-docs/reference/topics/client-cache.html.md.erb +++ b/geode-docs/reference/topics/client-cache.html.md.erb @@ -340,7 +340,7 @@ Provide a server list or `locator` list, but not both. ``` ## <socket-factory> -Defines a factory to create socket connections to locators and servers. A typical use of this element is to redirect connections to an ingress gateway such as Istio or HAProxy in a cluster where the TLS (SSL) Server Name Indication (SNI)field is set to indicate the actual locator or server the client is trying to reach. This allows you to expose only the gateway hostname:port without the client needing to be able to resolve the names of the locator and server machines. +Defines a factory to create socket connections to locators and servers. A typical use of this element is to redirect connections to an ingress gateway such as Istio or HAProxy in a cluster where the TLS (SSL) Server Name Indication (SNI) field is set to indicate the actual locator or server the client is trying to reach. This allows you to expose only the gateway hostname:port without the client needing to be able to resolve the names of the locator and server machines. **Note:** This setting may be used with either a Server list or a Locator list. It will be used to form connections to either. @@ -356,7 +356,7 @@ This setting may be used with either a Server list or a Locator list. It will b org.apache.geode.cache.client.proxy.SniSocketFactory - my-haproxy-address + my-gateway-address 12345 diff --git a/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb b/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb index c032cac82d96..a9c82c10c282 100644 --- a/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb +++ b/geode-docs/topologies_and_comm/cs_configuration/setting_up_a_client_server_system.html.md.erb @@ -70,7 +70,7 @@ Configure your server and client processes and data regions to run your client/s my-gateway-address - my-gateway-port-number + 12345 From 381714eb08c454894fe442d1df21c23adad5bc4e Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Wed, 8 Apr 2020 14:47:20 -0700 Subject: [PATCH 4/5] fixing new assertions in unit test and retriggering CI tasks --- .../cache/client/ClientCacheFactoryJUnitTest.java | 12 ++++++------ .../geode/cache/client/proxy/SniSocketFactory.java | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java index e5147df54947..0ca9a0c8d703 100644 --- a/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java +++ b/geode-core/src/integrationTest/java/org/apache/geode/cache/client/ClientCacheFactoryJUnitTest.java @@ -32,7 +32,6 @@ import java.io.PrintWriter; import java.net.InetAddress; import java.net.InetSocketAddress; -import java.net.Socket; import java.net.URL; import java.nio.charset.Charset; import java.util.Collections; @@ -64,7 +63,6 @@ import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID; import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator; import org.apache.geode.internal.cache.xmlcache.ClientCacheCreation; -import org.apache.geode.internal.inet.LocalHostUtil; import org.apache.geode.internal.serialization.Version; import org.apache.geode.internal.serialization.VersionedDataInputStream; import org.apache.geode.pdx.ReflectionBasedAutoSerializer; @@ -153,10 +151,12 @@ public void test001FindDefaultPoolFromXML() throws Exception { assertThat(defPool.getServers()).isEqualTo( Collections.singletonList(new InetSocketAddress("localhost", CacheServer.DEFAULT_PORT))); - assertThat(defPool.getSocketFactory()).isInstanceOf(SniSocketFactory.class); - Socket socket = defPool.getSocketFactory().createSocket(); - assertThat(socket.getPort()).isEqualTo(12345); - assertThat(socket.getInetAddress()).isEqualTo(LocalHostUtil.getLocalHost()); + // verify that the SocketCreator settings were correctly picked up from the xml file + SocketFactory factory = defPool.getSocketFactory(); + assertThat(factory).isInstanceOf(SniSocketFactory.class); + SniSocketFactory sniSocketFactory = (SniSocketFactory) factory; + assertThat(sniSocketFactory.getPort()).isEqualTo(40404); + assertThat(sniSocketFactory.getHostname()).isEqualTo("localhost"); } /** diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java b/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java index f7ee5c3ab1c3..f8059a312fba 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/proxy/SniSocketFactory.java @@ -44,6 +44,7 @@ public SniSocketFactory(String hostname, int port) { @Override // Declarable public void initialize(Cache cache, Properties properties) { + System.out.println("BRUCE: sni properties are " + properties); this.hostname = properties.getProperty("hostname"); String portString = properties.getProperty("port", "" + DistributionLocator.DEFAULT_LOCATOR_PORT); @@ -52,6 +53,15 @@ public void initialize(Cache cache, Properties properties) { @Override public Socket createSocket() throws IOException { + System.out.println("BRUCE: creating a socket with " + hostname + ":" + port); return new SniProxySocket(new InetSocketAddress(hostname, port)); } + + public String getHostname() { + return hostname; + } + + public int getPort() { + return port; + } } From 47cc74f22714ae8d598c337f4ab92e72fe02d28e Mon Sep 17 00:00:00 2001 From: Bruce Schuchardt Date: Wed, 8 Apr 2020 16:08:45 -0700 Subject: [PATCH 5/5] reverting ProcessManager changes - evidently the rolling upgrade test code needs some of this stuff --- .../apache/geode/test/dunit/internal/ProcessManager.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java index 73af9ddabe22..209c822c43a7 100755 --- a/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java +++ b/geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java @@ -257,11 +257,10 @@ private String[] buildJavaCommand(int vmNum, int namingPort, String version) { } else { // remove current-version product classes and resources from the classpath dunitClasspath = - removeModulesFromPath(dunitClasspath, "geode-common", "geode-core", "geode-cq", - "geode-http-service", "geode-json", "geode-log4j", "geode-lucene", "geode-tcp-server", - "geode-membership", "geode-management", "geode-logging", "geode-web", - "geode-rebalancer", - "geode-serialization", "geode-wan", "geode-gfsh", "geode-lucene"); + dunitClasspath = + removeModulesFromPath(dunitClasspath, "geode-common", "geode-core", "geode-cq", + "geode-http-service", "geode-json", "geode-log4j", "geode-lucene", + "geode-serialization", "geode-wan", "geode-gfsh"); classPath = versionManager.getClasspath(version) + File.pathSeparator + dunitClasspath; }