From f2e7d6b90ea515293fc7a549f6f0e39ddabf1cf3 Mon Sep 17 00:00:00 2001 From: Marcus Sorensen Date: Tue, 13 Dec 2022 02:58:43 -0700 Subject: [PATCH] Allow ssvm agent certs to contain host IP for NAT situations (#6864) Co-authored-by: Marcus Sorensen --- .../org/apache/cloudstack/ca/CAManager.java | 5 ++ .../api/routing/NetworkElementCommand.java | 1 + .../cloud/vm/VirtualMachineManagerImpl.java | 7 ++ .../vm/VirtualMachineManagerImplTest.java | 69 ++++++++++++++++--- .../apache/cloudstack/ca/CAManagerImpl.java | 2 +- 5 files changed, 72 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/ca/CAManager.java b/api/src/main/java/org/apache/cloudstack/ca/CAManager.java index c32cfbfe345..dbabfa2a484 100644 --- a/api/src/main/java/org/apache/cloudstack/ca/CAManager.java +++ b/api/src/main/java/org/apache/cloudstack/ca/CAManager.java @@ -62,6 +62,11 @@ public interface CAManager extends CAService, Configurable, PluggableService { "true", "Enable automatic renewal and provisioning of certificate to agents as supported by the configured CA plugin.", true, ConfigKey.Scope.Cluster); + ConfigKey AllowHostIPInSysVMAgentCert = new ConfigKey<>("Advanced", Boolean.class, + "ca.framework.cert.systemvm.allow.host.ip", + "false", + "Allow hypervisor host's IP to be a part of a system VM's agent cert", true, ConfigKey.Scope.Zone); + ConfigKey CABackgroundJobDelay = new ConfigKey<>("Advanced", Long.class, "ca.framework.background.task.delay", "3600", diff --git a/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java b/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java index de3843e2b83..4055876f26d 100644 --- a/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java +++ b/core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java @@ -39,6 +39,7 @@ public abstract class NetworkElementCommand extends Command { public static final String VPC_PRIVATE_GATEWAY = "vpc.gateway.private"; public static final String FIREWALL_EGRESS_DEFAULT = "firewall.egress.default"; public static final String NETWORK_PUB_LAST_IP = "network.public.last.ip"; + public static final String HYPERVISOR_HOST_PRIVATE_IP = "hypervisor.private.ip"; private String routerAccessIp; diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index df3c7ea04e5..a66b6cb2530 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1004,6 +1004,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (org.apache.commons.lang3.StringUtils.isNotEmpty(csr)) { final Map ipAddressDetails = new HashMap<>(sshAccessDetails); ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME); + addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddressDetails, CAManager.AllowHostIPInSysVMAgentCert); final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()), new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null); final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails); @@ -1015,6 +1016,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } + protected void addHostIpToCertDetailsIfConfigAllows(Host vmHost, Map ipAddressDetails, ConfigKey configKey) { + if (configKey.valueIn(vmHost.getDataCenterId())) { + ipAddressDetails.put(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP, vmHost.getPrivateIpAddress()); + } + } + @Override public void orchestrateStart(final String vmUuid, final Map params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index cb3a6b77b88..87b5b5c0c6f 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -17,6 +17,7 @@ package com.cloud.vm; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; @@ -31,9 +32,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.StorageManager; +import com.cloud.host.Host; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; @@ -152,6 +156,49 @@ public class VirtualMachineManagerImplTest { virtualMachineManagerImpl.setStoragePoolAllocators(storagePoolAllocators); } + @Test + public void testaddHostIpToCertDetailsIfConfigAllows() { + Host vmHost = mock(Host.class); + ConfigKey testConfig = mock(ConfigKey.class); + + Long dataCenterId = 5L; + String hostIp = "1.1.1.1"; + String routerIp = "2.2.2.2"; + Map ipAddresses = new HashMap<>(); + ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); + + when(testConfig.valueIn(dataCenterId)).thenReturn(true); + when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); + + virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); + assertTrue(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); + assertEquals(hostIp, ipAddresses.get(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); + assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP)); + assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP)); + } + + @Test + public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { + Host vmHost = mock(Host.class); + ConfigKey testConfig = mock(ConfigKey.class); + + Long dataCenterId = 5L; + String hostIp = "1.1.1.1"; + String routerIp = "2.2.2.2"; + Map ipAddresses = new HashMap<>(); + ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); + + when(testConfig.valueIn(dataCenterId)).thenReturn(false); + when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); + + virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); + assertFalse(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); + assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP)); + assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP)); + } + @Test(expected = CloudRuntimeException.class) public void testScaleVM3() throws Exception { when(vmInstanceMock.getHostId()).thenReturn(null); @@ -341,7 +388,7 @@ public class VirtualMachineManagerImplTest { Map volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); assertFalse(volumeToPoolObjectMap.isEmpty()); - Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); + assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); Mockito.verify(userDefinedVolumeToStoragePoolMap, times(1)).keySet(); } @@ -360,8 +407,8 @@ public class VirtualMachineManagerImplTest { Mockito.doReturn(volumesOfVm).when(volumeDaoMock).findUsableVolumesForInstance(vmInstanceVoMockId); List volumesNotMapped = virtualMachineManagerImpl.findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToStoragePoolObjectMap); - Assert.assertEquals(1, volumesNotMapped.size()); - Assert.assertEquals(volumeVoMock2, volumesNotMapped.get(0)); + assertEquals(1, volumesNotMapped.size()); + assertEquals(volumeVoMock2, volumesNotMapped.get(0)); } @Test @@ -407,8 +454,8 @@ public class VirtualMachineManagerImplTest { List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); - Assert.assertEquals(1, poolList.size()); - Assert.assertEquals(storagePoolVoMock, poolList.get(0)); + assertEquals(1, poolList.size()); + assertEquals(storagePoolVoMock, poolList.get(0)); } @Test @@ -426,8 +473,8 @@ public class VirtualMachineManagerImplTest { Mockito.doReturn(true).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); - Assert.assertEquals(1, poolList.size()); - Assert.assertEquals(storagePoolVoMock, poolList.get(0)); + assertEquals(1, poolList.size()); + assertEquals(storagePoolVoMock, poolList.get(0)); } @Test @@ -525,7 +572,7 @@ public class VirtualMachineManagerImplTest { virtualMachineManagerImpl.createVolumeToStoragePoolMappingIfPossible(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, volumeVoMock, storagePoolVoMock); assertFalse(volumeToPoolObjectMap.isEmpty()); - Assert.assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock)); + assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock)); } @Test @@ -582,7 +629,7 @@ public class VirtualMachineManagerImplTest { virtualMachineManagerImpl.createStoragePoolMappingsForVolumes(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, allVolumes); assertFalse(volumeToPoolObjectMap.isEmpty()); - Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); + assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); Mockito.verify(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); Mockito.verify(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock); @@ -603,7 +650,7 @@ public class VirtualMachineManagerImplTest { Map mappingVolumeAndStoragePool = virtualMachineManagerImpl.createMappingVolumeAndStoragePool(virtualMachineProfileMock, hostMock, new HashMap<>()); - Assert.assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap); + assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap); InOrder inOrder = Mockito.inOrder(virtualMachineManagerImpl); inOrder.verify(virtualMachineManagerImpl).buildMapUsingUserInformation(Mockito.eq(virtualMachineProfileMock), Mockito.eq(hostMock), Mockito.anyMapOf(Long.class, Long.class)); @@ -670,7 +717,7 @@ public class VirtualMachineManagerImplTest { boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l); - Assert.assertEquals(expected, result); + assertEquals(expected, result); } @Test diff --git a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java index 2e7e756c49a..facad1add16 100644 --- a/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java @@ -432,6 +432,6 @@ public class CAManagerImpl extends ManagerBase implements CAManager { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, CABackgroundJobDelay, CertExpiryAlertPeriod}; + return new ConfigKey[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, AllowHostIPInSysVMAgentCert, CABackgroundJobDelay, CertExpiryAlertPeriod}; } }