CLOUDSTACK-763: Added comments and removed unused imports
diff --git a/api/src/com/cloud/network/vpc/NetworkACLService.java b/api/src/com/cloud/network/vpc/NetworkACLService.java
index 9fc476f..0258333 100644
--- a/api/src/com/cloud/network/vpc/NetworkACLService.java
+++ b/api/src/com/cloud/network/vpc/NetworkACLService.java
@@ -17,19 +17,12 @@
package com.cloud.network.vpc;
-import java.util.List;
-
-import com.cloud.network.vpc.NetworkACL;
-import com.cloud.network.vpc.NetworkACLItem;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.utils.Pair;
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
-import org.apache.cloudstack.api.command.user.network.CreateNetworkACLListCmd;
-import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
-import com.cloud.exception.NetworkRuleConflictException;
-import com.cloud.exception.ResourceUnavailableException;
-import com.cloud.user.Account;
-import com.cloud.utils.Pair;
+import java.util.List;
public interface NetworkACLService {
/**
@@ -49,7 +42,7 @@
NetworkACL getNetworkACL(long id);
/**
- * List NeetworkACLs by Id/Name/Network or Vpc it belongs to
+ * List NetworkACLs by Id/Name/Network or Vpc it belongs to
* @param id
* @param name
* @param networkId
@@ -111,7 +104,21 @@
*/
boolean revokeNetworkACLItem(long ruleId);
-
+ /**
+ * Updates existing aclItem applies to associated networks
+ * @param id
+ * @param protocol
+ * @param sourceCidrList
+ * @param trafficType
+ * @param action
+ * @param number
+ * @param sourcePortStart
+ * @param sourcePortEnd
+ * @param icmpCode
+ * @param icmpType
+ * @return
+ * @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) throws ResourceUnavailableException;
diff --git a/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java b/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java
index 180c749..ac681ce 100644
--- a/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java
+++ b/engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java
@@ -404,15 +404,20 @@
//Fetch all VPC Tiers
//For each tier create a network ACL and move all the acl_items to network_acl_item table
// If there are no acl_items for a tier, associate it with default ACL
+
s_logger.debug("Updating network ACLs");
+
PreparedStatement pstmt = null;
PreparedStatement pstmtDelete = null;
ResultSet rs = null;
ResultSet rsAcls = null;
ResultSet rsCidr = null;
- //1,2 are default acl Ids, start Ids from 3
+
+ //1,2 are default acl Ids, start acl Ids from 3
long nextAclId = 3;
+
try {
+ //Get all VPC tiers
pstmt = conn.prepareStatement("SELECT id, vpc_id, uuid FROM `cloud`.`networks` where vpc_id is not null and removed is null");
rs = pstmt.executeQuery();
while (rs.next()) {
@@ -430,7 +435,7 @@
if(!hasAcls){
hasAcls = true;
aclId = nextAclId++;
- //create ACL
+ //create ACL for the tier
s_logger.debug("Creating network ACL for tier: "+tierUuid);
pstmt = conn.prepareStatement("INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (?, UUID(), ? , ?, ?)");
pstmt.setLong(1, aclId);
@@ -442,7 +447,7 @@
Long fwRuleId = rsAcls.getLong(1);
String cidr = null;
- //get cidr
+ //get cidr from firewall_rules_cidrs
pstmt = conn.prepareStatement("SELECT id, source_cidr FROM `cloud`.`firewall_rules_cidrs` where firewall_rule_id = ?");
pstmt.setLong(1, fwRuleId);
rsCidr = pstmt.executeQuery();
diff --git a/server/src/com/cloud/network/vpc/NetworkACLManager.java b/server/src/com/cloud/network/vpc/NetworkACLManager.java
index 58c26e3..0ff3e88 100644
--- a/server/src/com/cloud/network/vpc/NetworkACLManager.java
+++ b/server/src/com/cloud/network/vpc/NetworkACLManager.java
@@ -16,14 +16,11 @@
// under the License.
package com.cloud.network.vpc;
-import java.util.List;
-
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.dao.NetworkVO;
-import com.cloud.network.rules.FirewallRule;
import com.cloud.user.Account;
-import com.cloud.utils.db.DB;
-import org.apache.cloudstack.api.command.user.network.CreateNetworkACLListCmd;
+
+import java.util.List;
public interface NetworkACLManager{
@@ -108,11 +105,37 @@
* @throws ResourceUnavailableException
*/
boolean revokeACLItemsForNetwork(long networkId, long userId, Account caller) throws ResourceUnavailableException;
-
+
+ /**
+ * List network ACL items by network
+ * @param guestNtwkId
+ * @return
+ */
List<NetworkACLItemVO> listNetworkACLItems(long guestNtwkId);
+ /**
+ * Applies asscociated ACL to specified network
+ * @param networkId
+ * @return
+ * @throws ResourceUnavailableException
+ */
boolean applyACLToNetwork(long networkId) throws ResourceUnavailableException;
+ /**
+ * Updates and existing network ACL Item
+ * @param id
+ * @param protocol
+ * @param sourceCidrList
+ * @param trafficType
+ * @param action
+ * @param number
+ * @param sourcePortStart
+ * @param sourcePortEnd
+ * @param icmpCode
+ * @param icmpType
+ * @return
+ * @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) throws ResourceUnavailableException;
diff --git a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java
index 430e55d..71d6da4 100644
--- a/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java
+++ b/server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java
@@ -18,7 +18,6 @@
import com.cloud.event.ActionEvent;
import com.cloud.event.EventTypes;
-import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.ResourceUnavailableException;
import com.cloud.network.Network;
import com.cloud.network.Network.Service;
@@ -78,7 +77,7 @@
public boolean applyNetworkACL(long aclId) throws ResourceUnavailableException {
boolean handled = true;
List<NetworkACLItemVO> rules = _networkACLItemDao.listByACL(aclId);
- //Find all networks using this ACL
+ //Find all networks using this ACL and apply the ACL
List<NetworkVO> networks = _networkDao.listByAclId(aclId);
for(NetworkVO network : networks){
if(!applyACLItemsToNetwork(network.getId(), rules)) {
@@ -117,7 +116,9 @@
@Override
public boolean replaceNetworkACL(NetworkACL acl, NetworkVO network) throws ResourceUnavailableException {
network.setNetworkACLId(acl.getId());
+ //Update Network ACL
if(_networkDao.update(network.getId(), network)){
+ //Apply ACL to network
return applyACLToNetwork(network.getId());
}
return false;
@@ -133,7 +134,7 @@
if("deny".equalsIgnoreCase(action)){
ruleAction = NetworkACLItem.Action.Deny;
}
- // If number is null, set it to currentMax + 1
+ // If number is null, set it to currentMax + 1 (for backward compatibility)
if(number == null){
number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1;
}
diff --git a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java
index 94be0c7..7c50d90 100644
--- a/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java
+++ b/server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java
@@ -23,7 +23,6 @@
import com.cloud.network.Networks;
import com.cloud.network.dao.NetworkDao;
import com.cloud.network.dao.NetworkVO;
-import com.cloud.network.element.NetworkACLServiceProvider;
import com.cloud.network.vpc.dao.NetworkACLDao;
import com.cloud.projects.Project.ListProjectResourcesCriteria;
import com.cloud.server.ResourceTag.TaggedResourceType;
@@ -41,7 +40,6 @@
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.db.SearchCriteria.Op;
import com.cloud.utils.net.NetUtils;
-import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
@@ -140,6 +138,7 @@
throw new InvalidParameterValueException("Unable to find specified ACL");
}
+ //Do not allow deletion of default ACLs
if(acl.getId() == NetworkACL.DEFAULT_ALLOW || acl.getId() == NetworkACL.DEFAULT_DENY){
throw new InvalidParameterValueException("Default ACL cannot be removed");
}
@@ -218,6 +217,7 @@
}
_accountMgr.checkAccess(caller, null, true, vpc);
+ //Ensure that number is unique within the ACL
if(aclItemCmd.getNumber() != null){
if(_networkACLItemDao.findByAclAndNumber(aclId, aclItemCmd.getNumber()) != null){
throw new InvalidParameterValueException("ACL item with number "+aclItemCmd.getNumber()+" already exists in ACL: "+acl.getUuid());
@@ -293,6 +293,7 @@
}
}
+ //Check ofr valid action Allow/Deny
if(action != null){
try {
NetworkACLItem.Action.valueOf(action);
diff --git a/setup/db/db/schema-410to420.sql b/setup/db/db/schema-410to420.sql
index 85d17e7..8a42812 100644
--- a/setup/db/db/schema-410to420.sql
+++ b/setup/db/db/schema-410to420.sql
@@ -1176,10 +1176,12 @@
ALTER TABLE `cloud`.`networks` add column `network_acl_id` bigint unsigned COMMENT 'network acl id';
+-- Add Default ACL deny_all
INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (1, UUID(), 0, "Default Network ACL Deny All", "default_deny");
INSERT INTO `cloud`.`network_acl_item` (id, uuid, acl_id, state, protocol, created, traffic_type, cidr, number, action) values (1, UUID(), 1, "Active", "all", now(), "Ingress", "0.0.0.0/0", 1, "Deny");
INSERT INTO `cloud`.`network_acl_item` (id, uuid, acl_id, state, protocol, created, traffic_type, cidr, number, action) values (2, UUID(), 1, "Active", "all", now(), "Egress", "0.0.0.0/0", 2, "Deny");
+-- Add Default ACL allow_all
INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (2, UUID(), 0, "Default Network ACL Allow All", "default_allow");
INSERT INTO `cloud`.`network_acl_item` (id, uuid, acl_id, state, protocol, created, traffic_type, cidr, number, action) values (3, UUID(), 2, "Active", "all", now(), "Ingress", "0.0.0.0/0", 1, "Allow");
INSERT INTO `cloud`.`network_acl_item` (id, uuid, acl_id, state, protocol, created, traffic_type, cidr, number, action) values (4, UUID(), 2, "Active", "all", now(), "Egress", "0.0.0.0/0", 2, "Allow");
