(cloudstack) 09/17: NSX: Fix ACL rule removal on replacement and fix rule order (#11)

[pearl11594]

This is an automated email from the ASF dual-hosted git repository.

pearl11594 pushed a commit to branch fr03-nsx-reorder-acl-rules
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 9d5c8740699b000fa69d060d3ec04f8a7d50d230
Author: Pearl Dsilva <pearl1...@gmail.com>
AuthorDate: Fri Feb 2 10:16:08 2024 -0500

    NSX: Fix ACL rule removal on replacement and fix rule order (#11)
---
 .../java/org/apache/cloudstack/service/NsxElement.java   | 16 +++++++++++-----
 .../com/cloud/network/vpc/NetworkACLManagerImpl.java     |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
index dfb075aa312..05a7d141a86 100644
--- 
a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
+++ 
b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java
@@ -112,6 +112,7 @@ import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -706,8 +707,9 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
         if (!canHandle(network, Network.Service.NetworkACL)) {
             return false;
         }
-        List<NsxNetworkRule> nsxAddNetworkRules = new ArrayList<>();
+
         List<NsxNetworkRule> nsxDelNetworkRules = new ArrayList<>();
+        boolean success = true;
         for (NetworkACLItem rule : rules) {
             String privatePort = getPrivatePortRangeForACLRule(rule);
             NsxNetworkRule networkRule = new NsxNetworkRule.Builder()
@@ -723,22 +725,26 @@ public class NsxElement extends AdapterBase implements  
DhcpServiceProvider, Dns
                     .setService(Network.Service.NetworkACL)
                     .build();
             if (Arrays.asList(NetworkACLItem.State.Active, 
NetworkACLItem.State.Add).contains(rule.getState())) {
-                nsxAddNetworkRules.add(networkRule);
+                success = success && nsxService.addFirewallRules(network, 
List.of(networkRule));
             } else if (NetworkACLItem.State.Revoke == rule.getState()) {
                 nsxDelNetworkRules.add(networkRule);
             }
         }
-        boolean success = true;
+
         if (!nsxDelNetworkRules.isEmpty()) {
             success = nsxService.deleteFirewallRules(network, 
nsxDelNetworkRules);
             if (!success) {
                 LOGGER.warn("Not all firewall rules were successfully 
deleted");
             }
         }
-        return success && nsxService.addFirewallRules(network, 
nsxAddNetworkRules);
+        return success;
     }
 
-    @Override
+    private void reorderRules(List<? extends NetworkACLItem> rules) {
+        rules.sort((Comparator) (r1, r2) -> ((NetworkACLItem) r2).getNumber() 
- ((NetworkACLItem) r1).getNumber());
+
+    }
+        @Override
     public boolean applyFWRules(Network network, List<? extends FirewallRule> 
rules) throws ResourceUnavailableException {
 
         if (!canHandle(network, Network.Service.Firewall)) {
diff --git 
a/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java 
b/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
index d95cf9ac7af..2e54474772d 100644
--- a/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
+++ b/server/src/main/java/com/cloud/network/vpc/NetworkACLManagerImpl.java
@@ -206,6 +206,7 @@ public class NetworkACLManagerImpl extends ManagerBase 
implements NetworkACLMana
             final List<NetworkACLItemVO> aclItems = 
_networkACLItemDao.listByACL(acl.getId());
             if (aclItems == null || aclItems.isEmpty()) {
                 s_logger.debug("New network ACL is empty. Revoke existing 
rules before applying ACL");
+            } else {
                 if (!revokeACLItemsForNetwork(network.getId())) {
                     throw new CloudRuntimeException("Failed to replace network 
ACL. Error while removing existing ACL items for network: " + network.getId());
                 }