Fix listPublicIpAddresses NPE on shared networks with a VR (#14249)

* Fix listPublicIpAddresses NPE on shared network router placeholder NICs (#14248)

* Get the router's real NIC when the shared network IP lookup returns its placeholder

Author: MitchDrage

server/src/main/java/com/cloud/api/ApiDBUtils.java

@@ -2311,6 +2311,10 @@ public static NicVO findByIp4AddressAndNetworkId(String ip4Address, long network
         return s_nicDao.findByIp4AddressAndNetworkId(ip4Address, networkId);
     }
 
+    public static NicVO findNonPlaceHolderByIp4AddressAndNetworkId(String ip4Address, long networkId) {
+        return s_nicDao.findNonPlaceHolderByIp4AddressAndNetworkId(ip4Address, networkId);
+    }
+
     public static NicSecondaryIpVO findSecondaryIpByIp4AddressAndNetworkId(String ip4Address, long networkId) {
         return s_nicSecondaryIpDao.findByIp4AddressAndNetworkId(ip4Address, networkId);
     }

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -1235,7 +1235,7 @@ private void setVpcIdInResponse(Long vpcId, Consumer<String> vpcUuidSetter, Cons
         }
     }
 
-    private void showVmInfoForSharedNetworks(boolean forVirtualNetworks, IpAddress ipAddr, IPAddressResponse ipResponse) {
+    protected void showVmInfoForSharedNetworks(boolean forVirtualNetworks, IpAddress ipAddr, IPAddressResponse ipResponse) {
         if (!forVirtualNetworks) {
             NicVO nic = ApiDBUtils.findByIp4AddressAndNetworkId(ipAddr.getAddress().toString(), ipAddr.getNetworkId());
 
@@ -1266,11 +1266,19 @@ private void showVmInfoForSharedNetworks(boolean forVirtualNetworks, IpAddress i
                     }
                 }
             } else if (nic.getVmType() == Type.DomainRouter) {
-                VirtualMachine vm = ApiDBUtils.findVMInstanceById(nic.getInstanceId());
-                if (vm != null) {
-                    ipResponse.setVirtualMachineId(vm.getUuid());
-                    ipResponse.setVirtualMachineName(vm.getHostName());
-                    ipResponse.setVirtualMachineType(vm.getType().toString());
+                ipResponse.setIsSystem(true);
+                // The lookup above returns either the router's nic or its placeholder, whichever the db returns first.
+                NicVO routerNic = nic;
+                if (nic.getReservationStrategy() == Nic.ReservationStrategy.PlaceHolder) {  // found a placeholder nic, get the router's real nic
+                    routerNic = ApiDBUtils.findNonPlaceHolderByIp4AddressAndNetworkId(ipAddr.getAddress().toString(), ipAddr.getNetworkId());
+                }
+                if (routerNic != null) {  // continue if we have the router's real nic, i.e. the router exists
+                    VirtualMachine vm = ApiDBUtils.findVMInstanceById(routerNic.getInstanceId());
+                    if (vm != null) {
+                        ipResponse.setVirtualMachineId(vm.getUuid());
+                        ipResponse.setVirtualMachineName(vm.getHostName());
+                        ipResponse.setVirtualMachineType(vm.getType().toString());
+                    }
                 }
             } else if (nic.getVmType().isUsedBySystem()) {
                 ipResponse.setIsSystem(true);

server/src/test/java/com/cloud/api/ApiResponseHelperTest.java

@@ -42,6 +42,7 @@
 import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse;
 import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse;
 import org.apache.cloudstack.api.response.GuestOSCategoryResponse;
+import org.apache.cloudstack.api.response.IPAddressResponse;
 import org.apache.cloudstack.api.response.IpQuarantineResponse;
 import org.apache.cloudstack.api.response.NicSecondaryIpResponse;
 import org.apache.cloudstack.api.response.ResourceIconResponse;
@@ -68,6 +69,7 @@
 import com.cloud.configuration.Resource;
 import com.cloud.domain.DomainVO;
 import com.cloud.host.HostVO;
+import com.cloud.network.IpAddress;
 import com.cloud.network.PublicIpQuarantine;
 import com.cloud.network.as.AutoScaleVmGroup;
 import com.cloud.network.as.AutoScaleVmGroupVO;
@@ -95,8 +97,11 @@
 import com.cloud.user.dao.UserDataDao;
 import com.cloud.utils.net.Ip;
 import com.cloud.vm.ConsoleSessionVO;
+import com.cloud.vm.Nic;
 import com.cloud.vm.NicSecondaryIp;
+import com.cloud.vm.NicVO;
 import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.VirtualMachine;
 import org.apache.cloudstack.api.ResponseObject;
 import org.apache.cloudstack.api.response.ConsoleSessionResponse;
 
@@ -759,4 +764,50 @@ public void createConsoleSessionResponseTestShouldReturnFullResponse() {
             Assert.assertEquals(expected.getVmName(), response.getVmName());
         }
     }
+
+    @Test
+    public void showVmInfoForSharedNetworksTestRouterIp() {
+        IpAddress ipAddress = Mockito.mock(IpAddress.class);
+        when(ipAddress.getAddress()).thenReturn(new Ip("10.1.1.2"));
+        when(ipAddress.getNetworkId()).thenReturn(1L);
+        NicVO placeholderNic = new NicVO(null, null, 1L, VirtualMachine.Type.DomainRouter);
+        placeholderNic.setReservationStrategy(Nic.ReservationStrategy.PlaceHolder);
+        NicVO routerNic = new NicVO("DirectNetworkGuru", 2L, 1L, VirtualMachine.Type.DomainRouter);
+        IPAddressResponse ipResponse = Mockito.mock(IPAddressResponse.class);
+
+        try (MockedStatic<ApiDBUtils> ignored = Mockito.mockStatic(ApiDBUtils.class)) {
+            when(ApiDBUtils.findByIp4AddressAndNetworkId("10.1.1.2", 1L)).thenReturn(placeholderNic);
+            when(ApiDBUtils.findNonPlaceHolderByIp4AddressAndNetworkId("10.1.1.2", 1L)).thenReturn(routerNic);
+            when(ApiDBUtils.findVMInstanceById(2L)).thenReturn(vmInstanceVOMock);
+            when(vmInstanceVOMock.getUuid()).thenReturn("router-uuid");
+            when(vmInstanceVOMock.getHostName()).thenReturn("r-2-VM");
+            when(vmInstanceVOMock.getType()).thenReturn(VirtualMachine.Type.DomainRouter);
+
+            apiResponseHelper.showVmInfoForSharedNetworks(false, ipAddress, ipResponse);
+
+            verify(ipResponse).setIsSystem(true);
+            verify(ipResponse).setVirtualMachineId("router-uuid");
+            verify(ipResponse).setVirtualMachineName("r-2-VM");
+        }
+    }
+
+    @Test
+    public void showVmInfoForSharedNetworksTestRouterPlaceholderOnly() {
+        IpAddress ipAddress = Mockito.mock(IpAddress.class);
+        when(ipAddress.getAddress()).thenReturn(new Ip("10.1.1.2"));
+        when(ipAddress.getNetworkId()).thenReturn(1L);
+        NicVO placeholderNic = new NicVO(null, null, 1L, VirtualMachine.Type.DomainRouter);
+        placeholderNic.setReservationStrategy(Nic.ReservationStrategy.PlaceHolder);
+        IPAddressResponse ipResponse = Mockito.mock(IPAddressResponse.class);
+
+        try (MockedStatic<ApiDBUtils> ignored = Mockito.mockStatic(ApiDBUtils.class)) {
+            when(ApiDBUtils.findNonPlaceHolderByIp4AddressAndNetworkId("10.1.1.2", 1L)).thenReturn(null);
+            when(ApiDBUtils.findByIp4AddressAndNetworkId("10.1.1.2", 1L)).thenReturn(placeholderNic);
+
+            apiResponseHelper.showVmInfoForSharedNetworks(false, ipAddress, ipResponse);
+
+            verify(ipResponse).setIsSystem(true);
+            verify(ipResponse, Mockito.never()).setVirtualMachineId(Mockito.anyString());
+        }
+    }
 }