api: add ipaddress argument to disassociateIPAddress (#8222)

This PR adds argument 'ipadress' to the disassociateIpAddress api. IP address can be disassociated by directly giving the address instead of ID.

Fixes: #8125

Author: abh1sar

api/src/main/java/com/cloud/network/NetworkService.java

@@ -114,6 +114,8 @@ IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId, Long ne
 
     IpAddress getIp(long id);
 
+    IpAddress getIp(String ipAddress);
+
     Network updateGuestNetwork(final UpdateNetworkCmd cmd);
 
     /**

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

@@ -46,10 +46,14 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd {
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////
 
-    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IPAddressResponse.class, required = true, description = "the ID of the public IP address"
-        + " to disassociate")
+    @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IPAddressResponse.class, description = "the ID of the public IP address"
+        + " to disassociate. Mutually exclusive with the ipaddress parameter")
     private Long id;
 
+    @Parameter(name=ApiConstants.IP_ADDRESS, type=CommandType.STRING,  since="4.19.0", description="IP Address to be disassociated."
+        +  " Mutually exclusive with the id parameter")
+    private String ipAddress;
+
     // unexposed parameter needed for events logging
     @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, expose = false)
     private Long ownerId;
@@ -59,7 +63,18 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd {
     /////////////////////////////////////////////////////
 
     public Long getIpAddressId() {
-        return id;
+       if (id != null & ipAddress != null) {
+           throw new InvalidParameterValueException("id parameter is mutually exclusive with ipaddress parameter");
+       }
+
+       if (id != null) {
+            return id;
+        } else if (ipAddress != null) {
+            IpAddress ip = getIpAddressByIp(ipAddress);
+            return ip.getId();
+        }
+
+        throw new InvalidParameterValueException("Please specify either IP address or IP address ID");
     }
 
     /////////////////////////////////////////////////////
@@ -68,12 +83,13 @@ public Long getIpAddressId() {
 
     @Override
     public void execute() throws InsufficientAddressCapacityException {
-        CallContext.current().setEventDetails("IP ID: " + getIpAddressId());
+        Long ipAddressId = getIpAddressId();
+        CallContext.current().setEventDetails("IP ID: " + ipAddressId);
         boolean result = false;
-        if (!isPortable(id)) {
-            result = _networkService.releaseIpAddress(getIpAddressId());
+        if (!isPortable()) {
+            result = _networkService.releaseIpAddress(ipAddressId);
         } else {
-            result = _networkService.releasePortableIpAddress(getIpAddressId());
+            result = _networkService.releasePortableIpAddress(ipAddressId);
         }
         if (result) {
             SuccessResponse response = new SuccessResponse(getCommandName());
@@ -85,7 +101,7 @@ public void execute() throws InsufficientAddressCapacityException {
 
     @Override
     public String getEventType() {
-        if (!isPortable(id)) {
+        if (!isPortable()) {
             return EventTypes.EVENT_NET_IP_RELEASE;
         } else {
             return EventTypes.EVENT_PORTABLE_IP_RELEASE;
@@ -100,10 +116,7 @@ public String getEventDescription() {
     @Override
     public long getEntityOwnerId() {
         if (ownerId == null) {
-            IpAddress ip = getIpAddress(id);
-            if (ip == null) {
-                throw new InvalidParameterValueException("Unable to find IP address by ID=" + id);
-            }
+            IpAddress ip = getIpAddress();
             ownerId = ip.getAccountId();
         }
 
@@ -120,11 +133,11 @@ public String getSyncObjType() {
 
     @Override
     public Long getSyncObjId() {
-        IpAddress ip = getIpAddress(id);
+        IpAddress ip = getIpAddress();
         return ip.getAssociatedWithNetworkId();
     }
 
-    private IpAddress getIpAddress(long id) {
+    private IpAddress getIpAddressById(Long id) {
         IpAddress ip = _entityMgr.findById(IpAddress.class, id);
 
         if (ip == null) {
@@ -134,6 +147,29 @@ private IpAddress getIpAddress(long id) {
         }
     }
 
+    private IpAddress getIpAddressByIp(String ipAddress) {
+        IpAddress ip = _networkService.getIp(ipAddress);
+        if (ip == null) {
+            throw new InvalidParameterValueException("Unable to find IP address by IP address=" + ipAddress);
+        } else {
+            return ip;
+        }
+    }
+
+    private IpAddress getIpAddress() {
+        if (id != null & ipAddress != null) {
+            throw new InvalidParameterValueException("id parameter is mutually exclusive with ipaddress parameter");
+        }
+
+        if (id != null) {
+            return getIpAddressById(id);
+        } else if (ipAddress != null){
+            return getIpAddressByIp(ipAddress);
+        }
+
+        throw new InvalidParameterValueException("Please specify either IP address or IP address ID");
+    }
+
     @Override
     public ApiCommandResourceType getApiResourceType() {
         return ApiCommandResourceType.IpAddress;
@@ -144,8 +180,8 @@ public Long getApiResourceId() {
         return getIpAddressId();
     }
 
-    private boolean isPortable(long id) {
-        IpAddress ip = getIpAddress(id);
+    private boolean isPortable() {
+        IpAddress ip = getIpAddress();
         return ip.isPortable();
     }
 }

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

@@ -2798,6 +2798,11 @@ public IpAddress getIp(long ipAddressId) {
         return _ipAddressDao.findById(ipAddressId);
     }
 
+    @Override
+    public IpAddress getIp(String ipAddress) {
+        return _ipAddressDao.findByIp(ipAddress);
+    }
+
     protected boolean providersConfiguredForExternalNetworking(Collection<String> providers) {
         for (String providerStr : providers) {
             Provider provider = Network.Provider.getProvider(providerStr);

server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java

@@ -275,6 +275,15 @@ public IpAddress getIp(long id) {
         return null;
     }
 
+    /* (non-Javadoc)
+     * @see com.cloud.network.NetworkService#getIp(String)
+     */
+    @Override
+    public IpAddress getIp(String ipAddress) {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
     @Override
     public Network updateGuestNetwork(final UpdateNetworkCmd cmd) {
         // TODO Auto-generated method stub

test/integration/smoke/test_network.py

@@ -871,7 +871,7 @@ def tearDown(self):
 
     @attr(tags=["advanced", "advancedns", "smoke", "dvs"], required_hardware="false")
     def test_releaseIP(self):
-        """Test for release public IP address"""
+        """Test for release public IP address using the ID"""
 
         logger.debug("Deleting Public IP : %s" % self.ip_addr.id)
 
@@ -930,6 +930,66 @@ def test_releaseIP(self):
             )
         return
 
+    @attr(tags=["advanced", "advancedns", "smoke", "dvs"], required_hardware="false")
+    def test_releaseIP_using_IP(self):
+        """Test for release public IP address using the address"""
+
+        logger.debug("Deleting Public IP : %s" % self.ip_addr.ipaddress)
+        self.ip_address.delete_by_ip(self.apiclient)
+
+        retriesCount = 10
+        isIpAddressDisassociated = False
+        while retriesCount > 0:
+            listResponse = list_publicIP(
+                self.apiclient,
+                id=self.ip_addr.id,
+                state="Allocated"
+            )
+            if listResponse is None:
+                isIpAddressDisassociated = True
+                break
+            retriesCount -= 1
+            time.sleep(60)
+        # End while
+
+        self.assertTrue(
+            isIpAddressDisassociated,
+            "Failed to disassociate IP address")
+
+        # ListPortForwardingRules should not list
+        # associated rules with Public IP address
+        try:
+            list_nat_rule = list_nat_rules(
+                self.apiclient,
+                id=self.nat_rule.id
+            )
+            logger.debug("List NAT Rule response" + str(list_nat_rule))
+        except CloudstackAPIException:
+            logger.debug("Port Forwarding Rule is deleted")
+
+        # listLoadBalancerRules should not list
+        # associated rules with Public IP address
+        try:
+            list_lb_rule = list_lb_rules(
+                self.apiclient,
+                id=self.lb_rule.id
+            )
+            logger.debug("List LB Rule response" + str(list_lb_rule))
+        except CloudstackAPIException:
+            logger.debug("Port Forwarding Rule is deleted")
+
+        # SSH Attempt though public IP should fail
+        with self.assertRaises(Exception):
+            SshClient(
+                self.ip_addr.ipaddress,
+                self.services["natrule"]["publicport"],
+                self.virtual_machine.username,
+                self.virtual_machine.password,
+                retries=2,
+                delay=0
+            )
+        return
+
 
 class TestDeleteAccount(cloudstackTestCase):
 

tools/marvin/marvin/lib/base.py

@@ -1879,12 +1879,19 @@ def create(cls, apiclient, accountid=None, zoneid=None, domainid=None,
         return PublicIPAddress(apiclient.associateIpAddress(cmd).__dict__)
 
     def delete(self, apiclient):
-        """Dissociate Public IP address"""
+        """Dissociate Public IP address using the given ID"""
         cmd = disassociateIpAddress.disassociateIpAddressCmd()
         cmd.id = self.ipaddress.id
         apiclient.disassociateIpAddress(cmd)
         return
 
+    def delete_by_ip(self, apiclient):
+        """Dissociate Public IP address using the given IP address"""
+        cmd = disassociateIpAddress.disassociateIpAddressCmd()
+        cmd.ipaddress = self.ipaddress.ipaddress
+        apiclient.disassociateIpAddress(cmd)
+        return
+
     @classmethod
     def list(cls, apiclient, **kwargs):
         """List all Public IPs matching criteria"""