From fec31d53c362293087942f20fed974104fcbaa8b Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 12 Dec 2014 10:19:06 +0100 Subject: [PATCH] CLOUDSTACK-8064: UpdatePortForwardingRuleCmd implementation --- .../com/cloud/network/rules/RulesService.java | 2 +- .../firewall/UpdatePortForwardingRuleCmd.java | 60 +++++------------- .../network/rules/PortForwardingRuleVO.java | 12 ++++ .../cloud/network/rules/RulesManagerImpl.java | 62 ++++++++++++++++++- 4 files changed, 91 insertions(+), 45 deletions(-) diff --git a/api/src/com/cloud/network/rules/RulesService.java b/api/src/com/cloud/network/rules/RulesService.java index 2dd0182dbc4..3eca14d6543 100644 --- a/api/src/com/cloud/network/rules/RulesService.java +++ b/api/src/com/cloud/network/rules/RulesService.java @@ -81,6 +81,6 @@ public interface RulesService { boolean disableStaticNat(long ipId) throws ResourceUnavailableException, NetworkRuleConflictException, InsufficientAddressCapacityException; - PortForwardingRule updatePortForwardingRule(long id, String customId, Boolean forDisplay); + PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay); } diff --git a/api/src/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java b/api/src/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java index eb4d4688869..596b1d274e8 100644 --- a/api/src/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java +++ b/api/src/org/apache/cloudstack/api/command/user/firewall/UpdatePortForwardingRuleCmd.java @@ -23,7 +23,6 @@ import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.BaseAsyncCustomIdCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.response.FirewallRuleResponse; -import org.apache.cloudstack.api.response.IPAddressResponse; import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.log4j.Logger; @@ -32,6 +31,7 @@ import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.rules.FirewallRule; import com.cloud.network.rules.PortForwardingRule; import com.cloud.user.Account; +import com.cloud.utils.net.Ip; @APICommand(name = "updatePortForwardingRule", responseObject = FirewallRuleResponse.class, @@ -47,25 +47,8 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = FirewallRuleResponse.class, required = true, description = "the ID of the port forwarding rule", since = "4.4") private Long id; - @Parameter(name = ApiConstants.PRIVATE_IP, type = CommandType.STRING, description = "the private IP address of the port forwarding rule") - private String privateIp; - - @Parameter(name = ApiConstants.PRIVATE_PORT, type = CommandType.STRING, description = "the private port of the port forwarding rule") - private String privatePort; - - @Parameter(name = ApiConstants.PROTOCOL, - type = CommandType.STRING, - description = "the protocol for the port fowarding rule. Valid values are TCP or UDP.") - private String protocol; - - @Parameter(name = ApiConstants.IP_ADDRESS_ID, - type = CommandType.UUID, - entityType = IPAddressResponse.class, - description = "the IP address id of the port forwarding rule") - private Long publicIpId; - - @Parameter(name = ApiConstants.PUBLIC_PORT, type = CommandType.STRING, description = "the public port of the port forwarding rule") - private String publicPort; + @Parameter(name=ApiConstants.PRIVATE_PORT, type=CommandType.INTEGER, description="the private port of the port forwarding rule") + private Integer privatePort; @Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, type = CommandType.UUID, @@ -73,6 +56,9 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { description = "the ID of the virtual machine for the port forwarding rule") private Long virtualMachineId; + @Parameter(name = ApiConstants.VM_GUEST_IP, type = CommandType.STRING, required = false, description = "VM guest nic Secondary ip address for the port forwarding rule", since = "4.5") + private String vmGuestIp; + @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, description = "an optional field, whether to the display the rule to the end user or not", since = "4.4", authorized = {RoleType.Admin}) private Boolean display; @@ -80,32 +66,28 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// - public String getPrivateIp() { - return privateIp; + public Long getId() { + return id; } - public String getPrivatePort() { + public Integer getPrivatePort() { return privatePort; } - public String getProtocol() { - return protocol; - } - - public String getPublicPort() { - return publicPort; - } - public Long getVirtualMachineId() { return virtualMachineId; } + public Ip getVmGuestIp() { + if (vmGuestIp == null) { + return null; + } + return new Ip(vmGuestIp); + } + public Boolean getDisplay() { return display; } - public Long getId() { - return id; - } ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// @@ -148,21 +130,13 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { @Override public void execute() { - PortForwardingRule rule = _rulesService.updatePortForwardingRule(id, getCustomId(), getDisplay()); + PortForwardingRule rule = _rulesService.updatePortForwardingRule(id, getPrivatePort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(), getDisplay()); FirewallRuleResponse fwResponse = new FirewallRuleResponse(); if (rule != null) { fwResponse = _responseGenerator.createPortForwardingRuleResponse(rule); setResponseObject(fwResponse); } fwResponse.setResponseName(getCommandName()); -//FIXME: PortForwardingRule result = _mgr.updatePortForwardingRule(this); -// if (result != null) { -// FirewallRuleResponse response = _responseGenerator.createFirewallRuleResponse(result); -// response.setResponseName(getName()); -// this.setResponseObject(response); -// } else { -// throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to update port forwarding rule"); -// } } @Override diff --git a/engine/schema/src/com/cloud/network/rules/PortForwardingRuleVO.java b/engine/schema/src/com/cloud/network/rules/PortForwardingRuleVO.java index 2e71de1fbc9..4c27a3fc2b4 100644 --- a/engine/schema/src/com/cloud/network/rules/PortForwardingRuleVO.java +++ b/engine/schema/src/com/cloud/network/rules/PortForwardingRuleVO.java @@ -79,16 +79,28 @@ public class PortForwardingRuleVO extends FirewallRuleVO implements PortForwardi return destinationPortStart; } + public void setDestinationPortStart(int destinationPortStart) { + this.destinationPortStart = destinationPortStart; + } + @Override public int getDestinationPortEnd() { return destinationPortEnd; } + public void setDestinationPortEnd(int destinationPortEnd) { + this.destinationPortEnd = destinationPortEnd; + } + @Override public long getVirtualMachineId() { return virtualMachineId; } + public void setVirtualMachineId(long virtualMachineId) { + this.virtualMachineId = virtualMachineId; + } + @Override public Long getRelated() { return null; diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index dccb20bce49..e3d269dede5 100644 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -55,6 +55,7 @@ import com.cloud.network.dao.LoadBalancerVMMapDao; import com.cloud.network.dao.LoadBalancerVMMapVO; import com.cloud.network.rules.FirewallRule.FirewallRuleType; import com.cloud.network.rules.FirewallRule.Purpose; +import com.cloud.network.rules.FirewallRule.State; import com.cloud.network.rules.dao.PortForwardingRulesDao; import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.VpcService; @@ -1510,7 +1511,7 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules @Override @ActionEvent(eventType = EventTypes.EVENT_NET_RULE_MODIFY, eventDescription = "updating forwarding rule", async = true) - public PortForwardingRule updatePortForwardingRule(long id, String customId, Boolean forDisplay) { + public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay) { Account caller = CallContext.current().getCallingAccount(); PortForwardingRuleVO rule = _portForwardingDao.findById(id); if (rule == null) { @@ -1526,7 +1527,66 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules rule.setDisplay(forDisplay); } + if (rule.getSourcePortStart() != rule.getSourcePortEnd() && privatePort != null) { + throw new InvalidParameterValueException("Unable to update the private port of port forwarding rule as the rule has port range : " + rule.getSourcePortStart() + " to " + rule.getSourcePortEnd()); + } + if (virtualMachineId == null && vmGuestIp != null) { + throw new InvalidParameterValueException("vmguestip should be set along with virtualmachineid"); + } + Ip dstIp = rule.getDestinationIpAddress(); + if (virtualMachineId != null) { + // Verify that vm has nic in the network + Nic guestNic = _networkModel.getNicInNetwork(virtualMachineId, rule.getNetworkId()); + if (guestNic == null || guestNic.getIp4Address() == null) { + throw new InvalidParameterValueException("Vm doesn't belong to network associated with ipAddress"); + } else { + dstIp = new Ip(guestNic.getIp4Address()); + } + + if (vmGuestIp != null) { + //vm ip is passed so it can be primary or secondary ip addreess. + if (!dstIp.equals(vmGuestIp)) { + //the vm ip is secondary ip to the nic. + // is vmIp is secondary ip or not + NicSecondaryIp secondaryIp = _nicSecondaryDao.findByIp4AddressAndNicId(vmGuestIp.toString(), guestNic.getId()); + if (secondaryIp == null) { + throw new InvalidParameterValueException("IP Address is not in the VM nic's network "); + } + dstIp = vmGuestIp; + } + } + } + + // revoke old rules at first + List rules = new ArrayList(); + rule.setState(State.Revoke); _portForwardingDao.update(id, rule); + rules.add(rule); + try { + if (!_firewallMgr.applyRules(rules, true, false)) { + throw new CloudRuntimeException("Failed to revoke the existing port forwarding rule:" + id); + } + } catch (ResourceUnavailableException ex) { + throw new CloudRuntimeException("Failed to revoke the existing port forwarding rule:" + id + " due to ", ex); + } + + rule = _portForwardingDao.findById(id); + rule.setState(State.Add); + if (privatePort != null) { + rule.setDestinationPortStart(privatePort.intValue()); + rule.setDestinationPortEnd(privatePort.intValue()); + } + if (virtualMachineId != null) { + rule.setVirtualMachineId(virtualMachineId); + rule.setDestinationIpAddress(dstIp); + } + _portForwardingDao.update(id, rule); + + //apply new rules + if (!applyPortForwardingRules(rule.getSourceIpAddressId(), false, caller)) { + throw new CloudRuntimeException("Failed to apply the new port forwarding rule:" + id); + } + return _portForwardingDao.findById(id); } }