VNF: Configure network rules if management is Isolated networks or Sh…

…ared networks with SG

api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java

@@ -43,15 +43,16 @@
 public class DeployVnfApplianceCmd extends DeployVMCmd implements UserCmd {
 
     @Parameter(name = ApiConstants.VNF_CONFIGURE_MANAGEMENT, type = CommandType.BOOLEAN, required = false,
-            description = "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise.")
+            description = "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise. " +
+                    "Network rules are configured if management network is an isolated network or shared network with security groups.")
     private Boolean vnfConfigureManagement;
 
     @Parameter(name = ApiConstants.VNF_CIDR_LIST, type = CommandType.LIST, collectionType = CommandType.STRING,
-            description = "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,).")
+            description = "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,). The default value is 0.0.0.0/0.")
     private List<String> vnfCidrlist;
 
     public Boolean getVnfConfigureManagement() {
-        return vnfConfigureManagement == null || vnfConfigureManagement;
+        return vnfConfigureManagement != null && vnfConfigureManagement;
     }
 
     public List<String> getVnfCidrlist() {

api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java

@@ -70,8 +70,8 @@ public static List<VNF.VnfNic> getVnfNicsList(Map vnfNics) {
                 boolean management = StringUtils.isBlank(managementString) || Boolean.parseBoolean(managementString);
                 nicsList.add(new VNF.VnfNic(deviceId, name, required, management, description));
             }
+            Collections.sort(nicsList, Comparator.comparing(VNF.VnfNic::getDeviceId));
         }
-        Collections.sort(nicsList, Comparator.comparing(VNF.VnfNic::getDeviceId));
         return nicsList;
     }
 

server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java

@@ -274,7 +274,12 @@ private Map<Network, String> getManagementNetworkAndIp(VirtualMachineTemplate te
                             "skipping this network configuration for VNF appliance", network.getUuid()));
                     continue;
                 }
-                if (network.getVpcId() == null && !networkModel.areServicesSupportedInNetwork(network.getId(), Network.Service.Firewall)) {
+                if (network.getVpcId() != null) {
+                    LOGGER.info(String.format("Network ID: %s is a VPC tier, " +
+                            "skipping this network configuration for VNF appliance", network.getUuid()));
+                    continue;
+                }
+                if (!networkModel.areServicesSupportedInNetwork(network.getId(), Network.Service.Firewall)) {
                     LOGGER.info(String.format("Network ID: %s does not support firewall, " +
                             "skipping this network configuration for VNF appliance", network.getUuid()));
                     continue;
@@ -296,6 +301,10 @@ public SecurityGroup createSecurityGroupForVnfAppliance(DataCenter zone, Virtual
         }
         LOGGER.debug("Creating security group and rules for VNF appliance");
         Set<Integer> ports = getOpenPortsForVnfAppliance(template);
+        if (ports.size() == 0) {
+            LOGGER.debug("No need to create security group and rules for VNF appliance as there is no ports to be open");
+            return null;
+        }
         String securityGroupName = VNF_SECURITY_GROUP_NAME.concat(Long.toHexString(System.currentTimeMillis()));
         SecurityGroupVO securityGroupVO = securityGroupManager.createSecurityGroup(securityGroupName,
                 "Security group for VNF appliance", owner.getDomainId(), owner.getId(), owner.getAccountName());
@@ -327,10 +336,15 @@ public void createIsolatedNetworkRulesForVnfAppliance(DataCenter zone, VirtualMa
             if (publicIp == null) {
                 continue;
             }
-            if (network.getVpcId() != null) {
-                publicIp = vpcService.associateIPToVpc(publicIp.getId(), network.getVpcId());
-            }
             publicIp = ipAddressManager.associateIPToGuestNetwork(publicIp.getId(), network.getId(), false);
+            if (publicIp.isSourceNat()) {
+                // If isolated network is not implemented, the first acquired Public IP will be Source NAT IP
+                publicIp = networkService.allocateIP(owner, zone.getId(), network.getId(), null, null);
+                if (publicIp == null) {
+                    continue;
+                }
+                publicIp = ipAddressManager.associateIPToGuestNetwork(publicIp.getId(), network.getId(), false);
+            }
             final IpAddress publicIpFinal = publicIp;
             final List<String> cidrList = cmd.getVnfCidrlist();
             try {

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -39,6 +39,7 @@
 import com.cloud.network.NetworkModel;
 import com.cloud.network.dao.NetworkDao;
 import com.cloud.network.dao.NetworkVO;
+import com.cloud.network.security.SecurityGroupVO;
 import com.cloud.offering.ServiceOffering;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.service.dao.ServiceOfferingDao;
@@ -74,6 +75,7 @@
 import com.cloud.vm.dao.UserVmDetailsDao;
 import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
 import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
+import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd;
 import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd;
 import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
 import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
@@ -1119,4 +1121,22 @@ public void testUpdateVncPasswordIfItHasChangedNewPassword() {
         Mockito.verify(userVmDao).findById(vmId);
         Mockito.verify(userVmDao).update(vmId, userVmVoMock);
     }
+
+    @Test
+    public void testGetSecurityGroupIdList() {
+        DeployVnfApplianceCmd cmd = Mockito.mock(DeployVnfApplianceCmd.class);
+        Mockito.doReturn(new ArrayList<Long>()).when(userVmManagerImpl).getSecurityGroupIdList(cmd);
+        SecurityGroupVO securityGroupVO = Mockito.mock(SecurityGroupVO.class);
+        long securityGroupId = 100L;
+        when(securityGroupVO.getId()).thenReturn(securityGroupId);
+        Mockito.doReturn(securityGroupVO).when(vnfTemplateManager).createSecurityGroupForVnfAppliance(any(), any(), any(), any(DeployVnfApplianceCmd.class));
+
+        List<Long> securityGroupIds = userVmManagerImpl.getSecurityGroupIdList(cmd, null, null, null);
+
+        Assert.assertEquals(1, securityGroupIds.size());
+        Assert.assertEquals(securityGroupId, securityGroupIds.get(0).longValue());
+
+        Mockito.verify(userVmManagerImpl).getSecurityGroupIdList(cmd);
+        Mockito.verify(vnfTemplateManager).createSecurityGroupForVnfAppliance(any(), any(), any(), any(DeployVnfApplianceCmd.class));
+    }
 }

test/integration/smoke/test_vnf_templates.py

@@ -25,6 +25,7 @@
                              VirtualMachine,
                              Network,
                              NetworkOffering,
+                             VnfAppliance,
                              VnfTemplate,
                              Zone)
 from marvin.lib.common import get_zone, get_template
@@ -308,17 +309,18 @@ def test_04_deploy_vnf_appliance(self):
             pass
 
         # success deployment
-        self.virtual_machine = VirtualMachine.create(
+        self.vnf_appliance = VnfAppliance.create(
             self.user_apiclient,
             self.services["virtual_machine"],
             zoneid=self.zone.id,
             templateid=self.vnf_template.id,
             accountid=self.account.name,
             domainid=self.account.domainid,
             serviceofferingid=self.service_offering.id,
-            networkids=[isolated_network.id, l2_network_1.id, l2_network_2.id]
+            networkids=[isolated_network.id, l2_network_1.id, l2_network_2.id],
+            vnfconfiguremanagement='true'
         )
-        self.cleanup.append(self.virtual_machine)
+        self.cleanup.append(self.vnf_appliance)
 
     @attr(tags=["advanced"], required_hardware="false")
     def test_05_delete_vnf_template(self):

tools/marvin/marvin/lib/base.py

@@ -6817,3 +6817,198 @@ def list(cls, apiclient, **kwargs):
         if 'account' in list(kwargs.keys()) and 'domainid' in list(kwargs.keys()):
             cmd.listall = True
         return (apiclient.listVnfTemplates(cmd))
+
+class VnfAppliance:
+    """Manage VNF Appliance life cycle"""
+
+    def __init__(self, items):
+        self.__dict__.update(items)
+
+    @classmethod
+    def create(cls, apiclient, services, templateid=None, accountid=None,
+               domainid=None, zoneid=None, networkids=None,
+               serviceofferingid=None, securitygroupids=None,
+               projectid=None, startvm=None, diskofferingid=None,
+               affinitygroupnames=None, affinitygroupids=None, group=None,
+               hostid=None, clusterid=None, keypair=None, ipaddress=None, mode='default',
+               method='GET', hypervisor=None, customcpunumber=None,
+               customcpuspeed=None, custommemory=None, rootdisksize=None,
+               rootdiskcontroller=None, vpcid=None, macaddress=None, datadisktemplate_diskoffering_list={},
+               properties=None, nicnetworklist=None, bootmode=None, boottype=None, dynamicscalingenabled=None,
+               userdataid=None, userdatadetails=None, extraconfig=None,
+               vnfconfiguremanagement=None, vnfcidrlist=None):
+        """Create the VNF appliance"""
+
+        cmd = deployVnfAppliance.deployVnfApplianceCmd()
+
+        if serviceofferingid:
+            cmd.serviceofferingid = serviceofferingid
+        elif "serviceoffering" in services:
+            cmd.serviceofferingid = services["serviceoffering"]
+
+        if zoneid:
+            cmd.zoneid = zoneid
+        elif "zoneid" in services:
+            cmd.zoneid = services["zoneid"]
+
+        if hypervisor:
+            cmd.hypervisor = hypervisor
+
+        if "displayname" in services:
+            cmd.displayname = services["displayname"]
+
+        if "name" in services:
+            cmd.name = services["name"]
+
+        if accountid:
+            cmd.account = accountid
+        elif "account" in services:
+            cmd.account = services["account"]
+
+        if domainid:
+            cmd.domainid = domainid
+        elif "domainid" in services:
+            cmd.domainid = services["domainid"]
+
+        if networkids:
+            cmd.networkids = networkids
+            allow_egress = False
+        elif "networkids" in services:
+            cmd.networkids = services["networkids"]
+            allow_egress = False
+        else:
+            # When no networkids are passed, network
+            # is created using the "defaultOfferingWithSourceNAT"
+            # which has an egress policy of DENY. But guests in tests
+            # need access to test network connectivity
+            allow_egress = True
+
+        if templateid:
+            cmd.templateid = templateid
+        elif "template" in services:
+            cmd.templateid = services["template"]
+
+        if diskofferingid:
+            cmd.diskofferingid = diskofferingid
+        elif "diskoffering" in services:
+            cmd.diskofferingid = services["diskoffering"]
+
+        if keypair:
+            cmd.keypair = keypair
+        elif "keypair" in services:
+            cmd.keypair = services["keypair"]
+
+        if ipaddress:
+            cmd.ipaddress = ipaddress
+        elif "ipaddress" in services:
+            cmd.ipaddress = services["ipaddress"]
+
+        if securitygroupids:
+            cmd.securitygroupids = [str(sg_id) for sg_id in securitygroupids]
+
+        if "affinitygroupnames" in services:
+            cmd.affinitygroupnames = services["affinitygroupnames"]
+        elif affinitygroupnames:
+            cmd.affinitygroupnames = affinitygroupnames
+
+        if affinitygroupids:
+            cmd.affinitygroupids = affinitygroupids
+
+        if projectid:
+            cmd.projectid = projectid
+
+        if startvm is not None:
+            cmd.startvm = startvm
+
+        if hostid:
+            cmd.hostid = hostid
+
+        if clusterid:
+            cmd.clusterid = clusterid
+
+        if "userdata" in services:
+            cmd.userdata = base64.urlsafe_b64encode(services["userdata"].encode()).decode()
+
+        if userdataid is not None:
+            cmd.userdataid = userdataid
+
+        if userdatadetails is not None:
+            cmd.userdatadetails = userdatadetails
+
+        if "dhcpoptionsnetworklist" in services:
+            cmd.dhcpoptionsnetworklist = services["dhcpoptionsnetworklist"]
+
+        if dynamicscalingenabled is not None:
+            cmd.dynamicscalingenabled = dynamicscalingenabled
+
+        cmd.details = [{}]
+
+        if customcpunumber:
+            cmd.details[0]["cpuNumber"] = customcpunumber
+
+        if customcpuspeed:
+            cmd.details[0]["cpuSpeed"] = customcpuspeed
+
+        if custommemory:
+            cmd.details[0]["memory"] = custommemory
+
+        if not rootdisksize is None and rootdisksize >= 0:
+            cmd.details[0]["rootdisksize"] = rootdisksize
+
+        if rootdiskcontroller:
+            cmd.details[0]["rootDiskController"] = rootdiskcontroller
+
+        if "size" in services:
+            cmd.size = services["size"]
+
+        if group:
+            cmd.group = group
+
+        cmd.datadisktemplatetodiskofferinglist = []
+        for datadisktemplate, diskoffering in list(datadisktemplate_diskoffering_list.items()):
+            cmd.datadisktemplatetodiskofferinglist.append({
+                'datadisktemplateid': datadisktemplate,
+                'diskofferingid': diskoffering
+            })
+
+        # program default access to ssh
+        if mode.lower() == 'basic':
+            cls.ssh_access_group(apiclient, cmd)
+
+        if macaddress:
+            cmd.macaddress = macaddress
+        elif macaddress in services:
+            cmd.macaddress = services["macaddress"]
+
+        if properties:
+            cmd.properties = properties
+
+        if nicnetworklist:
+            cmd.nicnetworklist = nicnetworklist
+
+        if bootmode:
+            cmd.bootmode = bootmode
+
+        if boottype:
+            cmd.boottype = boottype
+
+        if extraconfig:
+            cmd.extraconfig = extraconfig
+
+        if vnfconfiguremanagement:
+            cmd.vnfconfiguremanagement = vnfconfiguremanagement
+
+        if vnfcidrlist:
+            cmd.vnfcidrlist = vnfcidrlist
+
+        vnf_app = apiclient.deployVnfAppliance(cmd, method=method)
+
+        return VnfAppliance(vnf_app.__dict__)
+
+    def delete(self, apiclient, expunge=True, **kwargs):
+        """Destroy an VNF appliance"""
+        cmd = destroyVirtualMachine.destroyVirtualMachineCmd()
+        cmd.id = self.id
+        cmd.expunge = expunge
+        [setattr(cmd, k, v) for k, v in list(kwargs.items())]
+        apiclient.destroyVirtualMachine(cmd)

ui/public/locales/en.json

@@ -2196,7 +2196,7 @@
 "label.vnf.app.action.reboot": "Reboot VNF appliance",
 "label.vnf.app.action.reinstall": "Reinstall VNF appliance",
 "label.vnf.cidr.list": "Source cidr list of rules",
-"label.vnf.cidr.list.tooltip": "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,).",
+"label.vnf.cidr.list.tooltip": "the CIDR list to forward traffic from to the VNF management interface. Multiple entries must be separated by a single comma character (,). The default value is 0.0.0.0/0.",
 "label.vnf.configure.management": "Configure rules for VNF management interfaces",
 "label.vnf.configure.management.tooltip": "True by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. False otherwise.",
 "label.vnf.detail.add": "Add VNF detail",