etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactor updateDiskOffering API (#8446) 

Co-authored-by: Henrique Sato <henrique.sato@scclouds.com.br>
This commit is contained in:
Henrique Sato 2024-02-19 06:29:45 -03:00 committed by GitHub
parent e74a72b4ef
commit 275abaff6b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 289 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

208
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View File

@ -3919,22 +3919,9 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
List<Long> existingZoneIds = diskOfferingDetailsDao.findZoneIds(diskOfferingId);
Collections.sort(existingZoneIds);
// check if valid domain
if (CollectionUtils.isNotEmpty(domainIds)) {
for (final Long domainId: domainIds) {
if (_domainDao.findById(domainId) == null) {
throw new InvalidParameterValueException("Please specify a valid domain id");
}
}
}
validateDomain(domainIds);
// check if valid zone
if (CollectionUtils.isNotEmpty(zoneIds)) {
for (Long zoneId : zoneIds) {
if (_zoneDao.findById(zoneId) == null)
throw new InvalidParameterValueException("Please specify a valid zone id");
}
}
validateZone(zoneIds);
Long userId = CallContext.current().getCallingUserId();
if (userId == null) {
@ -3957,35 +3944,16 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
Collections.sort(filteredZoneIds);
if (account.getType() == Account.Type.DOMAIN_ADMIN) {
if (!filteredZoneIds.equals(existingZoneIds)) { // Domain-admins cannot update zone(s) for offerings
throw new InvalidParameterValueException(String.format("Unable to update zone(s) for disk offering: %s by admin: %s as it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
}
if (existingDomainIds.isEmpty()) {
throw new InvalidParameterValueException(String.format("Unable to update public disk offering: %s by user: %s because it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
} else {
if (filteredDomainIds.isEmpty()) {
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s to a public offering by user: %s because it is domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
}
}
checkDomainAdminUpdateOfferingRestrictions(diskOfferingHandle, user, filteredZoneIds, existingZoneIds, existingDomainIds, filteredDomainIds);
if (StringUtils.isNotBlank(tags) && !ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS.valueIn(account.getAccountId())) {
throw new InvalidParameterValueException(String.format("User [%s] is unable to update disk offering tags.", user.getUuid()));
}
List<Long> nonChildDomains = new ArrayList<>();
for (Long domainId : existingDomainIds) {
if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
if (name != null || displayText != null || sortKey != null) { // Domain-admins cannot update name, display text, sort key for offerings with domain which are not child domains for domain-admin
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s as it has linked domain(s) which are not child domain for domain-admin: %s", diskOfferingHandle.getUuid(), user.getUuid()));
}
nonChildDomains.add(domainId);
}
}
for (Long domainId : filteredDomainIds) {
if (!_domainDao.isChildDomain(account.getDomainId(), domainId)) {
Domain domain = _entityMgr.findById(Domain.class, domainId);
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s by domain-admin: %s with domain: %3$s which is not a child domain", diskOfferingHandle.getUuid(), user.getUuid(), domain.getUuid()));
}
}
List<Long> nonChildDomains = getAccountNonChildDomains(diskOfferingHandle, account, user, cmd, existingDomainIds);
checkIfDomainIsChildDomain(diskOfferingHandle, account, user, filteredDomainIds);
filteredDomainIds.addAll(nonChildDomains); // Final list must include domains which were not child domain for domain-admin but specified for this offering prior to update
} else if (account.getType() != Account.Type.ADMIN) {
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s by id user: %s because it is not root-admin or domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
@ -4001,22 +3969,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
}
final DiskOfferingVO diskOffering = _diskOfferingDao.createForUpdate(diskOfferingId);
if (name != null) {
diskOffering.setName(name);
}
if (displayText != null) {
diskOffering.setDisplayText(displayText);
}
if (sortKey != null) {
diskOffering.setSortKey(sortKey);
}
if (displayDiskOffering != null) {
diskOffering.setDisplayOffering(displayDiskOffering);
}
updateDiskOfferingIfCmdAttributeNotNull(diskOffering, cmd);
updateOfferingTagsIfIsNotNull(tags, diskOffering);
@ -4039,26 +3992,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
}
List<DiskOfferingDetailVO> detailsVO = new ArrayList<>();
if(detailsUpdateNeeded) {
SearchBuilder<DiskOfferingDetailVO> sb = diskOfferingDetailsDao.createSearchBuilder();
sb.and("offeringId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ);
sb.done();
SearchCriteria<DiskOfferingDetailVO> sc = sb.create();
sc.setParameters("offeringId", String.valueOf(diskOfferingId));
if(!filteredDomainIds.equals(existingDomainIds)) {
sc.setParameters("detailName", ApiConstants.DOMAIN_ID);
diskOfferingDetailsDao.remove(sc);
for (Long domainId : filteredDomainIds) {
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.DOMAIN_ID, String.valueOf(domainId), false));
}
}
if(!filteredZoneIds.equals(existingZoneIds)) {
sc.setParameters("detailName", ApiConstants.ZONE_ID);
diskOfferingDetailsDao.remove(sc);
for (Long zoneId : filteredZoneIds) {
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.ZONE_ID, String.valueOf(zoneId), false));
}
}
updateDiskOfferingDetails(detailsVO, diskOfferingId, filteredDomainIds, existingDomainIds, filteredZoneIds, existingZoneIds);
}
if (!detailsVO.isEmpty()) {
for (DiskOfferingDetailVO detailVO : detailsVO) {
@ -4069,6 +4003,128 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
return _diskOfferingDao.findById(diskOfferingId);
}
protected void validateDomain(List<Long> domainIds) {
if (CollectionUtils.isEmpty(domainIds)) {
return;
}
for (final Long domainId: domainIds) {
if (_domainDao.findById(domainId) == null) {
throw new InvalidParameterValueException("Please specify a valid domain id.");
}
}
}
protected void validateZone(List<Long> zoneIds) {
if (CollectionUtils.isEmpty(zoneIds)) {
return;
}
for (Long zoneId : zoneIds) {
if (_zoneDao.findById(zoneId) == null) {
throw new InvalidParameterValueException("Please specify a valid zone id.");
}
}
}
protected void updateDiskOfferingIfCmdAttributeNotNull(DiskOfferingVO diskOffering, UpdateDiskOfferingCmd cmd) {
if (cmd.getDiskOfferingName() != null) {
diskOffering.setName(cmd.getDiskOfferingName());
}
if (cmd.getDisplayText() != null) {
diskOffering.setDisplayText(cmd.getDisplayText());
}
if (cmd.getSortKey() != null) {
diskOffering.setSortKey(cmd.getSortKey());
}
if (cmd.getDisplayOffering() != null) {
diskOffering.setDisplayOffering(cmd.getDisplayOffering());
}
}
protected void updateDiskOfferingDetails(List<DiskOfferingDetailVO> detailsVO, Long diskOfferingId, List<Long> filteredDomainIds,
List<Long> existingDomainIds, List<Long> filteredZoneIds, List<Long> existingZoneIds) {
SearchBuilder<DiskOfferingDetailVO> sb = diskOfferingDetailsDao.createSearchBuilder();
sb.and("offeringId", sb.entity().getResourceId(), SearchCriteria.Op.EQ);
sb.and("detailName", sb.entity().getName(), SearchCriteria.Op.EQ);
sb.done();
SearchCriteria<DiskOfferingDetailVO> sc = sb.create();
sc.setParameters("offeringId", String.valueOf(diskOfferingId));
updateDiskOfferingDetailsDomainIds(detailsVO, sc, diskOfferingId, filteredDomainIds, existingDomainIds);
updateDiskOfferingDetailsZoneIds(detailsVO, sc, diskOfferingId, filteredZoneIds, existingZoneIds);
}
protected void updateDiskOfferingDetailsDomainIds(List<DiskOfferingDetailVO> detailsVO, SearchCriteria<DiskOfferingDetailVO> sc, Long diskOfferingId, List<Long> filteredDomainIds, List<Long> existingDomainIds) {
if (filteredDomainIds.equals(existingDomainIds)) {
return;
}
sc.setParameters("detailName", ApiConstants.DOMAIN_ID);
diskOfferingDetailsDao.remove(sc);
for (Long domainId : filteredDomainIds) {
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.DOMAIN_ID, String.valueOf(domainId), false));
}
}
protected void updateDiskOfferingDetailsZoneIds(List<DiskOfferingDetailVO> detailsVO, SearchCriteria<DiskOfferingDetailVO> sc, Long diskOfferingId, List<Long> filteredZoneIds, List<Long> existingZoneIds) {
if (filteredZoneIds.equals(existingZoneIds)) {
return;
}
sc.setParameters("detailName", ApiConstants.ZONE_ID);
diskOfferingDetailsDao.remove(sc);
for (Long zoneId : filteredZoneIds) {
detailsVO.add(new DiskOfferingDetailVO(diskOfferingId, ApiConstants.ZONE_ID, String.valueOf(zoneId), false));
}
}
protected void checkDomainAdminUpdateOfferingRestrictions(DiskOffering diskOffering, User user, List<Long> filteredZoneIds, List<Long> existingZoneIds,
List<Long> existingDomainIds, List<Long> filteredDomainIds) {
if (!filteredZoneIds.equals(existingZoneIds)) {
throw new InvalidParameterValueException(String.format("Unable to update zone(s) for disk offering [%s] by admin [%s] as it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
}
if (existingDomainIds.isEmpty()) {
throw new InvalidParameterValueException(String.format("Unable to update public disk offering [%s] by user [%s] because it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
}
if (filteredDomainIds.isEmpty()) {
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] to a public offering by user [%s] because it is domain-admin.", diskOffering.getUuid(), user.getUuid()));
}
}
protected List<Long> getAccountNonChildDomains(DiskOffering diskOffering, Account account, User user,
UpdateDiskOfferingCmd cmd, List<Long> existingDomainIds) {
List<Long> nonChildDomains = new ArrayList<>();
String name = cmd.getDiskOfferingName();
String displayText = cmd.getDisplayText();
Integer sortKey = cmd.getSortKey();
for (Long domainId : existingDomainIds) {
if (_domainDao.isChildDomain(account.getDomainId(), domainId)) {
continue;
}
if (ObjectUtils.anyNotNull(name, displayText, sortKey)) {
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] as it has linked domain(s) which are not child domain for domain-admin [%s].", diskOffering.getUuid(), user.getUuid()));
}
nonChildDomains.add(domainId);
}
return nonChildDomains;
}
protected void checkIfDomainIsChildDomain(DiskOffering diskOffering, Account account, User user, List<Long> filteredDomainIds) {
for (Long domainId : filteredDomainIds) {
if (_domainDao.isChildDomain(account.getDomainId(), domainId)) {
continue;
}
Domain domain = _entityMgr.findById(Domain.class, domainId);
throw new InvalidParameterValueException(String.format("Unable to update disk offering [%s] by domain-admin [%s] with domain [%3$s] which is not a child domain.", diskOffering.getUuid(), user.getUuid(), domain.getUuid()));
}
}
/**
* Check the tags parameters to the disk/service offering
* <ul>

157
server/src/test/java/com/cloud/configuration/ConfigurationManagerImplTest.java
View File

@ -21,6 +21,18 @@ import com.cloud.storage.StorageManager;
import com.cloud.utils.net.NetUtils;
import org.apache.cloudstack.framework.config.ConfigDepot;
import org.apache.cloudstack.framework.config.ConfigKey;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.domain.Domain;
import com.cloud.domain.dao.DomainDao;
import com.cloud.offering.DiskOffering;
import com.cloud.storage.DiskOfferingVO;
import com.cloud.user.Account;
import com.cloud.user.User;
import com.cloud.utils.db.EntityManager;
import com.cloud.utils.db.SearchCriteria;
import org.apache.cloudstack.api.command.admin.offering.UpdateDiskOfferingCmd;
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@ -29,7 +41,10 @@ import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;
import org.mockito.InjectMocks;
import org.mockito.Spy;
import java.util.ArrayList;
import java.util.List;
@ -37,7 +52,40 @@ import java.util.List;
public class ConfigurationManagerImplTest {
@Mock
ConfigDepot configDepot;
@InjectMocks
ConfigurationManagerImpl configurationManagerImplSpy = Mockito.spy(new ConfigurationManagerImpl());
@Mock
SearchCriteria<DiskOfferingDetailVO> searchCriteriaDiskOfferingDetailMock;
@Mock
DiskOffering diskOfferingMock;
@Mock
Account accountMock;
@Mock
User userMock;
@Mock
Domain domainMock;
@Mock
DataCenterDao zoneDaoMock;
@Mock
DomainDao domainDaoMock;
@Mock
EntityManager entityManagerMock;
@Mock
DiskOfferingDetailsDao diskOfferingDetailsDao;
@Spy
DiskOfferingVO diskOfferingVOSpy;
@Mock
UpdateDiskOfferingCmd updateDiskOfferingCmdMock;
Long validId = 1L;
Long invalidId = 100L;
List<Long> filteredZoneIds = List.of(1L, 2L, 3L);
List<Long> existingZoneIds = List.of(1L, 2L, 3L);
List<Long> filteredDomainIds = List.of(1L, 2L, 3L);
List<Long> existingDomainIds = List.of(1L, 2L, 3L);
List<Long> emptyExistingZoneIds = new ArrayList<>();
List<Long> emptyExistingDomainIds = new ArrayList<>();
List<Long> emptyFilteredDomainIds = new ArrayList<>();
@Before
public void setUp() throws Exception {
@ -50,6 +98,7 @@ public class ConfigurationManagerImplTest {
Assert.assertNull(testVariable);
}
@Test
public void validateIfIntValueIsInRangeTestInvalidValueReturnString() {
String testVariable = configurationManagerImplSpy.validateIfIntValueIsInRange("String name", "9", "1-5");
@ -250,4 +299,112 @@ public class ConfigurationManagerImplTest {
Mockito.doReturn(key).when(configurationManagerImplSpy._configDepot).get("config.iprange");
configurationManagerImplSpy.validateIpAddressRelatedConfigValues("config.iprange", "192.168.1.1-192.168.1.100");
}
@Test
public void validateDomainTestInvalidIdThrowException() {
Mockito.doReturn(null).when(domainDaoMock).findById(invalidId);
Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.validateDomain(List.of(invalidId)));
}
@Test
public void validateZoneTestInvalidIdThrowException() {
Mockito.doReturn(null).when(zoneDaoMock).findById(invalidId);
Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.validateZone(List.of(invalidId)));
}
@Test
public void updateDiskOfferingIfCmdAttributeNotNullTestNotNullValueUpdateOfferingAttribute() {
Mockito.doReturn("DiskOfferingName").when(updateDiskOfferingCmdMock).getDiskOfferingName();
Mockito.doReturn("DisplayText").when(updateDiskOfferingCmdMock).getDisplayText();
Mockito.doReturn(1).when(updateDiskOfferingCmdMock).getSortKey();
Mockito.doReturn(false).when(updateDiskOfferingCmdMock).getDisplayOffering();
configurationManagerImplSpy.updateDiskOfferingIfCmdAttributeNotNull(diskOfferingVOSpy, updateDiskOfferingCmdMock);
Assert.assertEquals(updateDiskOfferingCmdMock.getDiskOfferingName(), diskOfferingVOSpy.getName());
Assert.assertEquals(updateDiskOfferingCmdMock.getDisplayText(), diskOfferingVOSpy.getDisplayText());
Assert.assertEquals(updateDiskOfferingCmdMock.getSortKey(), (Integer) diskOfferingVOSpy.getSortKey());
Assert.assertEquals(updateDiskOfferingCmdMock.getDisplayOffering(), diskOfferingVOSpy.getDisplayOffering());
}
@Test
public void updateDiskOfferingIfCmdAttributeNotNullTestNullValueDoesntUpdateOfferingAttribute() {
Mockito.doReturn("Name").when(diskOfferingVOSpy).getName();
Mockito.doReturn("DisplayText").when(diskOfferingVOSpy).getDisplayText();
Mockito.doReturn(1).when(diskOfferingVOSpy).getSortKey();
Mockito.doReturn(true).when(diskOfferingVOSpy).getDisplayOffering();
configurationManagerImplSpy.updateDiskOfferingIfCmdAttributeNotNull(diskOfferingVOSpy, updateDiskOfferingCmdMock);
Assert.assertNotEquals(updateDiskOfferingCmdMock.getDiskOfferingName(), diskOfferingVOSpy.getName());
Assert.assertNotEquals(updateDiskOfferingCmdMock.getDisplayText(), diskOfferingVOSpy.getDisplayText());
Assert.assertNotEquals(updateDiskOfferingCmdMock.getSortKey(), (Integer) diskOfferingVOSpy.getSortKey());
Assert.assertNotEquals(updateDiskOfferingCmdMock.getDisplayOffering(), diskOfferingVOSpy.getDisplayOffering());
}
@Test
public void updateDiskOfferingDetailsDomainIdsTestDifferentDomainIdsDiskOfferingDetailsAddDomainIds() {
List<DiskOfferingDetailVO> detailsVO = new ArrayList<>();
Long diskOfferingId = validId;
configurationManagerImplSpy.updateDiskOfferingDetailsDomainIds(detailsVO, searchCriteriaDiskOfferingDetailMock, diskOfferingId, filteredDomainIds, existingDomainIds);
for (int i = 0; i < detailsVO.size(); i++) {
Assert.assertEquals(filteredDomainIds.get(i), (Long) Long.parseLong(detailsVO.get(i).getValue()));
}
}
@Test
public void checkDomainAdminUpdateOfferingRestrictionsTestDifferentZoneIdsThrowException() {
Assert.assertThrows(InvalidParameterValueException.class,
() -> configurationManagerImplSpy.checkDomainAdminUpdateOfferingRestrictions(diskOfferingMock, userMock, filteredZoneIds, emptyExistingZoneIds, existingDomainIds, filteredDomainIds));
}
@Test
public void checkDomainAdminUpdateOfferingRestrictionsTestEmptyExistingDomainIdsThrowException() {
Assert.assertThrows(InvalidParameterValueException.class,
() -> configurationManagerImplSpy.checkDomainAdminUpdateOfferingRestrictions(diskOfferingMock, userMock, filteredZoneIds, existingZoneIds, emptyExistingDomainIds, filteredDomainIds));
}
@Test
public void checkDomainAdminUpdateOfferingRestrictionsTestEmptyFilteredDomainIdsThrowException() {
Assert.assertThrows(InvalidParameterValueException.class,
() -> configurationManagerImplSpy.checkDomainAdminUpdateOfferingRestrictions(diskOfferingMock, userMock, filteredZoneIds, existingZoneIds, existingDomainIds, emptyFilteredDomainIds));
}
@Test
public void getAccountNonChildDomainsTestValidValuesReturnChildDomains() {
Mockito.doReturn(null).when(updateDiskOfferingCmdMock).getSortKey();
List<Long> nonChildDomains = configurationManagerImplSpy.getAccountNonChildDomains(diskOfferingMock, accountMock, userMock, updateDiskOfferingCmdMock, existingDomainIds);
for (int i = 0; i < existingDomainIds.size(); i++) {
Assert.assertEquals(existingDomainIds.get(i), nonChildDomains.get(i));
}
}
@Test
public void getAccountNonChildDomainsTestAllDomainsAreChildDomainsReturnEmptyList() {
for (Long existingDomainId : existingDomainIds) {
Mockito.when(domainDaoMock.isChildDomain(accountMock.getDomainId(), existingDomainId)).thenReturn(true);
}
List<Long> nonChildDomains = configurationManagerImplSpy.getAccountNonChildDomains(diskOfferingMock, accountMock, userMock, updateDiskOfferingCmdMock, existingDomainIds);
Assert.assertTrue(nonChildDomains.isEmpty());
}
@Test
public void getAccountNonChildDomainsTestNotNullCmdAttributeThrowException() {
Mockito.doReturn("name").when(updateDiskOfferingCmdMock).getDiskOfferingName();
Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.getAccountNonChildDomains(diskOfferingMock, accountMock, userMock, updateDiskOfferingCmdMock, existingDomainIds));
}
@Test
public void checkIfDomainIsChildDomainTestNonChildDomainThrowException() {
Mockito.doReturn(false).when(domainDaoMock).isChildDomain(Mockito.anyLong(), Mockito.anyLong());
Mockito.doReturn(domainMock).when(entityManagerMock).findById(Domain.class, 1L);
Assert.assertThrows(InvalidParameterValueException.class, () -> configurationManagerImplSpy.checkIfDomainIsChildDomain(diskOfferingMock, accountMock, userMock, filteredDomainIds));
}
}