Skip to content

Commit 1ec09ea

Browse files
Pearl1594dhslove
Pearl1594
authored and
dhslove
committed
Support dedicating backup offerings to domains (apache#12194)
* Add support for dedicating backup offerings to domains * Add tests and UI support and update response params * add license header * exclude backupofferingdetailsvo from sonar * fix pre-commit checks - missing / extra EOF line * add test * EOF * filter backup offerings by domain id * add unit tests * add more unit tests and remove response file from code coverage check * update checks * address review comments: extract common code, fix tests * added bean definition * address comments * add unit tests to increase coverage * pre-commit check failure fix * address merge issue * allow updating backup offering when only domain id is modified
1 parent fb0cfe9 commit 1ec09ea

39 files changed

Lines changed: 1607 additions & 312 deletions

File tree

‎api/src/main/java/com/cloud/user/AccountService.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.cloud.offering.NetworkOffering;
3737
import com.cloud.offering.ServiceOffering;
3838
import org.apache.cloudstack.auth.UserTwoFactorAuthenticator;
39+
import org.apache.cloudstack.backup.BackupOffering;
3940

4041
public interface AccountService {
4142

@@ -115,6 +116,8 @@ User createUser(String userName, String password, String firstName, String lastN
115116

116117
void checkAccess(Account account, VpcOffering vof, DataCenter zone) throws PermissionDeniedException;
117118

119+
void checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException;
120+
118121
void checkAccess(User user, ControlledEntity entity);
119122

120123
void checkAccess(Account account, AccessType accessType, boolean sameOwner, String apiName, ControlledEntity... entities) throws PermissionDeniedException;

‎api/src/main/java/org/apache/cloudstack/acl/SecurityChecker.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import com.cloud.user.User;
2828
import com.cloud.utils.component.Adapter;
2929

30+
import org.apache.cloudstack.backup.BackupOffering;
31+
3032
/**
3133
* SecurityChecker checks the ownership and access control to objects within
3234
*/
@@ -145,4 +147,6 @@ boolean checkAccess(Account caller, AccessType accessType, String action, Contro
145147
boolean checkAccess(Account account, NetworkOffering nof, DataCenter zone) throws PermissionDeniedException;
146148

147149
boolean checkAccess(Account account, VpcOffering vof, DataCenter zone) throws PermissionDeniedException;
150+
151+
boolean checkAccess(Account account, BackupOffering bof) throws PermissionDeniedException;
148152
}

‎api/src/main/java/org/apache/cloudstack/api/BaseBackupListCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.apache.cloudstack.backup.BackupOffering;
2626
import org.apache.cloudstack.context.CallContext;
2727

28-
public abstract class BaseBackupListCmd extends BaseListCmd {
28+
public abstract class BaseBackupListCmd extends BaseListAccountResourcesCmd {
2929

3030
protected void setupResponseBackupOfferingsList(final List<BackupOffering> offerings, final Integer count) {
3131
final ListResponse<BackupOfferingResponse> response = new ListResponse<>();

‎api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.cloudstack.api.Parameter;
2828
import org.apache.cloudstack.api.ServerApiException;
2929
import org.apache.cloudstack.api.response.BackupOfferingResponse;
30+
import org.apache.cloudstack.api.response.DomainResponse;
3031
import org.apache.cloudstack.api.response.ZoneResponse;
3132
import org.apache.cloudstack.backup.BackupManager;
3233
import org.apache.cloudstack.backup.BackupOffering;
@@ -40,6 +41,11 @@
4041
import com.cloud.exception.ResourceAllocationException;
4142
import com.cloud.exception.ResourceUnavailableException;
4243
import com.cloud.utils.exception.CloudRuntimeException;
44+
import org.apache.commons.collections.CollectionUtils;
45+
46+
import java.util.LinkedHashSet;
47+
import java.util.List;
48+
import java.util.Set;
4349

4450
@APICommand(name = "importBackupOffering",
4551
description = "Imports a backup offering using a backup provider",
@@ -83,6 +89,13 @@ public class ImportBackupOfferingCmd extends BaseAsyncCmd {
8389
@Parameter(name = ApiConstants.RETENTION_PERIOD, type = CommandType.STRING, required = false,
8490
description = "Retention period of backup data")
8591
private String retentionPeriod;
92+
93+
@Parameter(name = ApiConstants.DOMAIN_ID,
94+
type = CommandType.LIST,
95+
collectionType = CommandType.UUID,
96+
entityType = DomainResponse.class,
97+
description = "the ID of the containing domain(s), null for public offerings")
98+
private List<Long> domainIds;
8699

87100
/////////////////////////////////////////////////////
88101
/////////////////// Accessors ///////////////////////
@@ -115,6 +128,15 @@ public Boolean getUserDrivenBackups() {
115128
public String getRetentionPeriod() {
116129
return retentionPeriod;
117130
}
131+
132+
public List<Long> getDomainIds() {
133+
if (CollectionUtils.isNotEmpty(domainIds)) {
134+
Set<Long> set = new LinkedHashSet<>(domainIds);
135+
domainIds.clear();
136+
domainIds.addAll(set);
137+
}
138+
return domainIds;
139+
}
118140

119141
/////////////////////////////////////////////////////
120142
/////////////// API Implementation///////////////////

‎api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,24 @@
2525
import org.apache.cloudstack.api.BaseCmd;
2626
import org.apache.cloudstack.api.Parameter;
2727
import org.apache.cloudstack.api.ServerApiException;
28+
import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver;
2829
import org.apache.cloudstack.api.response.BackupOfferingResponse;
2930
import org.apache.cloudstack.backup.BackupManager;
3031
import org.apache.cloudstack.backup.BackupOffering;
3132
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
33+
import org.apache.commons.collections.CollectionUtils;
3234
import org.apache.commons.lang3.StringUtils;
3335

3436
import com.cloud.exception.InvalidParameterValueException;
3537
import com.cloud.user.Account;
3638
import com.cloud.utils.exception.CloudRuntimeException;
3739

40+
import java.util.List;
41+
import java.util.function.LongFunction;
42+
3843
@APICommand(name = "updateBackupOffering", description = "Updates a backup offering.", responseObject = BackupOfferingResponse.class,
3944
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0")
40-
public class UpdateBackupOfferingCmd extends BaseCmd {
45+
public class UpdateBackupOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver {
4146

4247
@Inject
4348
private BackupManager backupManager;
@@ -59,6 +64,13 @@ public class UpdateBackupOfferingCmd extends BaseCmd {
5964

6065
@Parameter(name = ApiConstants.RETENTION_PERIOD, type = CommandType.STRING, required = false, description = "Retention period of backup data")
6166
private String retentionPeriod;
67+
68+
@Parameter(name = ApiConstants.DOMAIN_ID,
69+
type = CommandType.STRING,
70+
description = "the ID of the containing domain(s) as comma separated string, public for public offerings",
71+
since = "4.23.0",
72+
length = 4096)
73+
private String domainIds;
6274

6375
/////////////////////////////////////////////////////
6476
/////////////////// Accessors ///////////////////////
@@ -89,7 +101,7 @@ public String getRetentionPeriod() {
89101
@Override
90102
public void execute() {
91103
try {
92-
if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null) {
104+
if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null && CollectionUtils.isEmpty(getDomainIds())) {
93105
throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the",
94106
"following should be informed: name, description or allowUserDrivenBackups.", id));
95107
}
@@ -110,6 +122,18 @@ public void execute() {
110122
}
111123
}
112124

125+
public List<Long> getDomainIds() {
126+
// backupManager may be null in unit tests where the command is spied without injection.
127+
// Avoid creating a method reference to a null receiver which causes NPE. When backupManager
128+
// is null, pass null as the defaultDomainsProvider so resolveDomainIds will simply return
129+
// an empty list or parse the explicit domainIds string.
130+
LongFunction<List<Long>> defaultDomainsProvider = null;
131+
if (backupManager != null) {
132+
defaultDomainsProvider = backupManager::getBackupOfferingDomains;
133+
}
134+
return resolveDomainIds(domainIds, id, defaultDomainsProvider, "backup offering");
135+
}
136+
113137
@Override
114138
public long getEntityOwnerId() {
115139
return Account.ACCOUNT_ID_SYSTEM;

‎api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java‎

Lines changed: 5 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.admin.network;
1818

19-
import java.util.ArrayList;
2019
import java.util.List;
2120

2221
import org.apache.cloudstack.api.APICommand;
@@ -26,18 +25,16 @@
2625
import org.apache.cloudstack.api.BaseCmd;
2726
import org.apache.cloudstack.api.Parameter;
2827
import org.apache.cloudstack.api.ServerApiException;
28+
import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver;
2929
import org.apache.cloudstack.api.response.NetworkOfferingResponse;
30-
import org.apache.commons.lang3.StringUtils;
3130

32-
import com.cloud.dc.DataCenter;
33-
import com.cloud.domain.Domain;
34-
import com.cloud.exception.InvalidParameterValueException;
31+
3532
import com.cloud.offering.NetworkOffering;
3633
import com.cloud.user.Account;
3734

3835
@APICommand(name = "updateNetworkOffering", description = "Updates a network offering.", responseObject = NetworkOfferingResponse.class,
3936
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
40-
public class UpdateNetworkOfferingCmd extends BaseCmd {
37+
public class UpdateNetworkOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver {
4138

4239
/////////////////////////////////////////////////////
4340
//////////////// API parameters /////////////////////
@@ -129,63 +126,11 @@ public String getTags() {
129126
}
130127

131128
public List<Long> getDomainIds() {
132-
List<Long> validDomainIds = new ArrayList<>();
133-
if (StringUtils.isNotEmpty(domainIds)) {
134-
if (domainIds.contains(",")) {
135-
String[] domains = domainIds.split(",");
136-
for (String domain : domains) {
137-
Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim());
138-
if (validDomain != null) {
139-
validDomainIds.add(validDomain.getId());
140-
} else {
141-
throw new InvalidParameterValueException("Failed to create network offering because invalid domain has been specified.");
142-
}
143-
}
144-
} else {
145-
domainIds = domainIds.trim();
146-
if (!domainIds.matches("public")) {
147-
Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim());
148-
if (validDomain != null) {
149-
validDomainIds.add(validDomain.getId());
150-
} else {
151-
throw new InvalidParameterValueException("Failed to create network offering because invalid domain has been specified.");
152-
}
153-
}
154-
}
155-
} else {
156-
validDomainIds.addAll(_configService.getNetworkOfferingDomains(id));
157-
}
158-
return validDomainIds;
129+
return resolveDomainIds(domainIds, id, _configService::getNetworkOfferingDomains, "network offering");
159130
}
160131

161132
public List<Long> getZoneIds() {
162-
List<Long> validZoneIds = new ArrayList<>();
163-
if (StringUtils.isNotEmpty(zoneIds)) {
164-
if (zoneIds.contains(",")) {
165-
String[] zones = zoneIds.split(",");
166-
for (String zone : zones) {
167-
DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim());
168-
if (validZone != null) {
169-
validZoneIds.add(validZone.getId());
170-
} else {
171-
throw new InvalidParameterValueException("Failed to create network offering because invalid zone has been specified.");
172-
}
173-
}
174-
} else {
175-
zoneIds = zoneIds.trim();
176-
if (!zoneIds.matches("all")) {
177-
DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim());
178-
if (validZone != null) {
179-
validZoneIds.add(validZone.getId());
180-
} else {
181-
throw new InvalidParameterValueException("Failed to create network offering because invalid zone has been specified.");
182-
}
183-
}
184-
}
185-
} else {
186-
validZoneIds.addAll(_configService.getNetworkOfferingZones(id));
187-
}
188-
return validZoneIds;
133+
return resolveZoneIds(zoneIds, id, _configService::getNetworkOfferingZones, "network offering");
189134
}
190135

191136
/////////////////////////////////////////////////////

‎api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java‎

Lines changed: 4 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
// under the License.
1717
package org.apache.cloudstack.api.command.admin.offering;
1818

19-
import java.util.ArrayList;
2019
import java.util.List;
2120

2221
import com.cloud.offering.DiskOffering.State;
@@ -27,19 +26,18 @@
2726
import org.apache.cloudstack.api.BaseCmd;
2827
import org.apache.cloudstack.api.Parameter;
2928
import org.apache.cloudstack.api.ServerApiException;
29+
import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver;
3030
import org.apache.cloudstack.api.response.DiskOfferingResponse;
3131
import org.apache.commons.lang3.EnumUtils;
3232
import org.apache.commons.lang3.StringUtils;
3333

34-
import com.cloud.dc.DataCenter;
35-
import com.cloud.domain.Domain;
3634
import com.cloud.exception.InvalidParameterValueException;
3735
import com.cloud.offering.DiskOffering;
3836
import com.cloud.user.Account;
3937

4038
@APICommand(name = "updateDiskOffering", description = "Updates a disk offering.", responseObject = DiskOfferingResponse.class,
4139
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
42-
public class UpdateDiskOfferingCmd extends BaseCmd {
40+
public class UpdateDiskOfferingCmd extends BaseCmd implements DomainAndZoneIdResolver {
4341

4442
/////////////////////////////////////////////////////
4543
//////////////// API parameters /////////////////////
@@ -151,63 +149,11 @@ public Boolean getDisplayOffering() {
151149
}
152150

153151
public List<Long> getDomainIds() {
154-
List<Long> validDomainIds = new ArrayList<>();
155-
if (StringUtils.isNotEmpty(domainIds)) {
156-
if (domainIds.contains(",")) {
157-
String[] domains = domainIds.split(",");
158-
for (String domain : domains) {
159-
Domain validDomain = _entityMgr.findByUuid(Domain.class, domain.trim());
160-
if (validDomain != null) {
161-
validDomainIds.add(validDomain.getId());
162-
} else {
163-
throw new InvalidParameterValueException("Failed to create disk offering because invalid domain has been specified.");
164-
}
165-
}
166-
} else {
167-
domainIds = domainIds.trim();
168-
if (!domainIds.matches("public")) {
169-
Domain validDomain = _entityMgr.findByUuid(Domain.class, domainIds.trim());
170-
if (validDomain != null) {
171-
validDomainIds.add(validDomain.getId());
172-
} else {
173-
throw new InvalidParameterValueException("Failed to create disk offering because invalid domain has been specified.");
174-
}
175-
}
176-
}
177-
} else {
178-
validDomainIds.addAll(_configService.getDiskOfferingDomains(id));
179-
}
180-
return validDomainIds;
152+
return resolveDomainIds(domainIds, id, _configService::getDiskOfferingDomains, "disk offering");
181153
}
182154

183155
public List<Long> getZoneIds() {
184-
List<Long> validZoneIds = new ArrayList<>();
185-
if (StringUtils.isNotEmpty(zoneIds)) {
186-
if (zoneIds.contains(",")) {
187-
String[] zones = zoneIds.split(",");
188-
for (String zone : zones) {
189-
DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zone.trim());
190-
if (validZone != null) {
191-
validZoneIds.add(validZone.getId());
192-
} else {
193-
throw new InvalidParameterValueException("Failed to create disk offering because invalid zone has been specified.");
194-
}
195-
}
196-
} else {
197-
zoneIds = zoneIds.trim();
198-
if (!zoneIds.matches("all")) {
199-
DataCenter validZone = _entityMgr.findByUuid(DataCenter.class, zoneIds.trim());
200-
if (validZone != null) {
201-
validZoneIds.add(validZone.getId());
202-
} else {
203-
throw new InvalidParameterValueException("Failed to create disk offering because invalid zone has been specified.");
204-
}
205-
}
206-
}
207-
} else {
208-
validZoneIds.addAll(_configService.getDiskOfferingZones(id));
209-
}
210-
return validZoneIds;
156+
return resolveZoneIds(zoneIds, id, _configService::getDiskOfferingZones, "disk offering");
211157
}
212158

213159
public String getTags() {

0 commit comments

Comments
 (0)