From 4114c1b74abba0fe07bb341aeb3b73b6e5fb2522 Mon Sep 17 00:00:00 2001 From: abhishek Date: Tue, 17 Aug 2010 14:14:16 -0700 Subject: [PATCH] Adding the updatetemplateoriso command refactoring, --- .../UpdateTemplateOrIsoPermissionsCmd.java | 112 ++---------------- .../com/cloud/server/ManagementServer.java | 4 +- .../cloud/server/ManagementServerImpl.java | 93 +++++++++++++-- 3 files changed, 92 insertions(+), 117 deletions(-) diff --git a/server/src/com/cloud/api/commands/UpdateTemplateOrIsoPermissionsCmd.java b/server/src/com/cloud/api/commands/UpdateTemplateOrIsoPermissionsCmd.java index c33f639e794..510146454dd 100644 --- a/server/src/com/cloud/api/commands/UpdateTemplateOrIsoPermissionsCmd.java +++ b/server/src/com/cloud/api/commands/UpdateTemplateOrIsoPermissionsCmd.java @@ -1,37 +1,21 @@ package com.cloud.api.commands; -import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.StringTokenizer; import org.apache.log4j.Logger; import com.cloud.api.BaseCmd; +import com.cloud.api.Implementation; import com.cloud.api.Parameter; -import com.cloud.api.ServerApiException; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.exception.PermissionDeniedException; -import com.cloud.storage.VMTemplateVO; -import com.cloud.user.Account; -import com.cloud.utils.Pair; +import com.cloud.api.BaseCmd.Manager; +@Implementation(method="updateTemplatePermissions", manager=Manager.ManagementServer) public abstract class UpdateTemplateOrIsoPermissionsCmd extends BaseCmd { public Logger s_logger = getLogger(); - protected static final List> s_properties = new ArrayList>(); protected String s_name = getResponseName(); - static { - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT_OBJ, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.USER_ID, Boolean.FALSE)); - - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT_NAMES, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.ID, Boolean.TRUE)); - s_properties.add(new Pair(BaseCmd.Properties.IS_FEATURED, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.IS_PUBLIC, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.OP, Boolean.FALSE)); - } + ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// @@ -82,97 +66,19 @@ public abstract class UpdateTemplateOrIsoPermissionsCmd extends BaseCmd { @Override public String getName() { return s_name; - } - @Override - public List> getProperties() { - return s_properties; - } - - protected boolean templateIsCorrectType(VMTemplateVO template) { - return true; - } + } protected String getResponseName() { return "updatetemplateorisopermissionsresponse"; } - protected String getMediaType() { - return "templateOrIso"; - } - protected Logger getLogger() { return Logger.getLogger(UpdateTemplateOrIsoPermissionsCmd.class.getName()); } - + @Override - public List> execute(Map params) { - Long id = (Long)params.get(BaseCmd.Properties.ID.getName()); - Account account = (Account)params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName()); - Boolean isPublic = (Boolean)params.get(BaseCmd.Properties.IS_PUBLIC.getName()); - Boolean isFeatured = (Boolean)params.get(BaseCmd.Properties.IS_FEATURED.getName()); - String accoutNames = (String)params.get(BaseCmd.Properties.ACCOUNT_NAMES.getName()); - String operation = (String)params.get(BaseCmd.Properties.OP.getName()); - - Boolean publishTemplateResult = Boolean.FALSE; - - VMTemplateVO template = getManagementServer().findTemplateById(id.longValue()); - if (template == null || !templateIsCorrectType(template)) { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find " + getMediaType() + " with id " + id); - } - - if (account != null) { - if (!isAdmin(account.getType()) && (template.getAccountId() != account.getId())) { - throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id); - } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { - Long templateOwnerDomainId = getManagementServer().findDomainIdByAccountId(template.getAccountId()); - if (!getManagementServer().isChildDomain(account.getDomainId(), templateOwnerDomainId)) { - throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to update permissions for " + getMediaType() + " with id " + id); - } - } - } - - // If the template is removed throw an error. - if (template.getRemoved() != null){ - s_logger.error("unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed "); - throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed "); - } - - if (id == Long.valueOf(1)) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id); - } - - boolean isAdmin = ((account == null) || isAdmin(account.getType())); - boolean allowPublicUserTemplates = Boolean.parseBoolean(getManagementServer().getConfigurationValue("allow.public.user.templates")); - if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Only private " + getMediaType() + "s can be created."); - } - - // package up the accountNames as a list - List accountNameList = new ArrayList(); - if (accoutNames != null) { - if ((operation == null) || (!operation.equalsIgnoreCase("add") && !operation.equalsIgnoreCase("remove") && !operation.equalsIgnoreCase("reset"))) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid operation on accounts, the operation must be either 'add' or 'remove' in order to modify launch permissions." + - " Given operation is: '" + operation + "'"); - } - StringTokenizer st = new StringTokenizer(accoutNames, ","); - while (st.hasMoreTokens()) { - accountNameList.add(st.nextToken()); - } - } - - try { - publishTemplateResult = getManagementServer().updateTemplatePermissions(id, operation, isPublic, isFeatured, accountNameList); - } catch (InvalidParameterValueException ex) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ": internal error."); - } catch (PermissionDeniedException ex) { - throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ": internal error."); - } catch (Exception ex) { - s_logger.error("Exception editing template", ex); - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ": internal error."); - } - - List> returnValues = new ArrayList>(); - returnValues.add(new Pair(BaseCmd.Properties.SUCCESS.getName(), publishTemplateResult.toString())); - return returnValues; + public String getResponse() + { + return null;//return the response here } } diff --git a/server/src/com/cloud/server/ManagementServer.java b/server/src/com/cloud/server/ManagementServer.java index dc6990f3cdd..270411a74a5 100644 --- a/server/src/com/cloud/server/ManagementServer.java +++ b/server/src/com/cloud/server/ManagementServer.java @@ -32,6 +32,7 @@ import com.cloud.api.commands.GetCloudIdentifierCmd; import com.cloud.api.commands.UpdateAccountCmd; import com.cloud.api.commands.UpdateDomainCmd; import com.cloud.api.commands.UpdateTemplateCmd; +import com.cloud.api.commands.UpdateTemplateOrIsoPermissionsCmd; import com.cloud.api.commands.UpdateUserCmd; import com.cloud.async.AsyncJobResult; import com.cloud.async.AsyncJobVO; @@ -1784,7 +1785,7 @@ public interface ManagementServer { * @throws PermissionDeniedException * @throws InternalErrorException */ - boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List accountNames) throws InvalidParameterValueException, PermissionDeniedException, InternalErrorException; +// boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List accountNames) throws InvalidParameterValueException, PermissionDeniedException, InternalErrorException; /** * List the permissions on a template. This will return a list of account names that have been granted permission to launch instances from the template. @@ -2141,4 +2142,5 @@ public interface ManagementServer { boolean validateCustomVolumeSizeRange(long size) throws InvalidParameterValueException; boolean updateUser(UpdateUserCmd cmd) throws InvalidParameterValueException; + boolean updateTemplatePermissions(UpdateTemplateOrIsoPermissionsCmd cmd)throws InvalidParameterValueException, PermissionDeniedException,InternalErrorException; } diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index 408b769204f..44fe80cd7d1 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -38,6 +38,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.StringTokenizer; import java.util.TimeZone; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -84,6 +85,7 @@ import com.cloud.api.commands.StartVMCmd; import com.cloud.api.commands.UpdateAccountCmd; import com.cloud.api.commands.UpdateDomainCmd; import com.cloud.api.commands.UpdateTemplateCmd; +import com.cloud.api.commands.UpdateTemplateOrIsoPermissionsCmd; import com.cloud.api.commands.UpdateUserCmd; import com.cloud.api.commands.UpgradeVMCmd; import com.cloud.async.AsyncInstanceCreateStatus; @@ -6750,15 +6752,85 @@ public class ManagementServerImpl implements ManagementServer { public List findPrivateDiskOffering() { return _diskOfferingDao.findPrivateDiskOffering(); } + + protected boolean templateIsCorrectType(VMTemplateVO template) { + return true; + } + + protected String getMediaType() { + return "templateOrIso"; + } + + public static boolean isAdmin(short accountType) { + return ((accountType == Account.ACCOUNT_TYPE_ADMIN) || + (accountType == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) || + (accountType == Account.ACCOUNT_TYPE_READ_ONLY_ADMIN)); + } @Override @DB - public boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List accountNames) throws InvalidParameterValueException, + public boolean updateTemplatePermissions(UpdateTemplateOrIsoPermissionsCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, InternalErrorException { Transaction txn = Transaction.currentTxn(); - VMTemplateVO template = _templateDao.findById(templateId); - if (template == null) { - throw new InvalidParameterValueException("Unable to find template with id " + templateId); + + //Input validation + Long id = cmd.getId(); + Account account = (Account) UserContext.current().getAccountObject(); + List accountNames = cmd.getAccountNames(); + Long userId = UserContext.current().getUserId(); + Boolean isFeatured = cmd.isFeatured(); + Boolean isPublic = cmd.isPublic(); + String operation = cmd.getOperation(); + + Boolean publishTemplateResult = Boolean.FALSE; + + VMTemplateVO template = _templateDao.findById(id); + + if (template == null || !templateIsCorrectType(template)) { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find " + getMediaType() + " with id " + id); + } + + if (account != null) + { + if (!isAdmin(account.getType()) && (template.getAccountId() != account.getId())) { + throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id); + } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { + Long templateOwnerDomainId = findDomainIdByAccountId(template.getAccountId()); + if (!isChildDomain(account.getDomainId(), templateOwnerDomainId)) { + throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to update permissions for " + getMediaType() + " with id " + id); + } + } + } + + // If the template is removed throw an error. + if (template.getRemoved() != null){ + s_logger.error("unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed "); + throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed "); + } + + if (id == Long.valueOf(1)) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id); + } + + boolean isAdmin = ((account == null) || isAdmin(account.getType())); + boolean allowPublicUserTemplates = Boolean.parseBoolean(getConfigurationValue("allow.public.user.templates")); + if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "Only private " + getMediaType() + "s can be created."); + } + +// // package up the accountNames as a list +// List accountNameList = new ArrayList(); + if (accountNames != null) + { + if ((operation == null) || (!operation.equalsIgnoreCase("add") && !operation.equalsIgnoreCase("remove") && !operation.equalsIgnoreCase("reset"))) + { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid operation on accounts, the operation must be either 'add' or 'remove' in order to modify launch permissions." + + " Given operation is: '" + operation + "'"); + } +// StringTokenizer st = new StringTokenizer(accountNames, ","); +// while (st.hasMoreTokens()) { +// accountNameList.add(st.nextToken()); +// } } Long accountId = template.getAccountId(); @@ -6767,11 +6839,6 @@ public class ManagementServerImpl implements ManagementServer { throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName()); } - Account account = _accountDao.findById(accountId); - if (account == null) { - throw new PermissionDeniedException("Unable to verify owner of template " + template.getName()); - } - VMTemplateVO updatedTemplate = _templateDao.createForUpdate(); if (isPublic != null) { @@ -6793,9 +6860,9 @@ public class ManagementServerImpl implements ManagementServer { if (permittedAccount.getId().longValue() == account.getId().longValue()) { continue; // don't grant permission to the template owner, they implicitly have permission } - LaunchPermissionVO existingPermission = _launchPermissionDao.findByTemplateAndAccount(templateId, permittedAccount.getId().longValue()); + LaunchPermissionVO existingPermission = _launchPermissionDao.findByTemplateAndAccount(id, permittedAccount.getId().longValue()); if (existingPermission == null) { - LaunchPermissionVO launchPermission = new LaunchPermissionVO(templateId, permittedAccount.getId().longValue()); + LaunchPermissionVO launchPermission = new LaunchPermissionVO(id, permittedAccount.getId().longValue()); _launchPermissionDao.persist(launchPermission); } } else { @@ -6814,7 +6881,7 @@ public class ManagementServerImpl implements ManagementServer { accountIds.add(permittedAccount.getId()); } } - _launchPermissionDao.removePermissions(templateId, accountIds); + _launchPermissionDao.removePermissions(id, accountIds); } catch (CloudRuntimeException ex) { throw new InternalErrorException("Internal error removing launch permissions for template " + template.getName()); } @@ -6825,7 +6892,7 @@ public class ManagementServerImpl implements ManagementServer { updatedTemplate.setPublicTemplate(false); updatedTemplate.setFeatured(false); _templateDao.update(template.getId(), updatedTemplate); - _launchPermissionDao.removeAllPermissions(templateId); + _launchPermissionDao.removeAllPermissions(id); } return true; }