From f73ed920909540998656d5c8dd0e7fa876279e40 Mon Sep 17 00:00:00 2001 From: Abhinandan Prateek Date: Fri, 13 May 2011 12:52:15 +0530 Subject: [PATCH] bug 8115: Source IP filtering in Virtual Router We don't want to send an empty cidr param --- .../agent/api/to/PortForwardingRuleTO.java | 2 +- api/src/com/cloud/api/ApiConstants.java | 1 + .../commands/CreatePortForwardingRuleCmd.java | 3 - .../api/commands/UpdateHostPasswordCmd.java | 10 +-- .../com/cloud/resource/ResourceService.java | 2 - .../xen/resource/CitrixResourceBase.java | 2 +- .../cloud/agent/manager/AgentManagerImpl.java | 85 ------------------- .../cloud/server/ManagementServerImpl.java | 18 +++- 8 files changed, 19 insertions(+), 104 deletions(-) diff --git a/api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java b/api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java index 9d10bd6d94d..ead595ec984 100644 --- a/api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java +++ b/api/src/com/cloud/agent/api/to/PortForwardingRuleTO.java @@ -69,7 +69,7 @@ public class PortForwardingRuleTO extends FirewallRuleTO { } public String geStringSourceCidrs(){ - return sourceCidrs==null ? null : StringUtils.join(sourceCidrs, ","); + return StringUtils.join(sourceCidrs, ","); } } diff --git a/api/src/com/cloud/api/ApiConstants.java b/api/src/com/cloud/api/ApiConstants.java index e4de439beae..a125606e630 100755 --- a/api/src/com/cloud/api/ApiConstants.java +++ b/api/src/com/cloud/api/ApiConstants.java @@ -116,6 +116,7 @@ public class ApiConstants { public static final String OS_TYPE_ID = "ostypeid"; public static final String PARENT_DOMAIN_ID = "parentdomainid"; public static final String PASSWORD = "password"; + public static final String NEW_PASSWORD = "new_password"; public static final String PASSWORD_ENABLED = "passwordenabled"; public static final String PATH = "path"; public static final String POD_ID = "podid"; diff --git a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java index d95c1e33e47..94c93a097ac 100644 --- a/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java +++ b/api/src/com/cloud/api/commands/CreatePortForwardingRuleCmd.java @@ -120,9 +120,6 @@ public class CreatePortForwardingRuleCmd extends BaseAsyncCreateCmd implements P UserContext callerContext = UserContext.current(); boolean success = false; PortForwardingRule rule = _entityMgr.findById(PortForwardingRule.class, getEntityId()); - // load cidrs if any - rule.setSourceCidrList(_rulesService.getSourceCidrs(rule.getId())); - try { UserContext.current().setEventDetails("Rule Id: " + getEntityId()); success = _rulesService.applyPortForwardingRules(rule.getSourceIpAddressId(), callerContext.getCaller()); diff --git a/api/src/com/cloud/api/commands/UpdateHostPasswordCmd.java b/api/src/com/cloud/api/commands/UpdateHostPasswordCmd.java index 16b6b4ddc19..ec6849d6b46 100644 --- a/api/src/com/cloud/api/commands/UpdateHostPasswordCmd.java +++ b/api/src/com/cloud/api/commands/UpdateHostPasswordCmd.java @@ -89,13 +89,7 @@ public class UpdateHostPasswordCmd extends BaseCmd { @Override public void execute() { - boolean result = _resourceService.updateHostPassword(this); - if (result){ - _mgr.updateHostPassword(this); - this.setResponseObject(new SuccessResponse(getCommandName())); - } - else { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update host(s) password. Please, check the username and password."); - } + _mgr.updateHostPassword(this); + this.setResponseObject(new SuccessResponse(getCommandName())); } } \ No newline at end of file diff --git a/api/src/com/cloud/resource/ResourceService.java b/api/src/com/cloud/resource/ResourceService.java index d49e812aadb..429d976b0d7 100644 --- a/api/src/com/cloud/resource/ResourceService.java +++ b/api/src/com/cloud/resource/ResourceService.java @@ -81,8 +81,6 @@ public interface ResourceService { */ boolean deleteHost(long hostId, boolean isForced); - boolean updateHostPassword(UpdateHostPasswordCmd upasscmd); - Host getHost(long hostId); Cluster getCluster(Long clusterId); diff --git a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index fac67997478..4d9fce79870 100644 --- a/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/core/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -1218,7 +1218,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe args.append(" -p ").append(rule.getStringSrcPortRange()); args.append(" -r ").append(rule.getDstIp()); args.append(" -d ").append(rule.getStringDstPortRange()); - if (rule.geStringSourceCidrs() != null){ + if (rule.getSourceCidrs().size() > 0){ args.append(" -s " + rule.geStringSourceCidrs()); } String result = callHostPlugin(conn, "vmops", "setFirewallRule", "args", args.toString()); diff --git a/server/src/com/cloud/agent/manager/AgentManagerImpl.java b/server/src/com/cloud/agent/manager/AgentManagerImpl.java index 3c72a29f37a..e22a770f84f 100755 --- a/server/src/com/cloud/agent/manager/AgentManagerImpl.java +++ b/server/src/com/cloud/agent/manager/AgentManagerImpl.java @@ -84,7 +84,6 @@ import com.cloud.api.commands.DeleteClusterCmd; import com.cloud.api.commands.PrepareForMaintenanceCmd; import com.cloud.api.commands.ReconnectHostCmd; import com.cloud.api.commands.UpdateHostCmd; -import com.cloud.api.commands.UpdateHostPasswordCmd; import com.cloud.capacity.Capacity; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; @@ -1208,90 +1207,6 @@ public class AgentManagerImpl implements AgentManager, HandlerFactory, ResourceS return deleteHost(hostId, isForced, caller); } - @Override - public boolean updateHostPassword(UpdateHostPasswordCmd cmd){ - List hosts = _hostDao.listByCluster(cmd.getClusterId()); - for (HostVO host : hosts) { - String resourceName = host.getResource(); - ServerResource resource = null; - try { - Class clazz = Class.forName(resourceName); - Constructor constructor = clazz.getConstructor(); - resource = (ServerResource) constructor.newInstance(); - } catch (ClassNotFoundException e) { - s_logger.warn("Unable to find class " + host.getResource(), e); - return false; - } catch (InstantiationException e) { - s_logger.warn("Unablet to instantiate class " + host.getResource(), e); - return false; - } catch (IllegalAccessException e) { - s_logger.warn("Illegal access " + host.getResource(), e); - return false; - } catch (SecurityException e) { - s_logger.warn("Security error on " + host.getResource(), e); - return false; - } catch (NoSuchMethodException e) { - s_logger.warn("NoSuchMethodException error on " + host.getResource(), e); - return false; - } catch (IllegalArgumentException e) { - s_logger.warn("IllegalArgumentException error on " + host.getResource(), e); - return false; - } catch (InvocationTargetException e) { - s_logger.warn("InvocationTargetException error on " + host.getResource(), e); - return false; - } - - _hostDao.loadDetails(host); - - HashMap params = new HashMap(host.getDetails().size() + 5); - params.putAll(host.getDetails()); - - params.put("guid", host.getGuid()); - params.put("zone", Long.toString(host.getDataCenterId())); - if (host.getPodId() != null) { - params.put("pod", Long.toString(host.getPodId())); - } - if (host.getClusterId() != null) { - params.put("cluster", Long.toString(host.getClusterId())); - String guid = null; - ClusterVO cluster = _clusterDao.findById(host.getClusterId()); - if (cluster.getGuid() == null) { - guid = host.getDetail("pool"); - } else { - guid = cluster.getGuid(); - } - if (guid == null || guid.isEmpty()) { - throw new CloudRuntimeException("Can not find guid for cluster " + cluster.getId() + " name " + cluster.getName()); - } - params.put("pool", guid); - } - - params.put("ipaddress", host.getPrivateIpAddress()); - params.put("secondary.storage.vm", "false"); - params.put("max.template.iso.size", _configDao.getValue("max.template.iso.size")); - params.put("username", cmd.getUsername()); - params.put("password", cmd.getPassword()); - - try { - resource.configure(host.getName(), params); - } catch (ConfigurationException e) { - s_logger.warn("Unable to configure resource due to ", e); - return false; - } - - if (!resource.start()) { - s_logger.warn("Unable to start the resource"); - return false; - } - host.setLastPinged(System.currentTimeMillis() >> 10); - host.setManagementServerId(_nodeId); - _hostDao.update(host.getId(), host); - _executor.execute(new SimulateStartTask(host.getId(), resource, host.getDetails(), null)); - } - return true; - } - - @DB protected boolean deleteSecondaryStorageHost(HostVO secStorageHost) { long zoneId = secStorageHost.getDataCenterId(); diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index d701d8c1a09..73aef3b89fb 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -4697,8 +4697,13 @@ public class ManagementServerImpl implements ManagementServer { } DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME); if (nv.getValue().equals(cmd.getUsername())) { - DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD); - nvp.setValue(cmd.getPassword()); + DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.NEW_PASSWORD); + if (nvp==null){ + nvp = new DetailVO(h.getId(), ApiConstants.NEW_PASSWORD, cmd.getPassword()); + } + else { + nvp.setValue(cmd.getPassword()); + } _detailsDao.persist(nvp); } else { throw new InvalidParameterValueException("The username is not under use by management server."); @@ -4715,8 +4720,13 @@ public class ManagementServerImpl implements ManagementServer { // update password for this host DetailVO nv = _detailsDao.findDetail(h.getId(), ApiConstants.USERNAME); if (nv.getValue().equals(cmd.getUsername())) { - DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.PASSWORD); - nvp.setValue(cmd.getPassword()); + DetailVO nvp = _detailsDao.findDetail(h.getId(), ApiConstants.NEW_PASSWORD); + if (nvp==null){ + nvp = new DetailVO(h.getId(), ApiConstants.NEW_PASSWORD, cmd.getPassword()); + } + else { + nvp.setValue(cmd.getPassword()); + } _detailsDao.persist(nvp); } else { // if one host in the cluster has diff username then rollback to maintain consistency