diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java index 93b9cf6aaae9..4d50dd9c39bf 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVnfApplianceCmd.java +++ b/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 vnfCidrlist; public Boolean getVnfConfigureManagement() { - return vnfConfigureManagement == null || vnfConfigureManagement; + return vnfConfigureManagement != null && vnfConfigureManagement; } public List getVnfCidrlist() { diff --git a/api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java b/api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java index 3f0b33341054..e997a50cec03 100644 --- a/api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java +++ b/api/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateUtils.java @@ -70,8 +70,8 @@ public static List 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; } diff --git a/server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java b/server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java index ca6d54b59af1..997f124884ee 100644 --- a/server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/storage/template/VnfTemplateManagerImpl.java @@ -274,7 +274,12 @@ private Map 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 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 cidrList = cmd.getVnfCidrlist(); try { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 02a6b8a72ee2..5e10c4a38f23 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/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()).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 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)); + } } diff --git a/test/integration/smoke/test_vnf_templates.py b/test/integration/smoke/test_vnf_templates.py index df3bf82cf208..f963ce41b384 100644 --- a/test/integration/smoke/test_vnf_templates.py +++ b/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,7 +309,7 @@ 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, @@ -316,9 +317,10 @@ def test_04_deploy_vnf_appliance(self): 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): diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index d134154d7e50..dd26de840c77 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/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) diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 5c55f8b94246..879c0771bb49 100644 --- a/ui/public/locales/en.json +++ b/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", diff --git a/ui/src/views/compute/DeployVnfAppliance.vue b/ui/src/views/compute/DeployVnfAppliance.vue index 146934f90d41..db48d047a099 100644 --- a/ui/src/views/compute/DeployVnfAppliance.vue +++ b/ui/src/views/compute/DeployVnfAppliance.vue @@ -375,6 +375,20 @@ :networks="networks" @update-vnf-nic-networks="($event) => updateVnfNicNetworks($event)" /> +
+ + + + + + + + +
- - - - - - - - @@ -1304,6 +1306,22 @@ export default { showVnfNicsSection () { return this.networks && this.networks.length > 0 && this.vm.templateid && this.templateVnfNics && this.templateVnfNics.length > 0 }, + showVnfConfigureManagement () { + const managementDeviceIds = [] + for (const templateVnfNic of this.templateVnfNics) { + if (templateVnfNic.management) { + managementDeviceIds.push(templateVnfNic.deviceid) + } + } + for (const deviceId of managementDeviceIds) { + if (this.vnfNicNetworks && this.vnfNicNetworks[deviceId] && + ((this.vnfNicNetworks[deviceId].type === 'Isolated' && this.vnfNicNetworks[deviceId].vpcid === undefined) || + (this.vnfNicNetworks[deviceId].type === 'Shared' && this.zone.securitygroupsenabled))) { + return true + } + } + return false + }, showSecurityGroupSection () { return (this.networks.length > 0 && this.zone.securitygroupsenabled) || (this.zone && this.zone.networktype === 'Basic') }, @@ -1527,7 +1545,7 @@ export default { if (this.zoneSelected) { this.form.startvm = true - this.form.vnfconfiguremanagement = true + this.form.vnfconfiguremanagement = false } if (this.zone && this.zone.networktype !== 'Basic') { @@ -2014,7 +2032,7 @@ export default { }) return } - const networkId = this.vnfNicNetworks[String(templateVnfNic.deviceid)] + const networkId = this.vnfNicNetworks[String(templateVnfNic.deviceid)]?.id || null if (networkId) { if (templateVnfNic.deviceid !== nextDeviceId) { this.$notification.error({ @@ -2163,7 +2181,7 @@ export default { if (networkIds.length > 0) { if (this.templateVnfNics && this.templateVnfNics.length > 0) { for (const templateVnfNic of this.templateVnfNics) { - const vnfNicNetworkId = this.vnfNicNetworks[String(templateVnfNic.deviceid)] || null + const vnfNicNetworkId = this.vnfNicNetworks[String(templateVnfNic.deviceid)]?.id || null if (vnfNicNetworkId) { const ipToNetwork = { networkid: vnfNicNetworkId @@ -2527,7 +2545,7 @@ export default { this.zone = _.find(this.options.zones, (option) => option.id === value) this.zoneSelected = true this.form.startvm = true - this.form.vnfconfiguremanagement = true + this.form.vnfconfiguremanagement = false this.form.vnfcidrlist = '' this.selectedZone = this.zoneId this.form.zoneid = this.zoneId diff --git a/ui/src/views/compute/wizard/VnfNicsSelection.vue b/ui/src/views/compute/wizard/VnfNicsSelection.vue index 3b7cabb8401d..602a63236de2 100644 --- a/ui/src/views/compute/wizard/VnfNicsSelection.vue +++ b/ui/src/views/compute/wizard/VnfNicsSelection.vue @@ -40,6 +40,10 @@ {{ $t('label.yes') }} {{ $t('label.no') }} + @@ -102,6 +106,12 @@ export default { width: '10%', slots: { customRender: 'required' } }, + { + dataIndex: 'management', + title: this.$t('label.management'), + width: '15%', + slots: { customRender: 'management' } + }, { dataIndex: 'description', title: this.$t('label.description'), @@ -132,8 +142,8 @@ export default { this.form = reactive(form) this.rules = reactive(rules) }, - updateNicNetworkValue (value, networkid) { - this.values[networkid] = value + updateNicNetworkValue (value, deviceid) { + this.values[deviceid] = this.networks.filter(network => network.id === value)?.[0] || null this.$emit('update-vnf-nic-networks', this.values) } }