diff --git a/api/src/main/java/com/cloud/vm/VmDetailConstants.java b/api/src/main/java/com/cloud/vm/VmDetailConstants.java index 33cc6da70812..b1b70a6a1587 100644 --- a/api/src/main/java/com/cloud/vm/VmDetailConstants.java +++ b/api/src/main/java/com/cloud/vm/VmDetailConstants.java @@ -87,6 +87,7 @@ public interface VmDetailConstants { String NETWORK = "network"; String IP4_ADDRESS = "ip4Address"; String IP6_ADDRESS = "ip6Address"; + String NIC_MAC_ADDRESS = "macAddress"; String DISK = "disk"; String DISK_OFFERING = "diskOffering"; diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index ee10db031ba7..f923d305a09a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -418,6 +418,7 @@ public class ApiConstants { public static final String NICS = "nics"; public static final String NIC_NETWORK_LIST = "nicnetworklist"; public static final String NIC_IP_ADDRESS_LIST = "nicipaddresslist"; + public static final String NIC_MAC_ADDRESS_LIST = "nicmacaddresslist"; public static final String NIC_MULTIQUEUE_NUMBER = "nicmultiqueuenumber"; public static final String NIC_PACKED_VIRTQUEUES_ENABLED = "nicpackedvirtqueuesenabled"; public static final String NEW_START_IP = "newstartip"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index 3284dbafe7ca..1d0267750f79 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -135,6 +135,14 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { description = "VM NIC to ip address mapping using keys NIC, ip4Address") private Map nicIpAddressList; + @Parameter(name = ApiConstants.NIC_MAC_ADDRESS_LIST, + type = CommandType.MAP, + description = "VM NIC to MAC address mapping using keys nic and macAddress. " + + "Example: nicmacaddresslist[0].nic=nic1&nicmacaddresslist[0].macAddress=aa:bb:cc:dd:ee:ff. " + + "Overrides the MAC address reported by the hypervisor for the given NIC.", + since = "4.21.0") + private Map nicMacAddressList; + @Parameter(name = ApiConstants.DATADISK_OFFERING_LIST, type = CommandType.MAP, description = "Datadisk Template to disk-offering mapping using keys disk and diskOffering") @@ -234,6 +242,32 @@ public Map getNicIpAddressList() { return nicIpAddressMap; } + public Map getNicMacAddressList() { + Map nicMacMap = new HashMap<>(); + if (MapUtils.isEmpty(nicMacAddressList)) { + return nicMacMap; + } + for (Map entry : (Collection>) nicMacAddressList.values()) { + String nic = entry.get(VmDetailConstants.NIC); + String mac = entry.get(VmDetailConstants.NIC_MAC_ADDRESS); + logger.debug("Checking if MAC address '{}' can be mapped to NIC '{}'", mac, nic); + if (StringUtils.isEmpty(nic)) { + throw new InvalidParameterValueException(String.format("NIC ID: '%s' is invalid for MAC address mapping", nic)); + } + if (StringUtils.isEmpty(mac)) { + throw new InvalidParameterValueException(String.format("Empty MAC address for NIC ID: %s is invalid", nic)); + } + if (!NetUtils.isValidMac(mac)) { + throw new InvalidParameterValueException(String.format("MAC address '%s' for NIC ID: %s is not valid", mac, nic)); + } + if (!NetUtils.isUnicastMac(mac)) { + throw new InvalidParameterValueException(String.format("MAC address '%s' for NIC ID: %s is not a unicast address", mac, nic)); + } + nicMacMap.put(nic, NetUtils.standardizeMacAddress(mac)); + } + return nicMacMap; + } + public Map getDataDiskToDiskOfferingList() { Map dataDiskToDiskOfferingMap = new HashMap<>(); if (MapUtils.isNotEmpty(dataDiskToDiskOfferingList)) { diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmdTest.java new file mode 100644 index 000000000000..a37129f30d25 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmdTest.java @@ -0,0 +1,139 @@ +// 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. + +package org.apache.cloudstack.api.command.admin.vm; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.vm.VmDetailConstants; + +@RunWith(MockitoJUnitRunner.class) +public class ImportUnmanagedInstanceCmdTest { + + private ImportUnmanagedInstanceCmd buildCmd(Map nicMacAddressList) { + ImportUnmanagedInstanceCmd cmd = new ImportUnmanagedInstanceCmd(); + ReflectionTestUtils.setField(cmd, "nicMacAddressList", nicMacAddressList); + return cmd; + } + + private Map entry(String nic, String mac) { + Map e = new LinkedHashMap<>(); + if (nic != null) { + e.put(VmDetailConstants.NIC, nic); + } + if (mac != null) { + e.put(VmDetailConstants.NIC_MAC_ADDRESS, mac); + } + return e; + } + + @Test + public void testGetNicMacAddressListNullReturnsEmpty() { + ImportUnmanagedInstanceCmd cmd = buildCmd(null); + Map result = cmd.getNicMacAddressList(); + Assert.assertNotNull(result); + Assert.assertTrue(result.isEmpty()); + } + + @Test + public void testGetNicMacAddressListEmptyMapReturnsEmpty() { + ImportUnmanagedInstanceCmd cmd = buildCmd(new LinkedHashMap<>()); + Assert.assertTrue(cmd.getNicMacAddressList().isEmpty()); + } + + @Test + public void testGetNicMacAddressListValidMac() { + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "AA:BB:CC:DD:EE:FF")); + ImportUnmanagedInstanceCmd cmd = buildCmd(outer); + + Map result = cmd.getNicMacAddressList(); + + Assert.assertEquals(1, result.size()); + // standardizeMacAddress lowercases the address + Assert.assertEquals("aa:bb:cc:dd:ee:ff", result.get("nic1")); + } + + @Test + public void testGetNicMacAddressListMultipleNics() { + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "aa:bb:cc:dd:ee:01")); + outer.put("1", entry("nic2", "aa:bb:cc:dd:ee:02")); + ImportUnmanagedInstanceCmd cmd = buildCmd(outer); + + Map result = cmd.getNicMacAddressList(); + + Assert.assertEquals(2, result.size()); + Assert.assertEquals("aa:bb:cc:dd:ee:01", result.get("nic1")); + Assert.assertEquals("aa:bb:cc:dd:ee:02", result.get("nic2")); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListInvalidMacFormat() { + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "not-a-mac")); + buildCmd(outer).getNicMacAddressList(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListBroadcastMacRejected() { + // FF:FF:FF:FF:FF:FF is a broadcast (non-unicast) address + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "FF:FF:FF:FF:FF:FF")); + buildCmd(outer).getNicMacAddressList(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListMulticastMacRejected() { + // Bit 0 of first octet set → multicast, not unicast + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "01:bb:cc:dd:ee:ff")); + buildCmd(outer).getNicMacAddressList(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListEmptyNicIdRejected() { + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("", "aa:bb:cc:dd:ee:ff")); + buildCmd(outer).getNicMacAddressList(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListEmptyMacRejected() { + Map outer = new LinkedHashMap<>(); + outer.put("0", entry("nic1", "")); + buildCmd(outer).getNicMacAddressList(); + } + + @Test(expected = InvalidParameterValueException.class) + public void testGetNicMacAddressListNullMacRejected() { + Map outer = new LinkedHashMap<>(); + // entry without mac key + Map e = new LinkedHashMap<>(); + e.put(VmDetailConstants.NIC, "nic1"); + outer.put("0", e); + buildCmd(outer).getNicMacAddressList(); + } +} diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 846eab599fd1..ade40e92cb30 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -847,15 +847,49 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, return new Pair(profile, storagePool); } - private NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { + protected NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { + // Prefer caller-supplied MAC (from nicmacaddresslist); fall back to hypervisor-reported MAC + String macAddress = (ipAddresses != null && StringUtils.isNotEmpty(ipAddresses.getMacAddress())) + ? ipAddresses.getMacAddress() + : nic.getMacAddress(); DataCenterVO dataCenterVO = dataCenterDao.findById(network.getDataCenterId()); - Pair result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced); + Pair result = networkOrchestrationService.importNic(macAddress, deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced); if (result == null) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("NIC ID: %s import failed", nic.getNicId())); } return result.first(); } + + /** + * Merges caller-supplied MAC addresses into the NIC IP address map. + * For each NIC entry in {@code nicMacAddressMap}, the MAC address is set on the corresponding + * {@link Network.IpAddresses} object. If no IP address entry exists for a given NIC, a new + * {@link Network.IpAddresses} object is created to carry the MAC. + * + * @param nicIpAddressMap map of NIC ID to IP addresses (may be empty, never null after cmd getter) + * @param nicMacAddressMap map of NIC ID to validated/standardized MAC address strings + * @return merged map with MAC addresses populated where supplied by the caller + */ + protected Map mergeNicMacAddresses( + final Map nicIpAddressMap, + final Map nicMacAddressMap) { + if (MapUtils.isEmpty(nicMacAddressMap)) { + return nicIpAddressMap; + } + Map merged = new HashMap<>(nicIpAddressMap); + for (Map.Entry entry : nicMacAddressMap.entrySet()) { + String nicId = entry.getKey(); + String mac = entry.getValue(); + Network.IpAddresses existing = merged.get(nicId); + if (existing != null) { + existing.setMacAddress(mac); + } else { + merged.put(nicId, new Network.IpAddresses(null, null, mac)); + } + } + return merged; + } private void cleanupFailedImportVM(final UserVm userVm) { if (userVm == null) { return; @@ -1315,7 +1349,7 @@ private UserVmResponse baseImportInstance(ImportUnmanagedInstanceCmd cmd) { checkVmwareInstanceNameForImportInstance(cluster.getHypervisorType(), instanceName, hostName, zone); final Map nicNetworkMap = cmd.getNicNetworkList(); - final Map nicIpAddressMap = cmd.getNicIpAddressList(); + final Map nicIpAddressMap = mergeNicMacAddresses(cmd.getNicIpAddressList(), cmd.getNicMacAddressList()); final Map dataDiskOfferingMap = cmd.getDataDiskToDiskOfferingList(); final Map details = cmd.getDetails(); final boolean forced = cmd.isForced(); @@ -2600,7 +2634,7 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) { } final Map nicNetworkMap = cmd.getNicNetworkList(); - final Map nicIpAddressMap = cmd.getNicIpAddressList(); + final Map nicIpAddressMap = mergeNicMacAddresses(cmd.getNicIpAddressList(), cmd.getNicMacAddressList()); final Map dataDiskOfferingMap = cmd.getDataDiskToDiskOfferingList(); final Map details = cmd.getDetails(); diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index bee6c4ad257f..2ef43495109e 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -1609,4 +1609,140 @@ public void getDetailAsIntegerTestThrowsInvalidParameterValueExceptionWhenValueI details.put("key", "not-a-number"); unmanagedVMsManager.getDetailAsInteger("key", details); } + + // ------------------------------------------------------------------------- + // mergeNicMacAddresses tests + // ------------------------------------------------------------------------- + + @Test + public void mergeNicMacAddressesEmptyMacMapReturnsOriginal() { + Map ipMap = new HashMap<>(); + ipMap.put("nic1", new Network.IpAddresses("10.0.0.1", null)); + + Map result = unmanagedVMsManager.mergeNicMacAddresses(ipMap, new HashMap<>()); + + Assert.assertSame(ipMap, result); + Assert.assertNull(result.get("nic1").getMacAddress()); + } + + @Test + public void mergeNicMacAddressesNullMacMapReturnsOriginal() { + Map ipMap = new HashMap<>(); + ipMap.put("nic1", new Network.IpAddresses("10.0.0.1", null)); + + Map result = unmanagedVMsManager.mergeNicMacAddresses(ipMap, null); + + Assert.assertSame(ipMap, result); + } + + @Test + public void mergeNicMacAddressesMacMergedIntoExistingEntry() { + Map ipMap = new HashMap<>(); + ipMap.put("nic1", new Network.IpAddresses("10.0.0.1", null)); + + Map macMap = new HashMap<>(); + macMap.put("nic1", "aa:bb:cc:dd:ee:ff"); + + Map result = unmanagedVMsManager.mergeNicMacAddresses(ipMap, macMap); + + Assert.assertEquals("aa:bb:cc:dd:ee:ff", result.get("nic1").getMacAddress()); + Assert.assertEquals("10.0.0.1", result.get("nic1").getIp4Address()); + } + + @Test + public void mergeNicMacAddressesNewEntryCreatedWhenNicNotInIpMap() { + Map ipMap = new HashMap<>(); + + Map macMap = new HashMap<>(); + macMap.put("nic1", "aa:bb:cc:dd:ee:ff"); + + Map result = unmanagedVMsManager.mergeNicMacAddresses(ipMap, macMap); + + Assert.assertNotNull(result.get("nic1")); + Assert.assertEquals("aa:bb:cc:dd:ee:ff", result.get("nic1").getMacAddress()); + Assert.assertNull(result.get("nic1").getIp4Address()); + } + + @Test + public void mergeNicMacAddressesMultipleNicsMergedCorrectly() { + Map ipMap = new HashMap<>(); + ipMap.put("nic1", new Network.IpAddresses("10.0.0.1", null)); + ipMap.put("nic2", new Network.IpAddresses("10.0.0.2", null)); + + Map macMap = new HashMap<>(); + macMap.put("nic1", "aa:bb:cc:dd:ee:01"); + macMap.put("nic2", "aa:bb:cc:dd:ee:02"); + + Map result = unmanagedVMsManager.mergeNicMacAddresses(ipMap, macMap); + + Assert.assertEquals("aa:bb:cc:dd:ee:01", result.get("nic1").getMacAddress()); + Assert.assertEquals("10.0.0.1", result.get("nic1").getIp4Address()); + Assert.assertEquals("aa:bb:cc:dd:ee:02", result.get("nic2").getMacAddress()); + Assert.assertEquals("10.0.0.2", result.get("nic2").getIp4Address()); + } + + // ------------------------------------------------------------------------- + // importNic: caller MAC vs hypervisor MAC selection + // ------------------------------------------------------------------------- + + @Test + public void importNicUsesCallerSuppliedMacWhenPresent() throws Exception { + UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic(); + nic.setNicId("nic1"); + nic.setMacAddress("11:22:33:44:55:66"); + + Network.IpAddresses ipAddresses = new Network.IpAddresses(null, null, "aa:bb:cc:dd:ee:ff"); + + NetworkVO networkVO = Mockito.mock(NetworkVO.class); + when(networkVO.getDataCenterId()).thenReturn(1L); + + UserVmVO vm = Mockito.mock(UserVmVO.class); + NicProfile result = unmanagedVMsManager.importNic(nic, vm, networkVO, ipAddresses, 0, true, false); + + Assert.assertNotNull(result); + Mockito.verify(networkOrchestrationService).importNic( + Mockito.eq("aa:bb:cc:dd:ee:ff"), + Mockito.anyInt(), Mockito.any(), Mockito.anyBoolean(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean()); + } + + @Test + public void importNicFallsBackToHypervisorMacWhenIpAddressesHasNoMac() throws Exception { + UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic(); + nic.setNicId("nic1"); + nic.setMacAddress("11:22:33:44:55:66"); + + Network.IpAddresses ipAddresses = new Network.IpAddresses("10.0.0.1", null); // no MAC + + NetworkVO networkVO = Mockito.mock(NetworkVO.class); + when(networkVO.getDataCenterId()).thenReturn(1L); + + UserVmVO vm = Mockito.mock(UserVmVO.class); + NicProfile result = unmanagedVMsManager.importNic(nic, vm, networkVO, ipAddresses, 0, true, false); + + Assert.assertNotNull(result); + Mockito.verify(networkOrchestrationService).importNic( + Mockito.eq("11:22:33:44:55:66"), + Mockito.anyInt(), Mockito.any(), Mockito.anyBoolean(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean()); + } + + @Test + public void importNicFallsBackToHypervisorMacWhenIpAddressesIsNull() throws Exception { + UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic(); + nic.setNicId("nic1"); + nic.setMacAddress("11:22:33:44:55:66"); + + NetworkVO networkVO = Mockito.mock(NetworkVO.class); + when(networkVO.getDataCenterId()).thenReturn(1L); + + UserVmVO vm = Mockito.mock(UserVmVO.class); + NicProfile result = unmanagedVMsManager.importNic(nic, vm, networkVO, null, 0, true, false); + + Assert.assertNotNull(result); + Mockito.verify(networkOrchestrationService).importNic( + Mockito.eq("11:22:33:44:55:66"), + Mockito.anyInt(), Mockito.any(), Mockito.anyBoolean(), + Mockito.any(), Mockito.isNull(), Mockito.any(), Mockito.anyBoolean()); + } }