From fda366f8d6e83e4a40da2630c87972d09cfa620e Mon Sep 17 00:00:00 2001 From: Alena Prokharchyk Date: Mon, 5 Aug 2013 10:08:29 -0700 Subject: [PATCH] CLOUDSTACK-4087: updateTemplatePermissions - derive domainId from the template owner, not from the operation caller --- .../cloud/template/TemplateManagerImpl.java | 67 +++++++++++-------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java index d2b2afef3df..a5f389de1a8 100755 --- a/server/src/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/com/cloud/template/TemplateManagerImpl.java @@ -25,11 +25,11 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; + import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.utils.DateUtil; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.BaseListTemplateOrIsoPermissionsCmd; import org.apache.cloudstack.api.BaseUpdateTemplateOrIsoCmd; @@ -48,8 +48,22 @@ import org.apache.cloudstack.api.command.user.template.ListTemplatePermissionsCm import org.apache.cloudstack.api.command.user.template.RegisterTemplateCmd; import org.apache.cloudstack.api.command.user.template.UpdateTemplateCmd; import org.apache.cloudstack.api.command.user.template.UpdateTemplatePermissionsCmd; -import org.apache.cloudstack.engine.subsystem.api.storage.*; +import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; +import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; +import org.apache.cloudstack.engine.subsystem.api.storage.Scope; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.storage.command.AttachCommand; import org.apache.cloudstack.storage.command.CommandResult; @@ -67,7 +81,6 @@ import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; import com.cloud.agent.api.ComputeChecksumCommand; - import com.cloud.agent.api.storage.DestroyCommand; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DiskTO; @@ -83,6 +96,7 @@ import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; @@ -101,32 +115,29 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; - import com.cloud.resource.ResourceManager; import com.cloud.server.ConfigurationServer; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.GuestOSVO; import com.cloud.storage.LaunchPermissionVO; +import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; - import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.TemplateType; -import com.cloud.storage.DataStoreRole; -import com.cloud.storage.ScopeType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.TemplateProfile; import com.cloud.storage.Upload; -import com.cloud.storage.VMTemplateZoneVO; - import com.cloud.storage.VMTemplateHostVO; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeManager; import com.cloud.storage.VolumeVO; @@ -145,7 +156,6 @@ import com.cloud.storage.download.DownloadMonitor; import com.cloud.storage.secondary.SecondaryStorageVmManager; import com.cloud.storage.upload.UploadMonitor; import com.cloud.template.TemplateAdapter.TemplateAdapterType; - import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; @@ -157,13 +167,15 @@ import com.cloud.user.dao.AccountDao; import com.cloud.user.dao.UserAccountDao; import com.cloud.user.dao.UserDao; import com.cloud.uservm.UserVm; +import com.cloud.utils.DateUtil; import com.cloud.utils.EnumUtils; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.concurrency.NamedThreadFactory; -import com.cloud.utils.db.*; +import com.cloud.utils.db.DB; +import com.cloud.utils.db.Transaction; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; @@ -385,9 +397,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, if (template == null) { throw new InvalidParameterValueException("unable to find template with id " + templateId); } - TemplateAdapter adapter = getAdapter(template.getHypervisorType()); - TemplateProfile profile = adapter.prepareExtractTemplate(cmd); - + return extract(caller, templateId, url, zoneId, mode, eventId, false); } @@ -711,7 +721,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("There is no template " + templateId + " in zone " + sourceZoneId); } if (srcSecStore.getScope().getScopeType() == ScopeType.REGION) { - s_logger.debug("Template " + templateId + " is in region-wide secondary storage " + dstSecStore.getName() + " , don't need to copy"); + s_logger.debug("Template " + templateId + " is in region-wide secondary storage " + srcSecStore.getName() + " , don't need to copy"); return template; } @@ -809,9 +819,6 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, @Override public boolean configure(String name, Map params) throws ConfigurationException { - - final Map configs = _configDao.getConfiguration("AgentManager", params); - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); _primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); @@ -1237,8 +1244,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } } - Long accountId = template.getAccountId(); - if (accountId == null) { + Long ownerId = template.getAccountId(); + if (ownerId == null) { // if there is no owner of the template then it's probably already a // public template (or domain private template) so // publishing to individual users is irrelevant @@ -1270,12 +1277,19 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } _tmpltDao.update(template.getId(), updatedTemplate); + + //when operation is add/remove, accountNames can not be null + if (("add".equalsIgnoreCase(operation) || "remove".equalsIgnoreCase(operation)) && accountNames == null) { + throw new InvalidParameterValueException("Operation " + operation + " requires accounts or projectIds to be passed in"); + } - Long domainId = caller.getDomainId(); + //Derive the domain id from the template owner as updateTemplatePermissions is not cross domain operation + Account owner = _accountMgr.getAccount(ownerId); + Domain domain = _domainDao.findById(owner.getDomainId()); if ("add".equalsIgnoreCase(operation)) { txn.start(); for (String accountName : accountNames) { - Account permittedAccount = _accountDao.findActiveAccount(accountName, domainId); + Account permittedAccount = _accountDao.findActiveAccount(accountName, domain.getId()); if (permittedAccount != null) { if (permittedAccount.getId() == caller.getId()) { continue; // don't grant permission to the template @@ -1288,7 +1302,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } } else { txn.rollback(); - throw new InvalidParameterValueException("Unable to grant a launch permission to account " + accountName + throw new InvalidParameterValueException("Unable to grant a launch permission to account " + accountName + " in domain id=" + domain.getUuid() + ", account not found. " + "No permissions updated, please verify the account names and retry."); } } @@ -1296,7 +1310,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } else if ("remove".equalsIgnoreCase(operation)) { List accountIds = new ArrayList(); for (String accountName : accountNames) { - Account permittedAccount = _accountDao.findActiveAccount(accountName, domainId); + Account permittedAccount = _accountDao.findActiveAccount(accountName, domain.getId()); if (permittedAccount != null) { accountIds.add(permittedAccount.getId()); } @@ -1314,10 +1328,6 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, return true; } - private String getRandomPrivateTemplateName() { - return UUID.randomUUID().toString(); - } - @Override @DB @ActionEvent(eventType = EventTypes.EVENT_TEMPLATE_CREATE, eventDescription = "creating template", async = true) @@ -1526,7 +1536,6 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } volume = this._volumeDao.findById(snapshot.getVolumeId()); - VolumeVO snapshotVolume = this._volumeDao.findByIdIncludingRemoved(snapshot.getVolumeId()); // check permissions _accountMgr.checkAccess(caller, null, true, snapshot);