Save the account which deliberately removed a public IP from quarantine (#8339)

When a public IP gets removed from quarantine, the removal reason gets saved to the database; however, it may also be useful for operators to know who removed the public IP from quarantine. For that reason, this PR extends the public IP quarantine feature so that the account that deliberately removed an IP from quarantine also gets saved to the database.

Author: winterhazel

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

@@ -30,6 +30,8 @@ public interface PublicIpQuarantine extends InternalIdentity, Identity {
 
     String getRemovalReason();
 
+    Long getRemoverAccountId();
+
     Date getRemoved();
 
     Date getCreated();

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

@@ -803,6 +803,7 @@ public class ApiConstants {
     public static final String IPSEC_PSK = "ipsecpsk";
     public static final String GUEST_IP = "guestip";
     public static final String REMOVED = "removed";
+    public static final String REMOVER_ACCOUNT_ID = "removeraccountid";
     public static final String REMOVAL_REASON = "removalreason";
     public static final String COMPLETED = "completed";
     public static final String IKE_VERSION = "ikeversion";

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

@@ -60,6 +60,10 @@ public class IpQuarantineResponse extends BaseResponse {
     @Param(description = "The reason for removing the IP from quarantine prematurely.")
     private String removalReason;
 
+    @SerializedName(ApiConstants.REMOVER_ACCOUNT_ID)
+    @Param(description = "ID of the account that removed the IP from quarantine.")
+    private String removerAccountId;
+
     public IpQuarantineResponse() {
         super("quarantinedips");
     }
@@ -127,4 +131,12 @@ public String getRemovalReason() {
     public void setRemovalReason(String removalReason) {
         this.removalReason = removalReason;
     }
+
+    public String getRemoverAccountId() {
+        return removerAccountId;
+    }
+
+    public void setRemoverAccountId(String removerAccountId) {
+        this.removerAccountId = removerAccountId;
+    }
 }

engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java

@@ -63,6 +63,9 @@ public class PublicIpQuarantineVO implements PublicIpQuarantine {
     @Column(name = "removal_reason")
     private String removalReason = null;
 
+    @Column(name = "remover_account_id")
+    private Long removerAccountId = null;
+
     public PublicIpQuarantineVO() {
     }
 
@@ -98,6 +101,11 @@ public String getRemovalReason() {
         return removalReason;
     }
 
+    @Override
+    public Long getRemoverAccountId() {
+        return this.removerAccountId;
+    }
+
     @Override
     public String getUuid() {
         return uuid;
@@ -111,6 +119,10 @@ public void setRemovalReason(String removalReason) {
         this.removalReason = removalReason;
     }
 
+    public void setRemoverAccountId(Long removerAccountId) {
+        this.removerAccountId = removerAccountId;
+    }
+
     @Override
     public Date getRemoved() {
         return removed;

engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java

@@ -77,6 +77,7 @@ public void performDataMigration(Connection conn) {
         decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn);
         migrateBackupDates(conn);
         addIndexes(conn);
+        addRemoverAccountIdForeignKeyToQuarantinedIps(conn);
     }
 
     @Override
@@ -262,4 +263,7 @@ private void addIndexes(Connection conn) {
         DbUpgradeUtils.addIndexIfNeeded(conn, "event", "resource_type", "resource_id");
     }
 
+    private void addRemoverAccountIdForeignKeyToQuarantinedIps(Connection conn) {
+        DbUpgradeUtils.addForeignKey(conn, "quarantined_ips", "remover_account_id", "account", "id");
+    }
 }

engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql

@@ -314,3 +314,6 @@ CREATE TABLE `cloud_usage`.`bucket_statistics` (
   `size` bigint unsigned COMMENT 'total size of bucket objects',
    PRIMARY KEY(`id`)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+-- Add remover account ID to quarantined IPs table.
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.quarantined_ips', 'remover_account_id', 'bigint(20) unsigned DEFAULT NULL COMMENT "ID of the account that removed the IP from quarantine, foreign key to `account` table"');

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

@@ -5200,6 +5200,10 @@ public IpQuarantineResponse createQuarantinedIpsResponse(PublicIpQuarantine quar
         quarantinedIpsResponse.setRemoved(quarantinedIp.getRemoved());
         quarantinedIpsResponse.setEndDate(quarantinedIp.getEndDate());
         quarantinedIpsResponse.setRemovalReason(quarantinedIp.getRemovalReason());
+        if (quarantinedIp.getRemoverAccountId() != null) {
+            Account removerAccount = _accountMgr.getAccount(quarantinedIp.getRemoverAccountId());
+            quarantinedIpsResponse.setRemoverAccountId(removerAccount.getUuid());
+        }
         quarantinedIpsResponse.setResponseName("quarantinedip");
 
         return quarantinedIpsResponse;

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

@@ -2471,9 +2471,11 @@ public void removePublicIpAddressFromQuarantine(Long quarantineProcessId, String
         PublicIpQuarantineVO publicIpQuarantineVO = publicIpQuarantineDao.findById(quarantineProcessId);
         Ip ipAddress = _ipAddressDao.findById(publicIpQuarantineVO.getPublicIpAddressId()).getAddress();
         Date removedDate = new Date();
+        Long removerAccountId = CallContext.current().getCallingAccountId();
 
         publicIpQuarantineVO.setRemoved(removedDate);
         publicIpQuarantineVO.setRemovalReason(removalReason);
+        publicIpQuarantineVO.setRemoverAccountId(removerAccountId);
 
         s_logger.debug(String.format("Removing public IP Address [%s] from quarantine by updating the removed date to [%s].", ipAddress, removedDate));
         publicIpQuarantineDao.persist(publicIpQuarantineVO);

server/src/test/java/com/cloud/api/ApiResponseHelperTest.java

@@ -17,10 +17,12 @@
 package com.cloud.api;
 
 import com.cloud.domain.DomainVO;
+import com.cloud.network.PublicIpQuarantine;
 import com.cloud.network.as.AutoScaleVmGroup;
 import com.cloud.network.as.AutoScaleVmGroupVO;
 import com.cloud.network.as.AutoScaleVmProfileVO;
 import com.cloud.network.as.dao.AutoScaleVmGroupVmMapDao;
+import com.cloud.network.dao.IPAddressDao;
 import com.cloud.network.dao.IPAddressVO;
 import com.cloud.network.dao.LoadBalancerVO;
 import com.cloud.network.dao.NetworkServiceMapDao;
@@ -41,6 +43,7 @@
 import org.apache.cloudstack.api.response.AutoScaleVmGroupResponse;
 import org.apache.cloudstack.api.response.AutoScaleVmProfileResponse;
 import org.apache.cloudstack.api.response.DirectDownloadCertificateResponse;
+import org.apache.cloudstack.api.response.IpQuarantineResponse;
 import org.apache.cloudstack.api.response.NicSecondaryIpResponse;
 import org.apache.cloudstack.api.response.UnmanagedInstanceResponse;
 import org.apache.cloudstack.api.response.UsageRecordResponse;
@@ -97,6 +100,9 @@ public class ApiResponseHelperTest {
     @Mock
     UserDataDao userDataDaoMock;
 
+    @Mock
+    IPAddressDao ipAddressDaoMock;
+
     @Spy
     @InjectMocks
     ApiResponseHelper apiResponseHelper = new ApiResponseHelper();
@@ -396,4 +402,54 @@ public void testCreateUnmanagedInstanceResponseVmwareDcVms() {
         Assert.assertEquals(1, response.getDisks().size());
         Assert.assertEquals(1, response.getNics().size());
     }
+
+    @Test
+    public void createQuarantinedIpsResponseTestReturnsObject() {
+        String quarantinedIpUuid = "quarantined_ip_uuid";
+        Long previousOwnerId = 300L;
+        String previousOwnerUuid = "previous_owner_uuid";
+        String previousOwnerName = "previous_owner_name";
+        Long removerAccountId = 400L;
+        String removerAccountUuid = "remover_account_uuid";
+        Long publicIpAddressId = 500L;
+        String publicIpAddress = "1.2.3.4";
+        Date created = new Date(599L);
+        Date removed = new Date(600L);
+        Date endDate = new Date(601L);
+        String removalReason = "removalReason";
+
+        PublicIpQuarantine quarantinedIpMock = Mockito.mock(PublicIpQuarantine.class);
+        IPAddressVO ipAddressVoMock = Mockito.mock(IPAddressVO.class);
+        Account previousOwner = Mockito.mock(Account.class);
+        Account removerAccount = Mockito.mock(Account.class);
+
+        Mockito.when(quarantinedIpMock.getUuid()).thenReturn(quarantinedIpUuid);
+        Mockito.when(quarantinedIpMock.getPreviousOwnerId()).thenReturn(previousOwnerId);
+        Mockito.when(quarantinedIpMock.getPublicIpAddressId()).thenReturn(publicIpAddressId);
+        Mockito.doReturn(ipAddressVoMock).when(ipAddressDaoMock).findById(publicIpAddressId);
+        Mockito.when(ipAddressVoMock.getAddress()).thenReturn(new Ip(publicIpAddress));
+        Mockito.doReturn(previousOwner).when(accountManagerMock).getAccount(previousOwnerId);
+        Mockito.when(previousOwner.getUuid()).thenReturn(previousOwnerUuid);
+        Mockito.when(previousOwner.getName()).thenReturn(previousOwnerName);
+        Mockito.when(quarantinedIpMock.getCreated()).thenReturn(created);
+        Mockito.when(quarantinedIpMock.getRemoved()).thenReturn(removed);
+        Mockito.when(quarantinedIpMock.getEndDate()).thenReturn(endDate);
+        Mockito.when(quarantinedIpMock.getRemovalReason()).thenReturn(removalReason);
+        Mockito.when(quarantinedIpMock.getRemoverAccountId()).thenReturn(removerAccountId);
+        Mockito.when(removerAccount.getUuid()).thenReturn(removerAccountUuid);
+        Mockito.doReturn(removerAccount).when(accountManagerMock).getAccount(removerAccountId);
+
+        IpQuarantineResponse result = apiResponseHelper.createQuarantinedIpsResponse(quarantinedIpMock);
+
+        Assert.assertEquals(quarantinedIpUuid, result.getId());
+        Assert.assertEquals(publicIpAddress, result.getPublicIpAddress());
+        Assert.assertEquals(previousOwnerUuid, result.getPreviousOwnerId());
+        Assert.assertEquals(previousOwnerName, result.getPreviousOwnerName());
+        Assert.assertEquals(created, result.getCreated());
+        Assert.assertEquals(removed, result.getRemoved());
+        Assert.assertEquals(endDate, result.getEndDate());
+        Assert.assertEquals(removalReason, result.getRemovalReason());
+        Assert.assertEquals(removerAccountUuid, result.getRemoverAccountId());
+        Assert.assertEquals("quarantinedip", result.getResponseName());
+    }
 }

server/src/test/java/com/cloud/network/IpAddressManagerTest.java

@@ -36,12 +36,14 @@
 import com.cloud.network.vo.PublicIpQuarantineVO;
 import com.cloud.user.Account;
 import com.cloud.user.AccountManager;
+import org.apache.cloudstack.context.CallContext;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.runners.MockitoJUnitRunner;
@@ -397,6 +399,31 @@ public void checkIfPublicIpAddressIsNotInQuarantineAndCanBeAllocatedTestIpIsInQu
         Assert.assertFalse(result);
     }
 
+    @Test
+    public void removePublicIpAddressFromQuarantineTestPersistsObject() {
+        Long quarantineProcessId = 100L;
+        Long publicAddressId = 200L;
+        Long callingAccountId = 300L;
+        String removalReason = "removalReason";
+
+        try (MockedStatic<CallContext> callContextMockedStatic = Mockito.mockStatic(CallContext.class)) {
+            Mockito.doReturn(publicIpQuarantineVOMock).when(publicIpQuarantineDaoMock).findById(quarantineProcessId);
+            Mockito.when(publicIpQuarantineVOMock.getPublicIpAddressId()).thenReturn(publicAddressId);
+            Mockito.doReturn(ipAddressVO).when(ipAddressDao).findById(publicAddressId);
+
+            CallContext callContextMock = Mockito.mock(CallContext.class);
+            Mockito.when(callContextMock.getCallingAccountId()).thenReturn(callingAccountId);
+            callContextMockedStatic.when(CallContext::current).thenReturn(callContextMock);
+
+            ipAddressManager.removePublicIpAddressFromQuarantine(quarantineProcessId, removalReason);
+
+            Mockito.verify(publicIpQuarantineVOMock).setRemoved(Mockito.any());
+            Mockito.verify(publicIpQuarantineVOMock).setRemovalReason(removalReason);
+            Mockito.verify(publicIpQuarantineVOMock).setRemoverAccountId(callingAccountId);
+            Mockito.verify(publicIpQuarantineDaoMock).persist(publicIpQuarantineVOMock);
+        }
+    }
+
     @Test
     public void updateSourceNatIpAddress() throws Exception {
         IPAddressVO requestedIp = Mockito.mock(IPAddressVO.class);