diff --git a/api/src/com/cloud/network/rules/RulesService.java b/api/src/com/cloud/network/rules/RulesService.java index 3eca14d6543..0b4afeef945 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, Integer privatePort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay); + PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, 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 5f609b726f8..7de50761630 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 @@ -47,9 +47,13 @@ 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_PORT, type=CommandType.INTEGER, description="the private port of the port forwarding rule") + @Parameter(name=ApiConstants.PRIVATE_START_PORT, type=CommandType.INTEGER, description="the private start port of the port forwarding rule") private Integer privatePort; + + @Parameter(name=ApiConstants.PRIVATE_END_PORT, type=CommandType.INTEGER, description="the private end port of the port forwarding rule") + private Integer privateEndPort; + @Parameter(name = ApiConstants.VIRTUAL_MACHINE_ID, type = CommandType.UUID, entityType = UserVmResponse.class, @@ -74,6 +78,10 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { return privatePort; } + public Integer getPrivateEndPort() { + return privateEndPort; + } + public Long getVirtualMachineId() { return virtualMachineId; } @@ -130,7 +138,7 @@ public class UpdatePortForwardingRuleCmd extends BaseAsyncCustomIdCmd { @Override public void execute() { - PortForwardingRule rule = _rulesService.updatePortForwardingRule(id, getPrivatePort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(), getDisplay()); + PortForwardingRule rule = _rulesService.updatePortForwardingRule(getId(), getPrivatePort(), getPrivateEndPort(), getVirtualMachineId(), getVmGuestIp(), getCustomId(), getDisplay()); FirewallRuleResponse fwResponse = new FirewallRuleResponse(); if (rule != null) { fwResponse = _responseGenerator.createPortForwardingRuleResponse(rule); diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 629e35f5ba3..37a9ad90d33 100644 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -1525,7 +1525,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, Integer privatePort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay) { + public PortForwardingRule updatePortForwardingRule(long id, Integer privatePort, Integer privateEndPort, Long virtualMachineId, Ip vmGuestIp, String customId, Boolean forDisplay) { Account caller = CallContext.current().getCallingAccount(); PortForwardingRuleVO rule = _portForwardingDao.findById(id); if (rule == null) { @@ -1541,9 +1541,29 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules rule.setDisplay(forDisplay); } - if (!rule.getSourcePortStart().equals(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 (privatePort != null && !NetUtils.isValidPort(privatePort)) { + throw new InvalidParameterValueException("privatePort is an invalid value: " + privatePort); } + if (privateEndPort != null && !NetUtils.isValidPort(privateEndPort)) { + throw new InvalidParameterValueException("PrivateEndPort has an invalid value: " + privateEndPort); + } + + if (privatePort != null && privateEndPort != null && ((privateEndPort - privatePort) != (rule.getSourcePortEnd() - rule.getSourcePortStart()))) + { + throw new InvalidParameterValueException("Unable to update the private port range of port forwarding rule as " + + "the provided port range is not consistent with the port range : " + rule.getSourcePortStart() + " to " + rule.getSourcePortEnd()); + } + + //in case of port range + if (!rule.getSourcePortStart().equals(rule.getSourcePortEnd())) { + if ((privatePort == null || privateEndPort == null) && !(privatePort == null && privateEndPort == null)) { + throw new InvalidParameterValueException("Unable to update the private port range of port forwarding rule as " + + "the provided port range is not consistent with the port range : " + rule.getSourcePortStart() + " to " + rule.getSourcePortEnd()); + } + } + + + if (virtualMachineId == null && vmGuestIp != null) { throw new InvalidParameterValueException("vmguestip should be set along with virtualmachineid"); } @@ -1588,8 +1608,12 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules rule.setState(State.Add); if (privatePort != null) { rule.setDestinationPortStart(privatePort.intValue()); - rule.setDestinationPortEnd(privatePort.intValue()); + rule.setDestinationPortEnd((privateEndPort == null) ? privatePort.intValue() : privateEndPort.intValue()); + } else if (privateEndPort != null) { + rule.setDestinationPortStart(privateEndPort.intValue()); + rule.setDestinationPortEnd(privateEndPort); } + if (virtualMachineId != null) { rule.setVirtualMachineId(virtualMachineId); rule.setDestinationIpAddress(dstIp); diff --git a/test/integration/smoke/test_portforwardingrules.py b/test/integration/smoke/test_portforwardingrules.py new file mode 100644 index 00000000000..fbac0b442d3 --- /dev/null +++ b/test/integration/smoke/test_portforwardingrules.py @@ -0,0 +1,428 @@ +# 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. + +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase, unittest +from marvin.lib.base import (PublicIPAddress, + NetworkOffering, + Autoscale, + Network, + NetworkServiceProvider, + Template, + VirtualMachine, + VPC, + VpcOffering, + StaticNATRule, + FireWallRule, + NATRule, + Vpn, + VpnUser, + LoadBalancerRule, + Account, + ServiceOffering, + PhysicalNetwork, + User) +from marvin.lib.common import (get_domain, + get_zone, + get_template) +from marvin.lib.utils import validateList, cleanup_resources +from marvin.codes import PASS +from nose.plugins.attrib import attr + +class TestPortForwardingRules(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + try: + cls._cleanup = [] + cls.testClient = super(TestPortForwardingRules, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + cls.services = cls.testClient.getParsedTestDataConfig() + cls.hypervisor = cls.testClient.getHypervisorInfo() + # Get Domain, Zone, Template + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone( + cls.api_client, + cls.testClient.getZoneForTests()) + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.services["ostype"] + ) + if cls.zone.localstorageenabled: + cls.storagetype = 'local' + cls.services["service_offerings"][ + "tiny"]["storagetype"] = 'local' + else: + cls.storagetype = 'shared' + cls.services["service_offerings"][ + "tiny"]["storagetype"] = 'shared' + + cls.services['mode'] = cls.zone.networktype + cls.services["virtual_machine"][ + "hypervisor"] = cls.hypervisor + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + cls.services["virtual_machine"]["template"] = cls.template.id + cls.service_offering = ServiceOffering.create( + cls.api_client, + cls.services["service_offerings"]["tiny"] + ) + cls._cleanup.append(cls.service_offering) + cls.services['mode'] = cls.zone.networktype + + cls.vpc_offering = VpcOffering.create(cls.api_client, + cls.services["vpc_offering"] + ) + cls.vpc_offering.update(cls.api_client, state='Enabled') + cls._cleanup.append(cls.vpc_offering) + + except Exception as e: + cls.tearDownClass() + raise Exception("Warning: Exception in setup : %s" % e) + return + + def setUp(self): + + self.apiClient = self.testClient.getApiClient() + self.cleanup = [] + self.account = Account.create( + self.apiClient, + self.services["account"], + domainid=self.domain.id + ) + # Getting authentication for user in newly created Account + self.user = self.account.user[0] + self.userapiclient = self.testClient.getUserApiClient( + self.user.username, + self.domain.name) + + def tearDown(self): + # Clean up, terminate the created volumes + cleanup_resources(self.apiClient, self.cleanup) + return + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.api_client, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + def __verify_values(self, expected_vals, actual_vals): + """ + @summary: Function to verify expected and actual values + Step1: Initializing return flag to True + Step1: Verifying length of expected and actual dictionaries is matching + If not matching returning false + Step2: Listing all the keys from expected dictionary + Step3: Looping through each key from step2 and verifying expected + and actual dictionaries have same value + If not making return flag to False + Step4: returning the return flag after all the values are verified + """ + return_flag = True + + if len(expected_vals) != len(actual_vals): + return False + + keys = expected_vals.keys() + for i in range(0, len(expected_vals)): + exp_val = expected_vals[keys[i]] + act_val = actual_vals[keys[i]] + if exp_val == act_val: + return_flag = return_flag and True + else: + return_flag = return_flag and False + self.debug( + "expected Value: %s, is not matching with actual value: %s" + % + (exp_val, act_val)) + return return_flag + + + + @attr(tags=["advanced"], required_hardware="true") + def test_01_create_delete_portforwarding_fornonvpc(self): + """ + @summary: Test to list, create and delete Port Forwarding for + IP Address associated to Non VPC network + @Steps: + Step1: Creating a Network for the user + Step2: Associating an IP Addresses for Network + Step3: Launching Virtual Machine in network created in step 2 + Step4: Listing Port Forwarding Rules for the IP Address associated + in Step2 + Step5: Verifying that no Port Forwarding Rules are listed + Step6: Creating a Port Forwarding Rule for IP Address associated in + Step2 + Step7: Listing Port Forwarding Rules for the IP Address associated in + Step2 + Step8: Verifying 1 Port Forwarding Rule is listed + Step9: Deleting the Port Forwarding Rule created in Step6 + Step10: Listing Port Forwarding Rules for the IP Address associated in + Step2 + Step11: Verifying that no Port Forwarding Rules are listed + """ + # Listing all the Networks's for a user + list_networks_before = Network.list( + self.userapiclient, + listall=self.services["listall"], + type="Isolated" + ) + # Verifying No Networks are listed + self.assertIsNone( + list_networks_before, + "Networks listed for newly created User" + ) + # Listing Network Offerings + network_offerings_list = NetworkOffering.list( + self.apiClient, + forvpc="false", + guestiptype="Isolated", + state="Enabled", + supportedservices="SourceNat,PortForwarding", + zoneid=self.zone.id + ) + status = validateList(network_offerings_list) + self.assertEquals( + PASS, + status[0], + "Isolated Network Offerings with sourceNat,\ + PortForwarding enabled are not found" + ) + # Creating a network + network = Network.create( + self.userapiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.domain.id, + networkofferingid=network_offerings_list[0].id, + zoneid=self.zone.id + ) + self.assertIsNotNone( + network, + "Network creation failed" + ) + # Listing all the IP Addresses for a user + list_ipaddresses_before = PublicIPAddress.list( + self.userapiclient, + listall=self.services["listall"] + ) + # Verifying no IP Addresses are listed + self.assertIsNone( + list_ipaddresses_before, + "IP Addresses listed for newly created User" + ) + # Associating an IP Addresses to Network created + associated_ipaddress = PublicIPAddress.create( + self.userapiclient, + services=self.services["network"], + networkid=network.id + ) + self.assertIsNotNone( + associated_ipaddress, + "Failed to Associate IP Address" + ) + # Listing all the IP Addresses for a user + list_ipaddresses_after = PublicIPAddress.list( + self.userapiclient, + listall=self.services["listall"] + ) + status = validateList(list_ipaddresses_after) + self.assertEquals( + PASS, + status[0], + "IP Addresses Association Failed" + ) + # Verifying the length of the list is 1 + self.assertEqual( + 1, + len(list_ipaddresses_after), + "Number of IP Addresses associated are not matching expected" + ) + # Launching a Virtual Machine with above created Network + vm_created = VirtualMachine.create( + self.userapiclient, + self.services["virtual_machine"], + accountid=self.account.name, + domainid=self.account.domainid, + networkids=network.id, + serviceofferingid=self.service_offering.id, + ) + self.assertIsNotNone( + vm_created, + "Failed to launch a VM under network created" + ) + self.cleanup.append(network) + # Listing Virtual Machines in running state in above created network + list_vms_running = VirtualMachine.list( + self.userapiclient, + listall=self.services["listall"], + state="Running", + networkid=network.id + ) + status = validateList(list_vms_running) + self.assertEquals( + PASS, + status[0], + "VM Created is not in Running state" + ) + # Verifying the length of the list is 1 + self.assertEqual( + 1, + len(list_ipaddresses_after), + "VM Created is not in Running state" + ) + self.assertEquals( + vm_created.id, + list_vms_running[0].id, + "VM Created is not in Running state" + ) + # Listing Virtual Machines in stopped state in above created network + list_vms_stopped = VirtualMachine.list( + self.userapiclient, + listall=self.services["listall"], + state="Stopped", + networkid=network.id + ) + # Verifying no VMs are in stopped state + self.assertIsNone( + list_vms_stopped, + "VM Created is in stopped state" + ) + # Listing Port Forwarding Rules for the IP Address associated + list_prtfwdrule_before = NATRule.list( + self.userapiclient, + listall=self.services["listall"], + ipaddressid=associated_ipaddress.ipaddress.id + ) + # Verifying no port forwarding rules are listed + self.assertIsNone( + list_prtfwdrule_before, + "Port Forwarding Rules listed for newly associated IP Address" + ) + # Creating a Port Forwarding rule + portfwd_rule = NATRule.create( + self.userapiclient, + virtual_machine=vm_created, + services=self.services["natrule"], + ipaddressid=associated_ipaddress.ipaddress.id, + ) + self.assertIsNotNone( + portfwd_rule, + "Failed to create Port Forwarding Rule" + ) + # Verifying details of Sticky Policy created + # Creating expected and actual values dictionaries + expected_dict = { + "ipaddressid": associated_ipaddress.ipaddress.id, + "privateport": str(self.services["natrule"]["privateport"]), + "publicport": str(self.services["natrule"]["publicport"]), + "protocol": str(self.services["natrule"]["protocol"]).lower(), + } + actual_dict = { + "ipaddressid": portfwd_rule.ipaddressid, + "privateport": str(portfwd_rule.privateport), + "publicport": str(portfwd_rule.publicport), + "protocol": portfwd_rule.protocol, + } + portfwd_status = self.__verify_values( + expected_dict, + actual_dict + ) + self.assertEqual( + True, + portfwd_status, + "Created Port Forward Rule details are not as expected" + ) + # Listing Port Forwarding Rules for the IP Address associated + list_prtfwdrule_after = NATRule.list( + self.userapiclient, + listall=self.services["listall"], + ipaddressid=associated_ipaddress.ipaddress.id + ) + status = validateList(list_prtfwdrule_after) + self.assertEquals( + PASS, + status[0], + "Failed to create Port Forwarding Rule" + ) + # Verifying the length of the list is 1 + self.assertEqual( + 1, + len(list_prtfwdrule_after), + "Failed to create Port Forwarding Rule" + ) + # Deleting Port Forwarding Rule + portfwd_rule.delete(self.userapiclient) + + + # Creating a Port Forwarding rule with port range + portfwd_rule = NATRule.create( + self.userapiclient, + virtual_machine=vm_created, + services=self.services["natrulerange"], + ipaddressid=associated_ipaddress.ipaddress.id, + ) + self.assertIsNotNone( + portfwd_rule, + "Failed to create Port Forwarding Rule" + ) + #update the private port for port forwarding rule + updatefwd_rule = portfwd_rule.update(self.userapiclient, + portfwd_rule.id, + virtual_machine=vm_created, + services=self.services["updatenatrulerange"], + ) + + # Verifying details of Sticky Policy created + # Creating expected and actual values dictionaries + expected_dict = { + "privateport": str(self.services["updatenatrulerange"]["privateport"]), + "privateendport": str(self.services["updatenatrulerange"]["privateendport"]), + } + actual_dict = { + "privateport": str(updatefwd_rule.privateport), + "privateendport": str(updatefwd_rule.privateendport), + } + portfwd_status = self.__verify_values( + expected_dict, + actual_dict + ) + self.assertEqual( + True, + portfwd_status, + "Updated Port Forward Rule details are not as expected" + ) + # Deleting Port Forwarding Rule + portfwd_rule.delete(self.userapiclient) + # Listing Port Forwarding Rules for the IP Address associated + list_prtfwdrule_after = NATRule.list( + self.userapiclient, + listall=self.services["listall"], + ipaddressid=associated_ipaddress.ipaddress.id + ) + # Verifying no port forwarding rules are listed + self.assertIsNone( + list_prtfwdrule_after, + "Port Forwarding Rules listed after deletion" + ) + # Destroying the VM Launched + vm_created.delete(self.apiClient) + self.cleanup.append(self.account) + return + diff --git a/tools/marvin/marvin/config/test_data.py b/tools/marvin/marvin/config/test_data.py index 6519a7fbc32..13cd063bfa1 100644 --- a/tools/marvin/marvin/config/test_data.py +++ b/tools/marvin/marvin/config/test_data.py @@ -1,4 +1,4 @@ -# Licensed to the Apache Software Foundation (ASF) under one +#t 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 @@ -779,6 +779,17 @@ test_data = { "publicport": 22, "protocol": "TCP" }, + "natrulerange": { + "privateport": 70, + "privateendport": 75, + "publicport": 70, + "publicendport": 75, + "protocol": "TCP" + }, + "updatenatrulerange": { + "privateport": 50, + "privateendport": 55, + }, "egress_80": { "startport": 80, "endport": 80, diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 6a00c678c48..f96cd8b26d1 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1649,6 +1649,30 @@ class NATRule: return NATRule(apiclient.createPortForwardingRule(cmd).__dict__) + @classmethod + def update(self, apiclient, id, virtual_machine, services, fordisplay=False, + vmguestip=None): + """Create Port forwarding rule""" + cmd = updatePortForwardingRule.updatePortForwardingRuleCmd() + cmd.id = id + + if "privateport" in services: + cmd.privateport = services["privateport"] + + if "privateendport" in services: + cmd.privateendport = services["privateendport"] + + if vmguestip: + cmd.vmguestip = vmguestip + + if fordisplay: + cmd.fordisplay = fordisplay + + if virtual_machine.id: + cmd.virtualmachineid = virtual_machine.id + + return NATRule(apiclient.updatePortForwardingRule(cmd).__dict__) + def delete(self, apiclient): """Delete port forwarding""" cmd = deletePortForwardingRule.deletePortForwardingRuleCmd()