Skip to content

Commit f2e7d6b

Browse files
mlsorensenMarcus Sorensen
mlsorensen
and
Marcus Sorensen
authored
Allow ssvm agent certs to contain host IP for NAT situations (apache#6864)
Co-authored-by: Marcus Sorensen <mls@apple.com>
1 parent 72b6ab9 commit f2e7d6b

5 files changed

Lines changed: 72 additions & 12 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/ca/CAManager.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ public interface CAManager extends CAService, Configurable, PluggableService {
6262
"true",
6363
"Enable automatic renewal and provisioning of certificate to agents as supported by the configured CA plugin.", true, ConfigKey.Scope.Cluster);
6464

65+
ConfigKey<Boolean> AllowHostIPInSysVMAgentCert = new ConfigKey<>("Advanced", Boolean.class,
66+
"ca.framework.cert.systemvm.allow.host.ip",
67+
"false",
68+
"Allow hypervisor host's IP to be a part of a system VM's agent cert", true, ConfigKey.Scope.Zone);
69+
6570
ConfigKey<Long> CABackgroundJobDelay = new ConfigKey<>("Advanced", Long.class,
6671
"ca.framework.background.task.delay",
6772
"3600",

‎core/src/main/java/com/cloud/agent/api/routing/NetworkElementCommand.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public abstract class NetworkElementCommand extends Command {
3939
public static final String VPC_PRIVATE_GATEWAY = "vpc.gateway.private";
4040
public static final String FIREWALL_EGRESS_DEFAULT = "firewall.egress.default";
4141
public static final String NETWORK_PUB_LAST_IP = "network.public.last.ip";
42+
public static final String HYPERVISOR_HOST_PRIVATE_IP = "hypervisor.private.ip";
4243

4344
private String routerAccessIp;
4445

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,7 @@ private void setupAgentSecurity(final Host vmHost, final Map<String, String> ssh
10041004
if (org.apache.commons.lang3.StringUtils.isNotEmpty(csr)) {
10051005
final Map<String, String> ipAddressDetails = new HashMap<>(sshAccessDetails);
10061006
ipAddressDetails.remove(NetworkElementCommand.ROUTER_NAME);
1007+
addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddressDetails, CAManager.AllowHostIPInSysVMAgentCert);
10071008
final Certificate certificate = caManager.issueCertificate(csr, Arrays.asList(vm.getHostName(), vm.getInstanceName()),
10081009
new ArrayList<>(ipAddressDetails.values()), CAManager.CertValidityPeriod.value(), null);
10091010
final boolean result = caManager.deployCertificate(vmHost, certificate, false, sshAccessDetails);
@@ -1015,6 +1016,12 @@ private void setupAgentSecurity(final Host vmHost, final Map<String, String> ssh
10151016
}
10161017
}
10171018

1019+
protected void addHostIpToCertDetailsIfConfigAllows(Host vmHost, Map<String, String> ipAddressDetails, ConfigKey<Boolean> configKey) {
1020+
if (configKey.valueIn(vmHost.getDataCenterId())) {
1021+
ipAddressDetails.put(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP, vmHost.getPrivateIpAddress());
1022+
}
1023+
}
1024+
10181025
@Override
10191026
public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner)
10201027
throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {

‎engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java‎

Lines changed: 58 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
package com.cloud.vm;
1919

20+
import static org.junit.Assert.assertEquals;
2021
import static org.junit.Assert.assertFalse;
2122
import static org.junit.Assert.assertTrue;
2223
import static org.mockito.Matchers.any;
@@ -31,9 +32,12 @@
3132
import java.util.List;
3233
import java.util.Map;
3334

35+
import com.cloud.agent.api.routing.NetworkElementCommand;
3436
import com.cloud.exception.InvalidParameterValueException;
3537
import com.cloud.storage.StorageManager;
38+
import com.cloud.host.Host;
3639
import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
40+
import org.apache.cloudstack.framework.config.ConfigKey;
3741
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
3842
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
3943
import org.junit.Assert;
@@ -152,6 +156,49 @@ public void setup() {
152156
virtualMachineManagerImpl.setStoragePoolAllocators(storagePoolAllocators);
153157
}
154158

159+
@Test
160+
public void testaddHostIpToCertDetailsIfConfigAllows() {
161+
Host vmHost = mock(Host.class);
162+
ConfigKey testConfig = mock(ConfigKey.class);
163+
164+
Long dataCenterId = 5L;
165+
String hostIp = "1.1.1.1";
166+
String routerIp = "2.2.2.2";
167+
Map<String, String> ipAddresses = new HashMap<>();
168+
ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp);
169+
170+
when(testConfig.valueIn(dataCenterId)).thenReturn(true);
171+
when(vmHost.getDataCenterId()).thenReturn(dataCenterId);
172+
when(vmHost.getPrivateIpAddress()).thenReturn(hostIp);
173+
174+
virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig);
175+
assertTrue(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
176+
assertEquals(hostIp, ipAddresses.get(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
177+
assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP));
178+
assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP));
179+
}
180+
181+
@Test
182+
public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() {
183+
Host vmHost = mock(Host.class);
184+
ConfigKey testConfig = mock(ConfigKey.class);
185+
186+
Long dataCenterId = 5L;
187+
String hostIp = "1.1.1.1";
188+
String routerIp = "2.2.2.2";
189+
Map<String, String> ipAddresses = new HashMap<>();
190+
ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp);
191+
192+
when(testConfig.valueIn(dataCenterId)).thenReturn(false);
193+
when(vmHost.getDataCenterId()).thenReturn(dataCenterId);
194+
when(vmHost.getPrivateIpAddress()).thenReturn(hostIp);
195+
196+
virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig);
197+
assertFalse(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP));
198+
assertTrue(ipAddresses.containsKey(NetworkElementCommand.ROUTER_IP));
199+
assertEquals(routerIp, ipAddresses.get(NetworkElementCommand.ROUTER_IP));
200+
}
201+
155202
@Test(expected = CloudRuntimeException.class)
156203
public void testScaleVM3() throws Exception {
157204
when(vmInstanceMock.getHostId()).thenReturn(null);
@@ -341,7 +388,7 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() {
341388
Map<Volume, StoragePool> volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap);
342389

343390
assertFalse(volumeToPoolObjectMap.isEmpty());
344-
Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
391+
assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
345392

346393
Mockito.verify(userDefinedVolumeToStoragePoolMap, times(1)).keySet();
347394
}
@@ -360,8 +407,8 @@ public void findVolumesThatWereNotMappedByTheUserTest() {
360407
Mockito.doReturn(volumesOfVm).when(volumeDaoMock).findUsableVolumesForInstance(vmInstanceVoMockId);
361408
List<Volume> volumesNotMapped = virtualMachineManagerImpl.findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToStoragePoolObjectMap);
362409

363-
Assert.assertEquals(1, volumesNotMapped.size());
364-
Assert.assertEquals(volumeVoMock2, volumesNotMapped.get(0));
410+
assertEquals(1, volumesNotMapped.size());
411+
assertEquals(volumeVoMock2, volumesNotMapped.get(0));
365412
}
366413

367414
@Test
@@ -407,8 +454,8 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestLocalVolume() {
407454

408455
List<StoragePool> poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock);
409456

410-
Assert.assertEquals(1, poolList.size());
411-
Assert.assertEquals(storagePoolVoMock, poolList.get(0));
457+
assertEquals(1, poolList.size());
458+
assertEquals(storagePoolVoMock, poolList.get(0));
412459
}
413460

414461
@Test
@@ -426,8 +473,8 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestCrossClusterMigratio
426473
Mockito.doReturn(true).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock);
427474
List<StoragePool> poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock);
428475

429-
Assert.assertEquals(1, poolList.size());
430-
Assert.assertEquals(storagePoolVoMock, poolList.get(0));
476+
assertEquals(1, poolList.size());
477+
assertEquals(storagePoolVoMock, poolList.get(0));
431478
}
432479

433480
@Test
@@ -525,7 +572,7 @@ public void createVolumeToStoragePoolMappingIfPossibleTestTargetHostDoesNotAcces
525572
virtualMachineManagerImpl.createVolumeToStoragePoolMappingIfPossible(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, volumeVoMock, storagePoolVoMock);
526573

527574
assertFalse(volumeToPoolObjectMap.isEmpty());
528-
Assert.assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock));
575+
assertEquals(storagePoolMockOther, volumeToPoolObjectMap.get(volumeVoMock));
529576
}
530577

531578
@Test
@@ -582,7 +629,7 @@ public void createStoragePoolMappingsForVolumesTestNotCrossCluterMigrationWithCl
582629
virtualMachineManagerImpl.createStoragePoolMappingsForVolumes(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, allVolumes);
583630

584631
assertFalse(volumeToPoolObjectMap.isEmpty());
585-
Assert.assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
632+
assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock));
586633

587634
Mockito.verify(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock);
588635
Mockito.verify(virtualMachineManagerImpl).isStorageCrossClusterMigration(hostMockId, storagePoolVoMock);
@@ -603,7 +650,7 @@ public void createMappingVolumeAndStoragePoolTest() {
603650

604651
Map<Volume, StoragePool> mappingVolumeAndStoragePool = virtualMachineManagerImpl.createMappingVolumeAndStoragePool(virtualMachineProfileMock, hostMock, new HashMap<>());
605652

606-
Assert.assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap);
653+
assertEquals(mappingVolumeAndStoragePool, volumeToPoolObjectMap);
607654

608655
InOrder inOrder = Mockito.inOrder(virtualMachineManagerImpl);
609656
inOrder.verify(virtualMachineManagerImpl).buildMapUsingUserInformation(Mockito.eq(virtualMachineProfileMock), Mockito.eq(hostMock), Mockito.anyMapOf(Long.class, Long.class));
@@ -670,7 +717,7 @@ private void prepareAndTestIsRootVolumeOnLocalStorage(ScopeType scope, boolean e
670717

671718
boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l);
672719

673-
Assert.assertEquals(expected, result);
720+
assertEquals(expected, result);
674721
}
675722

676723
@Test

‎server/src/main/java/org/apache/cloudstack/ca/CAManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,6 @@ public String getConfigComponentName() {
432432

433433
@Override
434434
public ConfigKey<?>[] getConfigKeys() {
435-
return new ConfigKey<?>[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, CABackgroundJobDelay, CertExpiryAlertPeriod};
435+
return new ConfigKey<?>[] {CAProviderPlugin, CertKeySize, CertSignatureAlgorithm, CertValidityPeriod, AutomaticCertRenewal, AllowHostIPInSysVMAgentCert, CABackgroundJobDelay, CertExpiryAlertPeriod};
436436
}
437437
}

0 commit comments

Comments
 (0)