List only Netris Public IPs for NAT operations (#26)

* List only Netris Public IPs for NAT operations

* rename getter and change type

* fix failing unit tests

* list all IPs if forProvider is not passed

* fix list public IPs for external providers with additional IP range

* filter provider Ips in a zone with external provider setup

* Prevent acquiring IP that is not from the external provider range

* formating

---------

Co-authored-by: nvazquez <[email protected]>

Author: Pearl1594

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -210,6 +210,7 @@ public class ApiConstants {
     public static final String FORMAT = "format";
     public static final String FOR_VIRTUAL_NETWORK = "forvirtualnetwork";
     public static final String FOR_SYSTEM_VMS = "forsystemvms";
+    public static final String FOR_PROVIDER = "forprovider";
     public static final String FULL_PATH = "fullpath";
     public static final String GATEWAY = "gateway";
     public static final String IP6_GATEWAY = "ip6gateway";

api/src/main/java/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java

@@ -108,6 +108,9 @@ public class ListPublicIpAddressesCmd extends BaseListRetrieveOnlyResourceCountC
     @Parameter(name = ApiConstants.FOR_SYSTEM_VMS, type = CommandType.BOOLEAN, description = "true if range is dedicated for system VMs", since = "4.20.0")
     private Boolean forSystemVMs;
 
+    @Parameter(name = ApiConstants.FOR_PROVIDER, type = CommandType.BOOLEAN, description = "true if range is dedicated for external network provider", since = "4.20.0")
+    private Boolean forProvider;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -183,6 +186,10 @@ public boolean getForSystemVMs() {
         return BooleanUtils.isTrue(forSystemVMs);
     }
 
+    public boolean isForProvider() {
+        return BooleanUtils.isTrue(forProvider);
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////

api/src/main/java/org/apache/cloudstack/api/response/IPAddressResponse.java

@@ -175,6 +175,10 @@ public class IPAddressResponse extends BaseResponseWithAnnotations implements Co
     @Param(description="true if range is dedicated for System VMs")
     private boolean forSystemVms;
 
+    @SerializedName(ApiConstants.FOR_PROVIDER)
+    @Param(description="true if range is dedicated for external network providers")
+    private boolean forProvider;
+
     public void setIpAddress(String ipAddress) {
         this.ipAddress = ipAddress;
     }
@@ -332,4 +336,6 @@ public void setHasRules(final boolean hasRules) {
     public void setForSystemVms(boolean forSystemVms) {
         this.forSystemVms = forSystemVms;
     }
+
+    public void setForProvider(boolean forProvider) { this.forProvider = forProvider; }
 }

server/src/main/java/com/cloud/api/ApiResponseHelper.java

@@ -1168,6 +1168,9 @@ public IPAddressResponse createIPAddressResponse(ResponseView view, IpAddress ip
 
         ipResponse.setPortable(ipAddr.isPortable());
         ipResponse.setForSystemVms(ipAddr.isForSystemVms());
+        if (Objects.nonNull(getProviderFromVlanDetailKey(vlan))) {
+            ipResponse.setForProvider(true);
+        }
 
         //set tag information
         List<? extends ResourceTag> tags = ApiDBUtils.listByResourceTypeAndId(ResourceObjectType.PublicIpAddress, ipAddr.getId());

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

@@ -33,7 +33,13 @@
 
 import javax.inject.Inject;
 
+import com.cloud.dc.VlanDetailsVO;
+import com.cloud.dc.dao.VlanDetailsDao;
+import com.cloud.network.dao.NetrisProviderDao;
+import com.cloud.network.dao.NsxProviderDao;
 import com.cloud.network.dao.PublicIpQuarantineDao;
+import com.cloud.network.element.NetrisProviderVO;
+import com.cloud.network.element.NsxProviderVO;
 import com.cloud.network.vo.PublicIpQuarantineVO;
 import com.cloud.resourcelimit.CheckedReservation;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
@@ -189,6 +195,7 @@
 import com.cloud.vm.dao.NicSecondaryIpDao;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.commons.lang3.ObjectUtils;
 
 public class IpAddressManagerImpl extends ManagerBase implements IpAddressManager, Configurable {
 
@@ -313,6 +320,12 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
     private AnnotationDao annotationDao;
     @Inject
     MessageBus messageBus;
+    @Inject
+    NsxProviderDao nsxProviderDao;
+    @Inject
+    NetrisProviderDao netrisProviderDao;
+    @Inject
+    VlanDetailsDao vlanDetailsDao;
 
     @Inject
     PublicIpQuarantineDao publicIpQuarantineDao;
@@ -1301,6 +1314,30 @@ public void releasePodIp(Long id) throws CloudRuntimeException {
         }
     }
 
+    /**
+     * When the zone is linked to external provider NSX or Netris: check if the IP to be associated is from the suitable pool
+     * Otherwise, no checks are performed
+     */
+    private void checkPublicIpOnExternalProviderZone(DataCenter zone, String ip) {
+        long zoneId = zone.getId();
+        NetrisProviderVO netrisProvider = netrisProviderDao.findByZoneId(zoneId);
+        NsxProviderVO nsxProvider = nsxProviderDao.findByZoneId(zoneId);
+        if (ObjectUtils.allNull(netrisProvider, nsxProvider)) {
+            return;
+        }
+        IPAddressVO ipAddress = _ipAddressDao.findByIpAndDcId(zoneId, ip);
+        if (ipAddress != null) {
+            String detailKey = nsxProvider != null ? ApiConstants.NSX_DETAIL_KEY : ApiConstants.NETRIS_DETAIL_KEY;
+            VlanDetailsVO vlanDetailVO = vlanDetailsDao.findDetail(ipAddress.getVlanId(), detailKey);
+            if (vlanDetailVO == null || vlanDetailVO.getValue().equalsIgnoreCase("false")) {
+                String msg = String.format("Cannot acquire IP %s on the zone %s as the IP is not from the reserved pool " +
+                        "for the external provider", ip, zone.getName());
+                logger.error(msg);
+                throw new CloudRuntimeException(msg);
+            }
+        }
+    }
+
     @DB
     @Override
     public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Account caller, long callerUserId, final DataCenter zone, final Boolean displayIp, final String ipaddress)
@@ -1309,6 +1346,8 @@ public IpAddress allocateIp(final Account ipOwner, final boolean isSystem, Accou
         final VlanType vlanType = VlanType.VirtualNetwork;
         final boolean assign = false;
 
+        checkPublicIpOnExternalProviderZone(zone, ipaddress);
+
         if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) {
             // zone is of type DataCenter. See DataCenterVO.java.
             PermissionDeniedException ex = new PermissionDeniedException(generateErrorMessageForOperationOnDisabledZone("allocate IP addresses", zone));

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -44,6 +44,11 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.dc.VlanDetailsVO;
+import com.cloud.dc.dao.VlanDetailsDao;
+import com.cloud.network.dao.NetrisProviderDao;
+import com.cloud.network.dao.NsxProviderDao;
+
 import com.cloud.utils.security.CertificateHelper;
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.SecurityChecker;
@@ -885,6 +890,8 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private VlanDao _vlanDao;
     @Inject
+    private VlanDetailsDao vlanDetailsDao;
+    @Inject
     private AccountVlanMapDao _accountVlanMapDao;
     @Inject
     private PodVlanMapDao _podVlanMapDao;
@@ -927,7 +934,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private StoragePoolJoinDao _poolJoinDao;
     @Inject
-    private NetworkDao networkDao;
+    protected NetworkDao networkDao;
     @Inject
     private StorageManager _storageMgr;
     @Inject
@@ -997,7 +1004,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     @Inject
     private NetworkModel _networkMgr;
     @Inject
-    private VpcDao _vpcDao;
+    protected VpcDao _vpcDao;
     @Inject
     private DomainVlanMapDao _domainVlanMapDao;
     @Inject
@@ -1027,6 +1034,10 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe
     protected AffinityGroupVMMapDao _affinityGroupVMMapDao;
     @Inject
     ResourceLimitService resourceLimitService;
+    @Inject
+    NsxProviderDao nsxProviderDao;
+    @Inject
+    NetrisProviderDao netrisProviderDao;
 
     private LockControllerListener _lockControllerListener;
     private final ScheduledExecutorService _eventExecutor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("EventChecker"));
@@ -2571,6 +2582,7 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
         final String address = cmd.getIpAddress();
         final Boolean forLoadBalancing = cmd.isForLoadBalancing();
         final Map<String, String> tags = cmd.getTags();
+        boolean forProvider = cmd.isForProvider();
 
         sb.and("dataCenterId", sb.entity().getDataCenterId(), SearchCriteria.Op.EQ);
         sb.and("address", sb.entity().getAddress(), SearchCriteria.Op.EQ);
@@ -2616,13 +2628,21 @@ private void buildParameters(final SearchBuilder<IPAddressVO> sb, final ListPubl
         if (isAllocated != null && isAllocated) {
             sb.and("allocated", sb.entity().getAllocatedTime(), SearchCriteria.Op.NNULL);
         }
+
+        if (forProvider) {
+            SearchBuilder<VlanDetailsVO> vlanDetailsSearch = vlanDetailsDao.createSearchBuilder();
+            vlanDetailsSearch.and("name", vlanDetailsSearch.entity().getName(), SearchCriteria.Op.IN);
+            vlanDetailsSearch.and("value", vlanDetailsSearch.entity().getValue(), SearchCriteria.Op.EQ);
+            sb.join("vlanDetailSearch", vlanDetailsSearch, sb.entity().getVlanId(), vlanDetailsSearch.entity().getResourceId(), JoinType.LEFT);
+        }
     }
 
     protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpAddressesCmd cmd, VlanType vlanType, Boolean isAllocated) {
         final Object keyword = cmd.getKeyword();
         final Long physicalNetworkId = cmd.getPhysicalNetworkId();
         final Long sourceNetworkId = cmd.getNetworkId();
-        final Long zone = cmd.getZoneId();
+        final Long vpcId = cmd.getVpcId();
+        Long zone = cmd.getZoneId();
         final String address = cmd.getIpAddress();
         final Long vlan = cmd.getVlanId();
         final Long ipId = cmd.getId();
@@ -2631,6 +2651,7 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
         final Boolean forDisplay = cmd.getDisplay();
         final String state = cmd.getState();
         final Boolean forSystemVms = cmd.getForSystemVMs();
+        final boolean forProvider = cmd.isForProvider();
         final Map<String, String> tags = cmd.getTags();
 
         sc.setJoinParameters("vlanSearch", "vlanType", vlanType);
@@ -2696,6 +2717,11 @@ protected void setParameters(SearchCriteria<IPAddressVO> sc, final ListPublicIpA
         } else {
             sc.setParameters(FOR_SYSTEMVMS, forSystemVms);
         }
+
+        if (forProvider) {
+            sc.setJoinParameters("vlanDetailSearch", "name", ApiConstants.NETRIS_DETAIL_KEY, ApiConstants.NSX_DETAIL_KEY);
+            sc.setJoinParameters("vlanDetailSearch", "value", "true");
+        }
     }
 
     @Override

server/src/test/java/com/cloud/server/ManagementServerImplTest.java

@@ -25,6 +25,8 @@
 import com.cloud.network.IpAddress;
 import com.cloud.network.IpAddressManagerImpl;
 import com.cloud.network.dao.IPAddressVO;
+import com.cloud.network.dao.NetworkDao;
+import com.cloud.network.vpc.dao.VpcDao;
 import com.cloud.storage.VMTemplateVO;
 import com.cloud.storage.dao.VMTemplateDao;
 import com.cloud.user.Account;
@@ -123,6 +125,12 @@ public class ManagementServerImplTest {
     @Mock
     UserDataManager userDataManager;
 
+    @Mock
+    VpcDao vpcDao;
+
+    @Mock
+    NetworkDao networkDao;
+
     @Spy
     ManagementServerImpl spy = new ManagementServerImpl();
 
@@ -145,6 +153,8 @@ public void setup() throws IllegalAccessException, NoSuchFieldException {
         spy._UserVmDetailsDao = userVmDetailsDao;
         spy._detailsDao = hostDetailsDao;
         spy.userDataManager = userDataManager;
+        spy._vpcDao = vpcDao;
+        spy.networkDao = networkDao;
     }
 
     @After

ui/src/views/AutogenView.vue

@@ -1001,6 +1001,19 @@ export default {
         }
 
         this.items = json[responseName][objectName]
+        var filteredItems = []
+        if (this.apiName === 'listPublicIpAddresses') {
+          for (var zone of this.$store.getters.zones) {
+            const zoneIps = this.items.filter(item => item.zoneid === zone.id)
+            const providerIps = zoneIps.filter(item => item.forprovider === true)
+            if (providerIps.length === 0) {
+              filteredItems.push(...zoneIps)
+            } else {
+              filteredItems.push(...providerIps)
+            }
+          }
+          this.items = filteredItems
+        }
         if (!this.items || this.items.length === 0) {
           this.items = []
         }
@@ -1931,7 +1944,6 @@ export default {
           this.rules[field.name].push(rule)
           break
         case (this.currentAction.mapping && field.name in this.currentAction.mapping && 'options' in this.currentAction.mapping[field.name]):
-          console.log('op: ' + field)
           rule.required = field.required
           rule.message = this.$t('message.error.select')
           this.rules[field.name].push(rule)
@@ -1942,20 +1954,17 @@ export default {
           this.rules[field.name].push(rule)
           break
         case (field.type === 'uuid'):
-          console.log('uuid: ' + field)
           rule.required = field.required
           rule.message = this.$t('message.error.select')
           this.rules[field.name].push(rule)
           break
         case (field.type === 'list'):
-          console.log('list: ' + field)
           rule.type = 'array'
           rule.required = field.required
           rule.message = this.$t('message.error.select')
           this.rules[field.name].push(rule)
           break
         case (field.type === 'long'):
-          console.log(field)
           rule.type = 'number'
           rule.required = field.required
           rule.message = this.$t('message.validate.number')

ui/src/views/network/IpAddressesTab.vue

@@ -282,10 +282,12 @@ export default {
       acquireLoading: false,
       acquireIp: null,
       listPublicIpAddress: [],
-      changeSourceNat: false
+      changeSourceNat: false,
+      zoneExtNetProvider: ''
     }
   },
-  created () {
+  async created () {
+    await this.fetchZones()
     this.fetchData()
   },
   watch: {
@@ -321,6 +323,9 @@ export default {
       } else {
         params.associatednetworkid = this.resource.id
       }
+      if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
+        params.forprovider = true
+      }
       this.fetchLoading = true
       api('listPublicIpAddresses', params).then(json => {
         this.totalIps = json.listpublicipaddressesresponse.count || 0
@@ -329,6 +334,16 @@ export default {
         this.fetchLoading = false
       })
     },
+    fetchZones () {
+      return new Promise((resolve, reject) => {
+        api('listZones', {
+          id: this.resource.zoneid
+        }).then(json => {
+          this.zoneExtNetProvider = json?.listzonesresponse?.zone?.[0]?.provider || null
+          resolve(this.zoneExtNetProvider)
+        }).catch(reject)
+      })
+    },
     fetchListPublicIpAddress () {
       return new Promise((resolve, reject) => {
         const params = {
@@ -338,6 +353,9 @@ export default {
           forvirtualnetwork: true,
           allocatedonly: false
         }
+        if (['nsx', 'netris'].includes(this.zoneExtNetProvider?.toLowerCase())) {
+          params.forprovider = true
+        }
         api('listPublicIpAddresses', params).then(json => {
           const listPublicIps = json.listpublicipaddressesresponse.publicipaddress || []
           resolve(listPublicIps)