Skip to content

Commit a62de78

Browse files
shwstpprrohityadavcloud
authored andcommitted
server: fix listnetworkofferings with domain, refactor listvpofferings (apache#6748)
(cherry picked from commit 2dd2955) Signed-off-by: Rohit Yadav <[email protected]>
1 parent 9799582 commit a62de78

File tree

5 files changed

+71
-36
lines changed

5 files changed

+71
-36
lines changed

api/src/main/java/org/apache/cloudstack/api/command/user/vpc/ListVPCOfferingsCmd.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.cloudstack.api.ApiConstants;
2424
import org.apache.cloudstack.api.BaseListCmd;
2525
import org.apache.cloudstack.api.Parameter;
26+
import org.apache.cloudstack.api.response.DomainResponse;
2627
import org.apache.cloudstack.api.response.ListResponse;
2728
import org.apache.cloudstack.api.response.VpcOfferingResponse;
2829
import org.apache.cloudstack.api.response.ZoneResponse;
@@ -61,10 +62,17 @@ public class ListVPCOfferingsCmd extends BaseListCmd {
6162
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "list VPC offerings by state")
6263
private String state;
6364

65+
@Parameter(name = ApiConstants.DOMAIN_ID,
66+
type = CommandType.UUID,
67+
entityType = DomainResponse.class,
68+
description = "list VPC offerings available for VPC creation in specific domain",
69+
since = "4.18")
70+
private Long domainId;
71+
6472
@Parameter(name = ApiConstants.ZONE_ID,
6573
type = CommandType.UUID,
6674
entityType = ZoneResponse.class,
67-
description = "id of zone disk offering is associated with",
75+
description = "id of zone VPC offering is associated with",
6876
since = "4.13")
6977
private Long zoneId;
7078

@@ -95,6 +103,10 @@ public String getState() {
95103
return state;
96104
}
97105

106+
public Long getDomainId() {
107+
return domainId;
108+
}
109+
98110
public Long getZoneId() {
99111
return zoneId;
100112
}

engine/schema/src/main/java/com/cloud/domain/dao/DomainDao.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Set;
2121

2222
import com.cloud.domain.DomainVO;
23+
import com.cloud.user.Account;
2324
import com.cloud.utils.db.GenericDao;
2425

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

4243
List<Long> getDomainChildrenIds(String path);
44+
45+
boolean domainIdListContainsAccessibleDomain(String domainIdList, Account caller, Long domainId);
4346
}

engine/schema/src/main/java/com/cloud/domain/dao/DomainDaoImpl.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@
2424
import java.util.Set;
2525

2626

27+
import org.apache.commons.lang3.StringUtils;
2728
import org.apache.log4j.Logger;
2829
import org.springframework.stereotype.Component;
2930

3031
import com.cloud.domain.Domain;
3132
import com.cloud.domain.DomainVO;
33+
import com.cloud.user.Account;
3234
import com.cloud.utils.db.DB;
3335
import com.cloud.utils.db.GenericDaoBase;
3436
import com.cloud.utils.db.GenericSearchBuilder;
@@ -290,4 +292,27 @@ public Set<Long> getDomainParentIds(long domainId) {
290292

291293
return parentDomains;
292294
}
295+
296+
@Override
297+
public boolean domainIdListContainsAccessibleDomain(String domainIdList, Account caller, Long domainId) {
298+
if (StringUtils.isEmpty(domainIdList)) {
299+
return false;
300+
}
301+
String[] domainIdsArray = domainIdList.split(",");
302+
for (String domainIdString : domainIdsArray) {
303+
try {
304+
Long dId = Long.valueOf(domainIdString.trim());
305+
if (!Account.Type.ADMIN.equals(caller.getType()) &&
306+
isChildDomain(dId, caller.getDomainId())) {
307+
return true;
308+
}
309+
if (domainId != null && isChildDomain(dId, domainId)) {
310+
return true;
311+
}
312+
} catch (NumberFormatException nfe) {
313+
s_logger.debug(String.format("Unable to parse %s as domain ID from the list of domain IDs: %s", domainIdList.trim(), domainIdList), nfe);
314+
}
315+
}
316+
return false;
317+
}
293318
}

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6610,28 +6610,15 @@ public Pair<List<? extends NetworkOffering>, Integer> searchForNetworkOfferings(
66106610

66116611
final List<NetworkOfferingJoinVO> offerings = networkOfferingJoinDao.search(sc, searchFilter);
66126612
// Remove offerings that are not associated with caller's domain or domainId passed
6613-
if ((caller.getType() != Account.Type.ADMIN || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
6613+
if ((!Account.Type.ADMIN.equals(caller.getType()) || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
66146614
ListIterator<NetworkOfferingJoinVO> it = offerings.listIterator();
66156615
while (it.hasNext()) {
66166616
NetworkOfferingJoinVO offering = it.next();
6617-
if (StringUtils.isNotEmpty(offering.getDomainId())) {
6618-
boolean toRemove = false;
6619-
String[] domainIdsArray = offering.getDomainId().split(",");
6620-
for (String domainIdString : domainIdsArray) {
6621-
Long dId = Long.valueOf(domainIdString.trim());
6622-
if (caller.getType() != Account.Type.ADMIN &&
6623-
!_domainDao.isChildDomain(dId, caller.getDomainId())) {
6624-
toRemove = true;
6625-
break;
6626-
}
6627-
if (domainId != null && !_domainDao.isChildDomain(dId, domainId)) {
6628-
toRemove = true;
6629-
break;
6630-
}
6631-
}
6632-
if (toRemove) {
6633-
it.remove();
6634-
}
6617+
if (StringUtils.isEmpty(offering.getDomainId())) {
6618+
continue;
6619+
}
6620+
if (!_domainDao.domainIdListContainsAccessibleDomain(offering.getDomainId(), caller, domainId)) {
6621+
it.remove();
66356622
}
66366623
}
66376624
}

server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import com.cloud.dc.dao.DataCenterDao;
7171
import com.cloud.dc.dao.VlanDao;
7272
import com.cloud.deploy.DeployDestination;
73+
import com.cloud.domain.Domain;
7374
import com.cloud.domain.dao.DomainDao;
7475
import com.cloud.event.ActionEvent;
7576
import com.cloud.event.EventTypes;
@@ -667,6 +668,19 @@ public Map<Service, Set<Provider>> getVpcOffSvcProvidersMap(final long vpcOffId)
667668
return serviceProviderMap;
668669
}
669670

671+
private void verifyDomainId(Long domainId, Account caller) {
672+
if (domainId == null) {
673+
return;
674+
}
675+
Domain domain = _entityMgr.findById(Domain.class, domainId);
676+
if (domain == null) {
677+
throw new InvalidParameterValueException("Unable to find the domain by id=" + domainId);
678+
}
679+
if (!domainDao.isChildDomain(caller.getDomainId(), domainId)) {
680+
throw new InvalidParameterValueException(String.format("Unable to list VPC offerings for domain: %s as caller does not have access for it", domain.getUuid()));
681+
}
682+
}
683+
670684
@Override
671685
public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferingsCmd cmd) {
672686
Account caller = CallContext.current().getCallingAccount();
@@ -679,11 +693,14 @@ public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferi
679693
final String state = cmd.getState();
680694
final Long startIndex = cmd.getStartIndex();
681695
final Long pageSizeVal = cmd.getPageSizeVal();
696+
final Long domainId = cmd.getDomainId();
682697
final Long zoneId = cmd.getZoneId();
683698
final Filter searchFilter = new Filter(VpcOfferingJoinVO.class, "sortKey", QueryService.SortKeyAscending.value(), null, null);
684699
searchFilter.addOrderBy(VpcOfferingJoinVO.class, "id", true);
685700
final SearchCriteria<VpcOfferingJoinVO> sc = vpcOfferingJoinDao.createSearchCriteria();
686701

702+
verifyDomainId(domainId, caller);
703+
687704
if (keyword != null) {
688705
final SearchCriteria<VpcOfferingJoinVO> ssc = vpcOfferingJoinDao.createSearchCriteria();
689706
ssc.addOr("displayText", SearchCriteria.Op.LIKE, "%" + keyword + "%");
@@ -724,25 +741,16 @@ public Pair<List<? extends VpcOffering>, Integer> listVpcOfferings(ListVPCOfferi
724741

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

727-
// Remove offerings that are not associated with caller's domain
728-
// TODO: Better approach
729-
if (caller.getType() != Account.Type.ADMIN && CollectionUtils.isNotEmpty(offerings)) {
744+
// Remove offerings that are not associated with caller's domain or domainId passed
745+
if ((!Account.Type.ADMIN.equals(caller.getType()) || domainId != null) && CollectionUtils.isNotEmpty(offerings)) {
730746
ListIterator<VpcOfferingJoinVO> it = offerings.listIterator();
731747
while (it.hasNext()) {
732748
VpcOfferingJoinVO offering = it.next();
733-
if(org.apache.commons.lang3.StringUtils.isNotEmpty(offering.getDomainId())) {
734-
boolean toRemove = true;
735-
String[] domainIdsArray = offering.getDomainId().split(",");
736-
for (String domainIdString : domainIdsArray) {
737-
Long dId = Long.valueOf(domainIdString.trim());
738-
if (domainDao.isChildDomain(dId, caller.getDomainId())) {
739-
toRemove = false;
740-
break;
741-
}
742-
}
743-
if (toRemove) {
744-
it.remove();
745-
}
749+
if (org.apache.commons.lang3.StringUtils.isEmpty(offering.getDomainId())) {
750+
continue;
751+
}
752+
if (!domainDao.domainIdListContainsAccessibleDomain(offering.getDomainId(), caller, domainId)) {
753+
it.remove();
746754
}
747755
}
748756
}

0 commit comments

Comments
 (0)