Skip to content

Commit 10064a7

Browse files
vishesh92dhslove
vishesh92
authored and
dhslove
committed
server: Enforce strict host tag check (apache#9017)
Documentation PR: apache/cloudstack-documentation#398 Currently, an administrator can break host tag compatibility for a VM administrator by certain operations: * deploy/start VM on a specific host * migrate VM * restore VM * scale VM This PR allows the user to specify tags which must be checked during these operations. Global Settings 1. `vm.strict.host.tags` - A comma-separated list of tags for strict host check (Default - empty) 2. `vm.strict.resource.limit.host.tag.check` - Determines whether the resource limits tags are considered strict or not (Default - true) During the above operations, we now check and throw an error if host tags compatibility is being broken for tags specified in `vm.strict.host.tags`. If `vm.strict.resource.limit.host.tag.check` is set to `true`, tags set in `resource.limit.host.tags` are also checked during these operations.
1 parent a5c0f5d commit 10064a7

File tree

11 files changed

+766
-45
lines changed

11 files changed

+766
-45
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ jobs:
134134
smoke/test_usage
135135
smoke/test_usage_events
136136
smoke/test_vm_deployment_planner
137+
smoke/test_vm_strict_host_tags
137138
smoke/test_vm_schedule
138139
smoke/test_vm_life_cycle
139140
smoke/test_vm_lifecycle_unmanage_import

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

Lines changed: 34 additions & 12 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;
@@ -45,6 +45,7 @@
4545
import org.apache.cloudstack.util.HypervisorTypeConverter;
4646
import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper;
4747
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
48+
import org.apache.commons.collections.CollectionUtils;
4849
import org.apache.commons.lang.BooleanUtils;
4950
import org.apache.commons.lang3.StringUtils;
5051

@@ -768,27 +769,48 @@ public void setUuid(String uuid) {
768769
this.uuid = uuid;
769770
}
770771

771-
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template) {
772-
if (serviceOffering == null || template == null) {
773-
return false;
774-
}
772+
private Set<String> getHostServiceOfferingAndTemplateStrictTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
775773
if (StringUtils.isEmpty(serviceOffering.getHostTag()) && StringUtils.isEmpty(template.getTemplateTag())) {
776-
return true;
774+
return new HashSet<>();
777775
}
778-
if (getHostTags() == null) {
779-
return false;
780-
}
781-
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
782-
List<String> tags = new ArrayList<>();
776+
List<String> hostTagsList = getHostTags();
777+
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
778+
HashSet<String> tags = new HashSet<>();
783779
if (StringUtils.isNotEmpty(serviceOffering.getHostTag())) {
784780
tags.addAll(Arrays.asList(serviceOffering.getHostTag().split(",")));
785781
}
786-
if (StringUtils.isNotEmpty(template.getTemplateTag()) && !tags.contains(template.getTemplateTag())) {
782+
if (StringUtils.isNotEmpty(template.getTemplateTag())) {
787783
tags.add(template.getTemplateTag());
788784
}
785+
tags.removeIf(tag -> !strictHostTags.contains(tag));
786+
tags.removeAll(hostTagsSet);
787+
return tags;
788+
}
789+
790+
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
791+
if (serviceOffering == null || template == null) {
792+
return false;
793+
}
794+
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
795+
if (tags.isEmpty()) {
796+
return true;
797+
}
798+
List<String> hostTagsList = getHostTags();
799+
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
789800
return hostTagsSet.containsAll(tags);
790801
}
791802

803+
public Set<String> getHostServiceOfferingAndTemplateMissingTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
804+
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
805+
if (tags.isEmpty()) {
806+
return new HashSet<>();
807+
}
808+
List<String> hostTagsList = getHostTags();
809+
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
810+
tags.removeAll(hostTagsSet);
811+
return tags;
812+
}
813+
792814
public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering) {
793815
if (serviceOffering == null) {
794816
return false;

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

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,18 @@
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

32+
import static org.junit.Assert.assertEquals;
2633
import static org.junit.Assert.assertFalse;
2734
import static org.junit.Assert.assertTrue;
28-
import org.junit.Test;
29-
import org.junit.Before;
30-
import org.mockito.Mockito;
3135

3236
public class HostVOTest {
3337
HostVO host;
@@ -37,7 +41,7 @@ public class HostVOTest {
3741
public void setUp() throws Exception {
3842
host = new HostVO();
3943
offering = new ServiceOfferingVO("TestSO", 0, 0, 0, 0, 0,
40-
false, "TestSO", false,VirtualMachine.Type.User,false);
44+
false, "TestSO", false, VirtualMachine.Type.User, false);
4145
}
4246

4347
@Test
@@ -52,14 +56,14 @@ public void testNoTag() {
5256

5357
@Test
5458
public void testRightTag() {
55-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
59+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
5660
offering.setHostTag("tag2,tag1");
5761
assertTrue(host.checkHostServiceOfferingTags(offering));
5862
}
5963

6064
@Test
6165
public void testWrongTag() {
62-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
66+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
6367
offering.setHostTag("tag2,tag4");
6468
assertFalse(host.checkHostServiceOfferingTags(offering));
6569
}
@@ -87,40 +91,59 @@ public void checkHostServiceOfferingTagsTestRuleTagWithNullServiceTag() {
8791

8892
@Test
8993
public void testEitherNoSOOrTemplate() {
90-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class)));
91-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null));
94+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class), null));
95+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null, null));
9296
}
9397

9498
@Test
9599
public void testNoTagOfferingTemplate() {
96-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
100+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()));
101+
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()).isEmpty());
102+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")));
103+
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")).isEmpty());
97104
}
98105

99106
@Test
100107
public void testRightTagOfferingTemplate() {
101108
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
102109
offering.setHostTag("tag2,tag1");
103-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
110+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1")));
111+
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1"));
112+
assertTrue(actualMissingTags.isEmpty());
113+
104114
host.setHostTags(Arrays.asList("tag1", "tag2", "tag3"), false);
105115
offering.setHostTag("tag2,tag1");
106116
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
107117
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
108-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
118+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag3")));
119+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag3"));
120+
assertTrue(actualMissingTags.isEmpty());
109121
host.setHostTags(List.of("tag3"), false);
110122
offering.setHostTag(null);
111-
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
123+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag3")));
124+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag3"));
125+
assertTrue(actualMissingTags.isEmpty());
126+
127+
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag1")));
128+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag1"));
129+
assertTrue(actualMissingTags.isEmpty());
112130
}
113131

114132
@Test
115133
public void testWrongOfferingTag() {
116-
host.setHostTags(Arrays.asList("tag1","tag2"), false);
134+
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
117135
offering.setHostTag("tag2,tag4");
118136
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
119137
Mockito.when(template.getTemplateTag()).thenReturn("tag1");
120-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
138+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
139+
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
140+
assertEquals(Set.of("tag4"), actualMissingTags);
141+
121142
offering.setHostTag("tag1,tag2");
122143
template = Mockito.mock(VirtualMachineTemplate.class);
123144
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
124-
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
145+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
146+
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
147+
assertEquals(Set.of("tag3"), actualMissingTags);
125148
}
126149
}

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

Lines changed: 3 additions & 2 deletions
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;
@@ -282,7 +283,7 @@ protected void avoidOtherClustersForDeploymentIfMigrationDisabled(VirtualMachine
282283
boolean storageMigrationNeededDuringClusterMigration = false;
283284
for (Volume volume : volumes) {
284285
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
285-
if (List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
286+
if (pool != null && List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
286287
storageMigrationNeededDuringClusterMigration = true;
287288
break;
288289
}
@@ -777,7 +778,7 @@ protected boolean checkVmProfileAndHost(final VirtualMachineProfile vmProfile, f
777778
VirtualMachineTemplate template = vmProfile.getTemplate();
778779
if (offering.getHostTag() != null || template.getTemplateTag() != null) {
779780
_hostDao.loadHostTags(host);
780-
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template)) {
781+
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template, UserVmManager.getStrictHostTags())) {
781782
logger.debug("Service Offering host tag or template tag does not match the last host of this VM");
782783
return false;
783784
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
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

25+
import com.cloud.utils.StringUtils;
2326
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
2427
import org.apache.cloudstack.framework.config.ConfigKey;
2528

@@ -38,6 +41,8 @@
3841
import com.cloud.uservm.UserVm;
3942
import com.cloud.utils.Pair;
4043

44+
import static com.cloud.user.ResourceLimitService.ResourceLimitHostTags;
45+
4146
/**
4247
*
4348
*
@@ -59,6 +64,22 @@ public interface UserVmManager extends UserVmService {
5964
"Destroys the VM's root volume when the VM is destroyed.",
6065
true, ConfigKey.Scope.Domain);
6166

67+
ConfigKey<String> StrictHostTags = new ConfigKey<>(
68+
"Advanced",
69+
String.class,
70+
"vm.strict.host.tags",
71+
"",
72+
"A comma-separated list of tags which must match during operations like modifying the compute" +
73+
"offering for an instance, and starting or live migrating an instance to a specific host.",
74+
true);
75+
ConfigKey<Boolean> EnforceStrictResourceLimitHostTagCheck = new ConfigKey<Boolean>(
76+
"Advanced",
77+
Boolean.class,
78+
"vm.strict.resource.limit.host.tag.check",
79+
"true",
80+
"If set to true, tags specified in `resource.limit.host.tags` are also included in vm.strict.host.tags.",
81+
true);
82+
6283
static final int MAX_USER_DATA_LENGTH_BYTES = 2048;
6384

6485
public static final String CKS_NODE = "cksnode";
@@ -125,6 +146,18 @@ UserVm updateVirtualMachine(long id, String displayName, String group, Boolean h
125146

126147
void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType);
127148

149+
static Set<String> getStrictHostTags() {
150+
String strictHostTags = StrictHostTags.value();
151+
Set<String> strictHostTagsSet = new HashSet<>();
152+
if (StringUtils.isNotEmpty(strictHostTags)) {
153+
strictHostTagsSet.addAll(List.of(strictHostTags.split(",")));
154+
}
155+
if (EnforceStrictResourceLimitHostTagCheck.value() && StringUtils.isNotEmpty(ResourceLimitHostTags.value())) {
156+
strictHostTagsSet.addAll(List.of(ResourceLimitHostTags.value().split(",")));
157+
}
158+
return strictHostTagsSet;
159+
}
160+
128161
void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);
129162

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

0 commit comments

Comments
 (0)