Skip to content

Commit ba77dbd

Browse files
Pearl1594nvazquez
authored andcommitted
NSX: Fix ACL rule removal on replacement and fix rule order (#11)
1 parent 5792ece commit ba77dbd

File tree

2 files changed

+12
-5
lines changed

2 files changed

+12
-5
lines changed

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
import javax.naming.ConfigurationException;
113113
import java.util.ArrayList;
114114
import java.util.Arrays;
115+
import java.util.Comparator;
115116
import java.util.HashMap;
116117
import java.util.List;
117118
import java.util.Locale;
@@ -706,8 +707,9 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
706707
if (!canHandle(network, Network.Service.NetworkACL)) {
707708
return false;
708709
}
709-
List<NsxNetworkRule> nsxAddNetworkRules = new ArrayList<>();
710+
710711
List<NsxNetworkRule> nsxDelNetworkRules = new ArrayList<>();
712+
boolean success = true;
711713
for (NetworkACLItem rule : rules) {
712714
String privatePort = getPrivatePortRangeForACLRule(rule);
713715
NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
@@ -723,22 +725,26 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
723725
.setService(Network.Service.NetworkACL)
724726
.build();
725727
if (Arrays.asList(NetworkACLItem.State.Active, NetworkACLItem.State.Add).contains(rule.getState())) {
726-
nsxAddNetworkRules.add(networkRule);
728+
success = success && nsxService.addFirewallRules(network, List.of(networkRule));
727729
} else if (NetworkACLItem.State.Revoke == rule.getState()) {
728730
nsxDelNetworkRules.add(networkRule);
729731
}
730732
}
731-
boolean success = true;
733+
732734
if (!nsxDelNetworkRules.isEmpty()) {
733735
success = nsxService.deleteFirewallRules(network, nsxDelNetworkRules);
734736
if (!success) {
735737
LOGGER.warn("Not all firewall rules were successfully deleted");
736738
}
737739
}
738-
return success && nsxService.addFirewallRules(network, nsxAddNetworkRules);
740+
return success;
739741
}
740742

741-
@Override
743+
private void reorderRules(List<? extends NetworkACLItem> rules) {
744+
rules.sort((Comparator) (r1, r2) -> ((NetworkACLItem) r2).getNumber() - ((NetworkACLItem) r1).getNumber());
745+
746+
}
747+
@Override
742748
public boolean applyFWRules(Network network, List<? extends FirewallRule> rules) throws ResourceUnavailableException {
743749

744750
if (!canHandle(network, Network.Service.Firewall)) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ public boolean replaceNetworkACL(final NetworkACL acl, final NetworkVO network)
206206
final List<NetworkACLItemVO> aclItems = _networkACLItemDao.listByACL(acl.getId());
207207
if (aclItems == null || aclItems.isEmpty()) {
208208
s_logger.debug("New network ACL is empty. Revoke existing rules before applying ACL");
209+
} else {
209210
if (!revokeACLItemsForNetwork(network.getId())) {
210211
throw new CloudRuntimeException("Failed to replace network ACL. Error while removing existing ACL items for network: " + network.getId());
211212
}

0 commit comments

Comments
 (0)