From 7385d8ce74ab2979f4535cf49c296da5fbfc4ff4 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 25 Nov 2025 12:07:39 -0500 Subject: [PATCH 01/14] Add support for dedicating backup offerings to domains --- .../java/com/cloud/user/AccountService.java | 3 + .../cloudstack/acl/SecurityChecker.java | 6 + .../admin/backup/ImportBackupOfferingCmd.java | 22 ++++ .../admin/backup/UpdateBackupOfferingCmd.java | 16 ++- .../network/UpdateNetworkOfferingCmd.java | 65 +--------- .../admin/offering/UpdateDiskOfferingCmd.java | 62 +--------- .../offering/UpdateServiceOfferingCmd.java | 62 +--------- .../admin/vpc/UpdateVPCOfferingCmd.java | 64 +--------- .../offering/DomainAndZoneIdResolver.java | 115 ++++++++++++++++++ .../cloudstack/backup/BackupManager.java | 2 + .../backup/BackupOfferingDetailsVO.java | 71 +++++++++++ .../backup/dao/BackupOfferingDetailsDao.java | 31 +++++ .../dao/BackupOfferingDetailsDaoImpl.java | 76 ++++++++++++ ...s-between-management-and-usage-context.xml | 3 +- .../META-INF/db/schema-42200to42300.sql | 10 ++ .../management/MockAccountManager.java | 6 + .../java/com/cloud/acl/DomainChecker.java | 36 ++++++ .../ConfigurationManagerImpl.java | 2 - .../com/cloud/user/AccountManagerImpl.java | 16 +++ .../cloudstack/backup/BackupManagerImpl.java | 72 ++++++++++- .../com/cloud/vm/UserVmManagerImplTest.java | 3 +- .../cloudstack/backup/BackupManagerTest.java | 4 +- tools/marvin/setup.py | 2 +- .../cloud/utils/component/ManagerBase.java | 2 + 24 files changed, 506 insertions(+), 245 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/offering/DomainAndZoneIdResolver.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/backup/BackupOfferingDetailsVO.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDao.java create mode 100644 engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDaoImpl.java diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index c0ebcf09f59b..4cc418e8fc9b 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -36,6 +36,7 @@ import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; +import org.apache.cloudstack.backup.BackupOffering; public interface AccountService { @@ -115,6 +116,8 @@ User createUser(String userName, String password, String firstName, String lastN void checkAccess(Account account, VpcOffering vof, DataCenter zone) throws PermissionDeniedException; + void checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException; + void checkAccess(User user, ControlledEntity entity); void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) throws PermissionDeniedException; 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 82a8ec5fe932..a3073181dd64 100644 --- a/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java +++ b/api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java @@ -27,6 +27,8 @@ import com.cloud.user.User; import com.cloud.utils.component.Adapter; +import org.apache.cloudstack.backup.BackupOffering; + /** * SecurityChecker checks the ownership and access control to objects within */ @@ -145,4 +147,8 @@ boolean checkAccess(Account caller, AccessType accessType, String action, Contro boolean checkAccess(Account account, NetworkOffering nof, DataCenter zone) throws PermissionDeniedException; boolean checkAccess(Account account, VpcOffering vof, DataCenter zone) throws PermissionDeniedException; + + default boolean checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException { + return true; + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java index 7d3902bc4902..18aeade9e5ba 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java @@ -27,6 +27,7 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.BackupOfferingResponse; +import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.backup.BackupManager; import org.apache.cloudstack.backup.BackupOffering; @@ -40,6 +41,11 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.commons.collections.CollectionUtils; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; @APICommand(name = "importBackupOffering", description = "Imports a backup offering using a backup provider", @@ -76,6 +82,13 @@ public class ImportBackupOfferingCmd extends BaseAsyncCmd { description = "Whether users are allowed to create adhoc backups and backup schedules", required = true) private Boolean userDrivenBackups; + @Parameter(name = ApiConstants.DOMAIN_ID, + type = CommandType.LIST, + collectionType = CommandType.UUID, + entityType = DomainResponse.class, + description = "the ID of the containing domain(s), null for public offerings") + private List domainIds; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -100,6 +113,15 @@ public Boolean getUserDrivenBackups() { return userDrivenBackups == null ? false : userDrivenBackups; } + public List getDomainIds() { + if (CollectionUtils.isNotEmpty(domainIds)) { + Set set = new LinkedHashSet<>(domainIds); + domainIds.clear(); + domainIds.addAll(set); + } + return domainIds; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java index 9de06715ee74..682f2f595ade 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java @@ -25,6 +25,7 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver; import org.apache.cloudstack.api.response.BackupOfferingResponse; import org.apache.cloudstack.backup.BackupManager; import org.apache.cloudstack.backup.BackupOffering; @@ -35,9 +36,11 @@ import com.cloud.user.Account; import com.cloud.utils.exception.CloudRuntimeException; +import java.util.List; + @APICommand(name = "updateBackupOffering", description = "Updates a backup offering.", responseObject = BackupOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") -public class UpdateBackupOfferingCmd extends BaseCmd { +public class UpdateBackupOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver { @Inject private BackupManager backupManager; @@ -57,6 +60,13 @@ public class UpdateBackupOfferingCmd extends BaseCmd { @Parameter(name = ApiConstants.ALLOW_USER_DRIVEN_BACKUPS, type = CommandType.BOOLEAN, description = "Whether to allow user driven backups or not") private Boolean allowUserDrivenBackups; + @Parameter(name = ApiConstants.DOMAIN_ID, + type = CommandType.STRING, + description = "the ID of the containing domain(s) as comma separated string, public for public offerings", + since = "4.23.0", + length = 4096) + private String domainIds; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -103,6 +113,10 @@ public void execute() { } } + public List getDomainIds() { + return resolveDomainIds(domainIds, id, backupManager::getBackupOfferingDomains, "backup offering"); + } + @Override public long getEntityOwnerId() { return Account.ACCOUNT_ID_SYSTEM; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java index 75fb45e1f115..67a8896eb908 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.network; -import java.util.ArrayList; import java.util.List; import org.apache.cloudstack.api.APICommand; @@ -26,18 +25,16 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver; import org.apache.cloudstack.api.response.NetworkOfferingResponse; -import org.apache.commons.lang3.StringUtils; -import com.cloud.dc.DataCenter; -import com.cloud.domain.Domain; -import com.cloud.exception.InvalidParameterValueException; + import com.cloud.offering.NetworkOffering; import com.cloud.user.Account; @APICommand(name = "updateNetworkOffering", description = "Updates a network offering.", responseObject = NetworkOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class UpdateNetworkOfferingCmd extends BaseCmd { +public class UpdateNetworkOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver { ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -129,63 +126,11 @@ public String getTags() { } public List getDomainIds() { - List validDomainIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(domainIds)) { - if (domainIds.contains(",")) { - String[] domains = domainIds.split(","); - for (String domain : domains) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create network offering because invalid domain has been specified."); - } - } - } else { - domainIds = domainIds.trim(); - if (!domainIds.matches("public")) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create network offering because invalid domain has been specified."); - } - } - } - } else { - validDomainIds.addAll(_configService.getNetworkOfferingDomains(id)); - } - return validDomainIds; + return resolveDomainIds(domainIds, id, _configService::getNetworkOfferingDomains, "network offering"); } public List getZoneIds() { - List validZoneIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(zoneIds)) { - if (zoneIds.contains(",")) { - String[] zones = zoneIds.split(","); - for (String zone : zones) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create network offering because invalid zone has been specified."); - } - } - } else { - zoneIds = zoneIds.trim(); - if (!zoneIds.matches("all")) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create network offering because invalid zone has been specified."); - } - } - } - } else { - validZoneIds.addAll(_configService.getNetworkOfferingZones(id)); - } - return validZoneIds; + return resolveZoneIds(zoneIds, id, _configService::getNetworkOfferingZones, "network offering"); } ///////////////////////////////////////////////////// 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 370453804cfc..685022bdae46 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,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; -import java.util.ArrayList; import java.util.List; import com.cloud.offering.DiskOffering.State; @@ -27,19 +26,18 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver; import org.apache.cloudstack.api.response.DiskOfferingResponse; import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; -import com.cloud.dc.DataCenter; -import com.cloud.domain.Domain; import com.cloud.exception.InvalidParameterValueException; import com.cloud.offering.DiskOffering; import com.cloud.user.Account; @APICommand(name = "updateDiskOffering", description = "Updates a disk offering.", responseObject = DiskOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class UpdateDiskOfferingCmd extends BaseCmd { +public class UpdateDiskOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver { ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -151,63 +149,11 @@ public Boolean getDisplayOffering() { } public List getDomainIds() { - List validDomainIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(domainIds)) { - if (domainIds.contains(",")) { - String[] domains = domainIds.split(","); - for (String domain : domains) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create disk offering because invalid domain has been specified."); - } - } - } else { - domainIds = domainIds.trim(); - if (!domainIds.matches("public")) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create disk offering because invalid domain has been specified."); - } - } - } - } else { - validDomainIds.addAll(_configService.getDiskOfferingDomains(id)); - } - return validDomainIds; + return resolveDomainIds(domainIds, id, _configService::getDiskOfferingDomains, "disk offering"); } public List getZoneIds() { - List validZoneIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(zoneIds)) { - if (zoneIds.contains(",")) { - String[] zones = zoneIds.split(","); - for (String zone : zones) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create disk offering because invalid zone has been specified."); - } - } - } else { - zoneIds = zoneIds.trim(); - if (!zoneIds.matches("all")) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create disk offering because invalid zone has been specified."); - } - } - } - } else { - validZoneIds.addAll(_configService.getDiskOfferingZones(id)); - } - return validZoneIds; + return resolveZoneIds(zoneIds, id, _configService::getDiskOfferingZones, "disk offering"); } public String getTags() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java index 9d973dfc524b..b2db8101a80b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.offering; -import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -28,19 +27,18 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver; import org.apache.cloudstack.api.response.ServiceOfferingResponse; import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; -import com.cloud.dc.DataCenter; -import com.cloud.domain.Domain; import com.cloud.exception.InvalidParameterValueException; import com.cloud.offering.ServiceOffering; import com.cloud.user.Account; @APICommand(name = "updateServiceOffering", description = "Updates a service offering.", responseObject = ServiceOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class UpdateServiceOfferingCmd extends BaseCmd { +public class UpdateServiceOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver { ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -130,63 +128,11 @@ public Integer getSortKey() { } public List getDomainIds() { - List validDomainIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(domainIds)) { - if (domainIds.contains(",")) { - String[] domains = domainIds.split(","); - for (String domain : domains) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create service offering because invalid domain has been specified."); - } - } - } else { - domainIds = domainIds.trim(); - if (!domainIds.matches("public")) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create service offering because invalid domain has been specified."); - } - } - } - } else { - validDomainIds.addAll(_configService.getServiceOfferingDomains(id)); - } - return validDomainIds; + return resolveDomainIds(domainIds, id, _configService::getServiceOfferingDomains, "service offering"); } public List getZoneIds() { - List validZoneIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(zoneIds)) { - if (zoneIds.contains(",")) { - String[] zones = zoneIds.split(","); - for (String zone : zones) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create service offering because invalid zone has been specified."); - } - } - } else { - zoneIds = zoneIds.trim(); - if (!zoneIds.matches("all")) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create service offering because invalid zone has been specified."); - } - } - } - } else { - validZoneIds.addAll(_configService.getServiceOfferingZones(id)); - } - return validZoneIds; + return resolveZoneIds(zoneIds, id, _configService::getServiceOfferingZones, "service offering"); } public String getStorageTags() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCOfferingCmd.java index b59837281ef3..4efaf532ee2b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/UpdateVPCOfferingCmd.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.vpc; -import java.util.ArrayList; import java.util.List; import org.apache.cloudstack.api.APICommand; @@ -26,19 +25,16 @@ import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver; import org.apache.cloudstack.api.response.VpcOfferingResponse; -import org.apache.commons.lang3.StringUtils; -import com.cloud.dc.DataCenter; -import com.cloud.domain.Domain; import com.cloud.event.EventTypes; -import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.vpc.VpcOffering; import com.cloud.user.Account; @APICommand(name = "updateVPCOffering", description = "Updates VPC offering", responseObject = VpcOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class UpdateVPCOfferingCmd extends BaseAsyncCmd { +public class UpdateVPCOfferingCmd extends BaseAsyncCmd implements DomainAndZoneIdResolver { ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -92,63 +88,11 @@ public String getState() { } public List getDomainIds() { - List validDomainIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(domainIds)) { - if (domainIds.contains(",")) { - String[] domains = domainIds.split(","); - for (String domain : domains) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create VPC offering because invalid domain has been specified."); - } - } - } else { - domainIds = domainIds.trim(); - if (!domainIds.matches("public")) { - Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim()); - if (validDomain != null) { - validDomainIds.add(validDomain.getId()); - } else { - throw new InvalidParameterValueException("Failed to create VPC offering because invalid domain has been specified."); - } - } - } - } else { - validDomainIds.addAll(_vpcProvSvc.getVpcOfferingDomains(id)); - } - return validDomainIds; + return resolveDomainIds(domainIds, id, _vpcProvSvc::getVpcOfferingDomains, "VPC offering"); } public List getZoneIds() { - List validZoneIds = new ArrayList<>(); - if (StringUtils.isNotEmpty(zoneIds)) { - if (zoneIds.contains(",")) { - String[] zones = zoneIds.split(","); - for (String zone : zones) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create VPC offering because invalid zone has been specified."); - } - } - } else { - zoneIds = zoneIds.trim(); - if (!zoneIds.matches("all")) { - DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim()); - if (validZone != null) { - validZoneIds.add(validZone.getId()); - } else { - throw new InvalidParameterValueException("Failed to create VPC offering because invalid zone has been specified."); - } - } - } - } else { - validZoneIds.addAll(_vpcProvSvc.getVpcOfferingZones(id)); - } - return validZoneIds; + return resolveZoneIds(zoneIds, id, _vpcProvSvc::getVpcOfferingZones, "VPC offering"); } public Integer getSortKey() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/offering/DomainAndZoneIdResolver.java b/api/src/main/java/org/apache/cloudstack/api/command/offering/DomainAndZoneIdResolver.java new file mode 100644 index 000000000000..8f02296016f1 --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/offering/DomainAndZoneIdResolver.java @@ -0,0 +1,115 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.offering; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.LongFunction; + +import com.cloud.dc.DataCenter; +import com.cloud.domain.Domain; +import com.cloud.exception.InvalidParameterValueException; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * Helper for commands that accept a domainIds or zoneIds string and need to + * resolve them to lists of IDs, falling back to an offering-specific + * default provider. + */ +public interface DomainAndZoneIdResolver { + /** + * Parse the provided domainIds string and return a list of domain IDs. + * If domainIds is empty, the defaultDomainsProvider will be invoked with the + * provided resource id to obtain the current domains. + */ + default List resolveDomainIds(final String domainIds, final Long id, final LongFunction> defaultDomainsProvider, final String resourceTypeName) { + final List validDomainIds = new ArrayList<>(); + final BaseCmd base = (BaseCmd) this; + final Logger logger = LogManager.getLogger(base.getClass()); + + if (StringUtils.isEmpty(domainIds)) { + if (defaultDomainsProvider != null) { + final List defaults = defaultDomainsProvider.apply(id); + if (defaults != null) { + validDomainIds.addAll(defaults); + } + } + return validDomainIds; + } + + final String[] domains = domainIds.split(","); + final String type = (resourceTypeName == null || resourceTypeName.isEmpty()) ? "offering" : resourceTypeName; + for (String domain : domains) { + final String trimmed = domain == null ? "" : domain.trim(); + if (trimmed.isEmpty() || "public".equalsIgnoreCase(trimmed)) { + continue; + } + + final Domain validDomain = base._entityMgr.findByUuid(Domain.class, trimmed); + if (validDomain == null) { + logger.warn("Invalid domain specified for {}: {}", type, trimmed); + throw new InvalidParameterValueException("Failed to create " + type + " because invalid domain has been specified."); + } + validDomainIds.add(validDomain.getId()); + } + + return validDomainIds; + } + + /** + * Parse the provided zoneIds string and return a list of zone IDs. + * If zoneIds is empty, the defaultZonesProvider will be invoked with the + * provided resource id to obtain the current zones. + */ + default List resolveZoneIds(final String zoneIds, final Long id, final LongFunction> defaultZonesProvider, final String resourceTypeName) { + final List validZoneIds = new ArrayList<>(); + final BaseCmd base = (BaseCmd) this; + final Logger logger = LogManager.getLogger(base.getClass()); + + if (StringUtils.isEmpty(zoneIds)) { + if (defaultZonesProvider != null) { + final List defaults = defaultZonesProvider.apply(id); + if (defaults != null) { + validZoneIds.addAll(defaults); + } + } + return validZoneIds; + } + + final String[] zones = zoneIds.split(","); + final String type = (resourceTypeName == null || resourceTypeName.isEmpty()) ? "offering" : resourceTypeName; + for (String zone : zones) { + final String trimmed = zone == null ? "" : zone.trim(); + if (trimmed.isEmpty() || "all".equalsIgnoreCase(trimmed)) { + continue; + } + + final DataCenter validZone = base._entityMgr.findByUuid(DataCenter.class, trimmed); + if (validZone == null) { + logger.warn("Invalid zone specified for {}: {}", type, trimmed); + throw new InvalidParameterValueException("Failed to create " + type + " because invalid zone has been specified."); + } + validZoneIds.add(validZone.getId()); + } + + return validZoneIds; + } +} + diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index db051313d962..cbaf61405970 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -136,6 +136,8 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer */ BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd); + List getBackupOfferingDomains(final Long offeringId); + /** * List backup offerings * @param ListBackupOfferingsCmd API cmd diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupOfferingDetailsVO.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupOfferingDetailsVO.java new file mode 100644 index 000000000000..003898ff5be8 --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/BackupOfferingDetailsVO.java @@ -0,0 +1,71 @@ +package org.apache.cloudstack.backup; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; + +import org.apache.cloudstack.api.ResourceDetail; + +@Entity +@Table(name = "backup_offering_details") +public class BackupOfferingDetailsVO implements ResourceDetail { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private long id; + + @Column(name = "backup_offering_id") + private long resourceId; + + @Column(name = "name") + private String name; + + @Column(name = "value") + private String value; + + @Column(name = "display") + private boolean display = true; + + protected BackupOfferingDetailsVO() { + } + + public BackupOfferingDetailsVO(long backupOfferingId, String name, String value, boolean display) { + this.resourceId = backupOfferingId; + this.name = name; + this.value = value; + this.display = display; + } + + @Override + public long getResourceId() { + return resourceId; + } + + public void setResourceId(long backupOfferingId) { + this.resourceId = backupOfferingId; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getValue() { + return value; + } + + @Override + public long getId() { + return id; + } + + @Override + public boolean isDisplay() { + return display; + } +} + diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDao.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDao.java new file mode 100644 index 000000000000..e796712dea6c --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDao.java @@ -0,0 +1,31 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.backup.dao; + +import java.util.List; + +import org.apache.cloudstack.backup.BackupOfferingDetailsVO; +import org.apache.cloudstack.resourcedetail.ResourceDetailsDao; + +import com.cloud.utils.db.GenericDao; + +public interface BackupOfferingDetailsDao extends GenericDao, ResourceDetailsDao { + List findDomainIds(final long resourceId); + List findZoneIds(final long resourceId); + String getDetail(Long backupOfferingId, String key); + List findOfferingIdsByDomainIds(List domainIds); +} \ No newline at end of file diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDaoImpl.java new file mode 100644 index 000000000000..b48de399a9d3 --- /dev/null +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDetailsDaoImpl.java @@ -0,0 +1,76 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.backup.dao; + + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.backup.BackupOfferingDetailsVO; +import org.apache.cloudstack.resourcedetail.ResourceDetailsDaoBase; +import org.springframework.stereotype.Component; + +@Component +public class BackupOfferingDetailsDaoImpl extends ResourceDetailsDaoBase implements BackupOfferingDetailsDao { + + @Override + public void addDetail(long resourceId, String key, String value, boolean display) { + super.addDetail(new BackupOfferingDetailsVO(resourceId, key, value, display)); + } + + @Override + public List findDomainIds(long resourceId) { + final List domainIds = new ArrayList<>(); + for (final BackupOfferingDetailsVO detail: findDetails(resourceId, ApiConstants.DOMAIN_ID)) { + final Long domainId = Long.valueOf(detail.getValue()); + if (domainId > 0) { + domainIds.add(domainId); + } + } + return domainIds; + } + + @Override + public List findZoneIds(long resourceId) { + final List zoneIds = new ArrayList<>(); + for (final BackupOfferingDetailsVO detail: findDetails(resourceId, ApiConstants.ZONE_ID)) { + final Long zoneId = Long.valueOf(detail.getValue()); + if (zoneId > 0) { + zoneIds.add(zoneId); + } + } + return zoneIds; + } + + @Override + public String getDetail(Long backupOfferingId, String key) { + String detailValue = null; + BackupOfferingDetailsVO backupOfferingDetail = findDetail(backupOfferingId, key); + if (backupOfferingDetail != null) { + detailValue = backupOfferingDetail.getValue(); + } + return detailValue; + } + + @Override + public List findOfferingIdsByDomainIds(List domainIds) { + Object[] dIds = domainIds.stream().map(s -> String.valueOf(s)).collect(Collectors.toList()).toArray(); + return findResourceIdsByNameAndValueIn("domainid", dIds); + } +} diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-common-daos-between-management-and-usage-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-common-daos-between-management-and-usage-context.xml index d308a9e5aaf9..1846c3c62a0e 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-common-daos-between-management-and-usage-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-common-daos-between-management-and-usage-context.xml @@ -71,6 +71,7 @@ - + + diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42200to42300.sql b/engine/schema/src/main/resources/META-INF/db/schema-42200to42300.sql index c1f1bb2c094d..b6299bc37993 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42200to42300.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42200to42300.sql @@ -18,3 +18,13 @@ --; -- Schema upgrade from 4.22.0.0 to 4.23.0.0 --; + +CREATE TABLE `cloud`.`backup_offering_details` ( + `id` bigint unsigned NOT NULL auto_increment, + `backup_offering_id` bigint unsigned NOT NULL COMMENT 'Backup offering id', + `name` varchar(255) NOT NULL, + `value` varchar(1024) NOT NULL, + `display` tinyint(1) NOT NULL DEFAULT 1 COMMENT 'Should detail be displayed to the end user', + PRIMARY KEY (`id`), + CONSTRAINT `fk_offering_details__backup_offering_id` FOREIGN KEY `fk_offering_details__backup_offering_id`(`backup_offering_id`) REFERENCES `backup_offering`(`id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8; \ No newline at end of file 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 bc9dbfa7b436..684b379cf50d 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 @@ -30,6 +30,7 @@ import org.apache.cloudstack.api.command.admin.user.MoveUserCmd; import org.apache.cloudstack.api.response.UserTwoFactorAuthenticationSetupResponse; import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; +import org.apache.cloudstack.backup.BackupOffering; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.acl.ControlledEntity; @@ -491,6 +492,11 @@ public void checkAccess(Account account, VpcOffering vof, DataCenter zone) throw // TODO Auto-generated method stub } + @Override + public void checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException { + // TODO Auto-generated method stub + } + @Override public Pair> getKeys(GetUserKeysCmd cmd){ return null; diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 97832311b178..b436d0bdcf36 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -32,6 +32,8 @@ import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; +import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao; +import org.apache.cloudstack.backup.BackupOffering; import com.cloud.dc.DataCenter; import com.cloud.dc.DedicatedResourceVO; import com.cloud.dc.dao.DedicatedResourceDao; @@ -70,6 +72,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { @Inject DomainDao _domainDao; @Inject + BackupOfferingDetailsDao backupOfferingDetailsDao; + @Inject AccountDao _accountDao; @Inject LaunchPermissionDao _launchPermissionDao; @@ -474,6 +478,38 @@ else if (_accountService.isNormalUser(account.getId()) return hasAccess; } + @Override + public boolean checkAccess(Account account, BackupOffering backupOffering) throws PermissionDeniedException { + boolean hasAccess = false; + // Check for domains + if (account == null || backupOffering == null) { + hasAccess = true; + } else { + // admin has all permissions + if (_accountService.isRootAdmin(account.getId())) { + hasAccess = true; + } + // if account is normal user or domain admin or project + else if (_accountService.isNormalUser(account.getId()) + || account.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN + || _accountService.isDomainAdmin(account.getId()) + || account.getType() == Account.Type.PROJECT) { + final List boDomainIds = backupOfferingDetailsDao.findDomainIds(backupOffering.getId()); + if (boDomainIds.isEmpty()) { + hasAccess = true; + } else { + for (Long domainId : boDomainIds) { + if (_domainDao.isChildDomain(domainId, account.getDomainId())) { + hasAccess = true; + break; + } + } + } + } + } + return hasAccess; + } + @Override public boolean checkAccess(Account account, DataCenter zone) throws PermissionDeniedException { if (account == null || zone.getDomainId() == null) {//public zone diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index e2fc57b1b16d..3272a696a26d 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -8425,9 +8425,7 @@ private List filterChildSubDomains(final List 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--; diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index bbfc8fd36826..71df4fc1b01a 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -67,6 +67,7 @@ import org.apache.cloudstack.auth.UserAuthenticator; import org.apache.cloudstack.auth.UserAuthenticator.ActionOnFailedAuthentication; import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; +import org.apache.cloudstack.backup.BackupOffering; import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -3568,6 +3569,21 @@ public void checkAccess(Account account, VpcOffering vof, DataCenter zone) throw throw new PermissionDeniedException("There's no way to confirm " + account + " has access to " + vof); } + @Override + public void checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException { + for (SecurityChecker checker : _securityCheckers) { + if (checker.checkAccess(account, bof)) { + if (logger.isDebugEnabled()) { + logger.debug("Access granted to " + account + " to " + bof + " by " + checker.getName()); + } + return; + } + } + + assert false : "How can all of the security checkers pass on checking this caller?"; + throw new PermissionDeniedException("There's no way to confirm " + account + " has access to " + bof); + } + @Override public void checkAccess(User user, ControlledEntity entity) throws PermissionDeniedException { for (SecurityChecker checker : _securityCheckers) { diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index ef3ba917de74..e1c51813ad64 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -68,6 +68,7 @@ import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupDetailsDao; import org.apache.cloudstack.backup.dao.BackupOfferingDao; +import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao; import org.apache.cloudstack.backup.dao.BackupScheduleDao; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; @@ -184,6 +185,8 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { @Inject private BackupOfferingDao backupOfferingDao; @Inject + private BackupOfferingDetailsDao backupOfferingDetailsDao; + @Inject private VMInstanceDao vmInstanceDao; @Inject private AccountService accountService; @@ -280,6 +283,21 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { throw new CloudRuntimeException("A backup offering with the same name already exists in this zone"); } + if (org.apache.commons.collections.CollectionUtils.isNotEmpty(cmd.getDomainIds())) { + for (final Long domainId: cmd.getDomainIds()) { + if (domainDao.findById(domainId) == null) { + throw new InvalidParameterValueException("Please specify a valid domain id"); + } + } + } + + // enforce account permissions: domain-admins cannot create public offerings + final Account caller = CallContext.current().getCallingAccount(); + List filteredDomainIds = cmd.getDomainIds() == null ? new ArrayList<>() : new ArrayList<>(cmd.getDomainIds()); + if (filteredDomainIds.size() > 1) { + filteredDomainIds = filterChildSubDomains(filteredDomainIds); + } + final BackupProvider provider = getBackupProvider(cmd.getZoneId()); if (!provider.isValidProviderOffering(cmd.getZoneId(), cmd.getExternalId())) { throw new CloudRuntimeException("Backup offering '" + cmd.getExternalId() + "' does not exist on provider " + provider.getName() + " on zone " + cmd.getZoneId()); @@ -292,10 +310,50 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { if (savedOffering == null) { throw new CloudRuntimeException("Unable to create backup offering: " + cmd.getExternalId() + ", name: " + cmd.getName()); } + // Persist domain dedication details (if any) + if (org.apache.commons.collections.CollectionUtils.isNotEmpty(filteredDomainIds)) { + List detailsVOList = new ArrayList<>(); + for (Long domainId : filteredDomainIds) { + detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), org.apache.cloudstack.api.ApiConstants.DOMAIN_ID, String.valueOf(domainId), false)); + } + if (!detailsVOList.isEmpty()) { + backupOfferingDetailsDao.saveDetails(detailsVOList); + } + } logger.debug("Successfully created backup offering " + cmd.getName() + " mapped to backup provider offering " + cmd.getExternalId()); return savedOffering; } + private List filterChildSubDomains(final List domainIds) { + if (domainIds == null || domainIds.size() <= 1) { + return domainIds == null ? new ArrayList<>() : new ArrayList<>(domainIds); + } + final List result = new ArrayList<>(); + for (final Long candidate : domainIds) { + boolean isDescendant = false; + for (final Long other : domainIds) { + if (Objects.equals(candidate, other)) continue; + if (domainDao.isChildDomain(other, candidate)) { + isDescendant = true; + break; + } + } + if (!isDescendant) { + result.add(candidate); + } + } + return result; + } + + @Override + public List getBackupOfferingDomains(Long offeringId) { + final BackupOffering backupOffering = backupOfferingDao.findById(offeringId); + if (backupOffering == null) { + throw new InvalidParameterValueException("Unable to find backup offering " + backupOffering); + } + return backupOfferingDetailsDao.findDomainIds(offeringId); + } + @Override public Pair, Integer> listBackupOfferings(final ListBackupOfferingsCmd cmd) { final Long offeringId = cmd.getOfferingId(); @@ -342,6 +400,9 @@ public boolean deleteBackupOffering(final Long offeringId) { throw new CloudRuntimeException("Could not find a backup offering with id: " + offeringId); } + // Ensure caller has permission to delete this offering + accountManager.checkAccess(CallContext.current().getCallingAccount(), offering); + if (backupDao.listByOfferingId(offering.getId()).size() > 0) { throw new CloudRuntimeException("Backup Offering cannot be removed as it has backups associated with it."); } @@ -452,6 +513,8 @@ public boolean assignVMToBackupOffering(Long vmId, Long offeringId) { throw new CloudRuntimeException("Provided backup offering does not exist"); } + accountManager.checkAccess(CallContext.current().getCallingAccount(), offering); + final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); if (backupProvider == null) { throw new CloudRuntimeException("Failed to get the backup provider for the zone, please contact the administrator"); @@ -513,6 +576,8 @@ public boolean removeVMFromBackupOffering(final Long vmId, final boolean forced) throw new CloudRuntimeException("No previously configured backup offering found for the VM"); } + accountManager.checkAccess(CallContext.current().getCallingAccount(), offering); + final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); if (backupProvider == null) { throw new CloudRuntimeException("Failed to get the backup provider for the zone, please contact the administrator"); @@ -762,10 +827,11 @@ protected boolean deleteAllVmBackupSchedules(long vmId) { @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_CREATE, eventDescription = "creating VM backup", async = true) public boolean createBackup(CreateBackupCmd cmd, Object job) throws ResourceAllocationException { Long vmId = cmd.getVmId(); + Account caller = CallContext.current().getCallingAccount(); final VMInstanceVO vm = findVmById(vmId); validateBackupForZone(vm.getDataCenterId()); - accountManager.checkAccess(CallContext.current().getCallingAccount(), null, true, vm); + accountManager.checkAccess(caller, null, true, vm); if (vm.getBackupOfferingId() == null) { throw new CloudRuntimeException("VM has not backup offering configured, cannot create backup before assigning it to a backup offering"); @@ -775,6 +841,7 @@ public boolean createBackup(CreateBackupCmd cmd, Object job) throws ResourceAllo if (offering == null) { throw new CloudRuntimeException("VM backup offering not found"); } + accountManager.checkAccess(caller, offering); final BackupProvider backupProvider = getBackupProvider(offering.getProvider()); if (backupProvider == null) { @@ -2117,6 +2184,9 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO if (backupOfferingVO == null) { throw new InvalidParameterValueException(String.format("Unable to find Backup Offering with id: [%s].", id)); } + + accountManager.checkAccess(CallContext.current().getCallingAccount(), backupOfferingVO); + logger.debug("Trying to update Backup Offering {} to {}.", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(backupOfferingVO, "uuid", "name", "description", "userDrivenBackupAllowed"), ReflectionToStringBuilderUtils.reflectOnlySelectedFields(updateBackupOfferingCmd, "name", "description", "allowUserDrivenBackups")); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index a21477aeb80e..8b3e398cef4a 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -59,6 +59,7 @@ import java.util.TimeZone; import java.util.UUID; +import com.cloud.domain.Domain; import com.cloud.storage.dao.SnapshotPolicyDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; @@ -3105,7 +3106,7 @@ public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedExcep configureDoNothingForMethodsThatWeDoNotWantToTest(); - doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any()); + doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class)); Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); } diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index 8b13fd474947..6f6879b6fe34 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -1081,7 +1081,7 @@ public void testGetRootDiskInfoFromBackup() { assertEquals("root-disk-offering-uuid", VmDiskInfo.getDiskOffering().getUuid()); assertEquals(Long.valueOf(5), VmDiskInfo.getSize()); - assertEquals(null, VmDiskInfo.getDeviceId()); +// assertNull(com.cloud.vm.VmDiskInfo.getDeviceId()); } @Test @@ -1106,7 +1106,7 @@ public void testImportBackupOffering() { assertEquals("Test Offering", result.getName()); assertEquals("Test Description", result.getDescription()); - assertEquals(true, result.isUserDrivenBackupAllowed()); + assertTrue(result.isUserDrivenBackupAllowed()); assertEquals("external-id", result.getExternalId()); assertEquals("testbackupprovider", result.getProvider()); } diff --git a/tools/marvin/setup.py b/tools/marvin/setup.py index 05ce9d41023c..6c9aa087bc7d 100644 --- a/tools/marvin/setup.py +++ b/tools/marvin/setup.py @@ -27,7 +27,7 @@ raise RuntimeError("python setuptools is required to build Marvin") -VERSION = "4.23.0.0-SNAPSHOT" +VERSION = "4.23.0.0" setup(name="Marvin", version=VERSION, diff --git a/utils/src/main/java/com/cloud/utils/component/ManagerBase.java b/utils/src/main/java/com/cloud/utils/component/ManagerBase.java index 01e4f405d2b1..c076c26a5cd9 100644 --- a/utils/src/main/java/com/cloud/utils/component/ManagerBase.java +++ b/utils/src/main/java/com/cloud/utils/component/ManagerBase.java @@ -25,4 +25,6 @@ public ManagerBase() { // set default run level for manager components setRunLevel(ComponentLifecycle.RUN_LEVEL_COMPONENT_BOOTSTRAP); } + + } From 21bc89e07270fbcd7701976e74e1da2df236878c Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 4 Dec 2025 10:38:31 -0500 Subject: [PATCH 02/14] Add tests and UI support and update response params --- .../admin/backup/UpdateBackupOfferingCmd.java | 11 ++- .../api/response/BackupOfferingResponse.java | 17 ++++ .../backup/dao/BackupOfferingDaoImpl.java | 23 +++++- .../java/com/cloud/acl/DomainChecker.java | 3 - .../cloudstack/backup/BackupManagerImpl.java | 50 ++++++++++-- .../cloudstack/backup/BackupManagerTest.java | 78 ++++++++++++++++++- ui/src/config/section/offering.js | 6 +- .../views/offering/ImportBackupOffering.vue | 69 +++++++++++++++- 8 files changed, 241 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java index 682f2f595ade..2b8643819d46 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java @@ -37,6 +37,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import java.util.List; +import java.util.function.LongFunction; @APICommand(name = "updateBackupOffering", description = "Updates a backup offering.", responseObject = BackupOfferingResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") @@ -114,7 +115,15 @@ public void execute() { } public List getDomainIds() { - return resolveDomainIds(domainIds, id, backupManager::getBackupOfferingDomains, "backup offering"); + // backupManager may be null in unit tests where the command is spied without injection. + // Avoid creating a method reference to a null receiver which causes NPE. When backupManager + // is null, pass null as the defaultDomainsProvider so resolveDomainIds will simply return + // an empty list or parse the explicit domainIds string. + LongFunction> defaultDomainsProvider = null; + if (backupManager != null) { + defaultDomainsProvider = backupManager::getBackupOfferingDomains; + } + return resolveDomainIds(domainIds, id, defaultDomainsProvider, "backup offering"); } @Override diff --git a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java index 4120f68d9da3..0669afa604b4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/BackupOfferingResponse.java @@ -61,6 +61,14 @@ public class BackupOfferingResponse extends BaseResponse { @Param(description = "zone name") private String zoneName; + @SerializedName(ApiConstants.DOMAIN_ID) + @Param(description = "the domain ID(s) this disk offering belongs to. Ignore this information as it is not currently applicable.") + private String domainId; + + @SerializedName(ApiConstants.DOMAIN) + @Param(description = "the domain name(s) this disk offering belongs to. Ignore this information as it is not currently applicable.") + private String domain; + @SerializedName(ApiConstants.CROSS_ZONE_INSTANCE_CREATION) @Param(description = "the backups with this offering can be used to create Instances on all Zones", since = "4.22.0") private Boolean crossZoneInstanceCreation; @@ -108,4 +116,13 @@ public void setCrossZoneInstanceCreation(Boolean crossZoneInstanceCreation) { public void setCreated(Date created) { this.created = created; } + + public void setDomainId(String domainId) { + this.domainId = domainId; + } + + public void setDomain(String domain) { + this.domain = domain; + } + } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java index a41e4e70d339..708faeef4643 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupOfferingDaoImpl.java @@ -20,6 +20,8 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; import org.apache.cloudstack.api.response.BackupOfferingResponse; import org.apache.cloudstack.backup.BackupOffering; import org.apache.cloudstack.backup.BackupOfferingVO; @@ -30,10 +32,16 @@ import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; +import java.util.List; + public class BackupOfferingDaoImpl extends GenericDaoBase implements BackupOfferingDao { @Inject DataCenterDao dataCenterDao; + @Inject + BackupOfferingDetailsDao backupOfferingDetailsDao; + @Inject + DomainDao domainDao; private SearchBuilder backupPoliciesSearch; @@ -51,8 +59,9 @@ protected void init() { @Override public BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering, Boolean crossZoneInstanceCreation) { - DataCenterVO zone = dataCenterDao.findById(offering.getZoneId()); + DataCenterVO zone = dataCenterDao.findById(offering.getZoneId()); + List domainIds = backupOfferingDetailsDao.findDomainIds(offering.getId()); BackupOfferingResponse response = new BackupOfferingResponse(); response.setId(offering.getUuid()); response.setName(offering.getName()); @@ -64,6 +73,18 @@ public BackupOfferingResponse newBackupOfferingResponse(BackupOffering offering, response.setZoneId(zone.getUuid()); response.setZoneName(zone.getName()); } + if (domainIds != null && !domainIds.isEmpty()) { + String domainUUIDs = domainIds.stream().map(Long::valueOf).map(domainId -> { + DomainVO domain = domainDao.findById(domainId); + return domain != null ? domain.getUuid() : ""; + }).filter(name -> !name.isEmpty()).reduce((a, b) -> a + "," + b).orElse(""); + String domainNames = domainIds.stream().map(Long::valueOf).map(domainId -> { + DomainVO domain = domainDao.findById(domainId); + return domain != null ? domain.getName() : ""; + }).filter(name -> !name.isEmpty()).reduce((a, b) -> a + "," + b).orElse(""); + response.setDomain(domainNames); + response.setDomainId(domainUUIDs); + } if (crossZoneInstanceCreation) { response.setCrossZoneInstanceCreation(true); } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index b436d0bdcf36..99a1a4d5a20e 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -481,15 +481,12 @@ else if (_accountService.isNormalUser(account.getId()) @Override public boolean checkAccess(Account account, BackupOffering backupOffering) throws PermissionDeniedException { boolean hasAccess = false; - // Check for domains if (account == null || backupOffering == null) { hasAccess = true; } else { - // admin has all permissions if (_accountService.isRootAdmin(account.getId())) { hasAccess = true; } - // if account is normal user or domain admin or project else if (_accountService.isNormalUser(account.getId()) || account.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN || _accountService.isDomainAdmin(account.getId()) diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index e1c51813ad64..bc40719c7ec5 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -291,7 +291,6 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { } } - // enforce account permissions: domain-admins cannot create public offerings final Account caller = CallContext.current().getCallingAccount(); List filteredDomainIds = cmd.getDomainIds() == null ? new ArrayList<>() : new ArrayList<>(cmd.getDomainIds()); if (filteredDomainIds.size() > 1) { @@ -310,11 +309,10 @@ public BackupOffering importBackupOffering(final ImportBackupOfferingCmd cmd) { if (savedOffering == null) { throw new CloudRuntimeException("Unable to create backup offering: " + cmd.getExternalId() + ", name: " + cmd.getName()); } - // Persist domain dedication details (if any) if (org.apache.commons.collections.CollectionUtils.isNotEmpty(filteredDomainIds)) { List detailsVOList = new ArrayList<>(); for (Long domainId : filteredDomainIds) { - detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), org.apache.cloudstack.api.ApiConstants.DOMAIN_ID, String.valueOf(domainId), false)); + detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), false)); } if (!detailsVOList.isEmpty()) { backupOfferingDetailsDao.saveDetails(detailsVOList); @@ -400,7 +398,6 @@ public boolean deleteBackupOffering(final Long offeringId) { throw new CloudRuntimeException("Could not find a backup offering with id: " + offeringId); } - // Ensure caller has permission to delete this offering accountManager.checkAccess(CallContext.current().getCallingAccount(), offering); if (backupDao.listByOfferingId(offering.getId()).size() > 0) { @@ -2179,6 +2176,7 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO String name = updateBackupOfferingCmd.getName(); String description = updateBackupOfferingCmd.getDescription(); Boolean allowUserDrivenBackups = updateBackupOfferingCmd.getAllowUserDrivenBackups(); + List domainIds = updateBackupOfferingCmd.getDomainIds(); BackupOfferingVO backupOfferingVO = backupOfferingDao.findById(id); if (backupOfferingVO == null) { @@ -2209,16 +2207,58 @@ public BackupOffering updateBackupOffering(UpdateBackupOfferingCmd updateBackupO fields.add("allowUserDrivenBackups: " + allowUserDrivenBackups); } - if (!backupOfferingDao.update(id, offering)) { + if (org.apache.commons.collections.CollectionUtils.isNotEmpty(domainIds)) { + for (final Long domainId: domainIds) { + if (domainDao.findById(domainId) == null) { + throw new InvalidParameterValueException("Please specify a valid domain id"); + } + } + } + List filteredDomainIds = filterChildSubDomains(domainIds); + Collections.sort(filteredDomainIds); + + boolean success =backupOfferingDao.update(id, offering); + if (!success) { logger.warn(String.format("Couldn't update Backup offering (%s) with [%s].", backupOfferingVO, String.join(", ", fields))); } + if (success) { + List existingDomainIds = backupOfferingDetailsDao.findDomainIds(id); + Collections.sort(existingDomainIds); + updateBackupOfferingDomainDetails(id, filteredDomainIds, existingDomainIds); + } + BackupOfferingVO response = backupOfferingDao.findById(id); CallContext.current().setEventDetails(String.format("Backup Offering updated [%s].", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(response, "id", "name", "description", "userDrivenBackupAllowed", "externalId"))); return response; } + private void updateBackupOfferingDomainDetails(Long id, List filteredDomainIds, List existingDomainIds) { + if (existingDomainIds == null) { + existingDomainIds = new ArrayList<>(); + } + List detailsVO = new ArrayList<>(); + if(!filteredDomainIds.equals(existingDomainIds)) { + SearchBuilder sb = backupOfferingDetailsDao.createSearchBuilder(); + sb.and("offeringId", sb.entity().getResourceId(), SearchCriteria.Op.EQ); + sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("offeringId", String.valueOf(id)); + sc.setParameters("detailName", ApiConstants.DOMAIN_ID); + backupOfferingDetailsDao.remove(sc); + for (Long domainId : filteredDomainIds) { + detailsVO.add(new BackupOfferingDetailsVO(id, ApiConstants.DOMAIN_ID, String.valueOf(domainId), false)); + } + } + if (!detailsVO.isEmpty()) { + for (BackupOfferingDetailsVO detailVO : detailsVO) { + backupOfferingDetailsDao.persist(detailVO); + } + } + } + Map getDetailsFromBackupDetails(Long backupId) { Map details = backupDetailsDao.listDetailsKeyPairs(backupId, true); if (details == null) { diff --git a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java index 6f6879b6fe34..47c3c0d5d4cf 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -85,6 +85,7 @@ import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.backup.dao.BackupDetailsDao; import org.apache.cloudstack.backup.dao.BackupOfferingDao; +import org.apache.cloudstack.backup.dao.BackupOfferingDetailsDao; import org.apache.cloudstack.backup.dao.BackupScheduleDao; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; @@ -241,6 +242,9 @@ public class BackupManagerTest { @Mock private GuestOSDao _guestOSDao; + @Mock + private BackupOfferingDetailsDao backupOfferingDetailsDao; + private Gson gson; private String[] hostPossibleValues = {"127.0.0.1", "hostname"}; @@ -352,6 +356,7 @@ public void testUpdateBackupOfferingSuccess() { when(cmd.getName()).thenReturn("New name"); when(cmd.getDescription()).thenReturn("New description"); when(cmd.getAllowUserDrivenBackups()).thenReturn(true); + when(backupOfferingDetailsDao.findDomainIds(id)).thenReturn(Collections.emptyList()); BackupOffering updated = backupManager.updateBackupOffering(cmd); assertEquals("New name", updated.getName()); @@ -1081,7 +1086,7 @@ public void testGetRootDiskInfoFromBackup() { assertEquals("root-disk-offering-uuid", VmDiskInfo.getDiskOffering().getUuid()); assertEquals(Long.valueOf(5), VmDiskInfo.getSize()); -// assertNull(com.cloud.vm.VmDiskInfo.getDeviceId()); + assertNull(com.cloud.vm.VmDiskInfo.getDeviceId()); } @Test @@ -2156,4 +2161,75 @@ public void testRestoreBackupVolumeMismatch() { verify(vmInstanceDao, times(1)).findByIdIncludingRemoved(vmId); verify(volumeDao, times(1)).findByInstance(vmId); } + + @Test + public void getBackupOfferingDomainsTestOfferingNotFound() { + Long offeringId = 1L; + when(backupOfferingDao.findById(offeringId)).thenReturn(null); + + InvalidParameterValueException exception = Assert.assertThrows(InvalidParameterValueException.class, + () -> backupManager.getBackupOfferingDomains(offeringId)); + assertEquals("Unable to find backup offering null", exception.getMessage()); + } + + @Test + public void getBackupOfferingDomainsTestReturnsDomains() { + Long offeringId = 1L; + BackupOfferingVO offering = Mockito.mock(BackupOfferingVO.class); + when(backupOfferingDao.findById(offeringId)).thenReturn(offering); + when(backupOfferingDetailsDao.findDomainIds(offeringId)).thenReturn(List.of(10L, 20L)); + + List result = backupManager.getBackupOfferingDomains(offeringId); + + assertEquals(2, result.size()); + assertTrue(result.contains(10L)); + assertTrue(result.contains(20L)); + } + + @Test + public void testUpdateBackupOfferingThrowsWhenDomainIdInvalid() { + Long id = 1234L; + UpdateBackupOfferingCmd cmd = Mockito.spy(UpdateBackupOfferingCmd.class); + when(cmd.getId()).thenReturn(id); + when(cmd.getDomainIds()).thenReturn(List.of(99L)); + + when(domainDao.findById(99L)).thenReturn(null); + + InvalidParameterValueException exception = Assert.assertThrows(InvalidParameterValueException.class, + () -> backupManager.updateBackupOffering(cmd)); + assertEquals("Please specify a valid domain id", exception.getMessage()); + } + + @Test + public void testUpdateBackupOfferingPersistsDomainDetailsWhenProvided() { + Long id = 1234L; + Long domainId = 11L; + UpdateBackupOfferingCmd cmd = Mockito.spy(UpdateBackupOfferingCmd.class); + when(cmd.getId()).thenReturn(id); + when(cmd.getDomainIds()).thenReturn(List.of(domainId)); + + DomainVO domain = Mockito.mock(DomainVO.class); + when(domainDao.findById(domainId)).thenReturn(domain); + + when(backupOfferingDetailsDao.findDomainIds(id)).thenReturn(Collections.emptyList()); + + SearchBuilder sb = Mockito.mock(SearchBuilder.class); + SearchCriteria sc = Mockito.mock(SearchCriteria.class); + BackupOfferingDetailsVO entity = Mockito.mock(BackupOfferingDetailsVO.class); + when(backupOfferingDetailsDao.createSearchBuilder()).thenReturn(sb); + when(sb.entity()).thenReturn(entity); + when(sb.and(Mockito.anyString(), (Object) any(), Mockito.any())).thenReturn(sb); + when(sb.create()).thenReturn(sc); + + BackupOfferingVO offering = Mockito.mock(BackupOfferingVO.class); + BackupOfferingVO offeringUpdate = Mockito.mock(BackupOfferingVO.class); + when(backupOfferingDao.findById(id)).thenReturn(offering); + when(backupOfferingDao.createForUpdate(id)).thenReturn(offeringUpdate); + when(backupOfferingDao.update(id, offeringUpdate)).thenReturn(true); + + BackupOffering updated = backupManager.updateBackupOffering(cmd); + + verify(backupOfferingDetailsDao, times(1)).persist(Mockito.any(BackupOfferingDetailsVO.class)); + } + } diff --git a/ui/src/config/section/offering.js b/ui/src/config/section/offering.js index 4a32619b8c2f..bc95772d6f7a 100644 --- a/ui/src/config/section/offering.js +++ b/ui/src/config/section/offering.js @@ -340,9 +340,9 @@ export default { icon: 'cloud-upload-outlined', docHelp: 'adminguide/virtual_machines.html#backup-offerings', permission: ['listBackupOfferings'], - searchFilters: ['zoneid'], - columns: ['name', 'description', 'zonename'], - details: ['name', 'id', 'description', 'externalid', 'zone', 'allowuserdrivenbackups', 'created'], + searchFilters: ['zoneid', 'domainid'], + columns: ['name', 'description', 'domain', 'zonename'], + details: ['name', 'id', 'description', 'externalid', 'domain', 'zone', 'allowuserdrivenbackups', 'created'], related: [{ name: 'vm', title: 'label.instances', diff --git a/ui/src/views/offering/ImportBackupOffering.vue b/ui/src/views/offering/ImportBackupOffering.vue index b8ac7d8e8e65..f680eacd4a7d 100644 --- a/ui/src/views/offering/ImportBackupOffering.vue +++ b/ui/src/views/offering/ImportBackupOffering.vue @@ -85,6 +85,33 @@ + + + + + + + + + + + {{ opt.path || opt.name || opt.description }} + + + +
{{ this.$t('label.cancel') }} {{ this.$t('label.ok') }} @@ -96,6 +123,7 @@