mirror of https://github.com/apache/cloudstack.git
CLOUDSTACK-9245 - Deletes ACL items when destroying the VPC or deleting the ACL itself
This commit is contained in:
parent
1571e01994
commit
3ec37a0efd
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue