cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r...@apache.org
Subject [4/6] git commit: updated refs/heads/master to 886be62
Date Tue, 26 Jan 2016 06:56:49 GMT
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/master
Commit: 3ec37a0efd2f10a7ec6a5ff28bf0f71019b5eb94
Parents: 1571e01
Author: Wilder Rodrigues <wrodrigues@schubergphilis.com>
Authored: Thu Jan 21 16:16:11 2016 +0100
Committer: Wilder Rodrigues <wrodrigues@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);
             }
         }


Mime
View raw message