Skip to content

Enforce strict host tag check #9017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 25, 2024
Merged

Enforce strict host tag check #9017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e7ff032
Enforce strict host tag checking
vishesh92 Apr 24, 2024
1892426
Add e2e tests
vishesh92 Apr 24, 2024
958c92a
Update global settings descrption
vishesh92 May 1, 2024
5ffe5e2
fixup
vishesh92 May 2, 2024
74977c4
Fix e2e test teardown
vishesh92 May 4, 2024
5ae9fc3
Merge branch 'main' into enforce-strict-host-tag-check
vishesh92 May 21, 2024
73d6888
Merge branch 'main' into enforce-strict-host-tag-check
vishesh92 Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ jobs:
smoke/test_usage
smoke/test_usage_events
smoke/test_vm_deployment_planner
smoke/test_vm_strict_host_tags
smoke/test_vm_schedule
smoke/test_vm_life_cycle
smoke/test_vm_lifecycle_unmanage_import
Expand Down
46 changes: 34 additions & 12 deletions engine/schema/src/main/java/com/cloud/host/HostVO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
// under the License.
package com.cloud.host;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;

import javax.persistence.Column;
Expand All @@ -45,6 +45,7 @@
import org.apache.cloudstack.util.HypervisorTypeConverter;
import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.BooleanUtils;
import org.apache.commons.lang3.StringUtils;

Expand Down Expand Up @@ -768,27 +769,48 @@ public void setUuid(String uuid) {
this.uuid = uuid;
}

public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template) {
if (serviceOffering == null || template == null) {
return false;
}
private Set<String> getHostServiceOfferingAndTemplateStrictTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
if (StringUtils.isEmpty(serviceOffering.getHostTag()) && StringUtils.isEmpty(template.getTemplateTag())) {
return true;
return new HashSet<>();
}
if (getHostTags() == null) {
return false;
}
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
List<String> tags = new ArrayList<>();
List<String> hostTagsList = getHostTags();
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
HashSet<String> tags = new HashSet<>();
if (StringUtils.isNotEmpty(serviceOffering.getHostTag())) {
tags.addAll(Arrays.asList(serviceOffering.getHostTag().split(",")));
}
if (StringUtils.isNotEmpty(template.getTemplateTag()) && !tags.contains(template.getTemplateTag())) {
if (StringUtils.isNotEmpty(template.getTemplateTag())) {
tags.add(template.getTemplateTag());
}
tags.removeIf(tag -> !strictHostTags.contains(tag));
tags.removeAll(hostTagsSet);
return tags;
}

public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
if (serviceOffering == null || template == null) {
return false;
}
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
if (tags.isEmpty()) {
return true;
}
List<String> hostTagsList = getHostTags();
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
return hostTagsSet.containsAll(tags);
}

public Set<String> getHostServiceOfferingAndTemplateMissingTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
if (tags.isEmpty()) {
return new HashSet<>();
}
List<String> hostTagsList = getHostTags();
HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
tags.removeAll(hostTagsSet);
return tags;
}

public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering) {
if (serviceOffering == null) {
return false;
Expand Down
53 changes: 38 additions & 15 deletions engine/schema/src/test/java/com/cloud/host/HostVOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@
import com.cloud.service.ServiceOfferingVO;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.vm.VirtualMachine;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
import org.junit.Before;
import org.mockito.Mockito;

public class HostVOTest {
HostVO host;
Expand All @@ -37,7 +41,7 @@ public class HostVOTest {
public void setUp() throws Exception {
host = new HostVO();
offering = new ServiceOfferingVO("TestSO", 0, 0, 0, 0, 0,
false, "TestSO", false,VirtualMachine.Type.User,false);
false, "TestSO", false, VirtualMachine.Type.User, false);
}

@Test
Expand All @@ -52,14 +56,14 @@ public void testNoTag() {

@Test
public void testRightTag() {
host.setHostTags(Arrays.asList("tag1","tag2"), false);
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
offering.setHostTag("tag2,tag1");
assertTrue(host.checkHostServiceOfferingTags(offering));
}

@Test
public void testWrongTag() {
host.setHostTags(Arrays.asList("tag1","tag2"), false);
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
offering.setHostTag("tag2,tag4");
assertFalse(host.checkHostServiceOfferingTags(offering));
}
Expand Down Expand Up @@ -87,40 +91,59 @@ public void checkHostServiceOfferingTagsTestRuleTagWithNullServiceTag() {

@Test
public void testEitherNoSOOrTemplate() {
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class)));
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null));
assertFalse(host.checkHostServiceOfferingAndTemplateTags(null, Mockito.mock(VirtualMachineTemplate.class), null));
assertFalse(host.checkHostServiceOfferingAndTemplateTags(Mockito.mock(ServiceOffering.class), null, null));
}

@Test
public void testNoTagOfferingTemplate() {
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()));
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()).isEmpty());
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")));
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")).isEmpty());
}

@Test
public void testRightTagOfferingTemplate() {
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
offering.setHostTag("tag2,tag1");
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class)));
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1")));
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1"));
assertTrue(actualMissingTags.isEmpty());

host.setHostTags(Arrays.asList("tag1", "tag2", "tag3"), false);
offering.setHostTag("tag2,tag1");
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag3")));
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag3"));
assertTrue(actualMissingTags.isEmpty());
host.setHostTags(List.of("tag3"), false);
offering.setHostTag(null);
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template));
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag3")));
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag3"));
assertTrue(actualMissingTags.isEmpty());

assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag1")));
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag1"));
assertTrue(actualMissingTags.isEmpty());
}

@Test
public void testWrongOfferingTag() {
host.setHostTags(Arrays.asList("tag1","tag2"), false);
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
offering.setHostTag("tag2,tag4");
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
Mockito.when(template.getTemplateTag()).thenReturn("tag1");
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
assertEquals(Set.of("tag4"), actualMissingTags);

offering.setHostTag("tag1,tag2");
template = Mockito.mock(VirtualMachineTemplate.class);
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template));
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
assertEquals(Set.of("tag3"), actualMissingTags);
}
}
5 changes: 3 additions & 2 deletions server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;

import com.cloud.vm.UserVmManager;
import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO;
import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.dao.VMTemplateDao;
Expand Down Expand Up @@ -282,7 +283,7 @@ protected void avoidOtherClustersForDeploymentIfMigrationDisabled(VirtualMachine
boolean storageMigrationNeededDuringClusterMigration = false;
for (Volume volume : volumes) {
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
if (List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
if (pool != null && List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
storageMigrationNeededDuringClusterMigration = true;
break;
}
Expand Down Expand Up @@ -778,7 +779,7 @@ protected boolean checkVmProfileAndHost(final VirtualMachineProfile vmProfile, f
VirtualMachineTemplate template = vmProfile.getTemplate();
if (offering.getHostTag() != null || template.getTemplateTag() != null) {
_hostDao.loadHostTags(host);
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template)) {
if (!host.checkHostServiceOfferingAndTemplateTags(offering, template, UserVmManager.getStrictHostTags())) {
logger.debug("Service Offering host tag or template tag does not match the last host of this VM");
return false;
}
Expand Down
33 changes: 33 additions & 0 deletions server/src/main/java/com/cloud/vm/UserVmManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
package com.cloud.vm;

import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import com.cloud.utils.StringUtils;
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
import org.apache.cloudstack.framework.config.ConfigKey;

Expand All @@ -38,6 +41,8 @@
import com.cloud.uservm.UserVm;
import com.cloud.utils.Pair;

import static com.cloud.user.ResourceLimitService.ResourceLimitHostTags;

/**
*
*
Expand All @@ -59,6 +64,22 @@ public interface UserVmManager extends UserVmService {
"Destroys the VM's root volume when the VM is destroyed.",
true, ConfigKey.Scope.Domain);

ConfigKey<String> StrictHostTags = new ConfigKey<>(
"Advanced",
String.class,
"vm.strict.host.tags",
"",
"A comma-separated list of tags which must match during operations like modifying the compute" +
"offering for an instance, and starting or live migrating an instance to a specific host.",
true);
ConfigKey<Boolean> EnforceStrictResourceLimitHostTagCheck = new ConfigKey<Boolean>(
"Advanced",
Boolean.class,
"vm.strict.resource.limit.host.tag.check",
"true",
"If set to true, tags specified in `resource.limit.host.tags` are also included in vm.strict.host.tags.",
true);

static final int MAX_USER_DATA_LENGTH_BYTES = 2048;

public static final String CKS_NODE = "cksnode";
Expand Down Expand Up @@ -127,6 +148,18 @@ UserVm updateVirtualMachine(long id, String displayName, String group, Boolean h

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

static Set<String> getStrictHostTags() {
String strictHostTags = StrictHostTags.value();
Set<String> strictHostTagsSet = new HashSet<>();
if (StringUtils.isNotEmpty(strictHostTags)) {
strictHostTagsSet.addAll(List.of(strictHostTags.split(",")));
}
if (EnforceStrictResourceLimitHostTagCheck.value() && StringUtils.isNotEmpty(ResourceLimitHostTags.value())) {
strictHostTagsSet.addAll(List.of(ResourceLimitHostTags.value().split(",")));
}
return strictHostTagsSet;
}

void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);

boolean checkIfDynamicScalingCanBeEnabled(VirtualMachine vm, ServiceOffering offering, VirtualMachineTemplate template, Long zoneId);
Expand Down
Loading