From 17097929b666263c559ed18ad28e91105191fd8d Mon Sep 17 00:00:00 2001 From: Sven Vogel Date: Fri, 30 Nov 2018 21:08:01 +0100 Subject: [PATCH 1/3] packaging: correct permissions in spec file and fix class path specified variable (#3030) Install CentOS 7 e.g. Build 1804 and Java build 1.8.0_181 if you inspect systemd in debug mode you will see some errors 1. permission of the cloudstack-managment.service are not corretly set 2. invalid classpath specified. it seems the string which is used will be divided... we now we use ${..} like the lines above ... confused --- packaging/centos7/cloud.spec | 2 +- packaging/systemd/cloudstack-management.service | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packaging/centos7/cloud.spec b/packaging/centos7/cloud.spec index 38f90ce67cb..1cc89939cae 100644 --- a/packaging/centos7/cloud.spec +++ b/packaging/centos7/cloud.spec @@ -486,7 +486,7 @@ pip install --upgrade /usr/share/cloudstack-marvin/Marvin-*.tar.gz %config(noreplace) %{_sysconfdir}/%{name}/management/environment.properties %config(noreplace) %{_sysconfdir}/%{name}/management/java.security.ciphers %config(noreplace) %{_sysconfdir}/%{name}/management/commons-logging.properties -%attr(0755,root,root) %{_unitdir}/%{name}-management.service +%attr(0644,root,root) %{_unitdir}/%{name}-management.service %attr(0755,cloud,cloud) %{_localstatedir}/run/%{name}-management.pid %attr(0755,root,root) %{_bindir}/%{name}-setup-management %attr(0755,root,root) %{_bindir}/%{name}-update-xenserver-licenses diff --git a/packaging/systemd/cloudstack-management.service b/packaging/systemd/cloudstack-management.service index afcea8ca6f6..58c43437c10 100644 --- a/packaging/systemd/cloudstack-management.service +++ b/packaging/systemd/cloudstack-management.service @@ -28,8 +28,8 @@ Environment="NAME=cloudstack-management" EnvironmentFile=/etc/default/cloudstack-management ExecStartPre=/bin/bash -c "/bin/systemctl set-environment JAVA_HOME=$( readlink -f $( which java ) | sed s:bin/.*$:: )" ExecStartPre=/bin/bash -c "/bin/systemctl set-environment JARS=$(ls /usr/share/cloudstack-management/lib/*.jar | tr '\n' ':' | sed s'/.$//')" -ExecStart=/usr/bin/jsvc -home "${JAVA_HOME}" -user "${CLOUDSTACK_USER}" -cp "${JARS}:${CLASSPATH}" -errfile ${LOGDIR}/${NAME}.err -cwd ${LOGDIR} -pidfile "${CLOUDSTACK_PID}" ${JAVA_OPTS} "${BOOTSTRAP_CLASS}" -ExecStop=/usr/bin/jsvc -cp "$JARS:$CLASSPATH" -pidfile "$CLOUDSTACK_PID" -stop "$BOOTSTRAP_CLASS" +ExecStart=/usr/bin/jsvc -home "${JAVA_HOME}" -user "${CLOUDSTACK_USER}" -cp "${JARS}:${CLASSPATH}" -errfile "${LOGDIR}/${NAME}.err" -cwd "${LOGDIR}" -pidfile "${CLOUDSTACK_PID}" "${JAVA_OPTS}" "${BOOTSTRAP_CLASS}" +ExecStop=/usr/bin/jsvc -cp "${JARS}:${CLASSPATH}" -pidfile "${CLOUDSTACK_PID}" -stop "${BOOTSTRAP_CLASS}" SuccessExitStatus=143 [Install] From 89c567add8486ada7528dd7cb2482d7ea43b0d45 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Tue, 4 Dec 2018 01:28:24 +0530 Subject: [PATCH 2/3] security: increase keystore setup/import timeout (#3076) This increases and uses a default 15mins timeout for VR scripts and for KVM agent increases timeout from 60s to 5mins. The timeout can specifically occur when keystore does not get enough entropy from CPU and script gets killed due to timeout. This is a very specific corner case and generally should not happen on baremetal/prod environment, but sometimes seen in nested/test environments. Signed-off-by: Rohit Yadav --- agent/src/com/cloud/agent/Agent.java | 4 ++-- .../agent/resource/virtualnetwork/VirtualRoutingResource.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/src/com/cloud/agent/Agent.java b/agent/src/com/cloud/agent/Agent.java index f1f2116ffae..df0448dab22 100644 --- a/agent/src/com/cloud/agent/Agent.java +++ b/agent/src/com/cloud/agent/Agent.java @@ -729,7 +729,7 @@ public class Agent implements HandlerFactory, IAgentControl { _shell.setPersistentProperty(null, KeyStoreUtils.KS_PASSPHRASE_PROPERTY, storedPassword); } - Script script = new Script(_keystoreSetupPath, 60000, s_logger); + Script script = new Script(_keystoreSetupPath, 300000, s_logger); script.add(agentFile.getAbsolutePath()); script.add(keyStoreFile); script.add(storedPassword); @@ -773,7 +773,7 @@ public class Agent implements HandlerFactory, IAgentControl { throw new CloudRuntimeException("Unable to save received agent client and ca certificates", e); } - Script script = new Script(_keystoreCertImportPath, 60000, s_logger); + Script script = new Script(_keystoreCertImportPath, 300000, s_logger); script.add(agentFile.getAbsolutePath()); script.add(keyStoreFile); script.add(KeyStoreUtils.AGENT_MODE); diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java index 0ffe8cc0ea2..13672186653 100644 --- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java +++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java @@ -161,7 +161,7 @@ public class VirtualRoutingResource { cmd.getKeystorePassword(), cmd.getValidityDays(), KeyStoreUtils.CSR_FILENAME); - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), KeyStoreUtils.KS_SETUP_SCRIPT, args); + ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), KeyStoreUtils.KS_SETUP_SCRIPT, args, Duration.standardMinutes(15)); return new SetupKeystoreAnswer(result.getDetails()); } @@ -179,7 +179,7 @@ public class VirtualRoutingResource { cmd.getEncodedCaCertificates(), KeyStoreUtils.PKEY_FILENAME, cmd.getEncodedPrivateKey()); - ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), KeyStoreUtils.KS_IMPORT_SCRIPT, args); + ExecutionResult result = _vrDeployer.executeInVR(cmd.getRouterAccessIp(), KeyStoreUtils.KS_IMPORT_SCRIPT, args, Duration.standardMinutes(15)); return new SetupCertificateAnswer(result.isSuccess()); } From 290df5f42351f4c12197b2a71a4d6bb6dfc13529 Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Tue, 4 Dec 2018 02:29:48 -0600 Subject: [PATCH 3/3] api: Discover tags field on superclass of API responses (#3005) Updated ApiServiceDiscoveryImpl to check superclasses of API responses for fields. Fixes: #3002 --- .../api/response/ApiDiscoveryResponse.java | 12 ++-- .../api/response/ApiResponseResponse.java | 24 +++++-- .../discovery/ApiDiscoveryServiceImpl.java | 42 ++++++------ .../discovery/ApiDiscoveryTest.java | 66 ++++++++++++------- 4 files changed, 90 insertions(+), 54 deletions(-) diff --git a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java index 64e6ef9105c..dccf5a68e11 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiDiscoveryResponse.java @@ -16,15 +16,13 @@ // under the License. package org.apache.cloudstack.api.response; -import java.util.HashSet; -import java.util.Set; - +import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; -import com.cloud.serializer.Param; +import java.util.HashSet; +import java.util.Set; @SuppressWarnings("unused") public class ApiDiscoveryResponse extends BaseResponse { @@ -121,4 +119,8 @@ public class ApiDiscoveryResponse extends BaseResponse { public void addApiResponse(ApiResponseResponse apiResponse) { this.apiResponse.add(apiResponse); } + + public Set getApiResponse() { + return apiResponse; + } } diff --git a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java index ba1301b9ed9..9844df16841 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/api/response/ApiResponseResponse.java @@ -16,15 +16,13 @@ // under the License. package org.apache.cloudstack.api.response; -import java.util.HashSet; -import java.util.Set; - +import com.cloud.serializer.Param; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; -import com.cloud.serializer.Param; +import java.util.HashSet; +import java.util.Set; public class ApiResponseResponse extends BaseResponse { @SerializedName(ApiConstants.NAME) @@ -61,4 +59,20 @@ public class ApiResponseResponse extends BaseResponse { } this.apiResponse.add(childApiResponse); } + + public String getName() { + return name; + } + + public String getDescription() { + return description; + } + + public String getType() { + return type; + } + + public Set getApiResponse() { + return apiResponse; + } } diff --git a/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index 3408a77b850..62164db88ca 100644 --- a/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -16,21 +16,13 @@ // under the License. package org.apache.cloudstack.discovery; -import java.lang.reflect.Field; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import javax.inject.Inject; - -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - +import com.cloud.serializer.Param; +import com.cloud.user.User; +import com.cloud.utils.ReflectUtil; +import com.cloud.utils.StringUtils; +import com.cloud.utils.component.ComponentLifecycleBase; +import com.cloud.utils.component.PluggableService; import com.google.gson.annotations.SerializedName; - import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.BaseAsyncCmd; @@ -43,13 +35,18 @@ import org.apache.cloudstack.api.response.ApiDiscoveryResponse; import org.apache.cloudstack.api.response.ApiParameterResponse; import org.apache.cloudstack.api.response.ApiResponseResponse; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.log4j.Logger; +import org.reflections.ReflectionUtils; +import org.springframework.stereotype.Component; -import com.cloud.serializer.Param; -import com.cloud.user.User; -import com.cloud.utils.ReflectUtil; -import com.cloud.utils.StringUtils; -import com.cloud.utils.component.ComponentLifecycleBase; -import com.cloud.utils.component.PluggableService; +import javax.inject.Inject; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; @Component public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements ApiDiscoveryService { @@ -100,7 +97,8 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A } ApiDiscoveryResponse response = getCmdRequestMap(cmdClass, apiCmdAnnotation); - String responseName = apiCmdAnnotation.responseObject().getName(); + Class responseClass = apiCmdAnnotation.responseObject(); + String responseName = responseClass.getName(); if (!responseName.contains("SuccessResponse")) { if (!responseApiNameListMap.containsKey(responseName)) { responseApiNameListMap.put(responseName, new ArrayList()); @@ -109,7 +107,7 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A } response.setRelated(responseName); - Field[] responseFields = apiCmdAnnotation.responseObject().getDeclaredFields(); + Set responseFields = ReflectionUtils.getAllFields(responseClass); for (Field responseField : responseFields) { ApiResponseResponse responseResponse = getFieldResponseMap(responseField); response.addApiResponse(responseResponse); diff --git a/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java b/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java index 49bf5a55dc5..c261fb2538f 100644 --- a/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java +++ b/plugins/api/discovery/test/org/apache/cloudstack/discovery/ApiDiscoveryTest.java @@ -16,34 +16,36 @@ // under the License. package org.apache.cloudstack.discovery; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import com.cloud.user.User; +import com.cloud.user.UserVO; +import com.cloud.utils.component.PluggableService; +import org.apache.cloudstack.acl.APIChecker; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.command.user.discovery.ListApisCmd; +import org.apache.cloudstack.api.command.user.vm.ListVMsCmd; +import org.apache.cloudstack.api.response.ApiDiscoveryResponse; +import org.apache.cloudstack.api.response.ApiResponseResponse; +import org.apache.cloudstack.api.response.ListResponse; +import org.apache.commons.lang.StringUtils; +import org.junit.BeforeClass; +import org.junit.Test; +import javax.naming.ConfigurationException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; -import javax.naming.ConfigurationException; - -import org.junit.BeforeClass; -import org.junit.Test; - -import org.apache.cloudstack.acl.APIChecker; -import org.apache.cloudstack.api.APICommand; -import org.apache.cloudstack.api.command.user.discovery.ListApisCmd; -import org.apache.cloudstack.api.response.ApiDiscoveryResponse; -import org.apache.cloudstack.api.response.ListResponse; - -import com.cloud.user.User; -import com.cloud.user.UserVO; -import com.cloud.utils.component.PluggableService; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class ApiDiscoveryTest { private static APIChecker s_apiChecker = mock(APIChecker.class); @@ -51,14 +53,19 @@ public class ApiDiscoveryTest { private static ApiDiscoveryServiceImpl s_discoveryService = new ApiDiscoveryServiceImpl(); private static Class testCmdClass = ListApisCmd.class; + private static Class listVMsCmdClass = ListVMsCmd.class; private static User testUser; private static String testApiName; + private static String listVmsCmdName; private static String testApiDescription; private static String testApiSince; private static boolean testApiAsync; @BeforeClass public static void setUp() throws ConfigurationException { + + listVmsCmdName = listVMsCmdClass.getAnnotation(APICommand.class).name(); + testApiName = testCmdClass.getAnnotation(APICommand.class).name(); testApiDescription = testCmdClass.getAnnotation(APICommand.class).description(); testApiSince = testCmdClass.getAnnotation(APICommand.class).since(); @@ -75,6 +82,7 @@ public class ApiDiscoveryTest { Set> cmdClasses = new HashSet>(); cmdClasses.add(ListApisCmd.class); + cmdClasses.add(ListVMsCmd.class); s_discoveryService.start(); s_discoveryService.cacheResponseMap(cmdClasses); } @@ -82,6 +90,7 @@ public class ApiDiscoveryTest { @Test public void verifyListSingleApi() throws Exception { ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, testApiName); + assertNotNull("Responses should not be null", responses); if (responses != null) { ApiDiscoveryResponse response = responses.getResponses().get(0); assertTrue("No. of response items should be one", responses.getCount() == 1); @@ -95,12 +104,25 @@ public class ApiDiscoveryTest { @Test public void verifyListApis() throws Exception { ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, null); + assertNotNull("Responses should not be null", responses); if (responses != null) { - assertTrue("No. of response items > 1", responses.getCount().intValue() == 1); + assertTrue("No. of response items > 2", responses.getCount().intValue() == 2); for (ApiDiscoveryResponse response : responses.getResponses()) { assertFalse("API name is empty", response.getName().isEmpty()); assertFalse("API description is empty", response.getDescription().isEmpty()); } } } + + @Test + public void verifyListVirtualMachinesTagsField() throws Exception { + ListResponse responses = (ListResponse)s_discoveryService.listApis(testUser, listVmsCmdName); + assertNotNull("Response should not be null", responses); + if (responses != null) { + assertEquals("No. of response items should be one", 1, (int) responses.getCount()); + ApiDiscoveryResponse response = responses.getResponses().get(0); + List tagsResponse = response.getApiResponse().stream().filter(resp -> StringUtils.equals(resp.getName(), "tags")).collect(Collectors.toList()); + assertEquals("Tags field should be present in listVirtualMachines response fields", tagsResponse.size(), 1); + } + } }