From b28632c3e581371f3b0d670d376ab409a4b8fa0e Mon Sep 17 00:00:00 2001
From: Andrew Gunnerson <accounts+github@chiller3.com>
Date: Fri, 25 Feb 2022 13:28:54 -0500
Subject: 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>
---
.../vyos/config/firewall_rules/firewall_rules.py | 24 ++++++++++++++--------
.../vyos/facts/firewall_rules/firewall_rules.py | 3 ++-
2 files changed, 17 insertions(+), 10 deletions(-)
(limited to 'plugins')
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 1f1536c5..22973bd7 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 8e29dbd7..f1c080f6 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("'")
--
cgit v1.2.3