You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by re...@apache.org on 2016/01/26 07:56:03 UTC
[4/5] git commit: updated refs/heads/4.7 to 1e4bc9a
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 <wr...@schubergphilis.com>
Authored: Thu Jan 21 16:16:11 2016 +0100
Committer: Wilder Rodrigues <wr...@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);
}
}