Skip to content

Commit 5e009c4

Browse files
author
Kishan Kavala
committed
CLOUDSTACK-763: Added comments and removed unused imports
1 parent 2cdb540 commit 5e009c4

6 files changed

Lines changed: 65 additions & 26 deletions

File tree

‎api/src/com/cloud/network/vpc/NetworkACLService.java‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,12 @@
1717
package com.cloud.network.vpc;
1818

1919

20-
import java.util.List;
21-
22-
import com.cloud.network.vpc.NetworkACL;
23-
import com.cloud.network.vpc.NetworkACLItem;
20+
import com.cloud.exception.ResourceUnavailableException;
21+
import com.cloud.utils.Pair;
2422
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
25-
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLListCmd;
26-
import org.apache.cloudstack.api.command.user.network.ListNetworkACLListsCmd;
2723
import org.apache.cloudstack.api.command.user.network.ListNetworkACLsCmd;
2824

29-
import com.cloud.exception.NetworkRuleConflictException;
30-
import com.cloud.exception.ResourceUnavailableException;
31-
import com.cloud.user.Account;
32-
import com.cloud.utils.Pair;
25+
import java.util.List;
3326

3427
public interface NetworkACLService {
3528
/**
@@ -49,7 +42,7 @@ public interface NetworkACLService {
4942
NetworkACL getNetworkACL(long id);
5043

5144
/**
52-
* List NeetworkACLs by Id/Name/Network or Vpc it belongs to
45+
* List NetworkACLs by Id/Name/Network or Vpc it belongs to
5346
* @param id
5447
* @param name
5548
* @param networkId
@@ -111,7 +104,21 @@ public interface NetworkACLService {
111104
*/
112105
boolean revokeNetworkACLItem(long ruleId);
113106

114-
107+
/**
108+
* Updates existing aclItem applies to associated networks
109+
* @param id
110+
* @param protocol
111+
* @param sourceCidrList
112+
* @param trafficType
113+
* @param action
114+
* @param number
115+
* @param sourcePortStart
116+
* @param sourcePortEnd
117+
* @param icmpCode
118+
* @param icmpType
119+
* @return
120+
* @throws ResourceUnavailableException
121+
*/
115122
NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType,
116123
String action, Integer number, Integer sourcePortStart, Integer sourcePortEnd,
117124
Integer icmpCode, Integer icmpType) throws ResourceUnavailableException;

‎engine/schema/src/com/cloud/upgrade/dao/Upgrade410to420.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,15 +402,20 @@ private void updateNetworkACLs(Connection conn) {
402402
//Fetch all VPC Tiers
403403
//For each tier create a network ACL and move all the acl_items to network_acl_item table
404404
// If there are no acl_items for a tier, associate it with default ACL
405+
405406
s_logger.debug("Updating network ACLs");
407+
406408
PreparedStatement pstmt = null;
407409
PreparedStatement pstmtDelete = null;
408410
ResultSet rs = null;
409411
ResultSet rsAcls = null;
410412
ResultSet rsCidr = null;
411-
//1,2 are default acl Ids, start Ids from 3
413+
414+
//1,2 are default acl Ids, start acl Ids from 3
412415
long nextAclId = 3;
416+
413417
try {
418+
//Get all VPC tiers
414419
pstmt = conn.prepareStatement("SELECT id, vpc_id, uuid FROM `cloud`.`networks` where vpc_id is not null and removed is null");
415420
rs = pstmt.executeQuery();
416421
while (rs.next()) {
@@ -428,7 +433,7 @@ private void updateNetworkACLs(Connection conn) {
428433
if(!hasAcls){
429434
hasAcls = true;
430435
aclId = nextAclId++;
431-
//create ACL
436+
//create ACL for the tier
432437
s_logger.debug("Creating network ACL for tier: "+tierUuid);
433438
pstmt = conn.prepareStatement("INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (?, UUID(), ? , ?, ?)");
434439
pstmt.setLong(1, aclId);
@@ -440,7 +445,7 @@ private void updateNetworkACLs(Connection conn) {
440445

441446
Long fwRuleId = rsAcls.getLong(1);
442447
String cidr = null;
443-
//get cidr
448+
//get cidr from firewall_rules_cidrs
444449
pstmt = conn.prepareStatement("SELECT id, source_cidr FROM `cloud`.`firewall_rules_cidrs` where firewall_rule_id = ?");
445450
pstmt.setLong(1, fwRuleId);
446451
rsCidr = pstmt.executeQuery();

‎server/src/com/cloud/network/vpc/NetworkACLManager.java‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,11 @@
1616
// under the License.
1717
package com.cloud.network.vpc;
1818

19-
import java.util.List;
20-
2119
import com.cloud.exception.ResourceUnavailableException;
2220
import com.cloud.network.dao.NetworkVO;
23-
import com.cloud.network.rules.FirewallRule;
2421
import com.cloud.user.Account;
25-
import com.cloud.utils.db.DB;
26-
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLListCmd;
22+
23+
import java.util.List;
2724

2825

2926
public interface NetworkACLManager{
@@ -108,11 +105,37 @@ NetworkACLItem createNetworkACLItem(Integer sourcePortStart, Integer sourcePortE
108105
* @throws ResourceUnavailableException
109106
*/
110107
boolean revokeACLItemsForNetwork(long networkId, long userId, Account caller) throws ResourceUnavailableException;
111-
108+
109+
/**
110+
* List network ACL items by network
111+
* @param guestNtwkId
112+
* @return
113+
*/
112114
List<NetworkACLItemVO> listNetworkACLItems(long guestNtwkId);
113115

116+
/**
117+
* Applies asscociated ACL to specified network
118+
* @param networkId
119+
* @return
120+
* @throws ResourceUnavailableException
121+
*/
114122
boolean applyACLToNetwork(long networkId) throws ResourceUnavailableException;
115123

124+
/**
125+
* Updates and existing network ACL Item
126+
* @param id
127+
* @param protocol
128+
* @param sourceCidrList
129+
* @param trafficType
130+
* @param action
131+
* @param number
132+
* @param sourcePortStart
133+
* @param sourcePortEnd
134+
* @param icmpCode
135+
* @param icmpType
136+
* @return
137+
* @throws ResourceUnavailableException
138+
*/
116139
NetworkACLItem updateNetworkACLItem(Long id, String protocol, List<String> sourceCidrList, NetworkACLItem.TrafficType trafficType,
117140
String action, Integer number, Integer sourcePortStart, Integer sourcePortEnd,
118141
Integer icmpCode, Integer icmpType) throws ResourceUnavailableException;

‎server/src/com/cloud/network/vpc/NetworkACLManagerImpl.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import com.cloud.event.ActionEvent;
2020
import com.cloud.event.EventTypes;
21-
import com.cloud.exception.InvalidParameterValueException;
2221
import com.cloud.exception.ResourceUnavailableException;
2322
import com.cloud.network.Network;
2423
import com.cloud.network.Network.Service;
@@ -78,7 +77,7 @@ public NetworkACL createNetworkACL(String name, String description, long vpcId)
7877
public boolean applyNetworkACL(long aclId) throws ResourceUnavailableException {
7978
boolean handled = true;
8079
List<NetworkACLItemVO> rules = _networkACLItemDao.listByACL(aclId);
81-
//Find all networks using this ACL
80+
//Find all networks using this ACL and apply the ACL
8281
List<NetworkVO> networks = _networkDao.listByAclId(aclId);
8382
for(NetworkVO network : networks){
8483
if(!applyACLItemsToNetwork(network.getId(), rules)) {
@@ -117,7 +116,9 @@ public boolean deleteNetworkACL(NetworkACL acl) {
117116
@Override
118117
public boolean replaceNetworkACL(NetworkACL acl, NetworkVO network) throws ResourceUnavailableException {
119118
network.setNetworkACLId(acl.getId());
119+
//Update Network ACL
120120
if(_networkDao.update(network.getId(), network)){
121+
//Apply ACL to network
121122
return applyACLToNetwork(network.getId());
122123
}
123124
return false;
@@ -133,7 +134,7 @@ public NetworkACLItem createNetworkACLItem(Integer portStart, Integer portEnd, S
133134
if("deny".equalsIgnoreCase(action)){
134135
ruleAction = NetworkACLItem.Action.Deny;
135136
}
136-
// If number is null, set it to currentMax + 1
137+
// If number is null, set it to currentMax + 1 (for backward compatibility)
137138
if(number == null){
138139
number = _networkACLItemDao.getMaxNumberByACL(aclId) + 1;
139140
}

‎server/src/com/cloud/network/vpc/NetworkACLServiceImpl.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import com.cloud.network.Networks;
2424
import com.cloud.network.dao.NetworkDao;
2525
import com.cloud.network.dao.NetworkVO;
26-
import com.cloud.network.element.NetworkACLServiceProvider;
2726
import com.cloud.network.vpc.dao.NetworkACLDao;
2827
import com.cloud.projects.Project.ListProjectResourcesCriteria;
2928
import com.cloud.server.ResourceTag.TaggedResourceType;
@@ -41,7 +40,6 @@
4140
import com.cloud.utils.db.SearchCriteria;
4241
import com.cloud.utils.db.SearchCriteria.Op;
4342
import com.cloud.utils.net.NetUtils;
44-
import org.apache.cloudstack.acl.SecurityChecker;
4543
import org.apache.cloudstack.api.ApiErrorCode;
4644
import org.apache.cloudstack.api.ServerApiException;
4745
import org.apache.cloudstack.api.command.user.network.CreateNetworkACLCmd;
@@ -140,6 +138,7 @@ public boolean deleteNetworkACL(long id) {
140138
throw new InvalidParameterValueException("Unable to find specified ACL");
141139
}
142140

141+
//Do not allow deletion of default ACLs
143142
if(acl.getId() == NetworkACL.DEFAULT_ALLOW || acl.getId() == NetworkACL.DEFAULT_DENY){
144143
throw new InvalidParameterValueException("Default ACL cannot be removed");
145144
}
@@ -218,6 +217,7 @@ public NetworkACLItem createNetworkACLItem(CreateNetworkACLCmd aclItemCmd){
218217
}
219218
_accountMgr.checkAccess(caller, null, true, vpc);
220219

220+
//Ensure that number is unique within the ACL
221221
if(aclItemCmd.getNumber() != null){
222222
if(_networkACLItemDao.findByAclAndNumber(aclId, aclItemCmd.getNumber()) != null){
223223
throw new InvalidParameterValueException("ACL item with number "+aclItemCmd.getNumber()+" already exists in ACL: "+acl.getUuid());
@@ -293,6 +293,7 @@ private void validateNetworkACLItem(Integer portStart, Integer portEnd, List<Str
293293
}
294294
}
295295

296+
//Check ofr valid action Allow/Deny
296297
if(action != null){
297298
try {
298299
NetworkACLItem.Action.valueOf(action);

‎setup/db/db/schema-410to420.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,10 +1211,12 @@ CREATE TABLE `cloud`.`network_acl_item` (
12111211

12121212
ALTER TABLE `cloud`.`networks` add column `network_acl_id` bigint unsigned COMMENT 'network acl id';
12131213

1214+
-- Add Default ACL deny_all
12141215
INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (1, UUID(), 0, "Default Network ACL Deny All", "default_deny");
12151216
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");
12161217
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");
12171218

1219+
-- Add Default ACL allow_all
12181220
INSERT INTO `cloud`.`network_acl` (id, uuid, vpc_id, description, name) values (2, UUID(), 0, "Default Network ACL Allow All", "default_allow");
12191221
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");
12201222
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");

0 commit comments

Comments
 (0)