Skip to content

Commit 8e3bb7e

Browse files
Pearl1594nvazquez
Pearl1594
and
nvazquez
committed
NSX: Add support to re-order ACL rules (NSX FW rules) (#14)
* [WIP] NSX: Add support to re-order ACL rules (NSX FW rules) * fix reordering of acl rules on all networks that it is associated to * clean up and attempt test fix * Fix tests * Remove unused import * tweak reorder logic --------- Co-authored-by: nvazquez <nicovazquez90@gmail.com>
1 parent 09636cf commit 8e3bb7e

File tree

9 files changed

+84
-23
lines changed

9 files changed

+84
-23
lines changed

‎api/src/main/java/com/cloud/network/element/NetworkACLServiceProvider.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.cloud.exception.ResourceUnavailableException;
2222
import com.cloud.network.Network;
2323
import com.cloud.network.vpc.NetworkACLItem;
24+
import com.cloud.network.vpc.Vpc;
2425

2526
public interface NetworkACLServiceProvider extends NetworkElement {
2627

@@ -32,4 +33,6 @@ public interface NetworkACLServiceProvider extends NetworkElement {
3233
*/
3334
boolean applyNetworkACLs(Network config, List<? extends NetworkACLItem> rules) throws ResourceUnavailableException;
3435

36+
boolean reorderAclRules(Vpc vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems);
37+
3538
}

‎engine/components-api/src/main/java/com/cloud/network/vpc/NetworkACLManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.List;
2020

2121
import com.cloud.exception.ResourceUnavailableException;
22+
import com.cloud.network.Network;
2223
import com.cloud.network.dao.NetworkVO;
2324

2425
public interface NetworkACLManager {
@@ -91,4 +92,6 @@ public interface NetworkACLManager {
9192
boolean revokeACLItemsForPrivateGw(PrivateGateway gateway) throws ResourceUnavailableException;
9293

9394
boolean applyACLToPrivateGw(PrivateGateway gateway) throws ResourceUnavailableException;
95+
96+
boolean reorderAclRules(VpcVO vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems);
9497
}

‎plugins/network-elements/bigswitch/src/main/java/com/cloud/network/element/BigSwitchBcfElement.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,11 @@ public boolean applyNetworkACLs(Network network,
700700
return true;
701701
}
702702

703+
@Override
704+
public boolean reorderAclRules(Vpc vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems) {
705+
return true;
706+
}
707+
703708
@Override
704709
public boolean applyFWRules(Network network,
705710
List<? extends FirewallRule> rules)

‎plugins/network-elements/juniper-contrail/src/main/java/org/apache/cloudstack/network/contrail/management/ContrailVpcElementImpl.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,11 @@ public boolean applyNetworkACLs(Network net,
185185
return true;
186186
}
187187

188+
@Override
189+
public boolean reorderAclRules(Vpc vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems) {
190+
return true;
191+
}
192+
188193
@Override
189194
public boolean applyACLItemsToPrivateGw(PrivateGateway privateGateway,
190195
List<? extends NetworkACLItem> rules)

‎plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java‎

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@
112112
import javax.naming.ConfigurationException;
113113
import java.util.ArrayList;
114114
import java.util.Arrays;
115-
import java.util.Comparator;
116115
import java.util.HashMap;
117116
import java.util.List;
118117
import java.util.Locale;
@@ -712,18 +711,7 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
712711
boolean success = true;
713712
for (NetworkACLItem rule : rules) {
714713
String privatePort = getPrivatePortRangeForACLRule(rule);
715-
NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
716-
.setRuleId(rule.getId())
717-
.setSourceCidrList(Objects.nonNull(rule.getSourceCidrList()) ? transformCidrListValues(rule.getSourceCidrList()) : List.of("ANY"))
718-
.setAclAction(transformActionValue(rule.getAction()))
719-
.setTrafficType(rule.getTrafficType().toString())
720-
.setProtocol(rule.getProtocol().toUpperCase())
721-
.setPublicPort(String.valueOf(rule.getSourcePortStart()))
722-
.setPrivatePort(privatePort)
723-
.setIcmpCode(rule.getIcmpCode())
724-
.setIcmpType(rule.getIcmpType())
725-
.setService(Network.Service.NetworkACL)
726-
.build();
714+
NsxNetworkRule networkRule = getNsxNetworkRuleForAcl(rule, privatePort);
727715
if (Arrays.asList(NetworkACLItem.State.Active, NetworkACLItem.State.Add).contains(rule.getState())) {
728716
success = success && nsxService.addFirewallRules(network, List.of(networkRule));
729717
} else if (NetworkACLItem.State.Revoke == rule.getState()) {
@@ -740,9 +728,38 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
740728
return success;
741729
}
742730

743-
private void reorderRules(List<? extends NetworkACLItem> rules) {
744-
rules.sort((Comparator) (r1, r2) -> ((NetworkACLItem) r2).getNumber() - ((NetworkACLItem) r1).getNumber());
731+
@Override
732+
public boolean reorderAclRules(Vpc vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems) {
733+
List<NsxNetworkRule> aclRulesList = new ArrayList<>();
734+
for (NetworkACLItem rule : networkACLItems) {
735+
String privatePort = getPrivatePortRangeForACLRule(rule);
736+
aclRulesList.add(getNsxNetworkRuleForAcl(rule, privatePort));
737+
}
738+
for (Network network: networks) {
739+
nsxService.deleteFirewallRules(network, aclRulesList);
740+
}
741+
boolean success = true;
742+
for (Network network : networks) {
743+
for (NsxNetworkRule aclRule : aclRulesList) {
744+
success = success && nsxService.addFirewallRules(network, List.of(aclRule));
745+
}
746+
}
747+
return success;
748+
}
745749

750+
private NsxNetworkRule getNsxNetworkRuleForAcl(NetworkACLItem rule, String privatePort) {
751+
return new NsxNetworkRule.Builder()
752+
.setRuleId(rule.getId())
753+
.setSourceCidrList(Objects.nonNull(rule.getSourceCidrList()) ? transformCidrListValues(rule.getSourceCidrList()) : List.of("ANY"))
754+
.setAclAction(transformActionValue(rule.getAction()))
755+
.setTrafficType(rule.getTrafficType().toString())
756+
.setProtocol(rule.getProtocol().toUpperCase())
757+
.setPublicPort(String.valueOf(rule.getSourcePortStart()))
758+
.setPrivatePort(privatePort)
759+
.setIcmpCode(rule.getIcmpCode())
760+
.setIcmpType(rule.getIcmpType())
761+
.setService(Network.Service.NetworkACL)
762+
.build();
746763
}
747764
@Override
748765
public boolean applyFWRules(Network network, List<? extends FirewallRule> rules) throws ResourceUnavailableException {

‎server/src/main/java/com/cloud/network/element/VpcVirtualRouterElement.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,11 @@ public boolean applyNetworkACLs(final Network network, final List<? extends Netw
531531
return result;
532532
}
533533

534+
@Override
535+
public boolean reorderAclRules(Vpc vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems) {
536+
return true;
537+
}
538+
534539
@Override
535540
protected Type getVirtualRouterProvider() {
536541
return Type.VPCVirtualRouter;

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,20 @@ public boolean applyACLToPrivateGw(final PrivateGateway gateway) throws Resource
370370
return applyACLToPrivateGw(gateway, rules);
371371
}
372372

373+
@Override
374+
public boolean reorderAclRules(VpcVO vpc, List<? extends Network> networks, List<? extends NetworkACLItem> networkACLItems) {
375+
List<NetworkACLServiceProvider> nsxElements = new ArrayList<>();
376+
nsxElements.add((NetworkACLServiceProvider) _ntwkModel.getElementImplementingProvider(Network.Provider.Nsx.getName()));
377+
try {
378+
for (final NetworkACLServiceProvider provider : nsxElements) {
379+
return provider.reorderAclRules(vpc, networks, networkACLItems);
380+
}
381+
} catch (final Exception ex) {
382+
s_logger.debug("Failed to reorder ACLs on NSX due to: " + ex.getLocalizedMessage());
383+
}
384+
return false;
385+
}
386+
373387
private boolean applyACLToPrivateGw(final PrivateGateway gateway, final List<? extends NetworkACLItem> rules) throws ResourceUnavailableException {
374388
List<VpcProvider> vpcElements = new ArrayList<VpcProvider>();
375389
vpcElements.add((VpcProvider)_ntwkModel.getElementImplementingProvider(Network.Provider.VPCVirtualRouter.getName()));

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -993,14 +993,26 @@ public NetworkACLItem moveNetworkAclRuleToNewPosition(MoveNetworkAclItemCmd move
993993
NetworkACLVO lockedAcl = _networkACLDao.acquireInLockTable(ruleBeingMoved.getAclId());
994994
List<NetworkACLItemVO> allAclRules = getAllAclRulesSortedByNumber(lockedAcl.getId());
995995
validateAclConsistency(moveNetworkAclItemCmd, lockedAcl, allAclRules);
996-
996+
NetworkACLItem networkACLItem = null;
997997
if (previousRule == null) {
998-
return moveRuleToTheTop(ruleBeingMoved, allAclRules);
998+
networkACLItem = moveRuleToTheTop(ruleBeingMoved, allAclRules);
999+
} else if (nextRule == null) {
1000+
networkACLItem = moveRuleToTheBottom(ruleBeingMoved, allAclRules);
1001+
} else {
1002+
networkACLItem = moveRuleBetweenAclRules(ruleBeingMoved, allAclRules, previousRule, nextRule);
1003+
}
1004+
VpcVO vpc = _vpcDao.findById(lockedAcl.getVpcId());
1005+
if (Objects.isNull(vpc)) {
1006+
return networkACLItem;
9991007
}
1000-
if (nextRule == null) {
1001-
return moveRuleToTheBottom(ruleBeingMoved, allAclRules);
1008+
final DataCenter dc = _entityMgr.findById(DataCenter.class, vpc.getZoneId());
1009+
final NsxProviderVO nsxProvider = nsxProviderDao.findByZoneId(dc.getId());
1010+
List<NetworkVO> networks = _networkDao.listByAclId(lockedAcl.getId());
1011+
if (Objects.nonNull(nsxProvider) && !networks.isEmpty()) {
1012+
allAclRules = getAllAclRulesSortedByNumber(lockedAcl.getId());
1013+
_networkAclMgr.reorderAclRules(vpc, networks, allAclRules);
10021014
}
1003-
return moveRuleBetweenAclRules(ruleBeingMoved, allAclRules, previousRule, nextRule);
1015+
return networkACLItem;
10041016
} finally {
10051017
_networkACLDao.releaseFromLockTable(ruleBeingMoved.getAclId());
10061018
}

‎server/src/test/java/com/cloud/network/vpc/NetworkACLServiceImplTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import com.cloud.dc.DataCenter;
3535
import com.cloud.exception.PermissionDeniedException;
3636
import com.cloud.network.dao.NsxProviderDao;
37-
import com.cloud.network.element.NsxProviderVO;
3837
import com.cloud.network.vpc.dao.VpcDao;
3938
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
4039
import org.apache.cloudstack.api.ServerApiException;
@@ -138,8 +137,6 @@ public class NetworkACLServiceImplTest {
138137
private VpcVO vpcVOMock;
139138
@Mock
140139
DataCenter dataCenterVO;
141-
@Mock
142-
NsxProviderVO nsxProviderVO;
143140

144141
@Mock
145142
private Account accountMock;

0 commit comments

Comments
 (0)