diff --git a/api/src/com/cloud/network/vpc/NetworkACLService.java b/api/src/com/cloud/network/vpc/NetworkACLService.java index 7cd1d3b3141..f08fff5425d 100644 --- a/api/src/com/cloud/network/vpc/NetworkACLService.java +++ b/api/src/com/cloud/network/vpc/NetworkACLService.java @@ -96,9 +96,8 @@ public interface NetworkACLService { Pair, Integer> listNetworkACLItems(ListNetworkACLsCmd cmd); /** - * Revoked ACL Item with specified Id + * Revoke ACL Item with specified Id * @param ruleId - * @param apply * @return */ boolean revokeNetworkACLItem(long ruleId); @@ -121,7 +120,7 @@ public interface NetworkACLService { * @throws ResourceUnavailableException */ NetworkACLItem updateNetworkACLItem(Long id, String protocol, List sourceCidrList, NetworkACLItem.TrafficType trafficType, String action, Integer number, - Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String newUUID, Boolean forDisplay) throws ResourceUnavailableException; + Integer sourcePortStart, Integer sourcePortEnd, Integer icmpCode, Integer icmpType, String newUUID, Boolean forDisplay) throws ResourceUnavailableException; /** * Associates ACL with specified Network diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index 24193a4754e..8a9a799575b 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java @@ -141,18 +141,24 @@ public class NetworkACLManagerImpl extends ManagerBase implements NetworkACLMana @Override public boolean deleteNetworkACL(final NetworkACL acl) { - final List networks = _networkDao.listByAclId(acl.getId()); + final long aclId = acl.getId(); + final List networks = _networkDao.listByAclId(aclId); if (networks != null && networks.size() > 0) { throw new CloudRuntimeException("ACL is still associated with " + networks.size() + " tier(s). Cannot delete network ACL: " + acl.getUuid()); } - final List pvtGateways = _vpcGatewayDao.listByAclIdAndType(acl.getId(), VpcGateway.Type.Private); + final List pvtGateways = _vpcGatewayDao.listByAclIdAndType(aclId, VpcGateway.Type.Private); if (pvtGateways != null && pvtGateways.size() > 0) { throw new CloudRuntimeException("ACL is still associated with " + pvtGateways.size() + " private gateway(s). Cannot delete network ACL: " + acl.getUuid()); } - return _networkACLDao.remove(acl.getId()); + final List aclItems = _networkACLItemDao.listByACL(aclId); + for (final NetworkACLItemVO networkACLItem : aclItems) { + revokeNetworkACLItem(networkACLItem.getId()); + } + + return _networkACLDao.remove(aclId); } @Override diff --git a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java index f308b1d5f69..4132b606d4e 100644 --- a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java +++ b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java @@ -627,7 +627,6 @@ public class NetworkACLServiceImpl extends ManagerBase implements NetworkACLServ } @Override - @ActionEvent(eventType = EventTypes.EVENT_NETWORK_ACL_ITEM_UPDATE, eventDescription = "Updating Network ACL Item", async = true) public NetworkACLItem updateNetworkACLItem(final Long id, final String protocol, final List sourceCidrList, final NetworkACLItem.TrafficType trafficType, final String action, final Integer number, final Integer sourcePortStart, final Integer sourcePortEnd, final Integer icmpCode, final Integer icmpType, final String newUUID, final Boolean forDisplay) throws ResourceUnavailableException { final NetworkACLItemVO aclItem = _networkACLItemDao.findById(id); diff --git a/server/src/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/com/cloud/network/vpc/VpcManagerImpl.java index 2c3480207f9..18fbfe20226 100644 --- a/server/src/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/com/cloud/network/vpc/VpcManagerImpl.java @@ -211,7 +211,7 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis @Inject NetworkACLItemDao _networkACLItemDao; @Inject - NetworkACLService _networkACLService; + NetworkACLManager _networkAclMgr; @Inject IpAddressManager _ipAddrMgr; @Inject @@ -1473,6 +1473,22 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis } } + //5) Delete ACLs + final SearchBuilder searchBuilder = _networkAclDao.createSearchBuilder(); + + searchBuilder.and("vpcId", searchBuilder.entity().getVpcId(), Op.IN); + final SearchCriteria searchCriteria = searchBuilder.create(); + searchCriteria.setParameters("vpcId", vpcId, 0); + + final Filter filter = new Filter(NetworkACLVO.class, "id", false, null, null); + final Pair, Integer> aclsCountPair = _networkAclDao.searchAndCount(searchCriteria, filter); + + final List acls = aclsCountPair.first(); + for (final NetworkACLVO networkAcl : acls) { + if (networkAcl.getId() != NetworkACL.DEFAULT_ALLOW && networkAcl.getId() != NetworkACL.DEFAULT_DENY) { + _networkAclMgr.deleteNetworkACL(networkAcl); + } + } return success; } diff --git a/server/test/com/cloud/vpc/NetworkACLManagerTest.java b/server/test/com/cloud/vpc/NetworkACLManagerTest.java index cecdf3d2c78..9daf551e9ec 100644 --- a/server/test/com/cloud/vpc/NetworkACLManagerTest.java +++ b/server/test/com/cloud/vpc/NetworkACLManagerTest.java @@ -22,7 +22,6 @@ import java.util.UUID; import javax.inject.Inject; -import com.cloud.user.User; import junit.framework.TestCase; import org.apache.cloudstack.context.CallContext; @@ -53,6 +52,7 @@ import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.network.element.NetworkACLServiceProvider; import com.cloud.network.vpc.NetworkACLItem; +import com.cloud.network.vpc.NetworkACLItem.State; import com.cloud.network.vpc.NetworkACLItemDao; import com.cloud.network.vpc.NetworkACLItemVO; import com.cloud.network.vpc.NetworkACLManager; @@ -69,10 +69,10 @@ import com.cloud.tags.dao.ResourceTagDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountVO; +import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.db.EntityManager; -import com.cloud.utils.exception.CloudRuntimeException; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) @@ -110,8 +110,8 @@ public class NetworkACLManagerTest extends TestCase { @Before public void setUp() { ComponentContext.initComponentsLifeCycle(); - Account account = new AccountVO("testaccount", 1, "testdomain", (short)0, UUID.randomUUID().toString()); - UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); + final Account account = new AccountVO("testaccount", 1, "testdomain", (short)0, UUID.randomUUID().toString()); + final UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN); CallContext.register(user, account); acl = Mockito.mock(NetworkACLVO.class); @@ -133,10 +133,10 @@ public class NetworkACLManagerTest extends TestCase { @Test @SuppressWarnings("unchecked") public void testApplyACL() throws Exception { - NetworkVO network = Mockito.mock(NetworkVO.class); + final NetworkVO network = Mockito.mock(NetworkVO.class); Mockito.when(_networkDao.findById(Matchers.anyLong())).thenReturn(network); Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))) - .thenReturn(true); + .thenReturn(true); Mockito.when(_networkAclElements.get(0).applyNetworkACLs(Matchers.any(Network.class), Matchers.anyList())).thenReturn(true); assertTrue(_aclMgr.applyACLToNetwork(1L)); } @@ -149,21 +149,21 @@ public class NetworkACLManagerTest extends TestCase { } @SuppressWarnings("unchecked") - public void driveTestApplyNetworkACL(boolean result, boolean applyNetworkACLs, boolean applyACLToPrivateGw) throws Exception { + public void driveTestApplyNetworkACL(final boolean result, final boolean applyNetworkACLs, final boolean applyACLToPrivateGw) throws Exception { // In order to test ONLY our scope method, we mock the others - NetworkACLManager aclManager = Mockito.spy(_aclMgr); + final NetworkACLManager aclManager = Mockito.spy(_aclMgr); // Prepare // Reset mocked objects to reuse Mockito.reset(_networkACLItemDao); // Make sure it is handled - long aclId = 1L; - NetworkVO network = Mockito.mock(NetworkVO.class); - List networks = new ArrayList(); + final long aclId = 1L; + final NetworkVO network = Mockito.mock(NetworkVO.class); + final List networks = new ArrayList(); networks.add(network); Mockito.when(_networkDao.listByAclId(Matchers.anyLong())) - .thenReturn(networks); + .thenReturn(networks); Mockito.when(_networkDao.findById(Matchers.anyLong())).thenReturn(network); Mockito.when(_networkModel.isProviderSupportServiceInNetwork(Matchers.anyLong(), Matchers.any(Network.Service.class), Matchers.any(Network.Provider.class))) @@ -172,21 +172,21 @@ public class NetworkACLManagerTest extends TestCase { Matchers.anyList())).thenReturn(applyNetworkACLs); // Make sure it applies ACL to private gateway - List vpcGateways = new ArrayList(); - VpcGatewayVO vpcGateway = Mockito.mock(VpcGatewayVO.class); - PrivateGateway privateGateway = Mockito.mock(PrivateGateway.class); + final List vpcGateways = new ArrayList(); + final VpcGatewayVO vpcGateway = Mockito.mock(VpcGatewayVO.class); + final PrivateGateway privateGateway = Mockito.mock(PrivateGateway.class); Mockito.when(_vpcSvc.getVpcPrivateGateway(Mockito.anyLong())).thenReturn(privateGateway); vpcGateways.add(vpcGateway); Mockito.when(_vpcGatewayDao.listByAclIdAndType(aclId, VpcGateway.Type.Private)) - .thenReturn(vpcGateways); + .thenReturn(vpcGateways); // Create 4 rules to test all 4 scenarios: only revoke should // be deleted, only add should update - List rules = new ArrayList(); - NetworkACLItemVO ruleActive = Mockito.mock(NetworkACLItemVO.class); - NetworkACLItemVO ruleStaged = Mockito.mock(NetworkACLItemVO.class); - NetworkACLItemVO rule2Revoke = Mockito.mock(NetworkACLItemVO.class); - NetworkACLItemVO rule2Add = Mockito.mock(NetworkACLItemVO.class); + final List rules = new ArrayList(); + final NetworkACLItemVO ruleActive = Mockito.mock(NetworkACLItemVO.class); + final NetworkACLItemVO ruleStaged = Mockito.mock(NetworkACLItemVO.class); + final NetworkACLItemVO rule2Revoke = Mockito.mock(NetworkACLItemVO.class); + final NetworkACLItemVO rule2Add = Mockito.mock(NetworkACLItemVO.class); Mockito.when(ruleActive.getState()).thenReturn(NetworkACLItem.State.Active); Mockito.when(ruleStaged.getState()).thenReturn(NetworkACLItem.State.Staged); Mockito.when(rule2Add.getState()).thenReturn(NetworkACLItem.State.Add); @@ -196,15 +196,15 @@ public class NetworkACLManagerTest extends TestCase { rules.add(rule2Add); rules.add(rule2Revoke); - long revokeId = 8; + final long revokeId = 8; Mockito.when(rule2Revoke.getId()).thenReturn(revokeId); - long addId = 9; + final long addId = 9; Mockito.when(rule2Add.getId()).thenReturn(addId); Mockito.when(_networkACLItemDao.findById(addId)).thenReturn(rule2Add); Mockito.when(_networkACLItemDao.listByACL(aclId)) - .thenReturn(rules); + .thenReturn(rules); // Mock methods to avoid Mockito.doReturn(applyACLToPrivateGw).when(aclManager).applyACLToPrivateGw(privateGateway); @@ -212,7 +212,7 @@ public class NetworkACLManagerTest extends TestCase { assertEquals("Result was not congruent with applyNetworkACLs and applyACLToPrivateGw", result, aclManager.applyNetworkACL(aclId)); // Assert if conditions met, network ACL was applied - int timesProcessingDone = (applyNetworkACLs && applyACLToPrivateGw) ? 1 : 0; + final int timesProcessingDone = applyNetworkACLs && applyACLToPrivateGw ? 1 : 0; Mockito.verify(_networkACLItemDao, Mockito.times(timesProcessingDone)).remove(revokeId); Mockito.verify(rule2Add, Mockito.times(timesProcessingDone)).setState(NetworkACLItem.State.Active); Mockito.verify(_networkACLItemDao, Mockito.times(timesProcessingDone)).update(addId, rule2Add); @@ -232,17 +232,27 @@ public class NetworkACLManagerTest extends TestCase { assertNotNull(_aclMgr.updateNetworkACLItem(1L, "UDP", null, NetworkACLItem.TrafficType.Ingress, "Deny", 10, 22, 32, null, null, null, true)); } - @Test(expected = CloudRuntimeException.class) + @Test public void deleteNonEmptyACL() throws Exception { - List aclItems = new ArrayList(); + final List aclItems = new ArrayList(); aclItems.add(aclItem); Mockito.when(_networkACLItemDao.listByACL(Matchers.anyLong())).thenReturn(aclItems); - _aclMgr.deleteNetworkACL(acl); + Mockito.when(acl.getId()).thenReturn(3l); + Mockito.when(_networkACLItemDao.findById(Matchers.anyLong())).thenReturn(aclItem); + Mockito.when(aclItem.getState()).thenReturn(State.Add); + Mockito.when(aclItem.getId()).thenReturn(3l); + Mockito.when(_networkACLDao.remove(Matchers.anyLong())).thenReturn(true); + + final boolean result = _aclMgr.deleteNetworkACL(acl); + + Mockito.verify(aclItem, Mockito.times(4)).getState(); + + assertTrue("Operation should be successfull!", result); } @Configuration @ComponentScan(basePackageClasses = {NetworkACLManagerImpl.class}, includeFilters = {@ComponentScan.Filter(value = NetworkACLTestConfiguration.Library.class, - type = FilterType.CUSTOM)}, useDefaultFilters = false) + type = FilterType.CUSTOM)}, useDefaultFilters = false) public static class NetworkACLTestConfiguration extends SpringUtils.CloudStackTestConfiguration { @Bean @@ -317,9 +327,9 @@ public class NetworkACLManagerTest extends TestCase { public static class Library implements TypeFilter { @Override - public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOException { + public boolean match(final MetadataReader mdr, final MetadataReaderFactory arg1) throws IOException { mdr.getClassMetadata().getClassName(); - ComponentScan cs = NetworkACLTestConfiguration.class.getAnnotation(ComponentScan.class); + final ComponentScan cs = NetworkACLTestConfiguration.class.getAnnotation(ComponentScan.class); return SpringUtils.includedInBasePackageClasses(mdr.getClassMetadata().getClassName(), cs); } }