From f35174373b20263c4757f5e8eff5545fd1eca502 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 19 Mar 2019 18:08:47 +0530 Subject: [PATCH 01/12] server: ability to create disk offerings associated with zone(s) Allows craeting storage offerings associated with particular zones. In UI create disk/storage offfering form a mult-select control has been addded to select desired zone(s). createDiskOffering API has been modified to allow passing list of zoneids. This list is stored in DB in disk_offering_details table with 'zoneids' key. Response for create and list disk offering APIs will return zoneids and zonenames in details object of response of offering is associated with zone(s). listDiskOfferings API has been modified to allow passwing zoneid to return only offerings which are associated with the zone. Signed-off-by: Abhishek Kumar --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../admin/offering/CreateDiskOfferingCmd.java | 18 +++++- .../user/offering/ListDiskOfferingsCmd.java | 8 +++ .../api/response/DiskOfferingResponse.java | 16 +++++- .../java/com/cloud/dc/dao/DataCenterDao.java | 2 + .../com/cloud/dc/dao/DataCenterDaoImpl.java | 8 +++ .../main/java/com/cloud/api/ApiDBUtils.java | 29 +++++++++- .../com/cloud/api/query/QueryManagerImpl.java | 33 ++++++++++- .../ConfigurationManagerImpl.java | 29 ++++++++-- ui/scripts/configuration.js | 57 +++++++++++++++++++ ui/scripts/docs.js | 4 ++ ui/scripts/instanceWizard.js | 4 ++ 12 files changed, 195 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 0c1f2b11f327..29d256eed258 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -706,6 +706,7 @@ public class ApiConstants { public static final String NETSCALER_SERVICEPACKAGE_ID = "netscalerservicepackageid"; public static final String ZONE_ID_LIST = "zoneids"; + public static final String ZONE_NAME_LIST = "zonenames"; public static final String DESTINATION_ZONE_ID_LIST = "destzoneids"; public static final String ADMIN = "admin"; public static final String CHECKSUM_PARAMETER_PREFIX_DESCRIPTION = "The parameter containing the checksum will be considered a MD5sum if it is not prefixed\n" diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java index 469f714d0281..2833820dd5b5 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java @@ -16,7 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; -import org.apache.log4j.Logger; +import java.util.List; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -26,10 +26,12 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.log4j.Logger; -import com.cloud.storage.Storage.ProvisioningType; import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.storage.Storage.ProvisioningType; import com.cloud.user.Account; @APICommand(name = "createDiskOffering", description = "Creates a disk offering.", responseObject = DiskOfferingResponse.class, @@ -64,6 +66,14 @@ public class CreateDiskOfferingCmd extends BaseCmd { description = "the ID of the containing domain, null for public offerings") private Long domainId; + @Parameter(name = ApiConstants.ZONE_ID_LIST, + type=CommandType.LIST, + collectionType = CommandType.UUID, + entityType = ZoneResponse.class, + required = false, + description = "the ID of the zones offering is associated with, null for all zone offerings") + protected List zoneIds; + @Parameter(name = ApiConstants.STORAGE_TYPE, type = CommandType.STRING, description = "the storage type of the disk offering. Values are local and shared.") private String storageType = ServiceOffering.StorageType.shared.toString(); @@ -170,6 +180,10 @@ public Long getDomainId() { return domainId; } + public List getZoneIds() { + return zoneIds; + } + public Long getBytesReadRate() { return bytesReadRate; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java index e5f92c9e0de0..79192dad9371 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.user.offering; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; @@ -42,6 +43,9 @@ public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "name of the disk offering") private String diskOfferingName; + @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "id of zone disk offering is associated with") + private Long zoneId; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -54,6 +58,10 @@ public String getDiskOfferingName() { return diskOfferingName; } + public Long getZoneId() { + return zoneId; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java index 5f22c91488eb..2f86629043d8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/DiskOfferingResponse.java @@ -17,8 +17,7 @@ package org.apache.cloudstack.api.response; import java.util.Date; - -import com.google.gson.annotations.SerializedName; +import java.util.Map; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseResponse; @@ -26,6 +25,7 @@ import com.cloud.offering.DiskOffering; import com.cloud.serializer.Param; +import com.google.gson.annotations.SerializedName; @EntityReference(value = DiskOffering.class) public class DiskOfferingResponse extends BaseResponse { @@ -144,6 +144,10 @@ public class DiskOfferingResponse extends BaseResponse { @Param(description = "whether to display the offering to the end user or not.") private Boolean displayOffering; + @SerializedName(ApiConstants.DETAILS) + @Param(description = "the details of the disk offering", since = "4.13.0") + private Map details; + public Boolean getDisplayOffering() { return displayOffering; } @@ -328,4 +332,12 @@ public void setIopsWriteRateMax(Long iopsWriteRateMax) { public void setIopsWriteRateMaxLength(Long iopsWriteRateMaxLength) { this.iopsWriteRateMaxLength = iopsWriteRateMaxLength; } + + public Map getDetails() { + return details; + } + + public void setDetails(Map details) { + this.details = details; + } } diff --git a/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDao.java b/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDao.java index a6cd59f1cc35..e006b0e4573b 100644 --- a/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDao.java +++ b/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDao.java @@ -121,4 +121,6 @@ public Integer getVlan() { List findByKeyword(String keyword); List listAllZones(); + + List list(Object[] ids); } diff --git a/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDaoImpl.java index 385fb4061551..2db904f78c9f 100644 --- a/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/dc/dao/DataCenterDaoImpl.java @@ -437,4 +437,12 @@ public List listAllZones() { return dcs; } + + @Override + public List list(Object[] ids) { + SearchBuilder sb = createSearchBuilder(); + SearchCriteria sc = sb.create(); + sc.addAnd("id", SearchCriteria.Op.IN, ids); + return listBy(sc); + } } diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 22ed2437d4de..7be00c7692f5 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -33,6 +33,7 @@ import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; import org.apache.cloudstack.api.ApiCommandJobType; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiConstants.DomainDetails; import org.apache.cloudstack.api.ApiConstants.HostDetails; import org.apache.cloudstack.api.ApiConstants.VMDetails; @@ -68,6 +69,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.dao.AsyncJobDao; +import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -314,6 +316,7 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDao; import com.cloud.user.AccountManager; import com.cloud.network.dao.FirewallRulesDcidrsDao; +import com.google.common.base.Strings; public class ApiDBUtils { private static ManagementServer s_ms; @@ -335,6 +338,7 @@ public class ApiDBUtils { static CapacityDao s_capacityDao; static DiskOfferingDao s_diskOfferingDao; static DiskOfferingJoinDao s_diskOfferingJoinDao; + static DiskOfferingDetailsDao s_diskOfferingDetailsDao; static DataCenterJoinDao s_dcJoinDao; static DomainDao s_domainDao; static DomainJoinDao s_domainJoinDao; @@ -471,6 +475,8 @@ public class ApiDBUtils { @Inject private DiskOfferingJoinDao diskOfferingJoinDao; @Inject + private DiskOfferingDetailsDao diskOfferingDetailsDao; + @Inject private DomainDao domainDao; @Inject private DomainJoinDao domainJoinDao; @@ -688,6 +694,7 @@ void init() { s_dcJoinDao = dcJoinDao; s_diskOfferingDao = diskOfferingDao; s_diskOfferingJoinDao = diskOfferingJoinDao; + s_diskOfferingDetailsDao = diskOfferingDetailsDao; s_domainDao = domainDao; s_domainJoinDao = domainJoinDao; s_domainRouterDao = domainRouterDao; @@ -1906,7 +1913,27 @@ public static AsyncJobJoinVO newAsyncJobView(AsyncJob e) { } public static DiskOfferingResponse newDiskOfferingResponse(DiskOfferingJoinVO offering) { - return s_diskOfferingJoinDao.newDiskOfferingResponse(offering); + DiskOfferingResponse diskOfferingResponse = s_diskOfferingJoinDao.newDiskOfferingResponse(offering); + if(diskOfferingResponse!=null) { + Map details = s_diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); + if (details != null && !details.isEmpty()) { + if(details.containsKey(ApiConstants.ZONE_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { + String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); + List zones = s_zoneDao.list(zoneIdsArray); + List zoneIdsList = new ArrayList<>(); + List zoneNamesList = new ArrayList<>(); + for (DataCenterVO zone : zones) { + zoneIdsList.add(zone.getUuid()); + zoneNamesList.add(zone.getName()); + } + details.put(ApiConstants.ZONE_ID_LIST, String.join(",", zoneIdsList)); + details.put(ApiConstants.ZONE_NAME_LIST, String.join(", ", zoneNamesList)); + } + diskOfferingResponse.setDetails(details); + } + } + return diskOfferingResponse; } public static DiskOfferingJoinVO newDiskOfferingView(DiskOffering offering) { diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 91e0466d9dbb..edccb6fb3fcd 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -25,14 +25,13 @@ import javax.inject.Inject; -import com.cloud.cluster.ManagementServerHostVO; -import com.cloud.cluster.dao.ManagementServerHostDao; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.affinity.AffinityGroupDomainMapVO; import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.affinity.AffinityGroupVMMapVO; import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListProjectAndAccountResourcesCmd; import org.apache.cloudstack.api.ResourceDetail; import org.apache.cloudstack.api.ResponseObject.ResponseView; @@ -108,6 +107,7 @@ import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.query.QueryService; +import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -156,6 +156,8 @@ import com.cloud.api.query.vo.UserAccountJoinVO; import com.cloud.api.query.vo.UserVmJoinVO; import com.cloud.api.query.vo.VolumeJoinVO; +import com.cloud.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DedicatedResourceDao; import com.cloud.domain.Domain; @@ -223,6 +225,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import com.google.common.base.Strings; @Component public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements QueryService, Configurable { @@ -321,6 +324,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private DiskOfferingJoinDao _diskOfferingJoinDao; + @Inject + private DiskOfferingDetailsDao diskOfferingDetailsDao; + @Inject private ServiceOfferingJoinDao _srvOfferingJoinDao; @@ -2610,7 +2616,28 @@ private Pair, Integer> searchForDiskOfferingsInternal(L * domain.getPath() + "%"); // } */ - return _diskOfferingJoinDao.searchAndCount(sc, searchFilter); + Pair, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter); + // If zoneid is passed remove offer offerings that are not associated with the zone + // TODO: Needs better approach + if (cmd.getZoneId() != null && result.first() != null && !result.first().isEmpty()) { + final Long zoneId = cmd.getZoneId(); + List offerings = result.first(); + for (int i = offerings.size() - 1; i >= 0; i--) { + DiskOfferingJoinVO offering = offerings.get(i); + Map details = diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); + if (details.containsKey(ApiConstants.ZONE_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { + String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); + List zoneIds = new ArrayList<>(); + for (String zId : zoneIdsArray) + zoneIds.add(Long.valueOf(zId.trim())); + if (!zoneIds.contains(zoneId)) { + offerings.remove(i); + } + } + } + } + return result; } private List filterOfferingsOnCurrentTags(List offerings, ServiceOfferingVO currentVmOffering) { diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 30f2f7cd1e22..70342babe26b 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -35,12 +35,11 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.google.common.collect.Sets; - import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.affinity.dao.AffinityGroupDao; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.config.UpdateCfgCmd; import org.apache.cloudstack.api.command.admin.network.CreateManagementNetworkIpRangeCmd; import org.apache.cloudstack.api.command.admin.network.CreateNetworkOfferingCmd; @@ -87,6 +86,7 @@ import org.apache.cloudstack.region.Region; import org.apache.cloudstack.region.RegionVO; import org.apache.cloudstack.region.dao.RegionDao; +import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; @@ -232,6 +232,7 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.Sets; public class ConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService, Configurable { public static final Logger s_logger = Logger.getLogger(ConfigurationManagerImpl.class); @@ -267,6 +268,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati @Inject DiskOfferingDao _diskOfferingDao; @Inject + DiskOfferingDetailsDao diskOfferingDetailsDao; + @Inject NetworkOfferingDao _networkOfferingDao; @Inject VlanDao _vlanDao; @@ -2573,7 +2576,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd) } } - protected DiskOfferingVO createDiskOffering(final Long userId, final Long domainId, final String name, final String description, final String provisioningType, + protected DiskOfferingVO createDiskOffering(final Long userId, final Long domainId, final List zoneIds, final String name, final String description, final String provisioningType, final Long numGibibytes, String tags, boolean isCustomized, final boolean localStorageRequired, final boolean isDisplayOfferingEnabled, final Boolean isCustomizedIops, Long minIops, Long maxIops, Long bytesReadRate, Long bytesReadRateMax, Long bytesReadRateMaxLength, @@ -2640,6 +2643,13 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final Long domain throw new InvalidParameterValueException("Unable to create disk offering by id " + userId + " because it is not root-admin or domain-admin"); } + if (zoneIds != null) { + for (Long zoneId : zoneIds) { + if (_zoneDao.findById(zoneId) == null) + throw new InvalidParameterValueException("Unable to create disk offering associated with invalid zone, " + zoneId); + } + } + tags = StringUtils.cleanupTags(tags); final DiskOfferingVO newDiskOffering = new DiskOfferingVO(domainId, name, description, typedProvisioningType, diskSize, tags, isCustomized, isCustomizedIops, minIops, maxIops); @@ -2692,11 +2702,16 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final Long domain CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); final DiskOfferingVO offering = _diskOfferingDao.persist(newDiskOffering); if (offering != null) { + if(zoneIds!=null && !zoneIds.isEmpty()) { + List zoneIdsStringList = new ArrayList<>(); + for(Long zoneId : zoneIds) + zoneIdsStringList.add(String.valueOf(zoneId)); + diskOfferingDetailsDao.addDetail(offering.getId(), ApiConstants.ZONE_ID_LIST, String.join(",", zoneIdsStringList), true); + } CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); return offering; - } else { - return null; } + return null; } @Override @@ -2717,6 +2732,8 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) { // cmd final Long domainId = cmd.getDomainId(); + final List zoneIds = cmd.getZoneIds(); + if (!isCustomized && numGibibytes == null) { throw new InvalidParameterValueException("Disksize is required for a non-customized disk offering"); } @@ -2753,7 +2770,7 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) { final Integer hypervisorSnapshotReserve = cmd.getHypervisorSnapshotReserve(); final Long userId = CallContext.current().getCallingUserId(); - return createDiskOffering(userId, domainId, name, description, provisioningType, numGibibytes, tags, isCustomized, + return createDiskOffering(userId, domainId, zoneIds, name, description, provisioningType, numGibibytes, tags, isCustomized, localStorageRequired, isDisplayOfferingEnabled, isCustomizedIops, minIops, maxIops, bytesReadRate, bytesReadRateMax, bytesReadRateMaxLength, bytesWriteRate, bytesWriteRateMax, bytesWriteRateMaxLength, iopsReadRate, iopsReadRateMax, iopsReadRateMaxLength, iopsWriteRate, iopsWriteRateMax, iopsWriteRateMaxLength, diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js index de8f4726e66d..19bb6d4117e1 100644 --- a/ui/scripts/configuration.js +++ b/ui/scripts/configuration.js @@ -2026,6 +2026,44 @@ }); }, isHidden: true + }, + zone: { + label: 'label.zone', + docID: 'helpDiskOfferingZone', + isMultiple: true, + isHidden: true, + dependsOn: 'isPublic', + validation: { + allzonesonly: true + }, + select: function(args) { + $.ajax({ + url: createURL("listZones&available=true"), + dataType: "json", + async: true, + success: function(json) { + var zoneObjs = []; + var items = json.listzonesresponse.zone; + if (items != null) { + for (var i = 0; i < items.length; i++) { + zoneObjs.push({ + id: items[i].id, + description: items[i].name + }); + } + } + if (isAdmin()) { + zoneObjs.unshift({ + id: -1, + description: "All Zones" + }); + } + args.response.success({ + data: zoneObjs + }); + } + }); + } } } }, @@ -2109,6 +2147,16 @@ $.extend(data, { domainid: args.data.domainId }); + var zones = ""; + if (Object.prototype.toString.call( args.data.zone ) === '[object Array]') { + zones = args.data.zone.join(","); + } else + zones = args.data.zone; + if (zones != "") { + $.extend(data, { + zoneids: zones + }); + } } $.ajax({ @@ -2282,6 +2330,9 @@ domain: { label: 'label.domain' }, + zones: { + label: 'label.zones' + }, storagetype: { label: 'label.storage.type' }, @@ -2299,6 +2350,12 @@ data: data }; var diskOfferings = cloudStack.listDiskOfferings(listDiskOfferingsOptions); + var diskOffering = diskOfferings[0] + if(diskOffering.details && diskOffering.details.zonenames) { + $.extend(diskOffering, { + zones: diskOffering.details.zonenames + }); + } args.response.success({ actionFilter: diskOfferingActionfilter, data: diskOfferings[0] diff --git a/ui/scripts/docs.js b/ui/scripts/docs.js index bbe8f3e64b49..e55cc95e6dfd 100755 --- a/ui/scripts/docs.js +++ b/ui/scripts/docs.js @@ -386,6 +386,10 @@ cloudStack.docs = { desc: 'Select the subdomain in which this offering is available', externalLink: '' }, + helpDiskOfferingZone: { + desc: 'Select the zones in which this offering is available (Tip: Use Ctrl to choose multiple zones)', + externalLink: '' + }, // Add domain helpDomainName: { desc: 'Any desired name for the new sub-domain. Must be unique within the current domain.', diff --git a/ui/scripts/instanceWizard.js b/ui/scripts/instanceWizard.js index 351ca7b30208..771b47d62e64 100644 --- a/ui/scripts/instanceWizard.js +++ b/ui/scripts/instanceWizard.js @@ -377,6 +377,7 @@ // Step 4: Data disk offering function(args) { var isRequired = (args.currentData["select-template"] == "select-iso" ? true : false); + var zoneid = args.currentData["zoneid"] var templateFilter = 'executable' if (isAdmin()) { templateFilter = 'all' @@ -384,6 +385,9 @@ $.ajax({ url: createURL("listDiskOfferings"), dataType: "json", + data: { + zoneid: zoneid + }, async: true, success: function(json) { diskOfferingObjs = json.listdiskofferingsresponse.diskoffering; From 59543a292b0881f3c95eefe8a872bb6361829ed7 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 20 Mar 2019 14:04:27 +0530 Subject: [PATCH 02/12] api: update domain, zones(s) for disk offerings This change allows changing domain and zone(s) for a disk offering. Added 'domainid' and 'zoneids' for the 'updateDiskOffering' API. Signed-off-by: Abhishek Kumar --- .../admin/offering/UpdateDiskOfferingCmd.java | 26 +++++++++++++ .../ConfigurationManagerImpl.java | 37 ++++++++++++++++--- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java index 6e1fde55a30f..9b1b37f4071e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java @@ -16,6 +16,10 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; +import java.util.List; + +import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; @@ -59,6 +63,20 @@ public class UpdateDiskOfferingCmd extends BaseCmd { description = "an optional field, whether to display the offering to the end user or not.") private Boolean displayOffering; + @Parameter(name = ApiConstants.DOMAIN_ID, + type = CommandType.UUID, + entityType = DomainResponse.class, + description = "the ID of the containing domain, null for public offerings") + private Long domainId; + + @Parameter(name = ApiConstants.ZONE_ID_LIST, + type=CommandType.LIST, + collectionType = CommandType.UUID, + entityType = ZoneResponse.class, + required = false, + description = "the ID of the zones offering is associated with, null for all zone offerings") + protected List zoneIds; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -83,6 +101,14 @@ public Boolean getDisplayOffering() { return displayOffering; } + public Long getDomainId() { + return domainId; + } + + public List getZoneIds() { + return zoneIds; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 70342babe26b..3f20f88376d6 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2802,18 +2802,33 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { throw new InvalidParameterValueException("Unable to find active user by id " + userId); } final Account account = _accountDao.findById(user.getAccountId()); + final Long domainId = cmd.getDomainId(); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (diskOfferingHandle.getDomainId() == null) { - throw new InvalidParameterValueException("Unable to update public disk offering by id " + userId + " because it is domain-admin"); - } - if (! _domainDao.isChildDomain(account.getDomainId(), diskOfferingHandle.getDomainId() )) { - throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + if (domainId == null) { + if (diskOfferingHandle.getDomainId() == null) { + throw new InvalidParameterValueException("Unable to update public disk offering by id " + userId + " because it is domain-admin"); + } + if (!_domainDao.isChildDomain(account.getDomainId(), diskOfferingHandle.getDomainId())) { + throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + } + } else { + if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { + throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + } } } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { throw new InvalidParameterValueException("Unable to update disk offering by id " + userId + " because it is not root-admin or domain-admin"); } - final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null; + final List zoneIds = cmd.getZoneIds(); + if (zoneIds != null) { + for (Long zoneId : zoneIds) { + if (_zoneDao.findById(zoneId) == null) + throw new InvalidParameterValueException("Unable to create disk offering associated with invalid zone, " + zoneId); + } + } + + final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null || domainId != null || (zoneIds != null && !zoneIds.isEmpty()); if (!updateNeeded) { return _diskOfferingDao.findById(diskOfferingId); } @@ -2836,6 +2851,10 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { diskOffering.setDisplayOffering(displayDiskOffering); } + if (domainId != null) { + diskOffering.setDomainId(domainId); + } + // Note: tag editing commented out for now;keeping the code intact, // might need to re-enable in next releases // if (tags != null) @@ -2862,6 +2881,12 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { // } if (_diskOfferingDao.update(diskOfferingId, diskOffering)) { + if(zoneIds!=null && !zoneIds.isEmpty()) { + List zoneIdsStringList = new ArrayList<>(); + for(Long zoneId : zoneIds) + zoneIdsStringList.add(String.valueOf(zoneId)); + diskOfferingDetailsDao.addDetail(diskOfferingId, ApiConstants.ZONE_ID_LIST, String.join(",", zoneIdsStringList), true); + } CallContext.current().setEventDetails("Disk offering id=" + diskOffering.getId()); return _diskOfferingDao.findById(diskOfferingId); } else { From 5092f9c88605039ad26df2cbda2ca9424469b81c Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 20 Mar 2019 14:31:45 +0530 Subject: [PATCH 03/12] ui: fix for handling 'All zones' for disk offering zone(s) Signed-off-by: Abhishek Kumar --- ui/scripts/configuration.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js index 19bb6d4117e1..98e4e898bbb7 100644 --- a/ui/scripts/configuration.js +++ b/ui/scripts/configuration.js @@ -2148,10 +2148,22 @@ domainid: args.data.domainId }); var zones = ""; - if (Object.prototype.toString.call( args.data.zone ) === '[object Array]') { - zones = args.data.zone.join(","); - } else - zones = args.data.zone; + if (Object.prototype.toString.call(args.data.zone) === '[object Array]') { + var allZonesSelected = false; + args.data.zone.forEach(function (zone) { + if (zone === null) { + allZonesSelected = true; + break; + } + }); + if(!allZonesSelected) { + zones = args.data.zone.join(","); + } + } else { + if (args.data.zone != null) { + zones = args.data.zone; + } + } if (zones != "") { $.extend(data, { zoneids: zones From a04ad9a21dfa20456fe054da963f83e310a72b71 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Mar 2019 12:26:53 +0530 Subject: [PATCH 04/12] server: check access for disk offering and zone for volumes Associating zones with disk offering will require volumes to be created in storage pool of those associated zones only. This change adds check for validating allowed offering, zone combination while creating, resizing and migrating volumes. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/user/AccountService.java | 3 +- .../cloudstack/acl/SecurityChecker.java | 2 +- .../configuration/ConfigurationManager.java | 2 +- .../management/MockAccountManager.java | 3 +- .../java/com/cloud/acl/DomainChecker.java | 35 ++++++++++++++----- .../ConfigurationManagerImpl.java | 6 ++-- .../cloud/storage/VolumeApiServiceImpl.java | 27 +++++++------- .../com/cloud/user/AccountManagerImpl.java | 7 ++-- .../java/com/cloud/vm/UserVmManagerImpl.java | 6 ++-- .../cloud/user/MockAccountManagerImpl.java | 3 +- .../vpc/MockConfigurationManagerImpl.java | 2 +- 11 files changed, 59 insertions(+), 37 deletions(-) diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index 060861d18091..cda82f52a2ad 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.command.admin.user.RegisterCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; +import com.cloud.dc.DataCenter; import com.cloud.domain.Domain; import com.cloud.exception.PermissionDeniedException; import com.cloud.offering.DiskOffering; @@ -98,7 +99,7 @@ UserAccount createUserAccount(String userName, String password, String firstName void checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException; - void checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException; + void checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException; void checkAccess(User user, ControlledEntity entity); diff --git a/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java b/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java index 41708717548a..e4f3ca8075e2 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java @@ -138,5 +138,5 @@ boolean checkAccess(Account caller, AccessType accessType, String action, Contro public boolean checkAccess(Account account, ServiceOffering so) throws PermissionDeniedException; - boolean checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException; + boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException; } diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index 235b241264c8..58d43ae25960 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -177,7 +177,7 @@ DataCenterVO createZone(long userId, String zoneName, String dns1, String dns2, void checkZoneAccess(Account caller, DataCenter zone); - void checkDiskOfferingAccess(Account caller, DiskOffering dof); + void checkDiskOfferingAccess(Account caller, DiskOffering dof, DataCenter zone); /** * Creates a new network offering diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index 100f38060be4..7320773b858c 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -41,6 +41,7 @@ import com.cloud.api.query.vo.ControlledViewEntity; import com.cloud.configuration.ResourceLimit; import com.cloud.configuration.dao.ResourceCountDao; +import com.cloud.dc.DataCenter; import com.cloud.domain.Domain; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.PermissionDeniedException; @@ -435,7 +436,7 @@ public void checkAccess(Account account, ServiceOffering so) throws PermissionDe } @Override - public void checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException { + public void checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { // TODO Auto-generated method stub } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index c9e4087f6baf..15e16af61d02 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -16,13 +16,18 @@ // under the License. package com.cloud.acl; -import javax.inject.Inject; +import java.util.Arrays; +import java.util.List; +import java.util.Map; -import org.springframework.stereotype.Component; +import javax.inject.Inject; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; +import org.springframework.stereotype.Component; import com.cloud.dc.DataCenter; import com.cloud.dc.DedicatedResourceVO; @@ -44,6 +49,7 @@ import com.cloud.user.User; import com.cloud.user.dao.AccountDao; import com.cloud.utils.component.AdapterBase; +import com.google.common.base.Strings; @Component public class DomainChecker extends AdapterBase implements SecurityChecker { @@ -64,6 +70,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { private DedicatedResourceDao _dedicatedDao; @Inject AccountService _accountService; + @Inject + DiskOfferingDetailsDao diskOfferingDetailsDao; protected DomainChecker() { super(); @@ -167,13 +175,14 @@ public boolean checkAccess(User user, ControlledEntity entity) throws Permission } @Override - public boolean checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException { + public boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { + boolean isAccess = false; if (account == null || dof == null || dof.getDomainId() == null) {//public offering - return true; + isAccess = true; } else { //admin has all permissions if (_accountService.isRootAdmin(account.getId())) { - return true; + isAccess = true; } //if account is normal user or domain admin //check if account's domain is a child of zone's domain (Note: This is made consistent with the list command for disk offering) @@ -182,14 +191,14 @@ else if (_accountService.isNormalUser(account.getId()) || _accountService.isDomainAdmin(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_PROJECT) { if (account.getDomainId() == dof.getDomainId()) { - return true; //disk offering and account at exact node + isAccess = true; //disk offering and account at exact node } else { Domain domainRecord = _domainDao.findById(account.getDomainId()); if (domainRecord != null) { while (true) { if (domainRecord.getId() == dof.getDomainId()) { //found as a child - return true; + isAccess = true; } if (domainRecord.getParent() != null) { domainRecord = _domainDao.findById(domainRecord.getParent()); @@ -201,8 +210,18 @@ else if (_accountService.isNormalUser(account.getId()) } } } + + if (isAccess && dof != null && zone != null) { + Map details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); + if (details != null && !details.isEmpty() && + details.containsKey(ApiConstants.ZONE_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { + List zoneIds = Arrays.asList(details.get(ApiConstants.ZONE_ID_LIST).split(",")); + isAccess = zoneIds.contains(String.valueOf(zone.getId())); + } + } //not found - return false; + return isAccess; } @Override diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 3f20f88376d6..032d3f4a0c71 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -4236,15 +4236,15 @@ public boolean deleteVlanIpRange(final DeleteVlanIpRangeCmd cmd) { } @Override - public void checkDiskOfferingAccess(final Account caller, final DiskOffering dof) { + public void checkDiskOfferingAccess(final Account caller, final DiskOffering dof, DataCenter zone) { for (final SecurityChecker checker : _secChecker) { - if (checker.checkAccess(caller, dof)) { + if (checker.checkAccess(caller, dof, zone)) { if (s_logger.isDebugEnabled()) { s_logger.debug("Access granted to " + caller + " to disk offering:" + dof.getId() + " by " + checker.getName()); } return; } else { - throw new PermissionDeniedException("Access denied to " + caller + " by " + checker.getName()); + throw new PermissionDeniedException(String.format("Access denied to %s for disk offering: %s, zone: %s by %s", caller, dof.getUuid(), zone.getUuid(), checker.getName())); } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 23b56e98da38..f3b9e383019e 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -436,11 +436,7 @@ private boolean validateVolume(Account caller, long ownerId, Long zoneId, String throw new InvalidParameterValueException("Please specify a custom sized disk offering."); } - if (diskOffering.getDomainId() == null) { - // do nothing as offering is public - } else { - _configMgr.checkDiskOfferingAccess(volumeOwner, diskOffering); - } + _configMgr.checkDiskOfferingAccess(volumeOwner, diskOffering, zone); } return false; @@ -603,11 +599,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept throw new InvalidParameterValueException("This disk offering does not allow custom size"); } - if (diskOffering.getDomainId() == null) { - // do nothing as offering is public - } else { - _configMgr.checkDiskOfferingAccess(caller, diskOffering); - } + _configMgr.checkDiskOfferingAccess(caller, diskOffering, _dcDao.findById(zoneId)); if (diskOffering.getDiskSize() > 0) { size = diskOffering.getDiskSize(); @@ -666,6 +658,9 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept // if zoneId is not provided, we default to create volume in the same zone as the snapshot zone. zoneId = snapshotCheck.getDataCenterId(); } + + _configMgr.checkDiskOfferingAccess(null, diskOffering, _dcDao.findById(zoneId)); + size = snapshotCheck.getSize(); // ; disk offering is used for tags // purposes @@ -949,10 +944,7 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep throw new InvalidParameterValueException("There are no tags on the current disk offering. The new disk offering needs to have no tags, as well."); } - if (newDiskOffering.getDomainId() != null) { - // not a public offering; check access - _configMgr.checkDiskOfferingAccess(CallContext.current().getCallingAccount(), newDiskOffering); - } + _configMgr.checkDiskOfferingAccess(CallContext.current().getCallingAccount(), newDiskOffering, _dcDao.findById(volume.getDataCenterId())); if (newDiskOffering.isCustomized()) { newSize = cmd.getSize(); @@ -2182,7 +2174,12 @@ private DiskOfferingVO retrieveAndValidateNewDiskOffering(MigrateVolumeCmd cmd) throw new InvalidParameterValueException(String.format("We cannot assign a removed disk offering [id=%s] to a volume. ", newDiskOffering.getUuid())); } Account caller = CallContext.current().getCallingAccount(); - _accountMgr.checkAccess(caller, newDiskOffering); + DataCenter zone = null; + Volume volume = _volsDao.findById(cmd.getId()); + if (volume != null) { + zone = _dcDao.findById(volume.getDataCenterId()); + } + _accountMgr.checkAccess(caller, newDiskOffering, zone); return newDiskOffering; } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index bda4ccab4680..5783e4b65f9b 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -77,6 +77,7 @@ import com.cloud.configuration.ResourceLimit; import com.cloud.configuration.dao.ResourceCountDao; import com.cloud.configuration.dao.ResourceLimitDao; +import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DataCenterDao; @@ -2844,13 +2845,15 @@ public void checkAccess(Account account, ServiceOffering so) throws PermissionDe } @Override - public void checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException { + public void checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { for (SecurityChecker checker : _securityCheckers) { - if (checker.checkAccess(account, dof)) { + if (checker.checkAccess(account, dof, zone)) { if (s_logger.isDebugEnabled()) { s_logger.debug("Access granted to " + account + " to " + dof + " by " + checker.getName()); } return; + } else { + throw new PermissionDeniedException(String.format("Access denied to %s for disk offering: %s, zone: %s by %s", account, dof.getUuid(), zone.getUuid(), checker.getName())); } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 68b45e1af7c3..c30ccedef308 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -3009,7 +3009,7 @@ public UserVm createBasicSecurityGroupVirtualMachine(DataCenter zone, ServiceOff // Verify that owner can use the service offering _accountMgr.checkAccess(owner, serviceOffering); - _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId)); + _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); // Get default guest network in Basic zone Network defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); @@ -3068,7 +3068,7 @@ public UserVm createAdvancedSecurityGroupVirtualMachine(DataCenter zone, Service // Verify that owner can use the service offering _accountMgr.checkAccess(owner, serviceOffering); - _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId)); + _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); // If no network is specified, find system security group enabled network if (networkIdList == null || networkIdList.isEmpty()) { @@ -3176,7 +3176,7 @@ public UserVm createAdvancedVirtualMachine(DataCenter zone, ServiceOffering serv // Verify that owner can use the service offering _accountMgr.checkAccess(owner, serviceOffering); - _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId)); + _accountMgr.checkAccess(owner, _diskOfferingDao.findById(diskOfferingId), zone); List vpcSupportedHTypes = _vpcMgr.getSupportedVpcHypervisors(); if (networkIdList == null || networkIdList.isEmpty()) { diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index 4fbf7526f312..c6088521360a 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -36,6 +36,7 @@ import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import com.cloud.api.query.vo.ControlledViewEntity; +import com.cloud.dc.DataCenter; import com.cloud.domain.Domain; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.PermissionDeniedException; @@ -217,7 +218,7 @@ public void checkAccess(Account account, ServiceOffering so) throws PermissionDe } @Override - public void checkAccess(Account account, DiskOffering dof) throws PermissionDeniedException { + public void checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { // TODO Auto-generated method stub } diff --git a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java index 9057241cc78a..4c40f53afdc9 100644 --- a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java @@ -438,7 +438,7 @@ public void checkZoneAccess(Account caller, DataCenter zone) { * @see com.cloud.configuration.ConfigurationManager#checkDiskOfferingAccess(com.cloud.user.Account, com.cloud.offering.DiskOffering) */ @Override - public void checkDiskOfferingAccess(Account caller, DiskOffering dof) { + public void checkDiskOfferingAccess(Account caller, DiskOffering dof, DataCenter zone) { // TODO Auto-generated method stub } From 295b8f1d4fc72dbf241ad537255fa614a09a4ccc Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Mar 2019 18:12:06 +0530 Subject: [PATCH 05/12] server: multiple domains for disk offering This change allows associating multiple domains with a disk offering. ID of selected domains will be stored in disk_offering_details table as comma separated list with key 'domainids'. create, update disk offering API now accepts new parameter domainids to provide ID of domains. UI shows domains list as comma separated names string in offering details tab. Create disk offering form now allows multi-selection. Signed-off-by: Abhishek Kumar --- .../apache/cloudstack/api/ApiConstants.java | 2 + .../admin/offering/CreateDiskOfferingCmd.java | 18 +++- .../admin/offering/UpdateDiskOfferingCmd.java | 18 +++- .../user/offering/ListDiskOfferingsCmd.java | 6 +- .../java/com/cloud/domain/dao/DomainDao.java | 2 + .../com/cloud/domain/dao/DomainDaoImpl.java | 7 ++ .../com/cloud/storage/DiskOfferingVO.java | 20 ++++- .../java/com/cloud/acl/DomainChecker.java | 34 ++++--- .../main/java/com/cloud/api/ApiDBUtils.java | 13 +++ .../com/cloud/api/query/QueryManagerImpl.java | 73 ++++++++------- .../ConfigurationManagerImpl.java | 88 +++++++++++++------ ui/scripts/configuration.js | 48 +++++++--- 12 files changed, 233 insertions(+), 96 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 29d256eed258..cc7ca6437d5f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -112,6 +112,8 @@ public class ApiConstants { public static final String DOMAIN = "domain"; public static final String DOMAIN_ID = "domainid"; public static final String DOMAIN__ID = "domainId"; + public static final String DOMAIN_ID_LIST = "domainids"; + public static final String DOMAIN_NAME_LIST = "domainnames"; public static final String DURATION = "duration"; public static final String ELIGIBLE = "eligible"; public static final String EMAIL = "email"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java index 2833820dd5b5..78241ce62f21 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java @@ -66,12 +66,22 @@ public class CreateDiskOfferingCmd extends BaseCmd { description = "the ID of the containing domain, null for public offerings") private Long domainId; + @Parameter(name = ApiConstants.DOMAIN_ID_LIST, + type = CommandType.LIST, + collectionType = CommandType.UUID, + entityType = DomainResponse.class, + required = false, + description = "the ID of the domains offering is associated with, null for all domain offerings", + since = "4.13") + protected List domainIds; + @Parameter(name = ApiConstants.ZONE_ID_LIST, - type=CommandType.LIST, + type = CommandType.LIST, collectionType = CommandType.UUID, entityType = ZoneResponse.class, required = false, - description = "the ID of the zones offering is associated with, null for all zone offerings") + description = "the ID of the zones offering is associated with, null for all zone offerings", + since = "4.13") protected List zoneIds; @Parameter(name = ApiConstants.STORAGE_TYPE, type = CommandType.STRING, description = "the storage type of the disk offering. Values are local and shared.") @@ -180,6 +190,10 @@ public Long getDomainId() { return domainId; } + public List getDomainIds() { + return domainIds; + } + public List getZoneIds() { return zoneIds; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java index 9b1b37f4071e..5e28c7987f07 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java @@ -69,12 +69,22 @@ public class UpdateDiskOfferingCmd extends BaseCmd { description = "the ID of the containing domain, null for public offerings") private Long domainId; + @Parameter(name = ApiConstants.DOMAIN_ID_LIST, + type = CommandType.LIST, + collectionType = CommandType.UUID, + entityType = DomainResponse.class, + required = false, + description = "the ID of the domains offering is associated with, null for all domain offerings", + since = "4.13") + protected List domainIds; + @Parameter(name = ApiConstants.ZONE_ID_LIST, - type=CommandType.LIST, + type = CommandType.LIST, collectionType = CommandType.UUID, entityType = ZoneResponse.class, required = false, - description = "the ID of the zones offering is associated with, null for all zone offerings") + description = "the ID of the zones offering is associated with, null for all zone offerings", + since = "4.13") protected List zoneIds; ///////////////////////////////////////////////////// @@ -105,6 +115,10 @@ public Long getDomainId() { return domainId; } + public List getDomainIds() { + return domainIds; + } + public List getZoneIds() { return zoneIds; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java index 79192dad9371..92b8676b4693 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListDiskOfferingsCmd.java @@ -43,7 +43,11 @@ public class ListDiskOfferingsCmd extends BaseListDomainResourcesCmd { @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "name of the disk offering") private String diskOfferingName; - @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "id of zone disk offering is associated with") + @Parameter(name = ApiConstants.ZONE_ID, + type = CommandType.UUID, + entityType = ZoneResponse.class, + description = "id of zone disk offering is associated with", + since = "4.13") private Long zoneId; ///////////////////////////////////////////////////// diff --git a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java index 297fbfad6de9..45ff6b5df545 100644 --- a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java +++ b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java @@ -40,4 +40,6 @@ public interface DomainDao extends GenericDao { Set getDomainParentIds(long domainId); List getDomainChildrenIds(String path); + + List list(Object[] ids); } diff --git a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java index 4316e3bc8ed1..45be8ea2af17 100644 --- a/engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java @@ -291,4 +291,11 @@ public Set getDomainParentIds(long domainId) { return parentDomains; } + @Override + public List list(Object[] ids) { + SearchBuilder sb = createSearchBuilder(); + SearchCriteria sc = sb.create(); + sc.addAnd("id", SearchCriteria.Op.IN, ids); + return listBy(sc); + } } diff --git a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java index e8e91ea37814..2293553f22ea 100644 --- a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java @@ -51,7 +51,7 @@ public class DiskOfferingVO implements DiskOffering { long id; @Column(name = "domain_id") - Long domainId; + Long domainId = null; @Column(name = "unique_name") private String uniqueName; @@ -182,6 +182,24 @@ public DiskOfferingVO(Long domainId, String name, String displayText, Storage.Pr this.cacheMode = cacheMode; } + public DiskOfferingVO(String name, String displayText, Storage.ProvisioningType provisioningType, long diskSize, String tags, boolean isCustomized, Boolean isCustomizedIops, + Long minIops, Long maxIops) { + this.name = name; + this.displayText = displayText; + this.provisioningType = provisioningType; + this.diskSize = diskSize; + this.tags = tags; + recreatable = false; + type = Type.Disk; + useLocalStorage = false; + customized = isCustomized; + uuid = UUID.randomUUID().toString(); + customizedIops = isCustomizedIops; + this.minIops = minIops; + this.maxIops = maxIops; + state = State.Active; + } + public DiskOfferingVO(Long domainId, String name, String displayText, Storage.ProvisioningType provisioningType, long diskSize, String tags, boolean isCustomized, Boolean isCustomizedIops, Long minIops, Long maxIops) { this.domainId = domainId; diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 15e16af61d02..2fba4eaa53da 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -177,7 +177,8 @@ public boolean checkAccess(User user, ControlledEntity entity) throws Permission @Override public boolean checkAccess(Account account, DiskOffering dof, DataCenter zone) throws PermissionDeniedException { boolean isAccess = false; - if (account == null || dof == null || dof.getDomainId() == null) {//public offering + Map details = null; + if (account == null || dof == null) {//public offering isAccess = true; } else { //admin has all permissions @@ -190,31 +191,26 @@ else if (_accountService.isNormalUser(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN || _accountService.isDomainAdmin(account.getId()) || account.getType() == Account.ACCOUNT_TYPE_PROJECT) { - if (account.getDomainId() == dof.getDomainId()) { - isAccess = true; //disk offering and account at exact node - } else { - Domain domainRecord = _domainDao.findById(account.getDomainId()); - if (domainRecord != null) { - while (true) { - if (domainRecord.getId() == dof.getDomainId()) { - //found as a child - isAccess = true; - } - if (domainRecord.getParent() != null) { - domainRecord = _domainDao.findById(domainRecord.getParent()); - } else { - break; - } + details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); + isAccess = true; + if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { + List domainIds = Arrays.asList(details.get(ApiConstants.DOMAIN_ID_LIST).split(",")); + for (String domainId : domainIds) { + if (!_domainDao.isChildDomain(Long.valueOf(domainId), account.getDomainId())) { + isAccess = false; + break; } } } } } + // Check for zones if (isAccess && dof != null && zone != null) { - Map details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); - if (details != null && !details.isEmpty() && - details.containsKey(ApiConstants.ZONE_ID_LIST) && + if (details == null) + details = diskOfferingDetailsDao.listDetailsKeyPairs(dof.getId()); + if (details.containsKey(ApiConstants.ZONE_ID_LIST) && !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { List zoneIds = Arrays.asList(details.get(ApiConstants.ZONE_ID_LIST).split(",")); isAccess = zoneIds.contains(String.valueOf(zone.getId())); diff --git a/server/src/main/java/com/cloud/api/ApiDBUtils.java b/server/src/main/java/com/cloud/api/ApiDBUtils.java index 7be00c7692f5..89958673bd8d 100644 --- a/server/src/main/java/com/cloud/api/ApiDBUtils.java +++ b/server/src/main/java/com/cloud/api/ApiDBUtils.java @@ -1917,6 +1917,19 @@ public static DiskOfferingResponse newDiskOfferingResponse(DiskOfferingJoinVO of if(diskOfferingResponse!=null) { Map details = s_diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); if (details != null && !details.isEmpty()) { + if(details.containsKey(ApiConstants.DOMAIN_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { + String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); + List domains = s_domainDao.list(domainIdsArray); + List domainIdsList = new ArrayList<>(); + List domainNamesList = new ArrayList<>(); + for (DomainVO domain : domains) { + domainIdsList.add(domain.getUuid()); + domainNamesList.add(domain.getName()); + } + details.put(ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsList)); + details.put(ApiConstants.DOMAIN_NAME_LIST, String.join(", ", domainNamesList)); + } if(details.containsKey(ApiConstants.ZONE_ID_LIST) && !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index edccb6fb3fcd..0710e87f7633 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2544,7 +2544,6 @@ private Pair, Integer> searchForDiskOfferingsInternal(L } } - List domainIds = null; // For non-root users, only return all offerings for the user's domain, // and everything above till root if ((_accountMgr.isNormalUser(account.getId()) || _accountMgr.isDomainAdmin(account.getId())) || account.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) { @@ -2552,27 +2551,7 @@ private Pair, Integer> searchForDiskOfferingsInternal(L if (account.getType() == Account.ACCOUNT_TYPE_NORMAL) { throw new InvalidParameterValueException("Only ROOT admins and Domain admins can list disk offerings with isrecursive=true"); } - DomainVO domainRecord = _domainDao.findById(account.getDomainId()); - sc.addAnd("domainPath", SearchCriteria.Op.LIKE, domainRecord.getPath() + "%"); } else { // domain + all ancestors - // find all domain Id up to root domain for this account - domainIds = new ArrayList(); - DomainVO domainRecord = _domainDao.findById(account.getDomainId()); - if (domainRecord == null) { - s_logger.error("Could not find the domainId for account:" + account.getAccountName()); - throw new CloudAuthenticationException("Could not find the domainId for account:" + account.getAccountName()); - } - domainIds.add(domainRecord.getId()); - while (domainRecord.getParent() != null) { - domainRecord = _domainDao.findById(domainRecord.getParent()); - domainIds.add(domainRecord.getId()); - } - - SearchCriteria spc = _diskOfferingJoinDao.createSearchCriteria(); - - spc.addOr("domainId", SearchCriteria.Op.IN, domainIds.toArray()); - spc.addOr("domainId", SearchCriteria.Op.NULL); // include public offering as where - sc.addAnd("domainId", SearchCriteria.Op.SC, spc); sc.addAnd("systemUse", SearchCriteria.Op.EQ, false); // non-root users should not see system offering at all } @@ -2617,22 +2596,50 @@ private Pair, Integer> searchForDiskOfferingsInternal(L */ Pair, Integer> result = _diskOfferingJoinDao.searchAndCount(sc, searchFilter); - // If zoneid is passed remove offer offerings that are not associated with the zone - // TODO: Needs better approach - if (cmd.getZoneId() != null && result.first() != null && !result.first().isEmpty()) { - final Long zoneId = cmd.getZoneId(); + // Remove offerings that are not associated with caller's domain and passed zone + // TODO: Better approach + if (result.first() != null && !result.first().isEmpty()) { List offerings = result.first(); for (int i = offerings.size() - 1; i >= 0; i--) { DiskOfferingJoinVO offering = offerings.get(i); Map details = diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); - if (details.containsKey(ApiConstants.ZONE_ID_LIST) && - !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { - String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); - List zoneIds = new ArrayList<>(); - for (String zId : zoneIdsArray) - zoneIds.add(Long.valueOf(zId.trim())); - if (!zoneIds.contains(zoneId)) { - offerings.remove(i); + boolean toRemove = isRecursive; + if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && + details.containsKey(ApiConstants.DOMAIN_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { + toRemove = true; + String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); + for (String dIdStr : domainIdsArray) { + Long dId = Long.valueOf(dIdStr.trim()); + if(isRecursive) { + if (_domainDao.isChildDomain(account.getDomainId(), dId)) { + toRemove = false; + break; + } + } else { + if (_domainDao.isChildDomain(dId, account.getDomainId())) { + toRemove = false; + break; + } + } + } + } + if (toRemove) { + offerings.remove(i); + } else { + // If zoneid is passed remove offerings that are not associated with the zone + if (cmd.getZoneId() != null) { + final Long zoneId = cmd.getZoneId(); + if (details.containsKey(ApiConstants.ZONE_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.ZONE_ID_LIST))) { + String[] zoneIdsArray = details.get(ApiConstants.ZONE_ID_LIST).split(","); + List zoneIds = new ArrayList<>(); + for (String zId : zoneIdsArray) + zoneIds.add(Long.valueOf(zId.trim())); + if (!zoneIds.contains(zoneId)) { + offerings.remove(i); + } + } } } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 032d3f4a0c71..57dac2aa2c20 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2576,7 +2576,7 @@ public ServiceOffering updateServiceOffering(final UpdateServiceOfferingCmd cmd) } } - protected DiskOfferingVO createDiskOffering(final Long userId, final Long domainId, final List zoneIds, final String name, final String description, final String provisioningType, + protected DiskOfferingVO createDiskOffering(final Long userId, final List domainIds, final List zoneIds, final String name, final String description, final String provisioningType, final Long numGibibytes, String tags, boolean isCustomized, final boolean localStorageRequired, final boolean isDisplayOfferingEnabled, final Boolean isCustomizedIops, Long minIops, Long maxIops, Long bytesReadRate, Long bytesReadRateMax, Long bytesReadRateMaxLength, @@ -2630,14 +2630,16 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final Long domain } final Account account = _accountDao.findById(user.getAccountId()); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (domainId == null) { + if (domainIds == null || domainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to create public disk offering by id " + userId + " because it is domain-admin"); } if (tags != null) { throw new InvalidParameterValueException("Unable to create disk offering with storage tags by id " + userId + " because it is domain-admin"); } - if (! _domainDao.isChildDomain(account.getDomainId(), domainId)) { - throw new InvalidParameterValueException("Unable to create disk offering by another domain admin with id " + userId); + for (Long domainId : domainIds) { + if (domainId == null || !_domainDao.isChildDomain(account.getDomainId(), domainId)) { + throw new InvalidParameterValueException("Unable to create disk offering by another domain admin with id " + userId); + } } } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { throw new InvalidParameterValueException("Unable to create disk offering by id " + userId + " because it is not root-admin or domain-admin"); @@ -2651,7 +2653,7 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final Long domain } tags = StringUtils.cleanupTags(tags); - final DiskOfferingVO newDiskOffering = new DiskOfferingVO(domainId, name, description, typedProvisioningType, diskSize, tags, isCustomized, + final DiskOfferingVO newDiskOffering = new DiskOfferingVO(name, description, typedProvisioningType, diskSize, tags, isCustomized, isCustomizedIops, minIops, maxIops); newDiskOffering.setUseLocalStorage(localStorageRequired); newDiskOffering.setDisplayOffering(isDisplayOfferingEnabled); @@ -2702,6 +2704,12 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final Long domain CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); final DiskOfferingVO offering = _diskOfferingDao.persist(newDiskOffering); if (offering != null) { + if(domainIds!=null && !domainIds.isEmpty()) { + List domainIdsStringList = new ArrayList<>(); + for(Long domainId : domainIds) + domainIdsStringList.add(String.valueOf(domainId)); + diskOfferingDetailsDao.addDetail(offering.getId(), ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsStringList), true); + } if(zoneIds!=null && !zoneIds.isEmpty()) { List zoneIdsStringList = new ArrayList<>(); for(Long zoneId : zoneIds) @@ -2726,11 +2734,8 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) { // by // default final String tags = cmd.getTags(); - // Long domainId = cmd.getDomainId() != null ? cmd.getDomainId() : - // Long.valueOf(DomainVO.ROOT_DOMAIN); // disk offering - // always gets created under the root domain.Bug # 6055 if not passed in - // cmd - final Long domainId = cmd.getDomainId(); + + final List domainIds = cmd.getDomainIds(); final List zoneIds = cmd.getZoneIds(); @@ -2770,7 +2775,7 @@ public DiskOffering createDiskOffering(final CreateDiskOfferingCmd cmd) { final Integer hypervisorSnapshotReserve = cmd.getHypervisorSnapshotReserve(); final Long userId = CallContext.current().getCallingUserId(); - return createDiskOffering(userId, domainId, zoneIds, name, description, provisioningType, numGibibytes, tags, isCustomized, + return createDiskOffering(userId, domainIds, zoneIds, name, description, provisioningType, numGibibytes, tags, isCustomized, localStorageRequired, isDisplayOfferingEnabled, isCustomizedIops, minIops, maxIops, bytesReadRate, bytesReadRateMax, bytesReadRateMaxLength, bytesWriteRate, bytesWriteRateMax, bytesWriteRateMaxLength, iopsReadRate, iopsReadRateMax, iopsReadRateMaxLength, iopsWriteRate, iopsWriteRateMax, iopsWriteRateMaxLength, @@ -2788,6 +2793,15 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { // Check if diskOffering exists final DiskOffering diskOfferingHandle = _entityMgr.findById(DiskOffering.class, diskOfferingId); + List existingDomainIds = new ArrayList<>(); + Map details = diskOfferingDetailsDao.listDetailsKeyPairs(diskOfferingId); + if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { + String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); + for (String dIdStr : domainIdsArray) { + existingDomainIds.add(Long.valueOf(dIdStr.trim())); + } + } if (diskOfferingHandle == null) { throw new InvalidParameterValueException("Unable to find disk offering by id " + diskOfferingId); @@ -2802,18 +2816,22 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { throw new InvalidParameterValueException("Unable to find active user by id " + userId); } final Account account = _accountDao.findById(user.getAccountId()); - final Long domainId = cmd.getDomainId(); + final List domainIds = cmd.getDomainIds(); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (domainId == null) { - if (diskOfferingHandle.getDomainId() == null) { + if (domainIds == null) { + if (existingDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to update public disk offering by id " + userId + " because it is domain-admin"); } - if (!_domainDao.isChildDomain(account.getDomainId(), diskOfferingHandle.getDomainId())) { - throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + for (Long domainId : existingDomainIds) { + if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { + throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + } } } else { - if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { - throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + for (Long domainId : domainIds) { + if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { + throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); + } } } } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { @@ -2828,8 +2846,9 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { } } - final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null || domainId != null || (zoneIds != null && !zoneIds.isEmpty()); - if (!updateNeeded) { + final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null; + final boolean detailsUpdateNeeded = (domainIds != null && !domainIds.isEmpty()) || (zoneIds != null && !zoneIds.isEmpty()); + if (!updateNeeded && !detailsUpdateNeeded) { return _diskOfferingDao.findById(diskOfferingId); } @@ -2851,10 +2870,6 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { diskOffering.setDisplayOffering(displayDiskOffering); } - if (domainId != null) { - diskOffering.setDomainId(domainId); - } - // Note: tag editing commented out for now;keeping the code intact, // might need to re-enable in next releases // if (tags != null) @@ -2880,7 +2895,13 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { // } // } - if (_diskOfferingDao.update(diskOfferingId, diskOffering)) { + if (!updateNeeded || _diskOfferingDao.update(diskOfferingId, diskOffering)) { + if(domainIds!=null && !domainIds.isEmpty()) { + List domainIdsStringList = new ArrayList<>(); + for(Long domainId : domainIds) + domainIdsStringList.add(String.valueOf(domainId)); + diskOfferingDetailsDao.addDetail(diskOfferingId, ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsStringList), true); + } if(zoneIds!=null && !zoneIds.isEmpty()) { List zoneIdsStringList = new ArrayList<>(); for(Long zoneId : zoneIds) @@ -2915,11 +2936,22 @@ public boolean deleteDiskOffering(final DeleteDiskOfferingCmd cmd) { } final Account account = _accountDao.findById(user.getAccountId()); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (offering.getDomainId() == null) { + List existingDomainIds = new ArrayList<>(); + Map details = diskOfferingDetailsDao.listDetailsKeyPairs(diskOfferingId); + if (details.containsKey(ApiConstants.DOMAIN_ID_LIST) && + !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { + String[] domainIdsArray = details.get(ApiConstants.DOMAIN_ID_LIST).split(","); + for (String dIdStr : domainIdsArray) { + existingDomainIds.add(Long.valueOf(dIdStr.trim())); + } + } + if (existingDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to delete public disk offering by id " + userId + " because it is domain-admin"); } - if (! _domainDao.isChildDomain(account.getDomainId(), offering.getDomainId() )) { - throw new InvalidParameterValueException("Unable to delete disk offering by another domain admin with id " + userId); + for (Long domainId : existingDomainIds) { + if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { + throw new InvalidParameterValueException("Unable to delete disk offering by another domain admin with id " + userId); + } } } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) { throw new InvalidParameterValueException("Unable to delete disk offering by id " + userId + " because it is not root-admin or domain-admin"); diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js index 98e4e898bbb7..55ef33270f02 100644 --- a/ui/scripts/configuration.js +++ b/ui/scripts/configuration.js @@ -1994,10 +1994,11 @@ isChecked: false, docID: 'helpDiskOfferingPublic' }, - domainId: { + domain: { label: 'label.domain', docID: 'helpDiskOfferingDomain', dependsOn: 'isPublic', + isMultiple: true, select: function(args) { $.ajax({ url: createURL('listDomains'), @@ -2144,9 +2145,29 @@ } if (args.data.isPublic != "on") { - $.extend(data, { - domainid: args.data.domainId - }); + var domains = ""; + if (Object.prototype.toString.call(args.data.domain) === '[object Array]') { + var rootDomainSelected = false; + args.data.domain.forEach(function (domain) { + if (domain === null) { + rootDomainSelected = true; + break; + } + }); + if(!rootDomainSelected) { + domains = args.data.domain.join(","); + } + } else { + if (args.data.domain != null) { + domains = args.data.domain; + } + } + if (domains != "") { + $.extend(data, { + domainids: domains + }); + } + var zones = ""; if (Object.prototype.toString.call(args.data.zone) === '[object Array]') { var allZonesSelected = false; @@ -2339,8 +2360,8 @@ tags: { label: 'label.storage.tags' }, - domain: { - label: 'label.domain' + domains: { + label: 'label.domains' }, zones: { label: 'label.zones' @@ -2363,10 +2384,17 @@ }; var diskOfferings = cloudStack.listDiskOfferings(listDiskOfferingsOptions); var diskOffering = diskOfferings[0] - if(diskOffering.details && diskOffering.details.zonenames) { - $.extend(diskOffering, { - zones: diskOffering.details.zonenames - }); + if(diskOffering.details) { + if(diskOffering.details.domainnames) { + $.extend(diskOffering, { + domains: diskOffering.details.domainnames + }); + } + if(diskOffering.details.zonenames) { + $.extend(diskOffering, { + zones: diskOffering.details.zonenames + }); + } } args.response.success({ actionFilter: diskOfferingActionfilter, From 4c651e6ab035d9d09d28cb361485b4bd68efee5d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 27 Mar 2019 13:36:12 +0530 Subject: [PATCH 06/12] server: fix for empty listdiskofferings response for admin Signed-off-by: Abhishek Kumar --- server/src/main/java/com/cloud/api/query/QueryManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 0710e87f7633..dcc048019eda 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2603,7 +2603,7 @@ private Pair, Integer> searchForDiskOfferingsInternal(L for (int i = offerings.size() - 1; i >= 0; i--) { DiskOfferingJoinVO offering = offerings.get(i); Map details = diskOfferingDetailsDao.listDetailsKeyPairs(offering.getId()); - boolean toRemove = isRecursive; + boolean toRemove = account.getType() == Account.ACCOUNT_TYPE_ADMIN ? false : isRecursive; if (account.getType() != Account.ACCOUNT_TYPE_ADMIN && details.containsKey(ApiConstants.DOMAIN_ID_LIST) && !Strings.isNullOrEmpty(details.get(ApiConstants.DOMAIN_ID_LIST))) { From 3d533846cc7344503e0390b6c86a0455a886fab3 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 27 Mar 2019 14:10:41 +0530 Subject: [PATCH 07/12] server: filter child domains for disk offering create, update This change enables filtering child domains while creating, updating disk offerings when both parent and child domains are passed. Signed-off-by: Abhishek Kumar --- .../ConfigurationManagerImpl.java | 62 ++++++++++++++++--- 1 file changed, 53 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 57dac2aa2c20..d32039e98256 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2622,6 +2622,27 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List } } + // Filter child domains when both parent and child domains are present + List filteredDomainIds = new ArrayList<>(); + if (domainIds != null) { + filteredDomainIds.addAll(domainIds); + } + if (filteredDomainIds.size() > 1) { + for (int i = filteredDomainIds.size() - 1; i >= 1; i--) { + long first = filteredDomainIds.get(i); + for (int j = i - 1; j >= 0; j--) { + long second = filteredDomainIds.get(j); + if (_domainDao.isChildDomain(filteredDomainIds.get(i), filteredDomainIds.get(j))) { + filteredDomainIds.remove(j); + i--; + } + if (_domainDao.isChildDomain(filteredDomainIds.get(j), filteredDomainIds.get(i))) { + filteredDomainIds.remove(i); + break; + } + } + } + } // Check if user exists in the system final User user = _userDao.findById(userId); @@ -2630,13 +2651,13 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List } final Account account = _accountDao.findById(user.getAccountId()); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (domainIds == null || domainIds.isEmpty()) { + if (filteredDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to create public disk offering by id " + userId + " because it is domain-admin"); } if (tags != null) { throw new InvalidParameterValueException("Unable to create disk offering with storage tags by id " + userId + " because it is domain-admin"); } - for (Long domainId : domainIds) { + for (Long domainId : filteredDomainIds) { if (domainId == null || !_domainDao.isChildDomain(account.getDomainId(), domainId)) { throw new InvalidParameterValueException("Unable to create disk offering by another domain admin with id " + userId); } @@ -2704,9 +2725,9 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List CallContext.current().setEventDetails("Disk offering id=" + newDiskOffering.getId()); final DiskOfferingVO offering = _diskOfferingDao.persist(newDiskOffering); if (offering != null) { - if(domainIds!=null && !domainIds.isEmpty()) { + if(!filteredDomainIds.isEmpty()) { List domainIdsStringList = new ArrayList<>(); - for(Long domainId : domainIds) + for(Long domainId : filteredDomainIds) domainIdsStringList.add(String.valueOf(domainId)); diskOfferingDetailsDao.addDetail(offering.getId(), ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsStringList), true); } @@ -2817,8 +2838,31 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { } final Account account = _accountDao.findById(user.getAccountId()); final List domainIds = cmd.getDomainIds(); + + // Filter child domains when both parent and child domains are present + List filteredDomainIds = new ArrayList<>(); + if (domainIds != null) { + filteredDomainIds.addAll(domainIds); + } + if (filteredDomainIds.size() > 1) { + for (int i = filteredDomainIds.size() - 1; i >= 1; i--) { + long first = filteredDomainIds.get(i); + for (int j = i - 1; j >= 0; j--) { + long second = filteredDomainIds.get(j); + if (_domainDao.isChildDomain(filteredDomainIds.get(i), filteredDomainIds.get(j))) { + filteredDomainIds.remove(j); + i--; + } + if (_domainDao.isChildDomain(filteredDomainIds.get(j), filteredDomainIds.get(i))) { + filteredDomainIds.remove(i); + break; + } + } + } + } + if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { - if (domainIds == null) { + if (filteredDomainIds.isEmpty()) { if (existingDomainIds.isEmpty()) { throw new InvalidParameterValueException("Unable to update public disk offering by id " + userId + " because it is domain-admin"); } @@ -2828,7 +2872,7 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { } } } else { - for (Long domainId : domainIds) { + for (Long domainId : filteredDomainIds) { if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) { throw new InvalidParameterValueException("Unable to update disk offering by another domain admin with id " + userId); } @@ -2847,7 +2891,7 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { } final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null; - final boolean detailsUpdateNeeded = (domainIds != null && !domainIds.isEmpty()) || (zoneIds != null && !zoneIds.isEmpty()); + final boolean detailsUpdateNeeded = (!filteredDomainIds.isEmpty()) || (zoneIds != null && !zoneIds.isEmpty()); if (!updateNeeded && !detailsUpdateNeeded) { return _diskOfferingDao.findById(diskOfferingId); } @@ -2896,9 +2940,9 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { // } if (!updateNeeded || _diskOfferingDao.update(diskOfferingId, diskOffering)) { - if(domainIds!=null && !domainIds.isEmpty()) { + if(!filteredDomainIds.isEmpty()) { List domainIdsStringList = new ArrayList<>(); - for(Long domainId : domainIds) + for(Long domainId : filteredDomainIds) domainIdsStringList.add(String.valueOf(domainId)); diskOfferingDetailsDao.addDetail(diskOfferingId, ApiConstants.DOMAIN_ID_LIST, String.join(",", domainIdsStringList), true); } From fc38f7533d71aeff527bbdcca5540ad6b9dcfde9 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 27 Mar 2019 14:12:35 +0530 Subject: [PATCH 08/12] ui: fixes for domain, zone selection in create disk offering Fixes missing labels. Decouples zone selection with 'Public' check in create disk offering form. Signed-off-by: Abhishek Kumar --- ui/l10n/en.js | 1 + ui/scripts/configuration.js | 51 +++++++++++++++---------------------- ui/scripts/docs.js | 2 +- ui/scripts/storage.js | 20 +++++++++++++-- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/ui/l10n/en.js b/ui/l10n/en.js index eb7b82250891..cd0e01635f4d 100644 --- a/ui/l10n/en.js +++ b/ui/l10n/en.js @@ -705,6 +705,7 @@ var dictionary = { "label.dns.1":"DNS 1", "label.dns.2":"DNS 2", "label.domain":"Domain", +"label.domains":"Domains", "label.domain.admin":"Domain Admin", "label.domain.details":"Domain details", "label.domain.id":"Domain ID", diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js index 55ef33270f02..7417b26de9ee 100644 --- a/ui/scripts/configuration.js +++ b/ui/scripts/configuration.js @@ -2032,8 +2032,6 @@ label: 'label.zone', docID: 'helpDiskOfferingZone', isMultiple: true, - isHidden: true, - dependsOn: 'isPublic', validation: { allzonesonly: true }, @@ -2147,16 +2145,7 @@ if (args.data.isPublic != "on") { var domains = ""; if (Object.prototype.toString.call(args.data.domain) === '[object Array]') { - var rootDomainSelected = false; - args.data.domain.forEach(function (domain) { - if (domain === null) { - rootDomainSelected = true; - break; - } - }); - if(!rootDomainSelected) { - domains = args.data.domain.join(","); - } + domains = args.data.domain.join(","); } else { if (args.data.domain != null) { domains = args.data.domain; @@ -2167,30 +2156,30 @@ domainids: domains }); } + } - var zones = ""; - if (Object.prototype.toString.call(args.data.zone) === '[object Array]') { - var allZonesSelected = false; - args.data.zone.forEach(function (zone) { - if (zone === null) { - allZonesSelected = true; - break; - } - }); - if(!allZonesSelected) { - zones = args.data.zone.join(","); - } - } else { - if (args.data.zone != null) { - zones = args.data.zone; + var zones = ""; + if (Object.prototype.toString.call(args.data.zone) === '[object Array]') { + var allZonesSelected = false; + args.data.zone.forEach(function (zone) { + if (zone === null) { + allZonesSelected = true; + break; } + }); + if(!allZonesSelected) { + zones = args.data.zone.join(","); } - if (zones != "") { - $.extend(data, { - zoneids: zones - }); + } else { + if (args.data.zone != null) { + zones = args.data.zone; } } + if (zones != "") { + $.extend(data, { + zoneids: zones + }); + } $.ajax({ url: createURL('createDiskOffering'), diff --git a/ui/scripts/docs.js b/ui/scripts/docs.js index e55cc95e6dfd..e49fe3dd2a3a 100755 --- a/ui/scripts/docs.js +++ b/ui/scripts/docs.js @@ -383,7 +383,7 @@ cloudStack.docs = { externalLink: '' }, helpDiskOfferingDomain: { - desc: 'Select the subdomain in which this offering is available', + desc: 'Select the domains in which this offering is available (Tip: Use Ctrl to choose multiple domains)', externalLink: '' }, helpDiskOfferingZone: { diff --git a/ui/scripts/storage.js b/ui/scripts/storage.js index 06fe41e2f712..b3ad98f8e5e1 100644 --- a/ui/scripts/storage.js +++ b/ui/scripts/storage.js @@ -173,7 +173,7 @@ }); } - var selectedDiskOfferingObj = null; + var diskOfferingsObjList, selectedDiskOfferingObj = null; cloudStack.sections.storage = { title: 'label.storage', @@ -279,6 +279,21 @@ }); } }); + args.$select.change(function() { + var diskOfferingSelect = $(this).closest('form').find('select[name=diskOffering]'); + if(diskOfferingSelect) { + $(diskOfferingSelect).find('option').remove().end(); + var data = { + zoneid: $(this).val(), + }; + console.log(data); + var diskOfferings = cloudStack.listDiskOfferings({ data: data }); + diskOfferingsObjList = diskOfferings; + $(diskOfferings).each(function() { + $(diskOfferingSelect).append(new Option(this.displaytext, this.id)); + }); + } + }); } }, diskOffering: { @@ -286,6 +301,7 @@ docID: 'helpVolumeDiskOffering', select: function(args) { var diskOfferings = cloudStack.listDiskOfferings({}); + diskOfferingsObjList = diskOfferings; var items = []; $(diskOfferings).each(function() { items.push({ @@ -298,7 +314,7 @@ }); args.$select.change(function() { var diskOfferingId = $(this).val(); - $(diskOfferings).each(function() { + $(diskOfferingsObjList).each(function() { if (this.id == diskOfferingId) { selectedDiskOfferingObj = this; return false; //break the $.each() loop From 8e79cb18891f7283ae6f85614b3ee4f6cf34b8e1 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 27 Mar 2019 15:40:37 +0530 Subject: [PATCH 09/12] api: fix single domainid for create, update disk offering createDiskOffering and updateDiskOffering will allow using both 'domainid' for signle domain association with offering and 'domainds' for multiple domains. Signed-off-by: Abhishek Kumar --- .../api/command/admin/offering/CreateDiskOfferingCmd.java | 7 +++++++ .../api/command/admin/offering/UpdateDiskOfferingCmd.java | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java index 78241ce62f21..57616a6b3f02 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; +import java.util.ArrayList; import java.util.List; import org.apache.cloudstack.api.APICommand; @@ -191,6 +192,12 @@ public Long getDomainId() { } public List getDomainIds() { + if (domainId != null) { + if (domainIds == null) { + domainIds = new ArrayList<>(); + } + domainIds.add(domainId); + } return domainIds; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java index 5e28c7987f07..12a63f4b5aea 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; +import java.util.ArrayList; import java.util.List; import org.apache.cloudstack.api.response.DomainResponse; @@ -116,6 +117,12 @@ public Long getDomainId() { } public List getDomainIds() { + if (domainId != null) { + if (domainIds == null) { + domainIds = new ArrayList<>(); + } + domainIds.add(domainId); + } return domainIds; } From 09a2d3bf0cdf62c4c8f1b8b110d6fb92525aee8e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 28 Mar 2019 11:51:23 +0530 Subject: [PATCH 10/12] server: refactoring for child domain filtering multiple use Signed-off-by: Abhishek Kumar --- .../ConfigurationManagerImpl.java | 66 ++++++++----------- 1 file changed, 26 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index d32039e98256..fa7a2dba9ff0 100755 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -2623,26 +2623,7 @@ protected DiskOfferingVO createDiskOffering(final Long userId, final List } // Filter child domains when both parent and child domains are present - List filteredDomainIds = new ArrayList<>(); - if (domainIds != null) { - filteredDomainIds.addAll(domainIds); - } - if (filteredDomainIds.size() > 1) { - for (int i = filteredDomainIds.size() - 1; i >= 1; i--) { - long first = filteredDomainIds.get(i); - for (int j = i - 1; j >= 0; j--) { - long second = filteredDomainIds.get(j); - if (_domainDao.isChildDomain(filteredDomainIds.get(i), filteredDomainIds.get(j))) { - filteredDomainIds.remove(j); - i--; - } - if (_domainDao.isChildDomain(filteredDomainIds.get(j), filteredDomainIds.get(i))) { - filteredDomainIds.remove(i); - break; - } - } - } - } + List filteredDomainIds = filterChildSubDomains(domainIds); // Check if user exists in the system final User user = _userDao.findById(userId); @@ -2840,26 +2821,7 @@ public DiskOffering updateDiskOffering(final UpdateDiskOfferingCmd cmd) { final List domainIds = cmd.getDomainIds(); // Filter child domains when both parent and child domains are present - List filteredDomainIds = new ArrayList<>(); - if (domainIds != null) { - filteredDomainIds.addAll(domainIds); - } - if (filteredDomainIds.size() > 1) { - for (int i = filteredDomainIds.size() - 1; i >= 1; i--) { - long first = filteredDomainIds.get(i); - for (int j = i - 1; j >= 0; j--) { - long second = filteredDomainIds.get(j); - if (_domainDao.isChildDomain(filteredDomainIds.get(i), filteredDomainIds.get(j))) { - filteredDomainIds.remove(j); - i--; - } - if (_domainDao.isChildDomain(filteredDomainIds.get(j), filteredDomainIds.get(i))) { - filteredDomainIds.remove(i); - break; - } - } - } - } + List filteredDomainIds = filterChildSubDomains(domainIds); if (account.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) { if (filteredDomainIds.isEmpty()) { @@ -5841,6 +5803,30 @@ private boolean checkOverlapPortableIpRange(final int regionId, final String new return false; } + private List filterChildSubDomains(final List domainIds) { + List filteredDomainIds = new ArrayList<>(); + if (domainIds != null) { + filteredDomainIds.addAll(domainIds); + } + if (filteredDomainIds.size() > 1) { + for (int i = filteredDomainIds.size() - 1; i >= 1; i--) { + long first = filteredDomainIds.get(i); + for (int j = i - 1; j >= 0; j--) { + long second = filteredDomainIds.get(j); + if (_domainDao.isChildDomain(filteredDomainIds.get(i), filteredDomainIds.get(j))) { + filteredDomainIds.remove(j); + i--; + } + if (_domainDao.isChildDomain(filteredDomainIds.get(j), filteredDomainIds.get(i))) { + filteredDomainIds.remove(i); + break; + } + } + } + } + return filteredDomainIds; + } + public List getSecChecker() { return _secChecker; } From 270c76b78d7134b88ee4660af9875f1a6ff52c6d Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 28 Mar 2019 12:22:55 +0530 Subject: [PATCH 11/12] api: fixed create/update diskoffering API parameter access Signed-off-by: Abhishek Kumar --- .../api/command/admin/offering/CreateDiskOfferingCmd.java | 4 ++-- .../api/command/admin/offering/UpdateDiskOfferingCmd.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java index 57616a6b3f02..3976a7584eaa 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java @@ -74,7 +74,7 @@ public class CreateDiskOfferingCmd extends BaseCmd { required = false, description = "the ID of the domains offering is associated with, null for all domain offerings", since = "4.13") - protected List domainIds; + private List domainIds; @Parameter(name = ApiConstants.ZONE_ID_LIST, type = CommandType.LIST, @@ -83,7 +83,7 @@ public class CreateDiskOfferingCmd extends BaseCmd { required = false, description = "the ID of the zones offering is associated with, null for all zone offerings", since = "4.13") - protected List zoneIds; + private List zoneIds; @Parameter(name = ApiConstants.STORAGE_TYPE, type = CommandType.STRING, description = "the storage type of the disk offering. Values are local and shared.") private String storageType = ServiceOffering.StorageType.shared.toString(); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java index 12a63f4b5aea..39080652da72 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java @@ -77,7 +77,7 @@ public class UpdateDiskOfferingCmd extends BaseCmd { required = false, description = "the ID of the domains offering is associated with, null for all domain offerings", since = "4.13") - protected List domainIds; + private List domainIds; @Parameter(name = ApiConstants.ZONE_ID_LIST, type = CommandType.LIST, @@ -86,7 +86,7 @@ public class UpdateDiskOfferingCmd extends BaseCmd { required = false, description = "the ID of the zones offering is associated with, null for all zone offerings", since = "4.13") - protected List zoneIds; + private List zoneIds; ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// From ea163b824117ca7bfe8e6ec944995673120eb472 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 28 Mar 2019 15:50:44 +0530 Subject: [PATCH 12/12] api: removed domainid parameter for updateDiskOffering API Signed-off-by: Abhishek Kumar --- .../admin/offering/UpdateDiskOfferingCmd.java | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java index 39080652da72..14190b46916b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java @@ -16,13 +16,8 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; -import java.util.ArrayList; import java.util.List; -import org.apache.cloudstack.api.response.DomainResponse; -import org.apache.cloudstack.api.response.ZoneResponse; -import org.apache.log4j.Logger; - import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; @@ -30,6 +25,9 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DiskOfferingResponse; +import org.apache.cloudstack.api.response.DomainResponse; +import org.apache.cloudstack.api.response.ZoneResponse; +import org.apache.log4j.Logger; import com.cloud.offering.DiskOffering; import com.cloud.user.Account; @@ -64,12 +62,6 @@ public class UpdateDiskOfferingCmd extends BaseCmd { description = "an optional field, whether to display the offering to the end user or not.") private Boolean displayOffering; - @Parameter(name = ApiConstants.DOMAIN_ID, - type = CommandType.UUID, - entityType = DomainResponse.class, - description = "the ID of the containing domain, null for public offerings") - private Long domainId; - @Parameter(name = ApiConstants.DOMAIN_ID_LIST, type = CommandType.LIST, collectionType = CommandType.UUID, @@ -112,17 +104,7 @@ public Boolean getDisplayOffering() { return displayOffering; } - public Long getDomainId() { - return domainId; - } - public List getDomainIds() { - if (domainId != null) { - if (domainIds == null) { - domainIds = new ArrayList<>(); - } - domainIds.add(domainId); - } return domainIds; }