⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
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 = "False by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. True otherwise. " +
"Network rules are configured if management network is an isolated network or shared network with security groups.")
private Boolean vnfConfigureManagement;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd;
import org.apache.cloudstack.framework.config.ConfigKey;
import java.util.List;
import java.util.Map;

public interface VnfTemplateManager {

Expand All @@ -42,11 +43,12 @@ public interface VnfTemplateManager {

void updateVnfTemplate(long templateId, UpdateVnfTemplateCmd cmd);

void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds);
void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds, Map<Integer, Long> vmNetworkMap);

SecurityGroup createSecurityGroupForVnfAppliance(DataCenter zone, VirtualMachineTemplate template, Account owner, DeployVnfApplianceCmd cmd);

void createIsolatedNetworkRulesForVnfAppliance(DataCenter zone, VirtualMachineTemplate template, Account owner,
UserVm vm, DeployVnfApplianceCmd cmd)
throws InsufficientAddressCapacityException, ResourceAllocationException, ResourceUnavailableException;

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.
package org.apache.cloudstack.storage.template;

import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.network.VNF;
import com.cloud.storage.Storage;
Expand Down Expand Up @@ -124,6 +125,9 @@ public static void validateVnfNics(List<VNF.VnfNic> nicsList) {
public static void validateApiCommandParams(BaseCmd cmd, VirtualMachineTemplate template) {
if (cmd instanceof RegisterVnfTemplateCmd) {
RegisterVnfTemplateCmd registerCmd = (RegisterVnfTemplateCmd) cmd;
if (registerCmd.isDeployAsIs() && CollectionUtils.isNotEmpty(registerCmd.getVnfNics())) {
throw new InvalidParameterValueException("VNF nics cannot be specified when register a deploy-as-is Template. Please wait until Template settings are read from OVA.");
}
validateApiCommandParams(registerCmd.getVnfDetails(), registerCmd.getVnfNics(), registerCmd.getTemplateType());
} else if (cmd instanceof UpdateVnfTemplateCmd) {
UpdateVnfTemplateCmd updateCmd = (UpdateVnfTemplateCmd) cmd;
Expand All @@ -149,4 +153,18 @@ public static void validateVnfCidrList(List<String> cidrList) {
}
}
}

public static void validateDeployAsIsTemplateVnfNics(List<OVFNetworkTO> ovfNetworks, List<VNF.VnfNic> vnfNics) {
if (CollectionUtils.isEmpty(vnfNics)) {
return;
}
if (CollectionUtils.isEmpty(ovfNetworks)) {
throw new InvalidParameterValueException("The list of networks read from OVA is empty. Please wait until the template is fully downloaded and processed.");
}
for (VNF.VnfNic vnfNic : vnfNics) {
if (vnfNic.getDeviceId() < ovfNetworks.size() && !vnfNic.isRequired()) {
throw new InvalidParameterValueException(String.format("The VNF nic [device ID: %s ] is required as it is defined in the OVA template.", vnfNic.getDeviceId()));
}
Comment on lines +165 to +167
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation logic has a flaw. It checks if vnfNic.getDeviceId() < ovfNetworks.size() && !vnfNic.isRequired(), which throws an error if the vnfNic is NOT required. This is backwards - the error message says "The VNF nic [device ID: %s ] is required" but the condition checks !vnfNic.isRequired(). The logic should be: if the device ID corresponds to an OVF network, the VNF nic MUST be required. So the condition should be checking if it's defined in OVF but marked as not required in the VNF configuration.

Copilot uses AI. Check for mistakes.
}
}
Comment on lines +157 to +169
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new method validateDeployAsIsTemplateVnfNics is not covered by any unit tests. This is a new validation method for deploy-as-is VNF templates, and given that similar validation methods in the codebase have test coverage, unit tests should be added to ensure the validation logic works as expected.

Copilot uses AI. Check for mistakes.
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@
import com.cloud.agent.api.to.DiskTO;
import com.cloud.agent.api.to.NfsTO;
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.agent.api.to.deployasis.OVFNetworkTO;
import com.cloud.api.ApiDBUtils;
import com.cloud.api.query.dao.UserVmJoinDao;
import com.cloud.api.query.vo.UserVmJoinVO;
Expand All @@ -131,6 +132,7 @@
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.deploy.DeployDestination;
import com.cloud.deployasis.dao.TemplateDeployAsIsDetailsDao;
import com.cloud.domain.Domain;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.ActionEvent;
Expand Down Expand Up @@ -313,6 +315,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
protected SnapshotHelper snapshotHelper;
@Inject
VnfTemplateManager vnfTemplateManager;
@Inject
TemplateDeployAsIsDetailsDao templateDeployAsIsDetailsDao;

@Inject
private SecondaryStorageHeuristicDao secondaryStorageHeuristicDao;
Expand Down Expand Up @@ -2172,6 +2176,11 @@ private VMTemplateVO updateTemplateOrIso(BaseUpdateTemplateOrIsoCmd cmd) {
templateType = validateTemplateType(cmd, isAdmin, template.isCrossZones());
if (cmd instanceof UpdateVnfTemplateCmd) {
VnfTemplateUtils.validateApiCommandParams(cmd, template);
UpdateVnfTemplateCmd updateCmd = (UpdateVnfTemplateCmd) cmd;
if (template.isDeployAsIs() && CollectionUtils.isNotEmpty(updateCmd.getVnfNics())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvazquez
when register a deploy-as-is template, the template NICs are not available until the template is downloaded successfully.
I think it is better that user configures VNF nics only when template NICs are fetched from OVA template.

List<OVFNetworkTO> ovfNetworks = templateDeployAsIsDetailsDao.listNetworkRequirementsByTemplateId(template.getId());
VnfTemplateUtils.validateDeployAsIsTemplateVnfNics(ovfNetworks, updateCmd.getVnfNics());
}
vnfTemplateManager.updateVnfTemplate(template.getId(), (UpdateVnfTemplateCmd) cmd);
}
templateTag = ((UpdateTemplateCmd)cmd).getTemplateTag();
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -6127,7 +6127,7 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE
throw new InvalidParameterValueException("Unable to use template " + templateId);
}
if (TemplateType.VNF.equals(template.getTemplateType())) {
vnfTemplateManager.validateVnfApplianceNics(template, cmd.getNetworkIds());
vnfTemplateManager.validateVnfApplianceNics(template, cmd.getNetworkIds(), cmd.getVmNetworkMap());
} else if (cmd instanceof DeployVnfApplianceCmd) {
throw new InvalidParameterValueException("Can't deploy VNF appliance from a non-VNF template");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,14 @@ public ConfigKey<?>[] getConfigKeys() {
}

@Override
public void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds) {
public void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long> networkIds, Map<Integer, Long> vmNetworkMap) {
if (template.isDeployAsIs()) {
if (CollectionUtils.isNotEmpty(networkIds)) {
throw new InvalidParameterValueException("VNF nics mappings should be empty for deploy-as-is templates");
}
validateVnfApplianceNetworksMap(template, vmNetworkMap);
return;
}
if (CollectionUtils.isEmpty(networkIds)) {
throw new InvalidParameterValueException("VNF nics list is empty");
}
Expand All @@ -213,6 +220,18 @@ public void validateVnfApplianceNics(VirtualMachineTemplate template, List<Long>
}
}

private void validateVnfApplianceNetworksMap(VirtualMachineTemplate template, Map<Integer, Long> vmNetworkMap) {
if (MapUtils.isEmpty(vmNetworkMap)) {
throw new InvalidParameterValueException("VNF networks map is empty");
}
List<VnfTemplateNicVO> vnfNics = vnfTemplateNicDao.listByTemplateId(template.getId());
for (VnfTemplateNicVO vnfNic : vnfNics) {
if (vnfNic.isRequired() && vmNetworkMap.size() <= vnfNic.getDeviceId()) {
throw new InvalidParameterValueException("VNF nic is required but not found: " + vnfNic);
}
}
}

protected Set<Integer> getOpenPortsForVnfAppliance(VirtualMachineTemplate template) {
Set<Integer> ports = new HashSet<>();
VnfTemplateDetailVO accessMethodsDetail = vnfTemplateDetailsDao.findDetail(template.getId(), VNF.AccessDetail.ACCESS_METHODS.name().toLowerCase());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.cloud.api.query.dao.UserVmJoinDao;
import com.cloud.configuration.Resource;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.deployasis.dao.TemplateDeployAsIsDetailsDao;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.dao.UsageEventDao;
import com.cloud.exception.InvalidParameterValueException;
Expand Down Expand Up @@ -204,6 +205,8 @@ public class TemplateManagerImplTest {
AccountManager _accountMgr;
@Inject
VnfTemplateManager vnfTemplateManager;
@Inject
TemplateDeployAsIsDetailsDao templateDeployAsIsDetailsDao;

@Inject
HeuristicRuleHelper heuristicRuleHelperMock;
Expand Down Expand Up @@ -956,6 +959,11 @@ public VnfTemplateManager vnfTemplateManager() {
return Mockito.mock(VnfTemplateManager.class);
}

@Bean
public TemplateDeployAsIsDetailsDao templateDeployAsIsDetailsDao() {
return Mockito.mock(TemplateDeployAsIsDetailsDao.class);
}

@Bean
public SnapshotHelper snapshotHelper() {
return Mockito.mock(SnapshotHelper.class);
Expand Down
6 changes: 3 additions & 3 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ public void createVirtualMachine() throws ResourceUnavailableException, Insuffic
when(templateMock.isDeployAsIs()).thenReturn(false);
when(templateMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
when(templateMock.getUserDataId()).thenReturn(null);
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class));
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class), nullable(Map.class));

ServiceOfferingJoinVO svcOfferingMock = Mockito.mock(ServiceOfferingJoinVO.class);
when(serviceOfferingJoinDao.findById(anyLong())).thenReturn(svcOfferingMock);
Expand All @@ -1091,7 +1091,7 @@ public void createVirtualMachine() throws ResourceUnavailableException, Insuffic

UserVm result = userVmManagerImpl.createVirtualMachine(deployVMCmd);
assertEquals(userVmVoMock, result);
Mockito.verify(vnfTemplateManager).validateVnfApplianceNics(templateMock, null);
Mockito.verify(vnfTemplateManager).validateVnfApplianceNics(templateMock, null, null);
Mockito.verify(userVmManagerImpl).createBasicSecurityGroupVirtualMachine(any(), any(), any(), any(), any(), any(), any(),
any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), nullable(Boolean.class), any(), any(), any(),
any(), any(), any(), any(), eq(true), any());
Expand Down Expand Up @@ -1335,7 +1335,7 @@ public void createVirtualMachineWithCloudRuntimeException() throws ResourceUnava
when(templateMock.isDeployAsIs()).thenReturn(false);
when(templateMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
when(templateMock.getUserDataId()).thenReturn(null);
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class));
Mockito.doNothing().when(vnfTemplateManager).validateVnfApplianceNics(any(), nullable(List.class), nullable(Map.class));

ServiceOfferingJoinVO svcOfferingMock = Mockito.mock(ServiceOfferingJoinVO.class);
when(serviceOfferingJoinDao.findById(anyLong())).thenReturn(svcOfferingMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,25 +228,25 @@ public void testPersistVnfTemplateUpdateWithoutDetails() {
@Test
public void testValidateVnfApplianceNicsWithRequiredNics() {
List<Long> networkIds = Arrays.asList(200L, 201L);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds, null);
}

@Test
public void testValidateVnfApplianceNicsWithAllNics() {
List<Long> networkIds = Arrays.asList(200L, 201L, 202L);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds, null);
}

@Test(expected = InvalidParameterValueException.class)
public void testValidateVnfApplianceNicsWithEmptyList() {
List<Long> networkIds = new ArrayList<>();
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds, null);
}

@Test(expected = InvalidParameterValueException.class)
public void testValidateVnfApplianceNicsWithMissingNetworkId() {
List<Long> networkIds = Arrays.asList(200L);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds);
vnfTemplateManagerImpl.validateVnfApplianceNics(template, networkIds, null);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ui/public/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2528,7 +2528,7 @@
"label.vnf.cidr.list": "CIDR from which access to the VNF appliance's Management interface should be allowed from",
"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 network rules for VNF's 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. Learn what rules are configured at http://docs.cloudstack.apache.org/en/latest/adminguide/networking/vnf_templates_appliances.html#deploying-vnf-appliances",
"label.vnf.configure.management.tooltip": "False by default, security group or network rules (source nat and firewall rules) will be configured for VNF management interfaces. True otherwise. Learn what rules are configured at http://docs.cloudstack.apache.org/en/latest/adminguide/networking/vnf_templates_appliances.html#deploying-vnf-appliances",
"label.vnf.detail.add": "Add VNF detail",
"label.vnf.detail.remove": "Remove VNF detail",
"label.vnf.details": "VNF Details",
Expand Down
14 changes: 12 additions & 2 deletions ui/src/views/compute/DeployVnfAppliance.vue
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@
<div>
<vnf-nics-selection
:items="templateVnfNics"
:templateNics="templateNics"
:networks="networks"
@update-vnf-nic-networks="($event) => updateVnfNicNetworks($event)" />
</div>
Expand Down Expand Up @@ -1293,7 +1294,8 @@ export default {
return tabList
},
showVnfNicsSection () {
return this.networks && this.networks.length > 0 && this.vm.templateid && this.templateVnfNics && this.templateVnfNics.length > 0
return ((this.networks && this.networks.length > 0) || (this.templateNics && this.templateNics.length > 0)) &&
this.vm.templateid && this.templateVnfNics && this.templateVnfNics.length > 0
},
showVnfConfigureManagement () {
const managementDeviceIds = []
Expand All @@ -1303,6 +1305,11 @@ export default {
}
}
for (const deviceId of managementDeviceIds) {
if (this.templateNics && this.templateNics[deviceId] &&
((this.templateNics[deviceId].selectednetworktype === 'Isolated' && this.templateNics[deviceId].selectednetworkvpcid === undefined) ||
(this.templateNics[deviceId].selectednetworktype === 'Shared' && this.templateNics[deviceId].selectednetworkwithsg))) {
return true
}
if (this.vnfNicNetworks && this.vnfNicNetworks[deviceId] &&
((this.vnfNicNetworks[deviceId].type === 'Isolated' && this.vnfNicNetworks[deviceId].vpcid === undefined) ||
(this.vnfNicNetworks[deviceId].type === 'Shared' && this.vnfNicNetworks[deviceId].service.filter(svc => svc.name === 'SecurityGroupProvider')))) {
Expand Down Expand Up @@ -2005,7 +2012,7 @@ export default {
// All checked networks should be used and only once.
// Required NIC must be associated to a network
// DeviceID must be consequent
if (this.templateVnfNics && this.templateVnfNics.length > 0) {
if (this.templateVnfNics && this.templateVnfNics.length > 0 && (!this.templateNics || this.templateNics.length === 0)) {
let nextDeviceId = 0
const usedNetworkIds = []
const keys = Object.keys(this.vnfNicNetworks)
Expand Down Expand Up @@ -2629,6 +2636,9 @@ export default {
var network = this.options.networks[Math.min(i, this.options.networks.length - 1)]
nic.selectednetworkid = network.id
nic.selectednetworkname = network.name
nic.selectednetworktype = network.type
nic.selectednetworkvpcid = network.vpcid
nic.selectednetworkwithsg = network.service.filter(svc => svc.name === 'SecurityGroupProvider').length > 0
this.nicToNetworkSelection.push({ nic: nic.id, network: network.id })
}
}
Expand Down
5 changes: 5 additions & 0 deletions ui/src/views/compute/wizard/VnfNicsSelection.vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
<template #network="{ record }">
<a-form-item style="display: block" :name="'nic-' + record.deviceid">
<a-select
disabled="templateNics && templateNics.length > 0"
@change="updateNicNetworkValue($event, record.deviceid)"
optionFilterProp="label"
:filterOption="(input, option) => {
Expand All @@ -75,6 +76,10 @@ export default {
type: Array,
default: () => []
},
templateNics: {
type: Array,
default: () => []
},
networks: {
type: Array,
default: () => []
Expand Down
Loading