CLOUDSTACK-9245 - Deletes ACL items when destroying the VPC or deleting the ACL itself
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3ec37a0e Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3ec37a0e Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3ec37a0e Branch: refs/heads/4.7 Commit: 3ec37a0efd2f10a7ec6a5ff28bf0f71019b5eb94 Parents: 1571e01 Author: Wilder Rodrigues <wrodrig...@schubergphilis.com> Authored: Thu Jan 21 16:16:11 2016 +0100 Committer: Wilder Rodrigues <wrodrig...@schubergphilis.com> Committed: Fri Jan 22 12:49:48 2016 +0100 ---------------------------------------------------------------------- .../cloud/network/vpc/NetworkACLService.java | 5 +- .../network/vpc/NetworkACLManagerImpl.java | 12 +++- .../network/vpc/NetworkACLServiceImpl.java | 1 - .../com/cloud/network/vpc/VpcManagerImpl.java | 18 ++++- .../com/cloud/vpc/NetworkACLManagerTest.java | 72 +++++++++++--------- 5 files changed, 69 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3ec37a0e/api/src/com/cloud/network/vpc/NetworkACLService.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/vpc/NetworkACLService.java b/api/src/com/cloud/network/vpc/NetworkACLService.java index 7cd1d3b..f08fff5 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<List<? extends NetworkACLItem>, 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<String> 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 http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3ec37a0e/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java index 24193a4..8a9a799 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<NetworkVO> networks = _networkDao.listByAclId(acl.getId()); + final long aclId = acl.getId(); + final List<NetworkVO> 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<VpcGatewayVO> pvtGateways = _vpcGatewayDao.listByAclIdAndType(acl.getId(), VpcGateway.Type.Private); + final List<VpcGatewayVO> 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<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(aclId); + for (final NetworkACLItemVO networkACLItem : aclItems) { + revokeNetworkACLItem(networkACLItem.getId()); + } + + return _networkACLDao.remove(aclId); } @Override http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3ec37a0e/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java index f308b1d..4132b60 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<String> 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); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3ec37a0e/server/src/com/cloud/network/vpc/VpcManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/com/cloud/network/vpc/VpcManagerImpl.java index 2c34802..18fbfe2 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<NetworkACLVO> searchBuilder = _networkAclDao.createSearchBuilder(); + + searchBuilder.and("vpcId", searchBuilder.entity().getVpcId(), Op.IN); + final SearchCriteria<NetworkACLVO> searchCriteria = searchBuilder.create(); + searchCriteria.setParameters("vpcId", vpcId, 0); + + final Filter filter = new Filter(NetworkACLVO.class, "id", false, null, null); + final Pair<List<NetworkACLVO>, Integer> aclsCountPair = _networkAclDao.searchAndCount(searchCriteria, filter); + + final List<NetworkACLVO> acls = aclsCountPair.first(); + for (final NetworkACLVO networkAcl : acls) { + if (networkAcl.getId() != NetworkACL.DEFAULT_ALLOW && networkAcl.getId() != NetworkACL.DEFAULT_DENY) { + _networkAclMgr.deleteNetworkACL(networkAcl); + } + } return success; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3ec37a0e/server/test/com/cloud/vpc/NetworkACLManagerTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/vpc/NetworkACLManagerTest.java b/server/test/com/cloud/vpc/NetworkACLManagerTest.java index cecdf3d..9daf551 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<NetworkVO> networks = new ArrayList<NetworkVO>(); + final long aclId = 1L; + final NetworkVO network = Mockito.mock(NetworkVO.class); + final List<NetworkVO> networks = new ArrayList<NetworkVO>(); 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<VpcGatewayVO> vpcGateways = new ArrayList<VpcGatewayVO>(); - VpcGatewayVO vpcGateway = Mockito.mock(VpcGatewayVO.class); - PrivateGateway privateGateway = Mockito.mock(PrivateGateway.class); + final List<VpcGatewayVO> vpcGateways = new ArrayList<VpcGatewayVO>(); + 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<NetworkACLItemVO> rules = new ArrayList<NetworkACLItemVO>(); - 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<NetworkACLItemVO> rules = new ArrayList<NetworkACLItemVO>(); + 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<NetworkACLItemVO> aclItems = new ArrayList<NetworkACLItemVO>(); + final List<NetworkACLItemVO> aclItems = new ArrayList<NetworkACLItemVO>(); 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); } }
