Skip to content

server: fix listnetworkofferings with domain, refactor listvpofferings #6748

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 5 commits into from
Jan 24, 2023
Merged

server: fix listnetworkofferings with domain, refactor listvpofferings #6748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
875d2e8
server: fix listnetworkofferings with domain, refactor listvpofferings
shwstppr Sep 19, 2022
6f87729
address comments
shwstppr Dec 14, 2022
146d5da
consistency
shwstppr Dec 28, 2022
6c06e95
refactor
shwstppr Jan 13, 2023
744d839
refactor
shwstppr Jan 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListVPCOfferingsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.BaseListCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.response.DomainResponse;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.VpcOfferingResponse;
import org.apache.cloudstack.api.response.ZoneResponse;
Expand Down Expand Up @@ -60,10 +61,17 @@ public class ListVPCOfferingsCmd extends BaseListCmd {
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "list VPC offerings by state")
private String state;

@Parameter(name = ApiConstants.DOMAIN_ID,
type = CommandType.UUID,
entityType = DomainResponse.class,
description = "list VPC offerings available for VPC creation in specific domain",
since = "4.18")
private Long domainId;

@Parameter(name = ApiConstants.ZONE_ID,
type = CommandType.UUID,
entityType = ZoneResponse.class,
description = "id of zone disk offering is associated with",
description = "id of zone VPC offering is associated with",
since = "4.13")
private Long zoneId;

Expand Down Expand Up @@ -94,6 +102,10 @@ public String getState() {
return state;
}

public Long getDomainId() {
return domainId;
}

public Long getZoneId() {
return zoneId;
}
Expand Down
3 changes: 3 additions & 0 deletions engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Set;

import com.cloud.domain.DomainVO;
import com.cloud.user.Account;
import com.cloud.utils.db.GenericDao;

public interface DomainDao extends GenericDao<DomainVO, Long> {
Expand All @@ -40,4 +41,6 @@ public interface DomainDao extends GenericDao<DomainVO, Long> {
Set<Long> getDomainParentIds(long domainId);

List<Long> getDomainChildrenIds(String path);

boolean domainIdListContainsAccessibleDomain(String domainIdList, Account caller, Long domainId);
}
25 changes: 25 additions & 0 deletions engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
import java.util.Set;


import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;

import com.cloud.domain.Domain;
import com.cloud.domain.DomainVO;
import com.cloud.user.Account;
import com.cloud.utils.db.DB;
import com.cloud.utils.db.GenericDaoBase;
import com.cloud.utils.db.GenericSearchBuilder;
Expand Down Expand Up @@ -290,4 +292,27 @@ public Set<Long> getDomainParentIds(long domainId) {

return parentDomains;
}

@Override
public boolean domainIdListContainsAccessibleDomain(String domainIdList, Account caller, Long domainId) {
if (StringUtils.isEmpty(domainIdList)) {
return false;
}
String[] domainIdsArray = domainIdList.split(",");
for (String domainIdString : domainIdsArray) {
try {
Long dId = Long.valueOf(domainIdString.trim());
if (!Account.Type.ADMIN.equals(caller.getType()) &&
isChildDomain(dId, caller.getDomainId())) {
return true;
}
if (domainId != null && isChildDomain(dId, domainId)) {
return true;
}
} catch (NumberFormatException nfe) {
s_logger.debug(String.format("Unable to parse %s as domain ID from the list of domain IDs: %s", domainIdList.trim(), domainIdList), nfe);
}
}
return false;
}
}
25 changes: 6 additions & 19 deletions server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6705,28 +6705,15 @@ public Pair<List<? extends NetworkOffering>, Integer> searchForNetworkOfferings(

final List<NetworkOfferingJoinVO> offerings = networkOfferingJoinDao.search(sc, searchFilter);
// Remove offerings that are not associated with caller's domain or domainId passed
if ((caller.getType() != Account.Type.ADMIN || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
if ((!Account.Type.ADMIN.equals(caller.getType()) || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
ListIterator<NetworkOfferingJoinVO> it = offerings.listIterator();
while (it.hasNext()) {
NetworkOfferingJoinVO offering = it.next();
if (StringUtils.isNotEmpty(offering.getDomainId())) {
boolean toRemove = false;
String[] domainIdsArray = offering.getDomainId().split(",");
for (String domainIdString : domainIdsArray) {
Long dId = Long.valueOf(domainIdString.trim());
if (caller.getType() != Account.Type.ADMIN &&
!_domainDao.isChildDomain(dId, caller.getDomainId())) {
toRemove = true;
break;
}
if (domainId != null && !_domainDao.isChildDomain(dId, domainId)) {
toRemove = true;
break;
}
}
if (toRemove) {
it.remove();
}
if (StringUtils.isEmpty(offering.getDomainId())) {
continue;
}
if (!_domainDao.domainIdListContainsAccessibleDomain(offering.getDomainId(), caller, domainId)) {
it.remove();
}
}
}
Expand Down
40 changes: 24 additions & 16 deletions server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.VlanDao;
import com.cloud.deploy.DeployDestination;
import com.cloud.domain.Domain;
import com.cloud.domain.dao.DomainDao;
import com.cloud.event.ActionEvent;
import com.cloud.event.EventTypes;
Expand Down Expand Up @@ -703,6 +704,19 @@ public Map<Service, Set<Provider>> getVpcOffSvcProvidersMap(final long vpcOffId)
return serviceProviderMap;
}

private void verifyDomainId(Long domainId, Account caller) {
if (domainId == null) {
return;
}
Domain domain = _entityMgr.findById(Domain.class, domainId);
if (domain == null) {
throw new InvalidParameterValueException("Unable to find the domain by id=" + domainId);
}
if (!domainDao.isChildDomain(caller.getDomainId(), domainId)) {
throw new InvalidParameterValueException(String.format("Unable to list VPC offerings for domain: %s as caller does not have access for it", domain.getUuid()));
}
}

@Override
public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferingsCmd cmd) {
Account caller = CallContext.current().getCallingAccount();
Expand All @@ -715,11 +729,14 @@ public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferi
final String state = cmd.getState();
final Long startIndex = cmd.getStartIndex();
final Long pageSizeVal = cmd.getPageSizeVal();
final Long domainId = cmd.getDomainId();
final Long zoneId = cmd.getZoneId();
final Filter searchFilter = new Filter(VpcOfferingJoinVO.class, "sortKey", QueryService.SortKeyAscending.value(), null, null);
searchFilter.addOrderBy(VpcOfferingJoinVO.class, "id", true);
final SearchCriteria<VpcOfferingJoinVO> sc = vpcOfferingJoinDao.createSearchCriteria();

verifyDomainId(domainId, caller);

if (keyword != null) {
final SearchCriteria<VpcOfferingJoinVO> ssc = vpcOfferingJoinDao.createSearchCriteria();
ssc.addOr("displayText", SearchCriteria.Op.LIKE, "%" + keyword + "%");
Expand Down Expand Up @@ -760,25 +777,16 @@ public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferi

final List<VpcOfferingJoinVO> offerings = vpcOfferingJoinDao.search(sc, searchFilter);

// Remove offerings that are not associated with caller's domain
// TODO: Better approach
if (caller.getType() != Account.Type.ADMIN && CollectionUtils.isNotEmpty(offerings)) {
// Remove offerings that are not associated with caller's domain or domainId passed
if ((!Account.Type.ADMIN.equals(caller.getType()) || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
ListIterator<VpcOfferingJoinVO> it = offerings.listIterator();
while (it.hasNext()) {
VpcOfferingJoinVO offering = it.next();
if(org.apache.commons.lang3.StringUtils.isNotEmpty(offering.getDomainId())) {
boolean toRemove = true;
String[] domainIdsArray = offering.getDomainId().split(",");
for (String domainIdString : domainIdsArray) {
Long dId = Long.valueOf(domainIdString.trim());
if (domainDao.isChildDomain(dId, caller.getDomainId())) {
toRemove = false;
break;
}
}
if (toRemove) {
it.remove();
}
if (org.apache.commons.lang3.StringUtils.isEmpty(offering.getDomainId())) {
continue;
}
if (!domainDao.domainIdListContainsAccessibleDomain(offering.getDomainId(), caller, domainId)) {
it.remove();
}
}
}
Expand Down