Skip to content

Commit 3bedcc9

Browse files
vishesh92vishesh92
committed
Enforce strict host tag checking
1 parent d5241d3 commit 3bedcc9

File tree

8 files changed

+121
-32
lines changed

8 files changed

+121
-32
lines changed

‎engine/schema/src/main/java/com/cloud/host/HostVO.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
// under the License.
1717
package com.cloud.host;
1818

19-
import java.util.ArrayList;
2019
import java.util.Arrays;
2120
import java.util.Date;
2221
import java.util.HashMap;
2322
import java.util.HashSet;
2423
import java.util.List;
2524
import java.util.Map;
25+
import java.util.Set;
2626
import java.util.UUID;
2727

2828
import javax.persistence.Column;
@@ -768,24 +768,25 @@ public void setUuid(String uuid) {
768768
this.uuid = uuid;
769769
}
770770

771-
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template) {
771+
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
772772
if (serviceOffering == null || template == null) {
773773
return false;
774774
}
775775
if (StringUtils.isEmpty(serviceOffering.getHostTag()) && StringUtils.isEmpty(template.getTemplateTag())) {
776776
return true;
777777
}
778-
if (getHostTags() == null) {
779-
return false;
780-
}
781778
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
782-
List<String> tags = new ArrayList<>();
779+
HashSet<String> tags = new HashSet<>();
783780
if (StringUtils.isNotEmpty(serviceOffering.getHostTag())) {
784781
tags.addAll(Arrays.asList(serviceOffering.getHostTag().split(",")));
785782
}
786-
if (StringUtils.isNotEmpty(template.getTemplateTag()) && !tags.contains(template.getTemplateTag())) {
783+
if (StringUtils.isNotEmpty(template.getTemplateTag())) {
787784
tags.add(template.getTemplateTag());
788785
}
786+
tags.removeIf(tag -> !strictHostTags.contains(tag));
787+
if (tags.isEmpty()) {
788+
return true;
789+
}
789790
return hostTagsSet.containsAll(tags);
790791
}
791792

‎engine/schema/src/test/java/com/cloud/host/HostVOTest.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
import com.cloud.service.ServiceOfferingVO;
2121
import com.cloud.template.VirtualMachineTemplate;
2222
import com.cloud.vm.VirtualMachine;
23+
import org.junit.Before;
24+
import org.junit.Test;
25+
import org.mockito.Mockito;
26+
2327
import java.util.Arrays;
28+
import java.util.Collections;
2429
import java.util.List;
30+
import java.util.Set;
2531

2632
import static org.junit.Assert.assertFalse;
2733
import static org.junit.Assert.assertTrue;
28-
import org.junit.Test;
29-
import org.junit.Before;
30-
import org.mockito.Mockito;
3134

3235
public class HostVOTest {
3336
HostVO host;
@@ -37,7 +40,7 @@ public class HostVOTest {
3740
public void setUp() throws Exception {
3841
host = new HostVO();
3942
offering = new ServiceOfferingVO("TestSO", 0, 0, 0, 0, 0,
40-
false, "TestSO", false,VirtualMachine.Type.User,false);
43+
false, "TestSO", false, VirtualMachine.Type.User, false);
4144
}
4245

4346
@Test
@@ -52,14 +55,14 @@ public void testNoTag() {
5255

5356
@Test
5457
public void testRightTag() {
55-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
58+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
5659
offering.setHostTag("tag2,tag1");
5760
assertTrue(host.checkHostServiceOfferingTags(offering));
5861
}
5962

6063
@Test
6164
public void testWrongTag() {
62-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
65+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
6366
offering.setHostTag("tag2,tag4");
6467
assertFalse(host.checkHostServiceOfferingTags(offering));
6568
}
@@ -87,40 +90,42 @@ public void checkHostServiceOfferingTagsTestRuleTagWithNullServiceTag() {
8790

8891
@Test
8992
public void testEitherNoSOOrTemplate() {
90-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class)));
91-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null));
93+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class), null));
94+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null, null));
9295
}
9396

9497
@Test
9598
public void testNoTagOfferingTemplate() {
96-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
99+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()));
100+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")));
97101
}
98102

99103
@Test
100104
public void testRightTagOfferingTemplate() {
101105
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
102106
offering.setHostTag("tag2,tag1");
103-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
107+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1")));
104108
host.setHostTags(Arrays.asList("tag1", "tag2", "tag3"), false);
105109
offering.setHostTag("tag2,tag1");
106110
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
107111
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
108-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
112+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag3")));
109113
host.setHostTags(List.of("tag3"), false);
110114
offering.setHostTag(null);
111-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
115+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag3")));
116+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag1")));
112117
}
113118

114119
@Test
115120
public void testWrongOfferingTag() {
116-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
121+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
117122
offering.setHostTag("tag2,tag4");
118123
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
119124
Mockito.when(template.getTemplateTag()).thenReturn("tag1");
120-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
125+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
121126
offering.setHostTag("tag1,tag2");
122127
template = Mockito.mock(VirtualMachineTemplate.class);
123128
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
124-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
129+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
125130
}
126131
}

‎server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import javax.inject.Inject;
3636
import javax.naming.ConfigurationException;
3737

38+
import com.cloud.vm.UserVmManager;
3839
import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO;
3940
import com.cloud.storage.VMTemplateVO;
4041
import com.cloud.storage.dao.VMTemplateDao;
@@ -778,7 +779,7 @@ protected boolean checkVmProfileAndHost(final VirtualMachineProfile vmProfile, f
778779
VirtualMachineTemplate template = vmProfile.getTemplate();
779780
if (offering.getHostTag() != null || template.getTemplateTag() != null) {
780781
_hostDao.loadHostTags(host);
781-
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template)) {
782+
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template, UserVmManager.getStrictHostTags())) {
782783
logger.debug("Service Offering host tag or template tag does not match the last host of this VM");
783784
return false;
784785
}

‎server/src/main/java/com/cloud/vm/UserVmManager.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
package com.cloud.vm;
1818

1919
import java.util.HashMap;
20+
import java.util.HashSet;
2021
import java.util.List;
2122
import java.util.Map;
23+
import java.util.Set;
2224

2325
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
2426
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -38,6 +40,8 @@
3840
import com.cloud.uservm.UserVm;
3941
import com.cloud.utils.Pair;
4042

43+
import static com.cloud.user.ResourceLimitService.ResourceLimitHostTags;
44+
4145
/**
4246
*
4347
*
@@ -59,6 +63,12 @@ public interface UserVmManager extends UserVmService {
5963
"Destroys the VM's root volume when the VM is destroyed.",
6064
true, ConfigKey.Scope.Domain);
6165

66+
ConfigKey<Boolean> EnforceStrictResourceLimitHostTagCheck = new ConfigKey<Boolean>(
67+
"Advanced", Boolean.class, "vm.strict.resource.limit.host.tag.check", "true",
68+
"Determines whether the resource limits tags are considered strict or not", true);
69+
ConfigKey<String> StrictHostTags = new ConfigKey<>("Advanced", String.class, "vm.strict.host.tags", "",
70+
"A comma-separated list of tags for strict host check", true);
71+
6272
static final int MAX_USER_DATA_LENGTH_BYTES = 2048;
6373

6474
public static final String CKS_NODE = "cksnode";
@@ -127,6 +137,14 @@ UserVm updateVirtualMachine(long id, String displayName, String group, Boolean h
127137

128138
void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType);
129139

140+
static Set<String> getStrictHostTags() {
141+
Set<String> strictHostTags = new HashSet<>(List.of(StrictHostTags.value().split(",")));
142+
if (EnforceStrictResourceLimitHostTagCheck.value()) {
143+
strictHostTags.addAll(List.of(ResourceLimitHostTags.value().split(",")));
144+
}
145+
return strictHostTags;
146+
}
147+
130148
void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);
131149

132150
boolean checkIfDynamicScalingCanBeEnabled(VirtualMachine vm, ServiceOffering offering, VirtualMachineTemplate template, Long zoneId);

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,9 +1964,9 @@ public boolean upgradeVirtualMachine(Long vmId, Long newServiceOfferingId, Map<S
19641964
HostVO instanceHost = _hostDao.findById(vmInstance.getHostId());
19651965
_hostDao.loadHostTags(instanceHost);
19661966

1967-
if (!instanceHost.checkHostServiceOfferingAndTemplateTags(newServiceOfferingVO, template)) {
1968-
logger.error(String.format("Cannot upgrade VM [%s] as the new service offering [%s] does not have the required host tags %s.", vmInstance, newServiceOfferingVO,
1969-
instanceHost.getHostTags()));
1967+
if (!instanceHost.checkHostServiceOfferingAndTemplateTags(newServiceOfferingVO, template, UserVmManager.getStrictHostTags())) {
1968+
logger.error(String.format("Cannot upgrade VM [%s] as the new service offering [%s] does not have the required host tags %s.",
1969+
vmInstance, newServiceOfferingVO, instanceHost.getHostTags()));
19701970
return false;
19711971
}
19721972
}
@@ -5484,11 +5484,14 @@ public Pair<UserVmVO, Map<VirtualMachineProfile.Param, Object>> startVirtualMach
54845484
boolean isRootAdmin = _accountService.isRootAdmin(callerAccount.getId());
54855485
Pod destinationPod = getDestinationPod(podId, isRootAdmin);
54865486
Cluster destinationCluster = getDestinationCluster(clusterId, isRootAdmin);
5487-
Host destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost);
5487+
HostVO destinationHost = getDestinationHost(hostId, isRootAdmin, isExplicitHost);
54885488
DataCenterDeployment plan = null;
54895489
boolean deployOnGivenHost = false;
54905490
if (destinationHost != null) {
54915491
logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM");
5492+
_hostDao.loadHostTags(destinationHost);
5493+
checkEnforceStrictHostTagCheck(vm, destinationHost);
5494+
54925495
final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
54935496
Pair<Boolean, Boolean> cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false);
54945497
if (!cpuCapabilityAndCapacity.first() || !cpuCapabilityAndCapacity.second()) {
@@ -5659,8 +5662,8 @@ private Cluster getDestinationCluster(Long clusterId, boolean isRootAdmin) {
56595662
return destinationCluster;
56605663
}
56615664

5662-
private Host getDestinationHost(Long hostId, boolean isRootAdmin, boolean isExplicitHost) {
5663-
Host destinationHost = null;
5665+
private HostVO getDestinationHost(Long hostId, boolean isRootAdmin, boolean isExplicitHost) {
5666+
HostVO destinationHost = null;
56645667
if (hostId != null) {
56655668
if (isExplicitHost && !isRootAdmin) {
56665669
throw new PermissionDeniedException(
@@ -6702,6 +6705,21 @@ private DeployDestination chooseVmMigrationDestination(VMInstanceVO vm, Host src
67026705
}
67036706
}
67046707

6708+
protected void checkEnforceStrictHostTagCheck(VMInstanceVO vm, HostVO host) {
6709+
ServiceOffering serviceOffering = serviceOfferingDao.findByIdIncludingRemoved(vm.getServiceOfferingId());
6710+
VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId());
6711+
6712+
Set<String> strictHostTags = UserVmManager.getStrictHostTags();
6713+
if (!host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, strictHostTags)) {
6714+
s_logger.error(String.format(
6715+
"Cannot deploy VM: %s to host : %s due to tag mismatch." +
6716+
" strictHosts: %s serviceOffering tags: %s, template tags: %s",
6717+
vm, host, strictHostTags, serviceOffering.getHostTag(), template.getTemplateTag()));
6718+
throw new InvalidParameterValueException(String.format("Cannot deploy VM, destination host: %s " +
6719+
"is not compatible for the VM", host.getName()));
6720+
}
6721+
}
6722+
67056723
private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcHost, Host destinationHost) throws VirtualMachineMigrationException {
67066724
if (destinationHost == null) {
67076725
return null;
@@ -6727,6 +6745,10 @@ private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcH
67276745
throw new CloudRuntimeException("Cannot migrate VM, VM is DPDK enabled VM but destination host is not DPDK enabled");
67286746
}
67296747

6748+
HostVO destinationHostVO = _hostDao.findById(destinationHost.getId());
6749+
_hostDao.loadHostTags(destinationHostVO);
6750+
checkEnforceStrictHostTagCheck(vm, destinationHostVO);
6751+
67306752
checkHostsDedication(vm, srcHost.getId(), destinationHost.getId());
67316753

67326754
// call to core process
@@ -6736,7 +6758,6 @@ private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcH
67366758
DeployDestination dest = new DeployDestination(dcVO, pod, cluster, destinationHost);
67376759

67386760
// check max guest vm limit for the destinationHost
6739-
HostVO destinationHostVO = _hostDao.findById(destinationHost.getId());
67406761
if (_capacityMgr.checkIfHostReachMaxGuestLimit(destinationHostVO)) {
67416762
if (logger.isDebugEnabled()) {
67426763
logger.debug("Host name: " + destinationHost.getName() + ", hostId: " + destinationHost.getId()
@@ -8395,7 +8416,8 @@ public String getConfigComponentName() {
83958416
public ConfigKey<?>[] getConfigKeys() {
83968417
return new ConfigKey<?>[] {EnableDynamicallyScaleVm, AllowDiskOfferingChangeDuringScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax,
83978418
VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
8398-
KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction};
8419+
KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction,
8420+
EnforceStrictResourceLimitHostTagCheck, StrictHostTags};
83998421
}
84008422

84018423
@Override

‎server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ private boolean hostSupportsServiceOfferingAndTemplate(HostVO host, ServiceOffer
447447
return true;
448448
}
449449
hostDao.loadHostTags(host);
450-
return host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template);
450+
return host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, UserVmManager.getStrictHostTags());
451451
}
452452

453453
private boolean storagePoolSupportsDiskOffering(StoragePool pool, DiskOffering diskOffering) {

‎server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1563,4 +1563,45 @@ public void testCheckVolumesLimits() {
15631563
Assert.fail(e.getMessage());
15641564
}
15651565
}
1566+
1567+
@Test
1568+
public void testCheckEnforceStrictHostTagCheckPass() {
1569+
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
1570+
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
1571+
1572+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
1573+
HostVO destinationHostVO = Mockito.mock(HostVO.class);
1574+
1575+
Mockito.when(_serviceOfferingDao.findByIdIncludingRemoved(1L)).thenReturn(serviceOffering);
1576+
Mockito.when(templateDao.findByIdIncludingRemoved(2L)).thenReturn(template);
1577+
1578+
Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
1579+
Mockito.when(vm.getTemplateId()).thenReturn(2L);
1580+
1581+
Mockito.when(destinationHostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet())).thenReturn(true);
1582+
1583+
userVmManagerImpl.checkEnforceStrictHostTagCheck(vm, destinationHostVO);
1584+
1585+
Mockito.verify(
1586+
destinationHostVO, Mockito.times(1)
1587+
).checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet());
1588+
}
1589+
1590+
@Test(expected = InvalidParameterValueException.class)
1591+
public void testCheckEnforceStrictHostTagCheckFail() {
1592+
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
1593+
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
1594+
1595+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
1596+
HostVO destinationHostVO = Mockito.mock(HostVO.class);
1597+
1598+
Mockito.when(_serviceOfferingDao.findByIdIncludingRemoved(1L)).thenReturn(serviceOffering);
1599+
Mockito.when(templateDao.findByIdIncludingRemoved(2L)).thenReturn(template);
1600+
1601+
Mockito.when(vm.getServiceOfferingId()).thenReturn(1L);
1602+
Mockito.when(vm.getTemplateId()).thenReturn(2L);
1603+
1604+
Mockito.when(destinationHostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet())).thenReturn(false);
1605+
userVmManagerImpl.checkEnforceStrictHostTagCheck(vm, destinationHostVO);
1606+
}
15661607
}

‎server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,7 @@ public void setUp() throws Exception {
302302
HostVO hostVO = Mockito.mock(HostVO.class);
303303
when(hostVO.isInMaintenanceStates()).thenReturn(false);
304304
hosts.add(hostVO);
305+
when(hostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(true);
305306
when(resourceManager.listHostsInClusterByStatus(anyLong(), any(Status.class))).thenReturn(hosts);
306307
when(resourceManager.listAllUpAndEnabledHostsInOneZoneByHypervisor(any(Hypervisor.HypervisorType.class), anyLong())).thenReturn(hosts);
307308
List<VMTemplateStoragePoolVO> templates = new ArrayList<>();

0 commit comments

Comments
 (0)