From 10333dfd0ef8feb32d7d951d1a3bf8396994a068 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 21 Jan 2026 16:57:19 -0500 Subject: [PATCH] add recently added domain id for bkp offering to be inherited in clone operation --- .../admin/backup/CloneBackupOfferingCmd.java | 25 +++- .../admin/backup/ImportBackupOfferingCmd.java | 3 +- .../ConfigurationManagerImpl.java | 5 +- .../cloudstack/backup/BackupManagerImpl.java | 23 ++++ .../cloudstack/backup/BackupManagerTest.java | 104 ++++++++++++++++ ui/public/locales/en.json | 1 + ui/src/views/offering/CloneBackupOffering.vue | 113 ++++++++++++++++++ 7 files changed, 270 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java index 194357d0098..462fdb65289 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java @@ -26,6 +26,7 @@ import org.apache.cloudstack.api.BaseAsyncCmd; 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.api.response.ZoneResponse; import org.apache.cloudstack.backup.BackupManager; @@ -41,11 +42,15 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.utils.exception.CloudRuntimeException; +import java.util.Arrays; +import java.util.List; +import java.util.function.LongFunction; + @APICommand(name = "cloneBackupOffering", description = "Clones a backup offering from an existing offering", responseObject = BackupOfferingResponse.class, since = "4.14.0", authorized = {RoleType.Admin}) -public class CloneBackupOfferingCmd extends BaseAsyncCmd { +public class CloneBackupOfferingCmd extends BaseAsyncCmd implements DomainAndZoneIdResolver { @Inject protected BackupManager backupManager; @@ -74,6 +79,13 @@ public class CloneBackupOfferingCmd extends BaseAsyncCmd { description = "The zone ID", required = false) private Long zoneId; + @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; + @Parameter(name = ApiConstants.ALLOW_USER_DRIVEN_BACKUPS, type = BaseCmd.CommandType.BOOLEAN, description = "Whether users are allowed to create adhoc backups and backup schedules", required = false) private Boolean userDrivenBackups; @@ -106,6 +118,17 @@ public class CloneBackupOfferingCmd extends BaseAsyncCmd { return userDrivenBackups; } + public List getDomainIds() { + if (domainIds != null && !domainIds.isEmpty()) { + return Arrays.asList(Arrays.stream(domainIds.split(",")).map(domainId -> Long.parseLong(domainId.trim())).toArray(Long[]::new)); + } + LongFunction> defaultDomainsProvider = null; + if (backupManager != null) { + defaultDomainsProvider = backupManager::getBackupOfferingDomains; + } + return resolveDomainIds(domainIds, sourceOfferingId, defaultDomainsProvider, "backup offering"); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// 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 b44bdb8d64a..964968f94a0 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 @@ -86,7 +86,8 @@ public class ImportBackupOfferingCmd extends BaseAsyncCmd { type = CommandType.LIST, collectionType = CommandType.UUID, entityType = DomainResponse.class, - description = "the ID of the containing domain(s), null for public offerings") + description = "the ID of the containing domain(s), null for public offerings", + since = "4.23.0") private List domainIds; ///////////////////////////////////////////////////// diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 1e3015b5ece..1798101bb4c 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -22,6 +22,7 @@ import static com.cloud.offering.NetworkOffering.RoutingMode.Static; import static org.apache.cloudstack.framework.config.ConfigKey.CATEGORY_SYSTEM; import java.io.UnsupportedEncodingException; +import java.lang.reflect.Field; import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; @@ -8663,7 +8664,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } public static void setField(Object obj, String fieldName, Object value) throws Exception { - java.lang.reflect.Field field = findField(obj.getClass(), fieldName); + Field field = findField(obj.getClass(), fieldName); if (field == null) { throw new NoSuchFieldException("Field '" + fieldName + "' not found in class hierarchy of " + obj.getClass().getName()); } @@ -8671,7 +8672,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati field.set(obj, value); } - public static java.lang.reflect.Field findField(Class clazz, String fieldName) { + public static Field findField(Class clazz, String fieldName) { Class currentClass = clazz; while (currentClass != null) { try { 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 345e6a49f11..b83c1023662 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -372,10 +372,33 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { throw new CloudRuntimeException("Unable to clone backup offering from ID: " + cmd.getSourceOfferingId()); } + List filteredDomainIds = cmd.getDomainIds() == null ? new ArrayList<>() : new ArrayList<>(cmd.getDomainIds()); + Collections.sort(filteredDomainIds); + updateBackupOfferingDomainDetail(savedOffering, filteredDomainIds); + logger.debug("Successfully cloned backup offering '" + sourceOffering.getName() + "' (ID: " + cmd.getSourceOfferingId() + ") to '" + cmd.getName() + "' (ID: " + savedOffering.getId() + ")"); return savedOffering; } + private void updateBackupOfferingDomainDetail(BackupOfferingVO savedOffering, List filteredDomainIds) { + if (filteredDomainIds.size() > 1) { + filteredDomainIds = domainHelper.filterChildSubDomains(filteredDomainIds); + } + + if (CollectionUtils.isNotEmpty(filteredDomainIds)) { + List detailsVOList = new ArrayList<>(); + for (Long domainId : filteredDomainIds) { + if (domainDao.findById(domainId) == null) { + throw new InvalidParameterValueException("Please specify a valid domain id"); + } + detailsVOList.add(new BackupOfferingDetailsVO(savedOffering.getId(), ApiConstants.DOMAIN_ID, String.valueOf(domainId), false)); + } + if (!detailsVOList.isEmpty()) { + backupOfferingDetailsDao.saveDetails(detailsVOList); + } + } + } + @Override public List getBackupOfferingDomains(Long offeringId) { final BackupOffering backupOffering = backupOfferingDao.findById(offeringId); 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 a9c083228e2..cfac99e93ae 100644 --- a/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java +++ b/server/src/test/java/org/apache/cloudstack/backup/BackupManagerTest.java @@ -76,6 +76,7 @@ import com.cloud.vm.dao.VMInstanceDao; import com.google.gson.Gson; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.backup.CloneBackupOfferingCmd; import org.apache.cloudstack.api.command.admin.backup.ImportBackupOfferingCmd; import org.apache.cloudstack.api.command.admin.backup.UpdateBackupOfferingCmd; import org.apache.cloudstack.api.command.user.backup.CreateBackupCmd; @@ -132,6 +133,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.atLeastOnce; +import org.mockito.ArgumentCaptor; @RunWith(MockitoJUnitRunner.class) public class BackupManagerTest { @@ -2518,4 +2520,106 @@ public class BackupManagerTest { return offering; } + @Test + public void testCloneBackupOfferingUsesProvidedDomainIds() { + Long sourceOfferingId = 1L; + Long zoneId = 10L; + Long savedOfferingId = 2L; + List providedDomainIds = List.of(11L); + + // command + CloneBackupOfferingCmd cmd = Mockito.mock(CloneBackupOfferingCmd.class); + when(cmd.getSourceOfferingId()).thenReturn(sourceOfferingId); + when(cmd.getName()).thenReturn("Cloned Offering"); + when(cmd.getDescription()).thenReturn(null); + when(cmd.getExternalId()).thenReturn(null); + when(cmd.getUserDrivenBackups()).thenReturn(null); + when(cmd.getDomainIds()).thenReturn(providedDomainIds); + + // source offering + BackupOfferingVO sourceOffering = Mockito.mock(BackupOfferingVO.class); + when(sourceOffering.getZoneId()).thenReturn(zoneId); + when(sourceOffering.getExternalId()).thenReturn("ext-src"); + when(sourceOffering.getProvider()).thenReturn("testbackupprovider"); + when(sourceOffering.getDescription()).thenReturn("src desc"); + when(sourceOffering.isUserDrivenBackupAllowed()).thenReturn(true); + when(sourceOffering.getName()).thenReturn("Source Offering"); + + when(backupOfferingDao.findById(sourceOfferingId)).thenReturn(sourceOffering); + when(backupOfferingDao.findByName(cmd.getName(), zoneId)).thenReturn(null); + + BackupOfferingVO savedOffering = Mockito.mock(BackupOfferingVO.class); + when(savedOffering.getId()).thenReturn(savedOfferingId); + when(backupOfferingDao.persist(any(BackupOfferingVO.class))).thenReturn(savedOffering); + + DomainVO domain = Mockito.mock(DomainVO.class); + when(domainDao.findById(11L)).thenReturn(domain); + + overrideBackupFrameworkConfigValue(); + + BackupOffering result = backupManager.cloneBackupOffering(cmd); + + assertEquals(savedOffering, result); + + ArgumentCaptor captor = ArgumentCaptor.forClass(List.class); + verify(backupOfferingDetailsDao, times(1)).saveDetails(captor.capture()); + List savedDetails = captor.getValue(); + assertEquals(1, savedDetails.size()); + assertEquals(String.valueOf(11L), savedDetails.get(0).getValue()); + } + + @Test + public void testCloneBackupOfferingInheritsDomainIdsFromSource() { + Long sourceOfferingId = 3L; + Long zoneId = 20L; + Long savedOfferingId = 4L; + List sourceDomainIds = List.of(21L, 22L); + + CloneBackupOfferingCmd cmd = Mockito.mock(CloneBackupOfferingCmd.class); + when(cmd.getSourceOfferingId()).thenReturn(sourceOfferingId); + when(cmd.getName()).thenReturn("Cloned Inherit Offering"); + when(cmd.getDescription()).thenReturn(null); + when(cmd.getExternalId()).thenReturn(null); + when(cmd.getUserDrivenBackups()).thenReturn(null); + // Simulate resolver having provided the source offering domains (the real cmd#getDomainIds() would do this) + when(cmd.getDomainIds()).thenReturn(sourceDomainIds); + + BackupOfferingVO sourceOffering = Mockito.mock(BackupOfferingVO.class); + when(sourceOffering.getZoneId()).thenReturn(zoneId); + when(sourceOffering.getExternalId()).thenReturn("ext-src-2"); + when(sourceOffering.getProvider()).thenReturn("testbackupprovider"); + when(sourceOffering.getDescription()).thenReturn("src desc 2"); + when(sourceOffering.isUserDrivenBackupAllowed()).thenReturn(false); + when(sourceOffering.getName()).thenReturn("Source Offering 2"); + + when(backupOfferingDao.findById(sourceOfferingId)).thenReturn(sourceOffering); + when(backupOfferingDao.findByName(cmd.getName(), zoneId)).thenReturn(null); + + BackupOfferingVO savedOffering = Mockito.mock(BackupOfferingVO.class); + when(savedOffering.getId()).thenReturn(savedOfferingId); + when(backupOfferingDao.persist(any(BackupOfferingVO.class))).thenReturn(savedOffering); + + // domain handling + DomainVO domain21 = Mockito.mock(DomainVO.class); + DomainVO domain22 = Mockito.mock(DomainVO.class); + when(domainDao.findById(21L)).thenReturn(domain21); + when(domainDao.findById(22L)).thenReturn(domain22); + when(domainHelper.filterChildSubDomains(sourceDomainIds)).thenReturn(new ArrayList<>(sourceDomainIds)); + + overrideBackupFrameworkConfigValue(); + + BackupOffering result = backupManager.cloneBackupOffering(cmd); + assertEquals(savedOffering, result); + + ArgumentCaptor captor = ArgumentCaptor.forClass(List.class); + verify(backupOfferingDetailsDao, times(1)).saveDetails(captor.capture()); + List savedDetails = captor.getValue(); + assertEquals(2, savedDetails.size()); + List values = new ArrayList<>(); + for (BackupOfferingDetailsVO d : savedDetails) { + values.add(d.getValue()); + } + assertTrue(values.contains(String.valueOf(21L))); + assertTrue(values.contains(String.valueOf(22L))); + } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 3b660bd58a7..21fe35ea157 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -3357,6 +3357,7 @@ "message.disable.webhook.ssl.verification": "Disabling SSL verification is not recommended", "message.discovering.feature": "Discovering features, please wait...", "message.disk.offering.created": "Disk offering created:", +"message.success.clone.backup.offering": "Successfully cloned backup offering", "message.success.clone.disk.offering": "Successfully cloned disk offering:", "message.success.clone.network.offering": "Successfully cloned network offering:", "message.disk.usage.info.data.points": "Each data point represents the difference in read/write data since the last data point.", diff --git a/ui/src/views/offering/CloneBackupOffering.vue b/ui/src/views/offering/CloneBackupOffering.vue index 3f1c195faa9..c489286ce6c 100644 --- a/ui/src/views/offering/CloneBackupOffering.vue +++ b/ui/src/views/offering/CloneBackupOffering.vue @@ -101,6 +101,34 @@ + + + + + + + + + + + {{ opt.path || opt.name || opt.description }} + + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -116,6 +144,7 @@ import { getAPI, postAPI } from '@/api' import ResourceIcon from '@/components/view/ResourceIcon' import TooltipLabel from '@/components/widgets/TooltipLabel' import { GlobalOutlined } from '@ant-design/icons-vue' +import { isAdmin } from '@/role' export default { name: 'CloneBackupOffering', @@ -140,6 +169,10 @@ export default { externals: { loading: false, opts: [] + }, + domains: { + loading: false, + opts: [] } } }, @@ -163,6 +196,7 @@ export default { }, fetchData () { this.fetchZone() + this.fetchDomain() this.$nextTick(() => { this.populateFormFromResource() }) @@ -180,6 +214,20 @@ export default { this.zones.loading = false }) }, + fetchDomain () { + this.domains.loading = true + const params = { listAll: true, showicon: true, details: 'min' } + getAPI('listDomains', params).then(json => { + this.domains.opts = json.listdomainsresponse.domain || [] + this.$nextTick(() => { + this.populateFormFromResource() + }) + }).catch(error => { + this.$notifyError(error) + }).finally(() => { + this.domains.loading = false + }) + }, fetchExternal (zoneId) { if (!zoneId) { this.externals.opts = [] @@ -206,6 +254,27 @@ export default { this.form.allowuserdrivenbackups = r.allowuserdrivenbackups } + if (r.domainid) { + let offeringDomainIds = r.domainid + offeringDomainIds = (typeof offeringDomainIds === 'string' && offeringDomainIds.indexOf(',') !== -1) ? offeringDomainIds.split(',') : [offeringDomainIds] + const selected = [] + for (let i = 0; i < offeringDomainIds.length; i++) { + for (let j = 0; j < this.domains.opts.length; j++) { + if (String(offeringDomainIds[i]) === String(this.domains.opts[j].id)) { + selected.push(j) + } + } + } + if (selected.length > 0) { + this.form.ispublic = false + this.form.domainid = selected + } + } else { + if (isAdmin()) { + this.form.ispublic = true + } + } + if (r.zoneid && this.zones.opts.length > 0) { const zone = this.zones.opts.find(z => z.id === r.zoneid) if (zone) { @@ -255,6 +324,23 @@ export default { params.allowuserdrivenbackups = values.allowuserdrivenbackups } + // Include selected domain IDs when offering is not public + if (values.ispublic !== true) { + const domainIndexes = values.domainid + let domainId = null + if (domainIndexes && domainIndexes.length > 0) { + const domainIds = [] + const domains = this.domains.opts + for (let i = 0; i < domainIndexes.length; i++) { + domainIds.push(domains[domainIndexes[i]].id) + } + domainId = domainIds.join(',') + } + if (domainId) { + params.domainid = domainId + } + } + this.loading = true const title = this.$t('message.success.clone.backup.offering') @@ -301,8 +387,35 @@ export default { this.fetchExternal(zone.id) } }, + onChangeIsPublic (value) { + // when made public, clear any domain selection + try { + if (value === true) { + this.form.domainid = [] + } + } catch (e) { + // ignore + } + }, + onChangeDomain (value) { + // value is an array of selected indexes (as used in the form), when empty -> make offering public + try { + if (!value || (Array.isArray(value) && value.length === 0)) { + this.form.ispublic = true + // clear domain selection to avoid confusing hidden inputs + this.form.domainid = [] + } else { + this.form.ispublic = false + } + } catch (e) { + // defensive - do nothing on error + } + }, closeAction () { this.$emit('close-action') + }, + isAdmin () { + return isAdmin() } } }