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

Fix ISOs and templates listing pagination (#7974)

This commit is contained in:
Fabricio Duarte 2023-10-20 06:37:04 -03:00 committed by GitHub
parent 1bb6130795
commit d3d3027ba6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 149 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
View File

@ -29,7 +29,6 @@ import java.util.ListIterator;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@ -3823,11 +3822,62 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
}
}
applyPublicTemplateSharingRestrictions(sc, caller);
return templateChecks(isIso, hypers, tags, name, keyword, hyperType, onlyReady, bootable, zoneId, showDomr, caller,
showRemovedTmpl, parentTemplateId, showUnique, searchFilter, sc);
}
/**
* If the caller is not a root admin, restricts the search to return only public templates from the domain which
* the caller belongs to and domains with the setting 'share.public.templates.with.other.domains' enabled.
*/
protected void applyPublicTemplateSharingRestrictions(SearchCriteria<TemplateJoinVO> sc, Account caller) {
if (caller.getType() == Account.Type.ADMIN) {
s_logger.debug(String.format("Account [%s] is a root admin. Therefore, it has access to all public templates.", caller));
return;
}
List<TemplateJoinVO> publicTemplates = _templateJoinDao.listPublicTemplates();
Set<Long> unsharableDomainIds = new HashSet<>();
for (TemplateJoinVO template : publicTemplates) {
addDomainIdToSetIfDomainDoesNotShareTemplates(template.getDomainId(), caller, unsharableDomainIds);
}
if (!unsharableDomainIds.isEmpty()) {
s_logger.info(String.format("The public templates belonging to the domains [%s] will not be listed to account [%s] as they have the configuration [%s] marked as 'false'.", unsharableDomainIds, caller, QueryService.SharePublicTemplatesWithOtherDomains.key()));
sc.addAnd("domainId", SearchCriteria.Op.NOTIN, unsharableDomainIds.toArray());
}
}
/**
* Adds the provided domain ID to the set if the domain does not share templates with the account. That is, if:
* (1) the template does not belong to the domain of the account AND
* (2) the domain of the template has the setting 'share.public.templates.with.other.domains' disabled.
*/
protected void addDomainIdToSetIfDomainDoesNotShareTemplates(long domainId, Account account, Set<Long> unsharableDomainIds) {
if (domainId == account.getDomainId()) {
s_logger.trace(String.format("Domain [%s] will not be added to the set of domains with unshared templates since the account [%s] belongs to it.", domainId, account));
return;
}
if (unsharableDomainIds.contains(domainId)) {
s_logger.trace(String.format("Domain [%s] is already on the set of domains with unshared templates.", domainId));
return;
}
if (!checkIfDomainSharesTemplates(domainId)) {
s_logger.debug(String.format("Domain [%s] will be added to the set of domains with unshared templates as configuration [%s] is false.", domainId, QueryService.SharePublicTemplatesWithOtherDomains.key()));
unsharableDomainIds.add(domainId);
}
}
protected boolean checkIfDomainSharesTemplates(Long domainId) {
return QueryService.SharePublicTemplatesWithOtherDomains.valueIn(domainId);
}
private Pair<List<TemplateJoinVO>, Integer> templateChecks(boolean isIso, List<HypervisorType> hypers, Map<String, String> tags, String name, String keyword,
HypervisorType hyperType, boolean onlyReady, Boolean bootable, Long zoneId, boolean showDomr, Account caller,
boolean showRemovedTmpl, Long parentTemplateId, Boolean showUnique,
@ -3957,27 +4007,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
templates = _templateJoinDao.searchByTemplateZonePair(showRemoved, templateZonePairs);
}
if(caller.getType() != Account.Type.ADMIN) {
templates = applyPublicTemplateRestriction(templates, caller);
count = templates.size();
}
return new Pair<List<TemplateJoinVO>, Integer>(templates, count);
}
private List<TemplateJoinVO> applyPublicTemplateRestriction(List<TemplateJoinVO> templates, Account caller){
List<Long> unsharableDomainIds = templates.stream()
.map(TemplateJoinVO::getDomainId)
.distinct()
.filter(domainId -> domainId != caller.getDomainId())
.filter(Predicate.not(QueryService.SharePublicTemplatesWithOtherDomains::valueIn))
.collect(Collectors.toList());
return templates.stream()
.filter(Predicate.not(t -> unsharableDomainIds.contains(t.getDomainId())))
.collect(Collectors.toList());
}
@Override
public ListResponse<TemplateResponse> listIsos(ListIsosCmd cmd) {
Pair<List<TemplateJoinVO>, Integer> result = searchForIsosInternal(cmd);

2
server/src/main/java/com/cloud/api/query/dao/TemplateJoinDao.java
View File

@ -48,6 +48,8 @@ public interface TemplateJoinDao extends GenericDao<TemplateJoinVO, Long> {
List<TemplateJoinVO> listActiveTemplates(long storeId);
List<TemplateJoinVO> listPublicTemplates();
Pair<List<TemplateJoinVO>, Integer> searchIncludingRemovedAndCount(final SearchCriteria<TemplateJoinVO> sc, final Filter filter);
List<TemplateJoinVO> findByDistinctIds(Long... ids);

13
server/src/main/java/com/cloud/api/query/dao/TemplateJoinDaoImpl.java
View File

@ -104,6 +104,8 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa
private final SearchBuilder<TemplateJoinVO> activeTmpltSearch;
private final SearchBuilder<TemplateJoinVO> publicTmpltSearch;
protected TemplateJoinDaoImpl() {
tmpltIdPairSearch = createSearchBuilder();
@ -137,6 +139,10 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa
activeTmpltSearch.cp();
activeTmpltSearch.done();
publicTmpltSearch = createSearchBuilder();
publicTmpltSearch.and("public", publicTmpltSearch.entity().isPublicTemplate(), SearchCriteria.Op.EQ);
publicTmpltSearch.done();
// select distinct pair (template_id, zone_id)
_count = "select count(distinct temp_zone_pair) from template_view WHERE ";
}
@ -572,6 +578,13 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa
return searchIncludingRemoved(sc, null, null, false);
}
@Override
public List<TemplateJoinVO> listPublicTemplates() {
SearchCriteria<TemplateJoinVO> sc = publicTmpltSearch.create();
sc.setParameters("public", Boolean.TRUE);
return listBy(sc);
}
@Override
public Pair<List<TemplateJoinVO>, Integer> searchIncludingRemovedAndCount(final SearchCriteria<TemplateJoinVO> sc, final Filter filter) {
List<TemplateJoinVO> objects = searchIncludingRemoved(sc, filter, null, false);

83
server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java
View File

@ -17,7 +17,9 @@
package com.cloud.api.query;
import com.cloud.api.query.dao.TemplateJoinDao;
import com.cloud.api.query.vo.EventJoinVO;
import com.cloud.api.query.vo.TemplateJoinVO;
import com.cloud.event.dao.EventJoinDao;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
@ -48,10 +50,13 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.mockito.Spy;
import org.mockito.junit.MockitoJUnitRunner;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import static org.mockito.Mockito.when;
@ -61,13 +66,28 @@ public class QueryManagerImplTest {
public static final long USER_ID = 1;
public static final long ACCOUNT_ID = 1;
@Spy
@InjectMocks
private QueryManagerImpl queryManagerImplSpy = new QueryManagerImpl();
@Mock
EntityManager entityManager;
@Mock
AccountManager accountManager;
@Mock
EventJoinDao eventJoinDao;
@Mock
Account accountMock;
@Mock
TemplateJoinDao templateJoinDaoMock;
@Mock
SearchCriteria searchCriteriaMock;
private AccountVO account;
private UserVO user;
@ -176,4 +196,67 @@ public class QueryManagerImplTest {
Mockito.doThrow(new PermissionDeniedException("Denied")).when(accountManager).checkAccess(account, SecurityChecker.AccessType.ListEntry, false, network);
queryManager.searchForEvents(cmd);
}
@Test
public void applyPublicTemplateRestrictionsTestDoesNotApplyRestrictionsWhenCallerIsRootAdmin() {
Mockito.when(accountMock.getType()).thenReturn(Account.Type.ADMIN);
queryManagerImplSpy.applyPublicTemplateSharingRestrictions(searchCriteriaMock, accountMock);
Mockito.verify(searchCriteriaMock, Mockito.never()).addAnd(Mockito.anyString(), Mockito.any(), Mockito.any());
}
@Test
public void applyPublicTemplateRestrictionsTestAppliesRestrictionsWhenCallerIsNotRootAdmin() {
long callerDomainId = 1L;
long sharableDomainId = 2L;
long unsharableDomainId = 3L;
Mockito.when(accountMock.getType()).thenReturn(Account.Type.NORMAL);
Mockito.when(accountMock.getDomainId()).thenReturn(callerDomainId);
TemplateJoinVO templateMock1 = Mockito.mock(TemplateJoinVO.class);
Mockito.when(templateMock1.getDomainId()).thenReturn(callerDomainId);
Mockito.lenient().doReturn(false).when(queryManagerImplSpy).checkIfDomainSharesTemplates(callerDomainId);
TemplateJoinVO templateMock2 = Mockito.mock(TemplateJoinVO.class);
Mockito.when(templateMock2.getDomainId()).thenReturn(sharableDomainId);
Mockito.doReturn(true).when(queryManagerImplSpy).checkIfDomainSharesTemplates(sharableDomainId);
TemplateJoinVO templateMock3 = Mockito.mock(TemplateJoinVO.class);
Mockito.when(templateMock3.getDomainId()).thenReturn(unsharableDomainId);
Mockito.doReturn(false).when(queryManagerImplSpy).checkIfDomainSharesTemplates(unsharableDomainId);
List<TemplateJoinVO> publicTemplates = List.of(templateMock1, templateMock2, templateMock3);
Mockito.when(templateJoinDaoMock.listPublicTemplates()).thenReturn(publicTemplates);
queryManagerImplSpy.applyPublicTemplateSharingRestrictions(searchCriteriaMock, accountMock);
Mockito.verify(searchCriteriaMock).addAnd("domainId", SearchCriteria.Op.NOTIN, unsharableDomainId);
}
@Test
public void addDomainIdToSetIfDomainDoesNotShareTemplatesTestDoesNotAddWhenCallerBelongsToDomain() {
long domainId = 1L;
Set<Long> set = new HashSet<>();
Mockito.when(accountMock.getDomainId()).thenReturn(domainId);
queryManagerImplSpy.addDomainIdToSetIfDomainDoesNotShareTemplates(domainId, accountMock, set);
Assert.assertEquals(0, set.size());
}
@Test
public void addDomainIdToSetIfDomainDoesNotShareTemplatesTestAddsWhenDomainDoesNotShareTemplates() {
long domainId = 1L;
Set<Long> set = new HashSet<>();
Mockito.when(accountMock.getDomainId()).thenReturn(2L);
Mockito.doReturn(false).when(queryManagerImplSpy).checkIfDomainSharesTemplates(domainId);
queryManagerImplSpy.addDomainIdToSetIfDomainDoesNotShareTemplates(domainId, accountMock, set);
Assert.assertTrue(set.contains(domainId));
}
}