Repository: incubator-geode Updated Branches: refs/heads/develop 0c9002b20 -> 58e0026c6
GEODE-1955: properly disconnect gfsh session so that it won't leave heartbeat thread around to pollute other tests * fix the test so that it truely tests the jmx ssl Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/58e0026c Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/58e0026c Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/58e0026c Branch: refs/heads/develop Commit: 58e0026c6dda077fbcedea1289993f5d36b67a74 Parents: 0c9002b Author: Jinmei Liao <jil...@pivotal.io> Authored: Thu Nov 10 14:30:47 2016 -0800 Committer: Jinmei Liao <jil...@pivotal.io> Committed: Tue Nov 15 15:38:26 2016 -0600 ---------------------------------------------------------------------- .../internal/net/SSLConfigurationFactory.java | 20 ++++---- .../management/internal/JmxManagerAdvisee.java | 15 ++++-- .../internal/cli/shell/JmxOperationInvoker.java | 50 ++++++++++---------- .../ConnectToLocatorSSLDUnitTest.java | 45 ++++++++++-------- .../dunit/rules/GfshShellConnectionRule.java | 1 + 5 files changed, 70 insertions(+), 61 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/58e0026c/geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java b/geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java index 76fb041..3731d4d 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java +++ b/geode-core/src/main/java/org/apache/geode/internal/net/SSLConfigurationFactory.java @@ -15,18 +15,17 @@ package org.apache.geode.internal.net; -import java.util.HashMap; -import java.util.Map; -import java.util.Properties; - import org.apache.commons.lang.ArrayUtils; - import org.apache.commons.lang.StringUtils; import org.apache.geode.GemFireConfigException; import org.apache.geode.distributed.internal.DistributionConfig; import org.apache.geode.internal.admin.SSLConfig; import org.apache.geode.internal.security.SecurableCommunicationChannel; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + public class SSLConfigurationFactory { private static SSLConfigurationFactory instance = new SSLConfigurationFactory(); @@ -149,7 +148,7 @@ public class SSLConfigurationFactory { private SSLConfig createSSLConfig(final SecurableCommunicationChannel sslEnabledComponent) { SSLConfig sslConfig = new SSLConfig(); sslConfig.setCiphers(getDistributionConfig().getSSLCiphers()); - sslConfig.setEnabled(determineIfSSLEnabledForSSLComponent(sslEnabledComponent)); + sslConfig.setEnabled(isSSLEnabledForComponent(getDistributionConfig(), sslEnabledComponent)); sslConfig.setKeystore(getDistributionConfig().getSSLKeyStore()); sslConfig.setKeystorePassword(getDistributionConfig().getSSLKeyStorePassword()); sslConfig.setKeystoreType(getDistributionConfig().getSSLKeyStoreType()); @@ -161,18 +160,17 @@ public class SSLConfigurationFactory { return sslConfig; } - private boolean determineIfSSLEnabledForSSLComponent( + public static boolean isSSLEnabledForComponent(DistributionConfig dc, final SecurableCommunicationChannel sslEnabledComponent) { - if (ArrayUtils.contains(getDistributionConfig().getSecurableCommunicationChannels(), + if (ArrayUtils.contains(dc.getSecurableCommunicationChannels(), SecurableCommunicationChannel.NONE)) { return false; } - if (ArrayUtils.contains(getDistributionConfig().getSecurableCommunicationChannels(), + if (ArrayUtils.contains(dc.getSecurableCommunicationChannels(), SecurableCommunicationChannel.ALL)) { return true; } - return ArrayUtils.contains(getDistributionConfig().getSecurableCommunicationChannels(), - sslEnabledComponent) ? true : false; + return ArrayUtils.contains(dc.getSecurableCommunicationChannels(), sslEnabledComponent); } /** http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/58e0026c/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java b/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java index 0467c48..5952c4c 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/JmxManagerAdvisee.java @@ -15,10 +15,17 @@ package org.apache.geode.management.internal; import org.apache.geode.CancelCriterion; -import org.apache.geode.distributed.internal.*; +import org.apache.geode.distributed.internal.DM; +import org.apache.geode.distributed.internal.DistributionAdvisee; +import org.apache.geode.distributed.internal.DistributionAdvisor; import org.apache.geode.distributed.internal.DistributionAdvisor.Profile; -import org.apache.geode.internal.net.SocketCreator; +import org.apache.geode.distributed.internal.DistributionConfig; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.internal.admin.SSLConfig; import org.apache.geode.internal.cache.GemFireCacheImpl; +import org.apache.geode.internal.net.SSLConfigurationFactory; +import org.apache.geode.internal.net.SocketCreator; +import org.apache.geode.internal.security.SecurableCommunicationChannel; import org.apache.geode.management.ManagementService; import org.apache.geode.management.internal.JmxManagerAdvisor.JmxManagerProfile; @@ -110,7 +117,9 @@ public class JmxManagerAdvisee implements DistributionAdvisee { } if (port != 0) { if (!usingJdkConfig) { - ssl = dc.getJmxManagerSSLEnabled(); + SSLConfig jmxSSL = + SSLConfigurationFactory.getSSLConfigForComponent(SecurableCommunicationChannel.JMX); + ssl = jmxSSL.isEnabled(); host = dc.getJmxManagerHostnameForClients(); if (host == null || host.equals("")) { host = dc.getJmxManagerBindAddress(); http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/58e0026c/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java index 456f76b..0fa47d4 100644 --- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java +++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java @@ -14,6 +14,22 @@ */ package org.apache.geode.management.internal.cli.shell; +import static org.apache.geode.distributed.ConfigurationProperties.CLUSTER_SSL_PREFIX; +import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_SSL_PREFIX; + +import org.apache.geode.internal.lang.StringUtils; +import org.apache.geode.internal.util.ArrayUtils; +import org.apache.geode.internal.util.IOUtils; +import org.apache.geode.management.DistributedSystemMXBean; +import org.apache.geode.management.MemberMXBean; +import org.apache.geode.management.internal.MBeanJMXAdapter; +import org.apache.geode.management.internal.ManagementConstants; +import org.apache.geode.management.internal.cli.CliUtil; +import org.apache.geode.management.internal.cli.CommandRequest; +import org.apache.geode.management.internal.cli.LogWrapper; +import org.apache.geode.management.internal.cli.commands.ShellCommands; +import org.apache.geode.management.internal.cli.i18n.CliStrings; + import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -46,22 +62,6 @@ import javax.management.remote.JMXConnectorFactory; import javax.management.remote.JMXServiceURL; import javax.rmi.ssl.SslRMIClientSocketFactory; -import org.apache.geode.distributed.internal.DistributionConfig; -import org.apache.geode.internal.lang.StringUtils; -import org.apache.geode.internal.util.ArrayUtils; -import org.apache.geode.internal.util.IOUtils; -import org.apache.geode.management.DistributedSystemMXBean; -import org.apache.geode.management.MemberMXBean; -import org.apache.geode.management.internal.MBeanJMXAdapter; -import org.apache.geode.management.internal.ManagementConstants; -import org.apache.geode.management.internal.cli.CliUtil; -import org.apache.geode.management.internal.cli.CommandRequest; -import org.apache.geode.management.internal.cli.LogWrapper; -import org.apache.geode.management.internal.cli.commands.ShellCommands; -import org.apache.geode.management.internal.cli.i18n.CliStrings; - -import static org.apache.geode.distributed.ConfigurationProperties.*; - /** * OperationInvoker JMX Implementation * @@ -120,16 +120,13 @@ public class JmxOperationInvoker implements OperationInvoker { Entry<String, String> entry = it.next(); String key = entry.getKey(); String value = entry.getValue(); - if (key.startsWith("javax.") || key.startsWith(DistributionConfig.CLUSTER_SSL_PREFIX) - || key.startsWith(JMX_MANAGER_SSL_PREFIX)) { - key = checkforSystemPropertyPrefix(entry.getKey()); - if ((key.equals(Gfsh.SSL_ENABLED_CIPHERS) || key.equals(Gfsh.SSL_ENABLED_PROTOCOLS)) - && "any".equals(value)) { - continue; - } - System.setProperty(key, value); - propsToClear.add(key); + key = checkforSystemPropertyPrefix(key); + if ((key.equals(Gfsh.SSL_ENABLED_CIPHERS) || key.equals(Gfsh.SSL_ENABLED_PROTOCOLS)) + && "any".equals(value)) { + continue; } + System.setProperty(key, value); + propsToClear.add(key); } if (!sslConfigProps.isEmpty()) { @@ -242,7 +239,8 @@ public class JmxOperationInvoker implements OperationInvoker { if (key.startsWith("javax.")) { returnKey = key; } - if (key.startsWith(CLUSTER_SSL_PREFIX) || key.startsWith(JMX_MANAGER_SSL_PREFIX)) { + if (key.startsWith(CLUSTER_SSL_PREFIX) || key.startsWith(JMX_MANAGER_SSL_PREFIX) + || key.startsWith("ssl-")) { if (key.endsWith("keystore")) { returnKey = Gfsh.SSL_KEYSTORE; } else if (key.endsWith("keystore-password")) { http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/58e0026c/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java index b6afcca..0c16c1a 100644 --- a/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java @@ -37,16 +37,16 @@ import static org.apache.geode.distributed.ConfigurationProperties.SSL_TRUSTSTOR import static org.apache.geode.internal.Assert.assertTrue; import static org.apache.geode.util.test.TestUtil.getResourcePath; -import org.apache.geode.internal.security.SecurableCommunicationChannel; import org.apache.geode.management.internal.cli.i18n.CliStrings; +import org.apache.geode.security.SecurableCommunicationChannels; import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase; import org.apache.geode.test.dunit.rules.GfshShellConnectionRule; import org.apache.geode.test.dunit.rules.LocatorServerStartupRule; import org.apache.geode.test.junit.categories.DistributedTest; -import org.apache.geode.test.junit.categories.FlakyTest; import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -58,7 +58,9 @@ import java.io.OutputStream; import java.util.Properties; @Category(DistributedTest.class) + public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { + @Rule public TemporaryFolder folder = new SerializableTemporaryFolder(); @Rule @@ -80,10 +82,25 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { securityPropsFile.delete(); } + public void setUpLocatorAndConnect(Properties securityProps) throws Exception { + lsRule.getLocatorVM(0, securityProps); + + // saving the securityProps to a file + OutputStream out = new FileOutputStream(securityPropsFile); + securityProps.store(out, ""); + + GfshShellConnectionRule gfshConnector = + new GfshShellConnectionRule(lsRule.getPort(0), GfshShellConnectionRule.PortType.locator); + gfshConnector.connect(CliStrings.CONNECT__SECURITY_PROPERTIES, + securityPropsFile.getCanonicalPath()); + assertTrue(gfshConnector.isConnected()); + + gfshConnector.close(); + } + @Test - public void testConnectToLocatorWithSSL() throws Exception { - securityProps.setProperty(SSL_ENABLED_COMPONENTS, - SecurableCommunicationChannel.LOCATOR.getConstant()); + public void testConnectToLocatorWithSSLJMX() throws Exception { + securityProps.setProperty(SSL_ENABLED_COMPONENTS, SecurableCommunicationChannels.JMX); securityProps.setProperty(SSL_KEYSTORE, jks.getCanonicalPath()); securityProps.setProperty(SSL_KEYSTORE_PASSWORD, "password"); securityProps.setProperty(SSL_KEYSTORE_TYPE, "JKS"); @@ -94,8 +111,8 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { setUpLocatorAndConnect(securityProps); } - @Category(FlakyTest.class) // GEODE-2079 @Test + @Ignore("GEODE-2099") public void testConnectToLocatorWithLegacyClusterSSL() throws Exception { securityProps.setProperty(CLUSTER_SSL_ENABLED, "true"); securityProps.setProperty(CLUSTER_SSL_KEYSTORE, jks.getCanonicalPath()); @@ -108,6 +125,7 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { } @Test + @Ignore("GEODE-2099") public void testConnectToLocatorWithLegacyJMXSSL() throws Exception { securityProps.setProperty(JMX_MANAGER_SSL_ENABLED, "true"); securityProps.setProperty(JMX_MANAGER_SSL_KEYSTORE, jks.getCanonicalPath()); @@ -119,19 +137,4 @@ public class ConnectToLocatorSSLDUnitTest extends JUnit4DistributedTestCase { setUpLocatorAndConnect(securityProps); } - public void setUpLocatorAndConnect(Properties securityProps) throws Exception { - lsRule.getLocatorVM(0, securityProps); - - // saving the securityProps to a file - OutputStream out = new FileOutputStream(securityPropsFile); - securityProps.store(out, ""); - - GfshShellConnectionRule gfshConnector = - new GfshShellConnectionRule(lsRule.getPort(0), GfshShellConnectionRule.PortType.locator); - gfshConnector.connect(CliStrings.CONNECT__SECURITY_PROPERTIES, - securityPropsFile.getCanonicalPath()); - assertTrue(gfshConnector.isConnected()); - gfshConnector.close(); - } - } http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/58e0026c/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java index 44da08b..7490928 100644 --- a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java +++ b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java @@ -97,6 +97,7 @@ public class GfshShellConnectionRule extends DescribedExternalResource { public void close() throws Exception { if (gfsh != null) { gfsh.clear(); + gfsh.executeCommand("disconnect"); gfsh.executeCommand("exit"); gfsh.terminate(); gfsh = null;
