This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 16d45f731d7 Save the account which deliberately removed a public IP
from quarantine (#8339)
16d45f731d7 is described below
commit 16d45f731d7ae7d49287d33f21c2eb7ae141707e
Author: Fabricio Duarte <[email protected]>
AuthorDate: Mon Dec 18 03:36:31 2023 -0300
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.
---
.../java/com/cloud/network/PublicIpQuarantine.java | 2 +
.../org/apache/cloudstack/api/ApiConstants.java | 1 +
.../api/response/IpQuarantineResponse.java | 12 +++++
.../com/cloud/network/vo/PublicIpQuarantineVO.java | 12 +++++
.../com/cloud/upgrade/dao/Upgrade41810to41900.java | 4 ++
.../resources/META-INF/db/schema-41810to41900.sql | 3 ++
.../main/java/com/cloud/api/ApiResponseHelper.java | 4 ++
.../com/cloud/network/IpAddressManagerImpl.java | 2 +
.../java/com/cloud/api/ApiResponseHelperTest.java | 56 ++++++++++++++++++++++
.../com/cloud/network/IpAddressManagerTest.java | 27 +++++++++++
10 files changed, 123 insertions(+)
diff --git a/api/src/main/java/com/cloud/network/PublicIpQuarantine.java
b/api/src/main/java/com/cloud/network/PublicIpQuarantine.java
index d1ec98afe46..625a1bbbe50 100644
--- a/api/src/main/java/com/cloud/network/PublicIpQuarantine.java
+++ b/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();
diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index d7767721667..3ae0f319189 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/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";
diff --git
a/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java
b/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java
index 55720296315..c93f259382c 100644
---
a/api/src/main/java/org/apache/cloudstack/api/response/IpQuarantineResponse.java
+++
b/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 class IpQuarantineResponse extends BaseResponse {
public void setRemovalReason(String removalReason) {
this.removalReason = removalReason;
}
+
+ public String getRemoverAccountId() {
+ return removerAccountId;
+ }
+
+ public void setRemoverAccountId(String removerAccountId) {
+ this.removerAccountId = removerAccountId;
+ }
}
diff --git
a/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java
b/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java
index 56d167a0060..89e02610bd2 100644
--- a/engine/schema/src/main/java/com/cloud/network/vo/PublicIpQuarantineVO.java
+++ b/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 class PublicIpQuarantineVO implements
PublicIpQuarantine {
return removalReason;
}
+ @Override
+ public Long getRemoverAccountId() {
+ return this.removerAccountId;
+ }
+
@Override
public String getUuid() {
return uuid;
@@ -111,6 +119,10 @@ public class PublicIpQuarantineVO implements
PublicIpQuarantine {
this.removalReason = removalReason;
}
+ public void setRemoverAccountId(Long removerAccountId) {
+ this.removerAccountId = removerAccountId;
+ }
+
@Override
public Date getRemoved() {
return removed;
diff --git
a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
index bdfe58cbf89..13e30c0f6e2 100644
--- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
+++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41810to41900.java
@@ -77,6 +77,7 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
decryptConfigurationValuesFromAccountAndDomainScopesNotInSecureHiddenCategories(conn);
migrateBackupDates(conn);
addIndexes(conn);
+ addRemoverAccountIdForeignKeyToQuarantinedIps(conn);
}
@Override
@@ -262,4 +263,7 @@ public class Upgrade41810to41900 implements DbUpgrade,
DbUpgradeSystemVmTemplate
DbUpgradeUtils.addIndexIfNeeded(conn, "event", "resource_type",
"resource_id");
}
+ private void addRemoverAccountIdForeignKeyToQuarantinedIps(Connection
conn) {
+ DbUpgradeUtils.addForeignKey(conn, "quarantined_ips",
"remover_account_id", "account", "id");
+ }
}
diff --git
a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql
b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql
index 27170fcac14..308d48a311c 100644
--- a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql
+++ b/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"');
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
index 5424a36cdd8..e2b72f6175c 100644
--- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -5200,6 +5200,10 @@ public class ApiResponseHelper implements
ResponseGenerator {
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;
diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
index 75ea572491e..b5908935350 100644
--- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java
@@ -2471,9 +2471,11 @@ public class IpAddressManagerImpl extends ManagerBase
implements IpAddressManage
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);
diff --git a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java
b/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java
index f681669c789..1bea3ac78f3 100644
--- a/server/src/test/java/com/cloud/api/ApiResponseHelperTest.java
+++ b/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.annotation.dao.AnnotationDao;
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 class ApiResponseHelperTest {
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());
+ }
}
diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
index 935fb4e8c3b..5b8399ad000 100644
--- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
+++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java
@@ -36,12 +36,14 @@ import com.cloud.network.dao.PublicIpQuarantineDao;
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 class IpAddressManagerTest {
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);
