Don't allow adding of existing host to another zone, pod or cluster

server/src/main/java/com/cloud/resource/ResourceManagerImpl.java

@@ -19,9 +19,11 @@
 import static com.cloud.configuration.ConfigurationManagerImpl.MIGRATE_VM_ACROSS_CLUSTERS;
 import static com.cloud.configuration.ConfigurationManagerImpl.SET_HOST_DOWN_TO_MAINTENANCE;
 
+import java.net.InetAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLDecoder;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -31,6 +33,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Random;
 import java.util.stream.Collectors;
 
@@ -798,6 +801,8 @@ private List<HostVO> discoverHostsFull(final Long dcId, final Long podId, Long c
             throw new InvalidParameterValueException(url + " is not a valid uri");
         }
 
+        checkForDuplicateHost(url);
+
         final List<HostVO> hosts = new ArrayList<>();
         logger.info("Trying to add a new host at {} in data center {}", url, zone);
         boolean isHypervisorTypeSupported = false;
@@ -2281,6 +2286,29 @@ private HostVO getNewHost(StartupCommand[] startupCommands) {
         return null;
     }
 
+    protected void validateExistingHostLocationImmutable(final HostVO host, final boolean newHost,
+            final long dcId, final Long podId, final Long clusterId, final StartupCommand startup) {
+        if (newHost || host == null || host.getType() != Host.Type.Routing) {
+            return;
+        }
+        final long existingDcId = host.getDataCenterId();
+        final Long existingPodId = host.getPodId();
+        final Long existingClusterId = host.getClusterId();
+        if (existingPodId == null || existingClusterId == null) {
+            return;
+        }
+        if (existingDcId == dcId && Objects.equals(existingPodId, podId) && Objects.equals(existingClusterId, clusterId)) {
+            return;
+        }
+        final String identity = host.getUuid() != null ? host.getUuid() : host.getGuid();
+        final String ip = startup != null ? startup.getPrivateIpAddress() : "unknown";
+        throw new InvalidParameterValueException(
+                String.format("Host %s (ip: %s) is already registered in [zone: %s, pod: %s, cluster: %s] and cannot " +
+                                "be re-added or reconnected with [zone: %s, pod: %s, cluster: %s]. Zone, pod and " +
+                                "cluster of an existing host are immutable.",
+                        identity, ip, existingDcId, existingPodId, existingClusterId, dcId, podId, clusterId));
+    }
+
     protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource resource, final Map<String, String> details, List<String> hostTags,
             final ResourceStateAdapter.Event stateEvent) {
         boolean newHost = false;
@@ -2356,6 +2384,8 @@ protected HostVO createHostVO(final StartupCommand[] cmds, final ServerResource
             }
         }
 
+        validateExistingHostLocationImmutable(host, newHost, dcId, podId, clusterId, startup);
+
         host.setDataCenterId(dc.getId());
         host.setPodId(podId);
         host.setClusterId(clusterId);
@@ -2585,6 +2615,31 @@ private Host createHostAndAgent(final ServerResource resource, final Map<String,
         return host;
     }
 
+    void checkForDuplicateHost(final String url) {
+        String hostIpOrName = null;
+        String ipAddress = null;
+        try {
+            hostIpOrName = new URI(UriUtils.encodeURIComponent(url)).getHost();
+            if (StringUtils.isBlank(hostIpOrName)) {
+                return;
+            }
+            InetAddress ip = InetAddress.getByName(hostIpOrName);
+            ipAddress = ip.getHostAddress();
+        } catch (final URISyntaxException | UnknownHostException ignore) {
+            // unparseable URL or unknown host - discoverer will reject it shortly anyway
+            return;
+        }
+
+        if (StringUtils.isNotBlank(ipAddress)) {
+            final HostVO existingByIp = _hostDao.findByIp(ipAddress);
+            if (existingByIp != null) {
+                throw new InvalidParameterValueException(String.format(
+                        "A host with IP address '%s' (%s) already exists (id: %s). Remove it before adding again.",
+                        ipAddress, hostIpOrName, existingByIp));
+            }
+        }
+    }
+
     private Host createHostAndAgentDeferred(final ServerResource resource, final Map<String, String> details, final boolean old, final List<String> hostTags, final boolean forRebalance) {
         HostVO host = null;
         StartupCommand[] cmds = null;

server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java

@@ -18,6 +18,7 @@
 package com.cloud.resource;
 
 import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.StartupCommand;
 import com.cloud.agent.api.GetVncPortAnswer;
 import com.cloud.agent.api.GetVncPortCommand;
 import com.cloud.capacity.dao.CapacityDao;
@@ -218,6 +219,19 @@ public void tearDown() throws Exception {
         closeable.close();
     }
 
+    @Test(expected = InvalidParameterValueException.class)
+    public void testCheckForDuplicateHostThrowsWhenIpAlreadyExists() {
+        when(hostDao.findByIp("10.0.0.10")).thenReturn(host);
+        resourceManager.checkForDuplicateHost("http://10.0.0.10");
+    }
+
+    @Test
+    public void testCheckForDuplicateHostAllowsUniqueHost() {
+        when(hostDao.findByIp("10.0.0.30")).thenReturn(null);
+        resourceManager.checkForDuplicateHost("http://10.0.0.30");
+        verify(hostDao, times(1)).findByIp("10.0.0.30");
+    }
+
     @Test
     public void testCheckAndMaintainEnterMaintenanceModeNoVms() throws NoTransitionException {
         // Test entering into maintenance with no VMs running on host.
@@ -335,6 +349,74 @@ public void testGetHostCredentialsMissingParameter() {
         resourceManager.getHostCredentials(host);
     }
 
+    private HostVO mockExistingRoutingHost(long dcId, Long podId, Long clusterId) {
+        HostVO existing = Mockito.mock(HostVO.class);
+        when(existing.getType()).thenReturn(Host.Type.Routing);
+        when(existing.getDataCenterId()).thenReturn(dcId);
+        when(existing.getPodId()).thenReturn(podId);
+        when(existing.getClusterId()).thenReturn(clusterId);
+        when(existing.getUuid()).thenReturn("host-uuid");
+        return existing;
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testValidateExistingHostLocationImmutableRejectsZoneChange() {
+        HostVO existing = mockExistingRoutingHost(1L, 10L, 100L);
+        StartupCommand startup = Mockito.mock(StartupCommand.class);
+        when(startup.getPrivateIpAddress()).thenReturn("10.10.10.10");
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 2L, 10L, 100L, startup);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testValidateExistingHostLocationImmutableRejectsPodChange() {
+        HostVO existing = mockExistingRoutingHost(1L, 10L, 100L);
+        StartupCommand startup = Mockito.mock(StartupCommand.class);
+        when(startup.getPrivateIpAddress()).thenReturn("10.10.10.10");
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 1L, 11L, 100L, startup);
+    }
+
+    @Test(expected = InvalidParameterValueException.class)
+    public void testValidateExistingHostLocationImmutableRejectsClusterChange() {
+        HostVO existing = mockExistingRoutingHost(1L, 10L, 100L);
+        StartupCommand startup = Mockito.mock(StartupCommand.class);
+        when(startup.getPrivateIpAddress()).thenReturn("10.10.10.10");
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 1L, 10L, 101L, startup);
+    }
+
+    @Test
+    public void testValidateExistingHostLocationImmutableAllowsSameTupleReconnect() {
+        HostVO existing = mockExistingRoutingHost(1L, 10L, 100L);
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 1L, 10L, 100L, null);
+    }
+
+    @Test
+    public void testValidateExistingHostLocationImmutableAllowsNewHost() {
+        HostVO existing = mockExistingRoutingHost(2L, 20L, 200L);
+        resourceManager.validateExistingHostLocationImmutable(existing, true, 1L, 10L, 100L, null);
+    }
+
+    @Test
+    public void testValidateExistingHostLocationImmutableSkipsNonRoutingHost() {
+        HostVO existing = Mockito.mock(HostVO.class);
+        when(existing.getType()).thenReturn(Host.Type.SecondaryStorageVM);
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 1L, 10L, 100L, null);
+    }
+
+    @Test
+    public void testValidateExistingHostLocationImmutableSkipsPartialLocationRow() {
+        HostVO existing = Mockito.mock(HostVO.class);
+        when(existing.getType()).thenReturn(Host.Type.Routing);
+        when(existing.getDataCenterId()).thenReturn(1L);
+        when(existing.getPodId()).thenReturn(null);
+        when(existing.getClusterId()).thenReturn(null);
+        resourceManager.validateExistingHostLocationImmutable(existing, false, 2L, 10L, 100L, null);
+    }
+
+    @Test
+    public void testValidateExistingHostLocationImmutableSkipsNullExistingHost() {
+        resourceManager.validateExistingHostLocationImmutable(null, false, 2L, 10L, 100L, null);
+    }
+
     @Test
     public void testGetHostCredentials() {
         Ternary<String, String, String> credentials = resourceManager.getHostCredentials(host);