From cd5611273fd303737f500d0de379de288b690250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Beims=20Br=C3=A4scher?= Date: Sat, 20 May 2017 21:02:18 -0300 Subject: [PATCH] Resource dedicated to a domain is owned by the root domain When dedicating a resource (cluster or host) to a domain, the affinity group which is created is visible to everyone rather than only to domain that the cluster is dedicated to. --- .../affinity/AffinityGroupServiceImpl.java | 73 ++++++++++++------- .../AffinityGroupServiceImplTest.java | 43 ++++++++--- 2 files changed, 77 insertions(+), 39 deletions(-) diff --git a/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java b/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java index f2502442c18..f2c91c8addb 100644 --- a/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java +++ b/server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java @@ -117,7 +117,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro @DB @Override public AffinityGroup createAffinityGroup(final String accountName, final Long projectId, final Long domainId, final String affinityGroupName, final String affinityGroupType, - final String description) { + final String description) { // validate the affinityGroupType Map typeProcessorMap = getAffinityTypeToProcessorMap(); @@ -158,7 +158,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro verifyAffinityGroupNameInUse(owner.getAccountId(), owner.getDomainId(), affinityGroupName); verifyDomainLevelAffinityGroupName(domainLevel, owner.getDomainId(), affinityGroupName); - AffinityGroupVO group = createAffinityGroup(processor, owner, aclType, affinityGroupName, affinityGroupType, description); + AffinityGroupVO group = createAffinityGroup(processor, owner, aclType, affinityGroupName, affinityGroupType, description, domainLevel, domainId); if (s_logger.isDebugEnabled()) { s_logger.debug("Created affinity group =" + affinityGroupName); @@ -176,25 +176,27 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro } } - private AffinityGroupVO createAffinityGroup(final AffinityGroupProcessor processor, final Account owner, final ACLType aclType, final String affinityGroupName, final String affinityGroupType, final String description) { + private AffinityGroupVO createAffinityGroup(final AffinityGroupProcessor processor, final Account owner, final ACLType aclType, final String affinityGroupName, final String affinityGroupType, final String description, boolean domainLevel, Long domainId) { + + final Long affinityGroupDomainId = getDomainIdBasedOnDomainLevel(owner, domainLevel, domainId); + return Transaction.execute(new TransactionCallback() { @Override public AffinityGroupVO doInTransaction(TransactionStatus status) { - AffinityGroupVO group = - new AffinityGroupVO(affinityGroupName, affinityGroupType, description, owner.getDomainId(), owner.getId(), aclType); + AffinityGroupVO group = new AffinityGroupVO(affinityGroupName, affinityGroupType, description, affinityGroupDomainId, owner.getId(), aclType); _affinityGroupDao.persist(group); if (aclType == ACLType.Domain) { boolean subDomainAccess = false; subDomainAccess = processor.subDomainAccess(); - AffinityGroupDomainMapVO domainMap = new AffinityGroupDomainMapVO(group.getId(), owner.getDomainId(), + AffinityGroupDomainMapVO domainMap = new AffinityGroupDomainMapVO(group.getId(), affinityGroupDomainId, subDomainAccess); _affinityGroupDomainMapDao.persist(domainMap); //send event for storing the domain wide resource access Map params = new HashMap(); params.put(ApiConstants.ENTITY_TYPE, AffinityGroup.class); params.put(ApiConstants.ENTITY_ID, group.getId()); - params.put(ApiConstants.DOMAIN_ID, owner.getDomainId()); + params.put(ApiConstants.DOMAIN_ID, affinityGroupDomainId); params.put(ApiConstants.SUBDOMAIN_ACCESS, subDomainAccess); _messageBus.publish(_name, EntityManager.MESSAGE_ADD_DOMAIN_WIDE_ENTITY_EVENT, PublishScope.LOCAL, params); @@ -205,6 +207,20 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro }); } + /** + * If the account is null (domainLevel is true), then returns the domain id passed as a + * parameter; otherwise (domainLevel is false) it returns the domain id from the owner account. + * + * @note: this method fixes a critical bug. More details in JIRA ticket CLOUDSTACK-9432. + */ + protected Long getDomainIdBasedOnDomainLevel(final Account owner, boolean domainLevel, Long domainId) { + Long domainIdBasedOnDomainLevel = owner.getDomainId(); + if (domainLevel) { + domainIdBasedOnDomainLevel = domainId; + } + return domainIdBasedOnDomainLevel; + } + private DomainVO getDomain(Long domainId) { DomainVO domain = _domainDao.findById(domainId); if (domain == null) { @@ -225,6 +241,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro } } + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_AFFINITY_GROUP_DELETE, eventDescription = "Deleting affinity group") public boolean deleteAffinityGroup(Long affinityGroupId, String account, Long projectId, Long domainId, String affinityGroupName) { @@ -258,7 +275,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro throw new InvalidParameterValueException("Either the affinity group Id or group name must be specified to delete the group"); } if (group == null) { - throw new InvalidParameterValueException("Unable to find affinity group " + (affinityGroupId == null ? affinityGroupName : affinityGroupId)); + throw new InvalidParameterValueException("Unable to find affinity group " + (affinityGroupId == null ? affinityGroupName : affinityGroupId)); } return group; } @@ -292,10 +309,10 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro List affinityGroupVmMap = _affinityGroupVMMapDao.listByAffinityGroup(affinityGroupId); if (!affinityGroupVmMap.isEmpty()) { SearchBuilder listByAffinityGroup = _affinityGroupVMMapDao.createSearchBuilder(); - listByAffinityGroup.and("affinityGroupId", listByAffinityGroup.entity().getAffinityGroupId(), SearchCriteria.Op.EQ); + listByAffinityGroup.and("affinityGroupId", listByAffinityGroup.entity().getAffinityGroupId(), SearchCriteria.Op.EQ); listByAffinityGroup.done(); SearchCriteria sc = listByAffinityGroup.create(); - sc.setParameters("affinityGroupId", affinityGroupId); + sc.setParameters("affinityGroupId", affinityGroupId); _affinityGroupVMMapDao.lockRows(sc, null, true); _affinityGroupVMMapDao.remove(sc); @@ -323,12 +340,12 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro List types = new ArrayList(); for (AffinityGroupProcessor processor : _affinityProcessors) { - if (processor.isAdminControlledGroup()) { - continue; // we dont list the type if this group can be - // created only as an admin/system operation. - } - types.add(processor.getType()); + if (processor.isAdminControlledGroup()) { + continue; // we dont list the type if this group can be + // created only as an admin/system operation. } + types.add(processor.getType()); + } return types; } @@ -394,20 +411,20 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro @Override public boolean postStateTransitionEvent(StateMachine2.Transition transition, VirtualMachine vo, boolean status, Object opaque) { - if (!status) { - return false; - } - State newState = transition.getToState(); - if ((newState == State.Expunging) || (newState == State.Error)) { - // cleanup all affinity groups associations of the Expunged VM - SearchCriteria sc = _affinityGroupVMMapDao.createSearchCriteria(); - sc.addAnd("instanceId", SearchCriteria.Op.EQ, vo.getId()); - _affinityGroupVMMapDao.expunge(sc); - } - return true; + if (!status) { + return false; + } + State newState = transition.getToState(); + if ((newState == State.Expunging) || (newState == State.Error)) { + // cleanup all affinity groups associations of the Expunged VM + SearchCriteria sc = _affinityGroupVMMapDao.createSearchCriteria(); + sc.addAnd("instanceId", SearchCriteria.Op.EQ, vo.getId()); + _affinityGroupVMMapDao.expunge(sc); + } + return true; } - @Override + @Override public UserVm updateVMAffinityGroups(Long vmId, List affinityGroupIds) { // Verify input parameters UserVmVO vmInstance = _userVmDao.findById(vmId); @@ -419,7 +436,7 @@ public class AffinityGroupServiceImpl extends ManagerBase implements AffinityGro if (!vmInstance.getState().equals(State.Stopped)) { s_logger.warn("Unable to update affinity groups of the virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState()); throw new InvalidParameterValueException("Unable update affinity groups of the virtual machine " + vmInstance.toString() + " " + "in state " + - vmInstance.getState() + "; make sure the virtual machine is stopped and not in an error state before updating."); + vmInstance.getState() + "; make sure the virtual machine is stopped and not in an error state before updating."); } Account caller = CallContext.current().getCallingAccount(); diff --git a/server/test/org/apache/cloudstack/affinity/AffinityGroupServiceImplTest.java b/server/test/org/apache/cloudstack/affinity/AffinityGroupServiceImplTest.java index 448424716d5..6f45b908a43 100644 --- a/server/test/org/apache/cloudstack/affinity/AffinityGroupServiceImplTest.java +++ b/server/test/org/apache/cloudstack/affinity/AffinityGroupServiceImplTest.java @@ -32,12 +32,14 @@ import java.util.UUID; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.utils.db.EntityManager; -import com.cloud.event.ActionEventUtils; -import com.cloud.user.User; - +import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; +import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; +import org.apache.cloudstack.api.command.user.affinitygroup.CreateAffinityGroupCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.test.utils.SpringUtils; import org.junit.After; import org.junit.Before; @@ -57,33 +59,32 @@ import org.springframework.core.type.filter.TypeFilter; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.support.AnnotationConfigContextLoader; -import org.apache.cloudstack.acl.ControlledEntity; -import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao; -import org.apache.cloudstack.api.command.user.affinitygroup.CreateAffinityGroupCmd; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.framework.messagebus.MessageBus; import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.domain.dao.DomainDao; +import com.cloud.event.ActionEventUtils; import com.cloud.event.EventVO; import com.cloud.event.dao.EventDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceInUseException; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.projects.dao.ProjectDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; import com.cloud.user.AccountVO; import com.cloud.user.DomainManager; +import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; import com.cloud.user.dao.UserDao; import com.cloud.utils.component.ComponentContext; +import com.cloud.utils.db.EntityManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.dao.UserVmDao; -import com.cloud.projects.dao.ProjectDao; + +import org.junit.Assert; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) @@ -191,6 +192,26 @@ public class AffinityGroupServiceImplTest { } + private AccountVO mockOwnerForTestGetDomainIdBasedOnDomainLevel() { + AccountVO mockOwner = Mockito.mock(AccountVO.class); + when(mockOwner.getDomainId()).thenReturn(0l); + return mockOwner; + } + + @Test + public void getDomainIdBasedOnDomainLevelTestDomainLevelTrue() { + AccountVO owner = mockOwnerForTestGetDomainIdBasedOnDomainLevel(); + Long domainIdBasedOnDomainLevel = _affinityService.getDomainIdBasedOnDomainLevel(owner, true, 1l); + Assert.assertEquals(new Long(1), domainIdBasedOnDomainLevel); + } + + @Test + public void getDomainIdBasedOnDomainLevelTestDomainLevelFalse() { + AccountVO owner = mockOwnerForTestGetDomainIdBasedOnDomainLevel(); + Long domainIdBasedOnDomainLevel = _affinityService.getDomainIdBasedOnDomainLevel(owner, false, 1l); + Assert.assertEquals(new Long(0), domainIdBasedOnDomainLevel); + } + @Test public void shouldDeleteDomainLevelAffinityGroup() { AffinityGroupVO mockGroup = Mockito.mock(AffinityGroupVO.class);