You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2019/04/26 18:07:28 UTC
[geode] branch develop updated: GEODE-6709: Locators should not start when ClusterConfigurationServic… (#3503)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 8320fdc GEODE-6709: Locators should not start when ClusterConfigurationServic… (#3503)
8320fdc is described below
commit 8320fdc4f833501d7d64326bf9531b30e6d11753
Author: Peter Tran <Pe...@users.noreply.github.com>
AuthorDate: Fri Apr 26 14:07:18 2019 -0400
GEODE-6709: Locators should not start when ClusterConfigurationServic… (#3503)
Co-authored-by: Jinmei Liao <ji...@pivotal.io>
---
.../security/SecurityClusterConfigDUnitTest.java | 38 +++++++--------
.../cache/LocatorMisconfigurationTest.java | 36 ++++++++++++++
.../InternalConfigurationPersistenceService.java | 32 ++-----------
.../distributed/internal/InternalLocator.java | 6 +++
.../internal/cache/ClusterConfigurationLoader.java | 5 +-
.../geode/internal/cache/GemFireCacheImpl.java | 4 --
.../functions/DownloadJarFunction.java | 53 ++++++++++++---------
.../functions/DownloadJarFunctionTest.java | 55 ++++++++++++++++++++++
8 files changed, 152 insertions(+), 77 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
index 0cf5d42..4582bd9 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
@@ -14,9 +14,6 @@
*/
package org.apache.geode.security;
-import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER;
-import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
-import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_MANAGER;
import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST_PROCESSOR;
import static org.apache.geode.distributed.ConfigurationProperties.USE_CLUSTER_CONFIGURATION;
@@ -26,7 +23,8 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.util.Properties;
-import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -35,34 +33,32 @@ import org.apache.geode.GemFireConfigException;
import org.apache.geode.distributed.DistributedSystem;
import org.apache.geode.examples.SimpleSecurityManager;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
import org.apache.geode.test.junit.categories.SecurityTest;
import org.apache.geode.test.junit.rules.ServerStarterRule;
@Category({SecurityTest.class})
public class SecurityClusterConfigDUnitTest {
- @Rule
- public ClusterStartupRule lsRule = new ClusterStartupRule();
+ private static MemberVM locator;
+
+ @ClassRule
+ public static ClusterStartupRule cluster = new ClusterStartupRule();
@Rule
public ServerStarterRule serverStarter = new ServerStarterRule();
- @Before
- public void before() throws Exception {
+ @BeforeClass
+ public static void beforeClass() throws Exception {
addIgnoredException(
- "A server cannot specify its own security-manager or security-post-processor when using cluster configuration"
- .toString());
+ "A server cannot specify its own security-manager or security-post-processor when using cluster configuration");
addIgnoredException(
- "A server must use cluster configuration when joining a secured cluster.".toString());
+ "A server must use cluster configuration when joining a secured cluster.");
Properties props = new Properties();
- props.setProperty(JMX_MANAGER, "false");
- props.setProperty(JMX_MANAGER_START, "false");
- props.setProperty(JMX_MANAGER_PORT, 0 + "");
props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
props.setProperty(SECURITY_POST_PROCESSOR, PDXPostProcessor.class.getName());
-
- this.lsRule.startLocatorVM(0, props);
+ locator = cluster.startLocatorVM(0, props);
}
@Test
@@ -75,7 +71,7 @@ public class SecurityClusterConfigDUnitTest {
props.setProperty(USE_CLUSTER_CONFIGURATION, "true");
// initial security properties should only contain initial set of values
- this.serverStarter.startServer(props, this.lsRule.getMember(0).getPort());
+ this.serverStarter.startServer(props, locator.getPort());
DistributedSystem ds = this.serverStarter.getCache().getDistributedSystem();
// after cache is created, we got the security props passed in by cluster config
@@ -97,7 +93,7 @@ public class SecurityClusterConfigDUnitTest {
props.setProperty(USE_CLUSTER_CONFIGURATION, "true");
// initial security properties should only contain initial set of values
- this.serverStarter.startServer(props, this.lsRule.getMember(0).getPort());
+ this.serverStarter.startServer(props, locator.getPort());
DistributedSystem ds = this.serverStarter.getCache().getDistributedSystem();
// after cache is created, we got the security props passed in by cluster config
@@ -121,7 +117,7 @@ public class SecurityClusterConfigDUnitTest {
// initial security properties should only contain initial set of values
assertThatThrownBy(
- () -> this.serverStarter.startServer(props, this.lsRule.getMember(0).getPort()))
+ () -> this.serverStarter.startServer(props, locator.getPort()))
.isInstanceOf(GemFireConfigException.class).hasMessage(
"A server cannot specify its own security-manager or security-post-processor when using cluster configuration");
}
@@ -139,7 +135,7 @@ public class SecurityClusterConfigDUnitTest {
// initial security properties should only contain initial set of values
assertThatThrownBy(
- () -> this.serverStarter.startServer(props, this.lsRule.getMember(0).getPort()))
+ () -> this.serverStarter.startServer(props, locator.getPort()))
.isInstanceOf(GemFireConfigException.class).hasMessage(
"A server cannot specify its own security-manager or security-post-processor when using cluster configuration");
}
@@ -155,7 +151,7 @@ public class SecurityClusterConfigDUnitTest {
props.setProperty(USE_CLUSTER_CONFIGURATION, "false");
assertThatThrownBy(
- () -> this.serverStarter.startServer(props, this.lsRule.getMember(0).getPort()))
+ () -> this.serverStarter.startServer(props, this.cluster.getMember(0).getPort()))
.isInstanceOf(GemFireConfigException.class).hasMessage(
"A server must use cluster configuration when joining a secured cluster.");
}
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/LocatorMisconfigurationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/LocatorMisconfigurationTest.java
new file mode 100644
index 0000000..0d211e2
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/internal/cache/LocatorMisconfigurationTest.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.cache;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.test.junit.rules.LocatorStarterRule;
+
+public class LocatorMisconfigurationTest {
+
+ @Rule
+ public LocatorStarterRule locator = new LocatorStarterRule();
+
+ @Test
+ public void misconfiguredLocator() {
+ assertThatThrownBy(() -> locator.withProperty("jmx-manager", "false")
+ .withProperty("enable-cluster-configuration", "true").startLocator())
+ .isInstanceOf(IllegalStateException.class)
+ .hasMessageContaining("Cannot start cluster configuration without jmx-manager=true");
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
index f5b7681..fdcea8d 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalConfigurationPersistenceService.java
@@ -21,10 +21,8 @@ import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_POST
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileFilter;
-import java.io.FileOutputStream;
import java.io.FileWriter;
import java.io.IOException;
-import java.io.InputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.nio.file.Files;
@@ -33,7 +31,6 @@ import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -49,12 +46,9 @@ import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactoryConfigurationError;
-import com.healthmarketscience.rmiio.RemoteInputStream;
-import com.healthmarketscience.rmiio.RemoteInputStreamClient;
import joptsimple.internal.Strings;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.FilenameUtils;
-import org.apache.commons.io.IOUtils;
import org.apache.commons.io.filefilter.DirectoryFileFilter;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
@@ -74,12 +68,12 @@ import org.apache.geode.cache.Scope;
import org.apache.geode.cache.TimeoutException;
import org.apache.geode.cache.configuration.CacheConfig;
import org.apache.geode.cache.configuration.XSDRootElement;
-import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.ConfigurationPersistenceService;
import org.apache.geode.distributed.DistributedLockService;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.DistributedSystem;
import org.apache.geode.distributed.internal.locks.DLockService;
+import org.apache.geode.internal.cache.ClusterConfigurationLoader;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.InternalRegionArguments;
import org.apache.geode.internal.cache.persistence.PersistentMemberID;
@@ -89,14 +83,11 @@ import org.apache.geode.internal.cache.xmlcache.CacheXmlGenerator;
import org.apache.geode.internal.config.JAXBService;
import org.apache.geode.internal.lang.SystemPropertyHelper;
import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.management.internal.beans.FileUploader;
-import org.apache.geode.management.internal.cli.CliUtil;
import org.apache.geode.management.internal.cli.util.ClasspathScanLoadHelper;
import org.apache.geode.management.internal.configuration.callbacks.ConfigurationChangeListener;
import org.apache.geode.management.internal.configuration.domain.Configuration;
import org.apache.geode.management.internal.configuration.domain.SharedConfigurationStatus;
import org.apache.geode.management.internal.configuration.domain.XmlEntity;
-import org.apache.geode.management.internal.configuration.functions.DownloadJarFunction;
import org.apache.geode.management.internal.configuration.messages.ConfigurationResponse;
import org.apache.geode.management.internal.configuration.messages.SharedConfigurationStatusResponse;
import org.apache.geode.management.internal.configuration.utils.XmlUtils;
@@ -479,25 +470,8 @@ public class InternalConfigurationPersistenceService implements ConfigurationPer
*/
public File downloadJar(DistributedMember locator, String groupName, String jarName)
throws IOException {
- ResultCollector<RemoteInputStream, List<RemoteInputStream>> rc =
- (ResultCollector<RemoteInputStream, List<RemoteInputStream>>) CliUtil.executeFunction(
- new DownloadJarFunction(), new Object[] {groupName, jarName},
- Collections.singleton(locator));
-
- List<RemoteInputStream> result = rc.getResult();
- RemoteInputStream jarStream = result.get(0);
-
- Path tempDir = FileUploader.createSecuredTempDirectory("deploy-");
- Path tempJar = Paths.get(tempDir.toString(), jarName);
- FileOutputStream fos = new FileOutputStream(tempJar.toString());
- InputStream input = RemoteInputStreamClient.wrap(jarStream);
-
- IOUtils.copy(input, fos);
-
- fos.close();
- input.close();
-
- return tempJar.toFile();
+ ClusterConfigurationLoader loader = new ClusterConfigurationLoader();
+ return loader.downloadJar(locator, groupName, jarName);
}
/**
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
index 127a264..7ece981 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalLocator.java
@@ -1431,6 +1431,7 @@ public class InternalLocator extends Locator implements ConnectListener, LogConf
}
private void startConfigurationPersistenceService() {
+
installRequestHandlers();
if (!config.getEnableClusterConfiguration()) {
@@ -1438,6 +1439,11 @@ public class InternalLocator extends Locator implements ConnectListener, LogConf
return;
}
+ if (!config.getJmxManager()) {
+ throw new IllegalStateException(
+ "Cannot start cluster configuration without jmx-manager=true");
+ }
+
if (isSharedConfigurationStarted) {
logger.info("Cluster configuration service is already started.");
return;
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java b/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
index 3e15b00..7b8d1f8 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java
@@ -133,7 +133,7 @@ public class ClusterConfigurationLoader {
return results;
}
- public File downloadJar(DistributedMember locator, String groupName, String jarName)
+ public static File downloadJar(DistributedMember locator, String groupName, String jarName)
throws IOException {
ResultCollector<RemoteInputStream, List<RemoteInputStream>> rc =
(ResultCollector<RemoteInputStream, List<RemoteInputStream>>) CliUtil.executeFunction(
@@ -141,6 +141,9 @@ public class ClusterConfigurationLoader {
Collections.singleton(locator));
List<RemoteInputStream> result = rc.getResult();
+ if (result.get(0) instanceof Throwable) {
+ throw new IllegalStateException(((Throwable) result.get(0)).getMessage());
+ }
Path tempDir = FileUploader.createSecuredTempDirectory("deploy-");
Path tempJar = Paths.get(tempDir.toString(), jarName);
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index 3f676f5..47a87cb 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -1254,10 +1254,6 @@ public class GemFireCacheImpl implements InternalCache, InternalClientCache, Has
}
}
- private boolean isNotJmxManager() {
- return !this.system.getConfig().getJmxManagerStart();
- }
-
private boolean isServerNode() {
return this.system.getDistributedMember()
.getVmKind() != ClusterDistributionManager.LOCATOR_DM_TYPE
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunction.java
index 88712fb..1f29a90 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunction.java
@@ -18,8 +18,6 @@ package org.apache.geode.management.internal.configuration.functions;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
-import java.io.FileNotFoundException;
-import java.rmi.RemoteException;
import com.healthmarketscience.rmiio.RemoteInputStream;
import com.healthmarketscience.rmiio.RemoteInputStreamServer;
@@ -28,12 +26,12 @@ import com.healthmarketscience.rmiio.exporter.RemoteStreamExporter;
import org.apache.logging.log4j.Logger;
import org.apache.geode.cache.execute.FunctionContext;
-import org.apache.geode.cache.execute.FunctionException;
import org.apache.geode.distributed.Locator;
import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
import org.apache.geode.distributed.internal.InternalLocator;
import org.apache.geode.internal.cache.execute.InternalFunction;
import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.management.internal.ManagementAgent;
import org.apache.geode.management.internal.SystemManagementService;
public class DownloadJarFunction implements InternalFunction<Object[]> {
@@ -43,7 +41,7 @@ public class DownloadJarFunction implements InternalFunction<Object[]> {
@Override
public void execute(FunctionContext<Object[]> context) {
- InternalLocator locator = (InternalLocator) Locator.getLocator();
+ InternalLocator locator = getLocator();
Object[] args = context.getArguments();
String group = (String) args[0];
String jarName = (String) args[1];
@@ -53,30 +51,32 @@ public class DownloadJarFunction implements InternalFunction<Object[]> {
InternalConfigurationPersistenceService sharedConfig =
locator.getConfigurationPersistenceService();
if (sharedConfig != null) {
+ SystemManagementService managementService = getExistingManagementService(context);
+ ManagementAgent managementAgent = managementService.getManagementAgent();
+ if (managementAgent == null) {
+ throw new IllegalStateException(
+ "Failed to download jar because JMX Management agent is not available. Please ensure geode property jmx-manager is set to true.");
+ }
+
+ RemoteInputStreamServer istream = null;
try {
File jarFile = sharedConfig.getPathToJarOnThisLocator(group, jarName).toFile();
- RemoteStreamExporter exporter = ((SystemManagementService) SystemManagementService
- .getExistingManagementService(context.getCache())).getManagementAgent()
- .getRemoteStreamExporter();
- RemoteInputStreamServer istream = null;
- try {
- istream =
- new SimpleRemoteInputStream(new BufferedInputStream(new FileInputStream(jarFile)));
- result = exporter.export(istream);
- istream = null;
- } catch (FileNotFoundException | RemoteException ex) {
- throw new FunctionException(ex);
- } finally {
- // we will only close the stream here if the server fails before
- // returning an exported stream
- if (istream != null) {
- istream.close();
- }
- }
+ RemoteStreamExporter exporter = managementAgent.getRemoteStreamExporter();
+ istream =
+ new SimpleRemoteInputStream(new BufferedInputStream(new FileInputStream(jarFile)));
+ result = exporter.export(istream);
+ istream = null;
+
} catch (Exception e) {
logger.error(e);
throw new IllegalStateException(e.getMessage());
+ } finally {
+ // we will only close the stream here if the server fails before
+ // returning an exported stream
+ if (istream != null) {
+ istream.close();
+ }
}
}
}
@@ -84,6 +84,15 @@ public class DownloadJarFunction implements InternalFunction<Object[]> {
context.getResultSender().lastResult(result);
}
+ InternalLocator getLocator() {
+ return (InternalLocator) Locator.getLocator();
+ }
+
+ SystemManagementService getExistingManagementService(FunctionContext<Object[]> context) {
+ return (SystemManagementService) SystemManagementService
+ .getExistingManagementService(context.getCache());
+ }
+
@Override
public String getId() {
return DownloadJarFunction.class.getName();
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunctionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunctionTest.java
new file mode 100644
index 0000000..39a08ab
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/functions/DownloadJarFunctionTest.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.management.internal.configuration.functions;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.distributed.internal.InternalConfigurationPersistenceService;
+import org.apache.geode.distributed.internal.InternalLocator;
+import org.apache.geode.management.internal.SystemManagementService;
+
+public class DownloadJarFunctionTest {
+
+ private DownloadJarFunction function;
+
+ private FunctionContext<Object[]> context;
+
+ @Before
+ public void setUp() throws Exception {
+ function = spy(DownloadJarFunction.class);
+ context = mock(FunctionContext.class);
+ when(context.getArguments()).thenReturn(new String[] {"hello", "world"});
+ InternalLocator locator = mock(InternalLocator.class);
+ when(locator.getConfigurationPersistenceService()).thenReturn(mock(
+ InternalConfigurationPersistenceService.class));
+ doReturn(locator).when(function).getLocator();
+ doReturn(mock(SystemManagementService.class)).when(function)
+ .getExistingManagementService(context);
+ }
+
+ @Test
+ public void throwExceptionWhenManagementAgentIsNull() {
+ assertThatThrownBy(() -> function.execute(context)).isInstanceOf(IllegalStateException.class)
+ .hasMessageContaining("Failed to download jar");
+ }
+}