associate security policies with groups and not to DFW and add deleti…

…on of rules
nvazquez · Nov 30, 2023 · Nov 13, 2023 · Nov 13, 2023 · Nov 13, 2023 · Nov 13, 2023
commit db11f0dc6eb82fb193c3880775051d6a5c283afc
diff --git a/.../main/java/org/apache/cloudstack/agent/api/DeletedNsxDistributedFirewallRulesCommand.java b/.../main/java/org/apache/cloudstack/agent/api/DeletedNsxDistributedFirewallRulesCommand.java
@@ -0,0 +1,28 @@
+
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.agent.api;
+
+import org.apache.cloudstack.resource.NsxNetworkRule;
+
+import java.util.List;
+
+public class DeletedNsxDistributedFirewallRulesCommand extends CreateNsxDistributedFirewallRulesCommand {
+    public DeletedNsxDistributedFirewallRulesCommand(long domainId, long accountId, long zoneId, Long vpcId, long networkId, List<NsxNetworkRule> rules) {
+        super(domainId, accountId, zoneId, vpcId, networkId, rules);
+    }
+}
diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java
@@ -47,6 +47,7 @@
 import org.apache.cloudstack.agent.api.DeleteNsxSegmentCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxNatRuleCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxTier1GatewayCommand;
+import org.apache.cloudstack.agent.api.DeletedNsxDistributedFirewallRulesCommand;
 import org.apache.cloudstack.service.NsxApiClient;
 import org.apache.cloudstack.utils.NsxControllerUtils;
 import org.apache.commons.collections.CollectionUtils;
@@ -124,6 +125,8 @@ public Answer executeRequest(Command cmd) {
             return executeRequest((CreateNsxLoadBalancerRuleCommand) cmd);
         } else if (cmd instanceof DeleteNsxLoadBalancerRuleCommand) {
             return executeRequest((DeleteNsxLoadBalancerRuleCommand) cmd);
+        }  else  if (cmd instanceof DeletedNsxDistributedFirewallRulesCommand) {
+            return executeRequest((DeletedNsxDistributedFirewallRulesCommand) cmd);
         } else if (cmd instanceof CreateNsxDistributedFirewallRulesCommand) {
             return executeRequest((CreateNsxDistributedFirewallRulesCommand) cmd);
         } else {
@@ -471,6 +474,19 @@ private NsxAnswer executeRequest(CreateNsxDistributedFirewallRulesCommand cmd) {
         return new NsxAnswer(cmd, true, null);
     }
 
+    private NsxAnswer executeRequest(DeletedNsxDistributedFirewallRulesCommand cmd) {
+        String segmentName = NsxControllerUtils.getNsxSegmentId(cmd.getDomainId(), cmd.getAccountId(),
+                cmd.getZoneId(), cmd.getVpcId(), cmd.getNetworkId());
+        List<NsxNetworkRule> rules = cmd.getRules();
+        try {
+            nsxApiClient.deleteDistributedFirewallRules(segmentName, rules);
+        } catch (Exception e) {
+            LOGGER.error(String.format("Failed to create NSX distributed firewall %s: %s", segmentName, e.getMessage()), e);
+            return new NsxAnswer(cmd, new CloudRuntimeException(e.getMessage()));
+        }
+        return new NsxAnswer(cmd, true, null);
+    }
+
     @Override
     public boolean start() {
         return true;

diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java
@@ -31,13 +31,15 @@
 import com.vmware.nsx_policy.infra.Tier1s;
 import com.vmware.nsx_policy.infra.domains.Groups;
 import com.vmware.nsx_policy.infra.domains.SecurityPolicies;
+import com.vmware.nsx_policy.infra.domains.security_policies.Rules;
 import com.vmware.nsx_policy.infra.sites.EnforcementPoints;
 import com.vmware.nsx_policy.infra.tier_0s.LocaleServices;
 import com.vmware.nsx_policy.infra.tier_1s.nat.NatRules;
 import com.vmware.nsx_policy.model.ApiError;
 import com.vmware.nsx_policy.model.DhcpRelayConfig;
 import com.vmware.nsx_policy.model.EnforcementPointListResult;
 import com.vmware.nsx_policy.model.Group;
+import com.vmware.nsx_policy.model.GroupListResult;
 import com.vmware.nsx_policy.model.ICMPTypeServiceEntry;
 import com.vmware.nsx_policy.model.L4PortSetServiceEntry;
 import com.vmware.nsx_policy.model.LBAppProfileListResult;
@@ -789,13 +791,18 @@ private void removeSegmentDistributedFirewallRules(String segmentName) {
 
     public void createSegmentDistributedFirewall(String segmentName, List<NsxNetworkRule> nsxRules) {
         try {
+            String groupPath = getGroupPath(segmentName);
+            if (Objects.isNull(groupPath)) {
+                throw new CloudRuntimeException(String.format("Failed to find group for segment %s", segmentName));
+            }
             SecurityPolicies services = (SecurityPolicies) nsxService.apply(SecurityPolicies.class);
             List<Rule> rules = getRulesForDistributedFirewall(segmentName, nsxRules);
             SecurityPolicy policy = new SecurityPolicy.Builder()
                     .setDisplayName(segmentName)
                     .setId(segmentName)
                     .setCategory("Application")
                     .setRules(rules)
+                    .setScope(List.of(groupPath))
                     .build();
             services.patch(DEFAULT_DOMAIN, segmentName, policy);
         } catch (Error error) {
@@ -806,8 +813,25 @@ public void createSegmentDistributedFirewall(String segmentName, List<NsxNetwork
         }
     }
 
+    public void deleteDistributedFirewallRules(String segmentName, List<NsxNetworkRule> nsxRules) {
+        for(NsxNetworkRule rule : nsxRules) {
+            String ruleId = NsxControllerUtils.getNsxDistributedFirewallPolicyRuleId(segmentName, rule.getRuleId());
+            String svcName = getServiceName(ruleId, rule.getPrivatePort(), rule.getProtocol(), rule.getIcmpType(), rule.getIcmpCode());
+            // delete rules
+            Rules rules = (Rules) nsxService.apply(Rules.class);
+            rules.delete(DEFAULT_DOMAIN, segmentName, ruleId);
+            // delete service - if any
+            Services services = (Services) nsxService.apply(Services.class);
+            services.delete(svcName);
+        }
+    }
+
     private List<Rule> getRulesForDistributedFirewall(String segmentName, List<NsxNetworkRule> nsxRules) {
         List<Rule> rules = new ArrayList<>();
+        String groupPath = getGroupPath(segmentName);
+        if (Objects.isNull(groupPath)) {
+            throw new CloudRuntimeException(String.format("Failed to find group for segment %s", segmentName));
+        }
         for (NsxNetworkRule rule : nsxRules) {
             String ruleId = NsxControllerUtils.getNsxDistributedFirewallPolicyRuleId(segmentName, rule.getRuleId());
             Rule ruleToAdd = new Rule.Builder()
@@ -818,7 +842,7 @@ private List<Rule> getRulesForDistributedFirewall(String segmentName, List<NsxNe
                     .setSourceGroups(getGroupsForTraffic(rule, segmentName, true))
                     .setDestinationGroups(getGroupsForTraffic(rule, segmentName, false))
                     .setServices(getServicesListForDistributedFirewallRule(rule, segmentName))
-                    .setScope(List.of("ANY"))
+                    .setScope(List.of(groupPath))
                     .build();
             rules.add(ruleToAdd);
         }
@@ -852,4 +876,24 @@ protected List<String> getGroupsForTraffic(NsxNetworkRule rule,
         throw new CloudRuntimeException(err);
     }
 
+    private List<Group> listNsxGroups() {
+        try {
+           Groups groups = (Groups) nsxService.apply(Groups.class);
+           GroupListResult result = groups.list(DEFAULT_DOMAIN, null, false, null, null, null, null, null);
+           return result.getResults();
+        } catch (Error error) {
+            ApiError ae = error.getData()._convertTo(ApiError.class);
+            String msg = String.format("Failed to list NSX groups, due to: %s", ae.getErrorMessage());
+            LOGGER.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
+    }
+
+    private String getGroupPath(String segmentName) {
+        List<Group> groups = listNsxGroups();
+        Optional<Group> matchingGroup = groups.stream().filter(group -> group.getDisplayName().equals(segmentName)).findFirst();
+        return matchingGroup.map(Group::getPath).orElse(null);
+
+    }
+
 }
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
@@ -667,7 +667,8 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
         if (!canHandle(network, Network.Service.NetworkACL)) {
             return false;
         }
-        List<NsxNetworkRule> nsxNetworkRules = new ArrayList<>();
+        List<NsxNetworkRule> nsxAddNetworkRules = new ArrayList<>();
+        List<NsxNetworkRule> nsxDelNetworkRules = new ArrayList<>();
         for (NetworkACLItem rule : rules) {
             String privatePort = getPrivatePortRangeForACLRule(rule);
 
@@ -683,9 +684,20 @@ public boolean applyNetworkACLs(Network network, List<? extends NetworkACLItem>
                     .setIcmpType(rule.getIcmpType())
                     .setService(Network.Service.NetworkACL)
                     .build();
-            nsxNetworkRules.add(networkRule);
+            if (NetworkACLItem.State.Add == rule.getState()) {
+                nsxAddNetworkRules.add(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 nsxService.addFirewallRules(network, nsxNetworkRules);
+        return success && nsxService.addFirewallRules(network, nsxAddNetworkRules);
     }
 
     protected NsxNetworkRule.NsxRuleAction transformActionValue(NetworkACLItem.Action action) {

diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxServiceImpl.java
@@ -36,6 +36,7 @@
 import org.apache.cloudstack.agent.api.DeleteNsxSegmentCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxNatRuleCommand;
 import org.apache.cloudstack.agent.api.DeleteNsxTier1GatewayCommand;
+import org.apache.cloudstack.agent.api.DeletedNsxDistributedFirewallRulesCommand;
 import org.apache.cloudstack.resource.NsxNetworkRule;
 import org.apache.cloudstack.utils.NsxControllerUtils;
 import org.apache.cloudstack.utils.NsxHelper;
@@ -181,4 +182,11 @@ public boolean addFirewallRules(Network network, List<NsxNetworkRule> netRules)
         NsxAnswer result = nsxControllerUtils.sendNsxCommand(command, network.getDataCenterId());
         return result.getResult();
     }
+
+    public boolean deleteFirewallRules(Network network, List<NsxNetworkRule> netRules) {
+        DeletedNsxDistributedFirewallRulesCommand command = new DeletedNsxDistributedFirewallRulesCommand(network.getDomainId(),
+                network.getAccountId(), network.getDataCenterId(), network.getVpcId(), network.getId(), netRules);
+        NsxAnswer result = nsxControllerUtils.sendNsxCommand(command, network.getDataCenterId());
+        return result.getResult();
+    }
 }