server: fix listnetworkofferings with domain, refactor listvpofferings (apache#6748)

Author: shwstppr

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

@@ -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;
@@ -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;
 
@@ -94,6 +102,10 @@ public String getState() {
         return state;
     }
 
+    public Long getDomainId() {
+        return domainId;
+    }
+
     public Long getZoneId() {
         return zoneId;
     }

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

@@ -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> {
@@ -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);
 }

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

@@ -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;
@@ -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;
+    }
 }

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

@@ -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();
                 }
             }
         }

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

@@ -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;
@@ -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();
@@ -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 + "%");
@@ -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();
                 }
             }
         }