api: Bug fixes for primate (#4214)

Adding the following fixes so primate can work without issues :
- Adding pagination for listNetworkAclLists
- Adding pagination for listRoles
- Returning mshost uuid rather than msid in list hosts response
- Allowing listVirtualMachinesMetrics to respect hostid
- Fixing return all details in template response
This commit is contained in:
davidjumani 2020-07-29 12:26:39 +05:30 committed by GitHub
parent e225db46e4
commit eec56025c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 123 additions and 54 deletions

View File

@ -19,6 +19,8 @@ package org.apache.cloudstack.acl;
import java.util.List;
import com.cloud.utils.Pair;
import org.apache.cloudstack.acl.RolePermission.Permission;
import org.apache.cloudstack.framework.config.ConfigKey;
@ -65,16 +67,22 @@ public interface RoleService {
*/
List<Role> listRoles();
Pair<List<Role>, Integer> listRoles(Long startIndex, Long limit);
/**
* Find all roles that have the giving {@link String} as part of their name.
* If the user calling the method is not a 'root admin', roles of type {@link RoleType#Admin} wil lbe removed of the returned list.
*/
List<Role> findRolesByName(String name);
Pair<List<Role>, Integer> findRolesByName(String name, Long startIndex, Long limit);
/**
* Find all roles by {@link RoleType}. If the role type is {@link RoleType#Admin}, the calling account must be a root admin, otherwise we return an empty list.
*/
List<Role> findRolesByType(RoleType roleType);
Pair<List<Role>, Integer> findRolesByType(RoleType roleType, Long startIndex, Long limit);
List<RolePermission> findAllPermissionsBy(Long roleId);
}

View File

@ -25,18 +25,19 @@ import org.apache.cloudstack.acl.Role;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.BaseListCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.RoleResponse;
import org.apache.commons.lang3.StringUtils;
import com.cloud.user.Account;
import com.cloud.utils.Pair;
import com.google.common.base.Strings;
@APICommand(name = ListRolesCmd.APINAME, description = "Lists dynamic roles in CloudStack", responseObject = RoleResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.9.0", authorized = {
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin})
public class ListRolesCmd extends BaseCmd {
RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin })
public class ListRolesCmd extends BaseListCmd {
public static final String APINAME = "listRoles";
/////////////////////////////////////////////////////
@ -77,7 +78,7 @@ public class ListRolesCmd extends BaseCmd {
@Override
public String getCommandName() {
return APINAME.toLowerCase() + BaseCmd.RESPONSE_SUFFIX;
return APINAME.toLowerCase() + BaseListCmd.RESPONSE_SUFFIX;
}
@Override
@ -85,10 +86,10 @@ public class ListRolesCmd extends BaseCmd {
return Account.ACCOUNT_ID_SYSTEM;
}
private void setupResponse(final List<Role> roles) {
private void setupResponse(final Pair<List<Role>, Integer> roles) {
final ListResponse<RoleResponse> response = new ListResponse<>();
final List<RoleResponse> roleResponses = new ArrayList<>();
for (final Role role : roles) {
for (final Role role : roles.first()) {
if (role == null) {
continue;
}
@ -100,22 +101,22 @@ public class ListRolesCmd extends BaseCmd {
roleResponse.setObjectName("role");
roleResponses.add(roleResponse);
}
response.setResponses(roleResponses);
response.setResponses(roleResponses, roles.second());
response.setResponseName(getCommandName());
setResponseObject(response);
}
@Override
public void execute() {
List<Role> roles;
Pair<List<Role>, Integer> roles;
if (getId() != null && getId() > 0L) {
roles = Collections.singletonList(roleService.findRole(getId()));
roles = new Pair<List<Role>, Integer>(Collections.singletonList(roleService.findRole(getId())), 1);
} else if (StringUtils.isNotBlank(getName())) {
roles = roleService.findRolesByName(getName());
roles = roleService.findRolesByName(getName(), getStartIndex(), getPageSizeVal());
} else if (getRoleType() != null) {
roles = roleService.findRolesByType(getRoleType());
roles = roleService.findRolesByType(getRoleType(), getStartIndex(), getPageSizeVal());
} else {
roles = roleService.listRoles();
roles = roleService.listRoles(getStartIndex(), getPageSizeVal());
}
setupResponse(roles);
}

View File

@ -200,6 +200,18 @@ public class ListVMsCmd extends BaseListTaggedResourcesCmd implements UserCmd {
return keypair;
}
public Long getHostId() {
return hostId;
}
public Long getPodId() {
return podId;
}
public Long getStorageId() {
return storageId;
}
public EnumSet<VMDetails> getDetails() throws InvalidParameterValueException {
EnumSet<VMDetails> dv;
if (viewDetails == null || viewDetails.size() <= 0) {

View File

@ -166,7 +166,7 @@ public class HostResponse extends BaseResponse {
@SerializedName("managementserverid")
@Param(description = "the management server ID of the host")
private Long managementServerId;
private String managementServerId;
@SerializedName("clusterid")
@Param(description = "the cluster ID of the host")
@ -381,7 +381,7 @@ public class HostResponse extends BaseResponse {
this.lastPinged = lastPinged;
}
public void setManagementServerId(Long managementServerId) {
public void setManagementServerId(String managementServerId) {
this.managementServerId = managementServerId;
}
@ -633,7 +633,7 @@ public class HostResponse extends BaseResponse {
return lastPinged;
}
public Long getManagementServerId() {
public String getManagementServerId() {
return managementServerId;
}

View File

@ -17,6 +17,7 @@
package org.apache.cloudstack.acl.dao;
import com.cloud.utils.Pair;
import com.cloud.utils.db.GenericDao;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.RoleVO;
@ -25,5 +26,10 @@ import java.util.List;
public interface RoleDao extends GenericDao<RoleVO, Long> {
List<RoleVO> findAllByName(String roleName);
Pair<List<RoleVO>, Integer> findAllByName(final String roleName, Long offset, Long limit);
List<RoleVO> findAllByRoleType(RoleType type);
Pair<List<RoleVO>, Integer> findAllByRoleType(RoleType type, Long offset, Long limit);
}

View File

@ -17,6 +17,8 @@
package org.apache.cloudstack.acl.dao;
import com.cloud.utils.Pair;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
@ -45,15 +47,24 @@ public class RoleDaoImpl extends GenericDaoBase<RoleVO, Long> implements RoleDao
@Override
public List<RoleVO> findAllByName(final String roleName) {
return findAllByName(roleName, null, null).first();
}
@Override
public Pair<List<RoleVO>, Integer> findAllByName(final String roleName, Long offset, Long limit) {
SearchCriteria<RoleVO> sc = RoleByNameSearch.create();
sc.setParameters("roleName", "%" + roleName + "%");
return listBy(sc);
return searchAndCount(sc, new Filter(RoleVO.class, "id", true, offset, limit));
}
@Override
public List<RoleVO> findAllByRoleType(final RoleType type) {
return findAllByRoleType(type, null, null).first();
}
public Pair<List<RoleVO>, Integer> findAllByRoleType(final RoleType type, Long offset, Long limit) {
SearchCriteria<RoleVO> sc = RoleByTypeSearch.create();
sc.setParameters("roleType", type);
return listBy(sc);
return searchAndCount(sc, new Filter(RoleVO.class, "id", true, offset, limit));
}
}

View File

@ -57,7 +57,6 @@ import org.apache.cloudstack.api.command.admin.storage.ListStoragePoolsCmd;
import org.apache.cloudstack.api.command.admin.storage.ListStorageTagsCmd;
import org.apache.cloudstack.api.command.admin.template.ListTemplatesCmdByAdmin;
import org.apache.cloudstack.api.command.admin.user.ListUsersCmd;
import org.apache.cloudstack.api.command.admin.vm.ListVMsCmdByAdmin;
import org.apache.cloudstack.api.command.admin.zone.ListZonesCmdByAdmin;
import org.apache.cloudstack.api.command.user.account.ListAccountsCmd;
import org.apache.cloudstack.api.command.user.account.ListProjectAccountsCmd;
@ -910,11 +909,10 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
Object pod = null;
Object hostId = null;
Object storageId = null;
if (cmd instanceof ListVMsCmdByAdmin) {
ListVMsCmdByAdmin adCmd = (ListVMsCmdByAdmin)cmd;
pod = adCmd.getPodId();
hostId = adCmd.getHostId();
storageId = adCmd.getStorageId();
if (_accountMgr.isRootAdmin(caller.getId())) {
pod = cmd.getPodId();
hostId = cmd.getHostId();
storageId = cmd.getStorageId();
}
sb.and("displayName", sb.entity().getDisplayName(), SearchCriteria.Op.LIKE);
@ -1064,9 +1062,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q
sc.setParameters("keyPairName", keyPairName);
}
if (cmd instanceof ListVMsCmdByAdmin) {
ListVMsCmdByAdmin aCmd = (ListVMsCmdByAdmin)cmd;
if (aCmd.getPodId() != null) {
if (_accountMgr.isRootAdmin(caller.getId())) {
if (cmd.getPodId() != null) {
sc.setParameters("podId", pod);
if (state == null) {

View File

@ -39,6 +39,8 @@ import org.apache.cloudstack.outofbandmanagement.dao.OutOfBandManagementDao;
import com.cloud.api.ApiDBUtils;
import com.cloud.api.query.vo.HostJoinVO;
import com.cloud.cluster.ManagementServerHostVO;
import com.cloud.cluster.dao.ManagementServerHostDao;
import com.cloud.gpu.HostGpuGroupsVO;
import com.cloud.gpu.VGPUTypesVO;
import com.cloud.host.Host;
@ -65,6 +67,8 @@ public class HostJoinDaoImpl extends GenericDaoBase<HostJoinVO, Long> implements
private HAConfigDao haConfigDao;
@Inject
private OutOfBandManagementDao outOfBandManagementDao;
@Inject
private ManagementServerHostDao managementServerHostDao;
private final SearchBuilder<HostJoinVO> hostSearch;
@ -103,7 +107,13 @@ public class HostJoinDaoImpl extends GenericDaoBase<HostJoinVO, Long> implements
hostResponse.setHypervisor(host.getHypervisorType());
hostResponse.setHostType(host.getType());
hostResponse.setLastPinged(new Date(host.getLastPinged()));
hostResponse.setManagementServerId(host.getManagementServerId());
Long mshostId = host.getManagementServerId();
if (mshostId != null) {
ManagementServerHostVO managementServer = managementServerHostDao.findByMsid(host.getManagementServerId());
if (managementServer != null) {
hostResponse.setManagementServerId(managementServer.getUuid());
}
}
hostResponse.setName(host.getName());
hostResponse.setPodId(host.getPodUuid());
hostResponse.setRemoved(host.getRemoved());

View File

@ -48,6 +48,7 @@ import com.cloud.storage.VMTemplateHostVO;
import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
import com.cloud.storage.VMTemplateVO;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VMTemplateDetailsDao;
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.user.Account;
import com.cloud.user.AccountService;
@ -68,6 +69,8 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa
private AccountService _accountService;
@Inject
private VMTemplateDao _vmTemplateDao;
@Inject
private VMTemplateDetailsDao _templateDetailsDao;
private final SearchBuilder<TemplateJoinVO> tmpltIdPairSearch;
@ -209,11 +212,8 @@ public class TemplateJoinDaoImpl extends GenericDaoBaseWithTagInformation<Templa
}
// set details map
if (template.getDetailName() != null) {
Map<String, String> details = new HashMap<>();
details.put(template.getDetailName(), template.getDetailValue());
templateResponse.setDetails(details);
}
Map<String, String> details = _templateDetailsDao.listDetailsKeyPairs(template.getId());
templateResponse.setDetails(details);
// update tag information
long tag_id = template.getTagId();

View File

@ -204,7 +204,7 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ
sc.setJoinParameters("networkJoin", "networkId", networkId);
}
final Filter filter = new Filter(NetworkACLVO.class, "id", false, null, null);
final Filter filter = new Filter(NetworkACLVO.class, "id", false, cmd.getStartIndex(), cmd.getPageSizeVal());
final Pair<List<NetworkACLVO>, Integer> acls = _networkACLDao.searchAndCount(sc, filter);
return new Pair<List<? extends NetworkACL>, Integer>(acls.first(), acls.second());
}

View File

@ -49,8 +49,10 @@ import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.user.dao.AccountDao;
import com.cloud.utils.ListUtils;
import com.cloud.utils.Pair;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.component.PluggableService;
import com.cloud.utils.db.Filter;
import com.cloud.utils.db.Transaction;
import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionStatus;
@ -244,49 +246,62 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
@Override
public List<Role> findRolesByName(String name) {
List<? extends Role> roles = null;
return findRolesByName(name, null, null).first();
}
@Override
public Pair<List<Role>, Integer> findRolesByName(String name, Long startIndex, Long limit) {
if (StringUtils.isNotBlank(name)) {
roles = roleDao.findAllByName(name);
Pair<List<RoleVO>, Integer> data = roleDao.findAllByName(name, startIndex, limit);
int removed = removeRootAdminRolesIfNeeded(data.first());
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
}
removeRootAdminRolesIfNeeded(roles);
return ListUtils.toListOfInterface(roles);
return new Pair<List<Role>, Integer>(new ArrayList<Role>(), 0);
}
/**
* Removes roles of the given list that have the type '{@link RoleType#Admin}' if the user calling the method is not a 'root admin'.
* The actual removal is executed via {@link #removeRootAdminRoles(List)}. Therefore, if the method is called by a 'root admin', we do nothing here.
*/
protected void removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
protected int removeRootAdminRolesIfNeeded(List<? extends Role> roles) {
Account account = getCurrentAccount();
if (!accountManager.isRootAdmin(account.getId())) {
removeRootAdminRoles(roles);
return removeRootAdminRoles(roles);
}
return 0;
}
/**
* Remove all roles that have the {@link RoleType#Admin}.
*/
protected void removeRootAdminRoles(List<? extends Role> roles) {
protected int removeRootAdminRoles(List<? extends Role> roles) {
if (CollectionUtils.isEmpty(roles)) {
return;
return 0;
}
Iterator<? extends Role> rolesIterator = roles.iterator();
int count = 0;
while (rolesIterator.hasNext()) {
Role role = rolesIterator.next();
if (RoleType.Admin == role.getRoleType()) {
count++;
rolesIterator.remove();
}
}
return count;
}
@Override
public List<Role> findRolesByType(RoleType roleType) {
return findRolesByType(roleType, null, null).first();
}
@Override
public Pair<List<Role>, Integer> findRolesByType(RoleType roleType, Long startIndex, Long limit) {
if (roleType == null || RoleType.Admin == roleType && !accountManager.isRootAdmin(getCurrentAccount().getId())) {
return Collections.emptyList();
return new Pair<List<Role>, Integer>(Collections.emptyList(), 0);
}
List<? extends Role> roles = roleDao.findAllByRoleType(roleType);
return ListUtils.toListOfInterface(roles);
Pair<List<RoleVO>, Integer> data = roleDao.findAllByRoleType(roleType, startIndex, limit);
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second()));
}
@Override
@ -296,6 +311,14 @@ public class RoleManagerImpl extends ManagerBase implements RoleService, Configu
return ListUtils.toListOfInterface(roles);
}
@Override
public Pair<List<Role>, Integer> listRoles(Long startIndex, Long limit) {
Pair<List<RoleVO>, Integer> data = roleDao.searchAndCount(null,
new Filter(RoleVO.class, "id", Boolean.TRUE, startIndex, limit));
int removed = removeRootAdminRolesIfNeeded(data.first());
return new Pair<List<Role>,Integer>(ListUtils.toListOfInterface(data.first()), Integer.valueOf(data.second() - removed));
}
@Override
public List<RolePermission> findAllPermissionsBy(final Long roleId) {
List<? extends RolePermission> permissions = rolePermissionsDao.findAllByRoleIdSorted(roleId);

View File

@ -34,6 +34,7 @@ import org.mockito.runners.MockitoJUnitRunner;
import com.cloud.user.Account;
import com.cloud.user.AccountManager;
import com.cloud.utils.Pair;
@RunWith(MockitoJUnitRunner.class)
public class RoleManagerImplTest {
@ -163,12 +164,12 @@ public class RoleManagerImplTest {
@Test
public void findRolesByNameTest() {
String roleName = "roleName";
ArrayList<Role> toBeReturned = new ArrayList<>();
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByName(roleName);
List<Role> roles = new ArrayList<>();
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 0);
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByName(roleName, null, null);
roleManagerImpl.findRolesByName(roleName);
Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(toBeReturned);
Mockito.verify(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
}
@Test
@ -248,12 +249,12 @@ public class RoleManagerImplTest {
List<Role> roles = new ArrayList<>();
roles.add(Mockito.mock(Role.class));
Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.Admin);
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 1);
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByRoleType(RoleType.Admin, null, null);
List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.Admin);
Assert.assertEquals(1, returnedRoles.size());
Mockito.verify(accountManagerMock, Mockito.times(1)).isRootAdmin(Mockito.anyLong());
Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
}
@Test
@ -263,12 +264,12 @@ public class RoleManagerImplTest {
List<Role> roles = new ArrayList<>();
roles.add(Mockito.mock(Role.class));
Mockito.doReturn(roles).when(roleDaoMock).findAllByRoleType(RoleType.User);
Pair<ArrayList<RoleVO>, Integer> toBeReturned = new Pair(roles, 1);
Mockito.doReturn(toBeReturned).when(roleDaoMock).findAllByRoleType(RoleType.User, null, null);
List<Role> returnedRoles = roleManagerImpl.findRolesByType(RoleType.User);
Assert.assertEquals(1, returnedRoles.size());
Mockito.verify(accountManagerMock, Mockito.times(0)).isRootAdmin(Mockito.anyLong());
Mockito.verify(roleDaoMock, Mockito.times(1)).findAllByRoleType(Mockito.any(RoleType.class));
}
@Test
@ -277,7 +278,7 @@ public class RoleManagerImplTest {
roles.add(Mockito.mock(Role.class));
Mockito.doReturn(roles).when(roleDaoMock).listAll();
Mockito.doNothing().when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
Mockito.doReturn(0).when(roleManagerImpl).removeRootAdminRolesIfNeeded(roles);
List<Role> returnedRoles = roleManagerImpl.listRoles();