firewall_rules: Fix incorrect behavior when IPv4 and IPv6 rule sets have the same name (#236)

firewall_rules: Fix incorrect behavior when IPv4 and IPv6 rule sets have the same name

SUMMARY
VyOS supports IPv4 and IPv6 rule sets having the same name, but there are a couple places in the Ansible module that don't handle this situation.

The fact gathering for ansible_network_resources.firewall_rules has been updated to look for name <name> or ipv6-name <name> instead of just <name>.
The vyos_firewall_rules module has been updated to take the afi into consideration when comparing the have and want states.
V4-EGRESS and V6-EGRESS have been renamed to just EGRESS in the tests. The existing tests seem to be complete enough to test this same-name situation. (V4-INGRESS and V6-INGRESS were not renamed.)

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
vyos_facts and vyos_firewall_rules
ADDITIONAL INFORMATION
An example of a configuration that was originally causing an issue:

(Click to expand):
name wan-lan {
    default-action drop
    rule 1 {
        action accept
        state {
            established enable
            related enable
        }
    }
    rule 2 {
        action drop
        log enable
        state {
            invalid enable
        }
    }
}

ipv6-name wan-lan {
    default-action drop
    rule 1 {
        action accept
        state {
            established enable
            related enable
        }
    }
    rule 2 {
        action drop
        log enable
        state {
            invalid enable
        }
    }
    rule 10 {
        action accept
        protocol icmpv6
    }
}


With this configuration, ansible_network_resources.firewall_rules would show the icmpv6 rule under both ipv4 and ipv6:

(Click to expand):
[
    {
        "afi": "ipv4",
        "rule_sets": [
            {
                "default_action": "drop",
                "name": "wan-lan",
                "rules": [
                    {
                        "action": "accept",
                        "number": 1,
                        "state": {
                            "established": true,
                            "related": true
                        }
                    },
                    {
                        "action": "drop",
                        "number": 2,
                        "state": {
                            "invalid": true
                        }
                    },
                    {
                        "action": "accept",
                        "number": 10,
                        "protocol": "icmpv6"
                    }
                ]
            },
        ]
    },
    {
        "afi": "ipv6",
        "rule_sets": [
            {
                "default_action": "drop",
                "name": "wan-lan",
                "rules": [
                    {
                        "action": "accept",
                        "number": 1,
                        "state": {
                            "established": true,
                            "related": true
                        }
                    },
                    {
                        "action": "drop",
                        "number": 2,
                        "state": {
                            "invalid": true
                        }
                    },
                    {
                        "action": "accept",
                        "number": 10,
                        "protocol": "icmpv6"
                    }
                ]
            },
        ]
    }
]

A similar issue would happen when using vyos_firewall_rules as well, where it would attempt to change rules for the wrong afi.

Reviewed-by: GomathiselviS <None>
Reviewed-by: None <None>

2 files changed, 17 insertions, 10 deletions

diff --git a/plugins/module_utils/network/vyos/config/firewall_rules/firewall_rules.py b/plugins/module_utils/network/vyos/config/firewall_rules/firewall_rules.py
index 1f1536c..22973bd 100644
--- a/plugins/module_utils/network/vyos/config/firewall_rules/firewall_rules.py
+++ b/plugins/module_utils/network/vyos/config/firewall_rules/firewall_rules.py
@@ -176,7 +176,7 @@ class Firewall_rules(ConfigBase):
                     # already have (to be replaced by our desired
                     # configuration's rule set).
                     wanted_rule_set = self.search_r_sets_in_have(
-                        want, rs["name"], "r_list"
+                        want, rs["name"], "r_list", h["afi"]
                     )
                     if wanted_rule_set is not None:
                         # Remove the rules that we already have if the wanted
@@ -205,7 +205,9 @@ class Firewall_rules(ConfigBase):
             for h in have:
                 r_sets = self._get_r_sets(h)
                 for rs in r_sets:
-                    w = self.search_r_sets_in_have(want, rs["name"], "r_list")
+                    w = self.search_r_sets_in_have(
+                        want, rs["name"], "r_list", h["afi"]
+                    )
                     if not w:
                         commands.append(
                             self._compute_command(
@@ -230,7 +232,9 @@ class Firewall_rules(ConfigBase):
         for w in want:
             r_sets = self._get_r_sets(w)
             for rs in r_sets:
-                h = self.search_r_sets_in_have(have, rs["name"], "r_list")
+                h = self.search_r_sets_in_have(
+                    have, rs["name"], "r_list", w["afi"]
+                )
                 commands.extend(self._add_r_sets(w["afi"], rs, h))
         return commands
 
@@ -248,7 +252,7 @@ class Firewall_rules(ConfigBase):
                 if r_sets:
                     for rs in r_sets:
                         h = self.search_r_sets_in_have(
-                            have, rs["name"], "r_list"
+                            have, rs["name"], "r_list", w["afi"]
                         )
                         if h:
                             commands.append(
@@ -842,12 +846,13 @@ class Firewall_rules(ConfigBase):
                             )
         return commands
 
-    def search_r_sets_in_have(self, have, w_name, type="rule_sets"):
+    def search_r_sets_in_have(self, have, w_name, type="rule_sets", afi=None):
         """
         This function  returns the rule-set/rule if it is present in target config.
         :param have: target config.
         :param w_name: rule-set name.
         :param type: rule_sets/rule/r_list.
+        :param afi: address family (when type is r_list).
         :return: rule-set/rule.
         """
         if have:
@@ -859,10 +864,11 @@ class Firewall_rules(ConfigBase):
                         return r
             elif type == "r_list":
                 for h in have:
-                    r_sets = self._get_r_sets(h)
-                    for rs in r_sets:
-                        if rs[key] == w_name:
-                            return rs
+                    if h["afi"] == afi:
+                        r_sets = self._get_r_sets(h)
+                        for rs in r_sets:
+                            if rs[key] == w_name:
+                                return rs
             else:
                 for rs in have:
                     if rs[key] == w_name:
diff --git a/plugins/module_utils/network/vyos/facts/firewall_rules/firewall_rules.py b/plugins/module_utils/network/vyos/facts/firewall_rules/firewall_rules.py
index 8e29dbd..f1c080f 100644
--- a/plugins/module_utils/network/vyos/facts/firewall_rules/firewall_rules.py
+++ b/plugins/module_utils/network/vyos/facts/firewall_rules/firewall_rules.py
@@ -100,7 +100,8 @@ class Firewall_rulesFacts(object):
         r_v4 = []
         r_v6 = []
         for r in set(rules):
-            rule_regex = r" %s .+$" % r.strip("'")
+            name_key = "ipv6-name" if type == "ipv6" else "name"
+            rule_regex = r" %s %s .+$" % (name_key, r.strip("'"))
             cfg = findall(rule_regex, data, M)
             fr = self.render_config(cfg, r.strip("'"))
             fr["name"] = r.strip("'")