You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2022/12/13 09:58:51 UTC

[cloudstack] branch main updated: Allow ssvm agent certs to contain host IP for NAT situations (#6864)

This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/main by this push:
     new f2e7d6b90ea Allow ssvm agent certs to contain host IP for NAT situations (#6864)
f2e7d6b90ea is described below

commit f2e7d6b90ea515293fc7a549f6f0e39ddabf1cf3
Author: Marcus Sorensen <sh...@gmail.com>
AuthorDate: Tue Dec 13 02:58:43 2022 -0700

    Allow ssvm agent certs to contain host IP for NAT situations (#6864)
    
    Co-authored-by: Marcus Sorensen <ml...@apple.com>
---
 .../java/org/apache/cloudstack/ca/CAManager.java   |  5 ++
 .../agent/api/routing/NetworkElementCommand.java   |  1 +
 .../com/cloud/vm/VirtualMachineManagerImpl.java    |  7 +++
 .../cloud/vm/VirtualMachineManagerImplTest.java    | 69 ++++++++++++++++++----
 .../org/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<Boolean> 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<Long> 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<String, String> 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<String, String> ipAddressDetails, ConfigKey<Boolean> configKey) {
+        if (configKey.valueIn(vmHost.getDataCenterId())) {
+            ipAddressDetails.put(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP, vmHost.getPrivateIpAddress());
+        }
+    }
+
     @Override
     public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> 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<String, String> 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<String, String> 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<Volume, StoragePool> 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<Volume> 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<StoragePool> 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<StoragePool> 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<Volume, StoragePool> 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};
     }
 }