From 9e829e1b75996b6b8268ab29b105d5dfa97da441 Mon Sep 17 00:00:00 2001 From: Shawn Wilsher <656602+sdwilsh@users.noreply.github.com> Date: Mon, 20 Sep 2021 16:25:42 -0700 Subject: Fix `vyos.vyos.vyos_firewall_rules` `state: replaced` to match documentation (#203) Fix `vyos.vyos.vyos_firewall_rules` `state: replaced` to match documentation SUMMARY vyos.vyos.vyos_firewall_rules should only try to change listed firewall rules, as documented, when the state is set to replaced. As currently implemented (prior to this PR), it better matches what overridden is meant to do. Fixes #201 ISSUE TYPE Bugfix Pull Request COMPONENT NAME vyos.vyos.vyos_firewall_rules ADDITIONAL INFORMATION Cleanup and document existing code for clarity Add a failing idempotent test Add a failing change test Fix failing tests Add change fragment Reviewed-by: GomathiselviS Reviewed-by: Shawn Wilsher Reviewed-by: None --- .../fix-firewall_rules-state-replaced.yaml | 3 ++ .../vyos/config/firewall_rules/firewall_rules.py | 22 ++++++++++++-- .../network/vyos/test_vyos_firewall_rules.py | 34 ++++++++++++++++++++-- 3 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/fix-firewall_rules-state-replaced.yaml diff --git a/changelogs/fragments/fix-firewall_rules-state-replaced.yaml b/changelogs/fragments/fix-firewall_rules-state-replaced.yaml new file mode 100644 index 0000000..231cd71 --- /dev/null +++ b/changelogs/fragments/fix-firewall_rules-state-replaced.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - Fix vyos_firewall_rules with state replaced to only replace the specified rules. 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 fd5a4f5..3c56626 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 @@ -167,13 +167,29 @@ class Firewall_rules(ConfigBase): """ commands = [] if have: + # Iterate over the afi rule sets we already have. for h in have: r_sets = self._get_r_sets(h) + # Iterate over each rule set we already have. for rs in r_sets: - w = self.search_r_sets_in_have(want, rs["name"], "r_list") - commands.extend( - self._add_r_sets(h["afi"], rs, w, opr=False) + # In the desired configuration, search for the rule set we + # 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" ) + if wanted_rule_set is not None: + # Remove the rules that we already have if the wanted + # rules exist under the same name. + commands.extend( + self._add_r_sets( + h["afi"], + want=rs, + have=wanted_rule_set, + opr=False, + ) + ) + # Merge the desired configuration into what we already have. commands.extend(self._state_merged(want, have)) return commands diff --git a/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py b/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py index 520446e..dd3dbce 100644 --- a/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py +++ b/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py @@ -788,7 +788,6 @@ class TestVyosFirewallRulesModule(TestVyosModule): ) commands = [ "delete firewall name V4-INGRESS rule 101 disabled", - "delete firewall name V4-EGRESS default-action", "set firewall name V4-INGRESS description 'This is IPv4 INGRESS rule set'", "set firewall name V4-INGRESS rule 101 protocol 'tcp'", "set firewall name V4-INGRESS rule 101 description 'Rule 101 is configured by Ansible RM'", @@ -854,7 +853,6 @@ class TestVyosFirewallRulesModule(TestVyosModule): ) commands = [ "delete firewall name V4-INGRESS enable-default-log", - "delete firewall name V4-EGRESS default-action", ] self.execute_module(changed=True, commands=commands) @@ -913,6 +911,38 @@ class TestVyosFirewallRulesModule(TestVyosModule): ) self.execute_module(changed=False, commands=[]) + def test_vyos_firewall_v4v6_rule_sets_rule_rep_idem_02(self): + set_module_args( + dict( + config=[ + dict( + afi="ipv4", + rule_sets=[ + dict( + name="V4-INGRESS", + description="This is IPv4 V4-INGRESS rule set", + default_action="accept", + enable_default_log=True, + rules=[ + dict( + number="101", + action="accept", + description="Rule 101 is configured by Ansible", + ipsec="match-ipsec", + protocol="icmp", + fragment="match-frag", + disabled=True, + ), + ], + ), + ], + ), + ], + state="replaced", + ) + ) + self.execute_module(changed=False, commands=[]) + def test_vyos_firewall_v4v6_rule_sets_rule_mer_idem_01(self): set_module_args( dict( -- cgit v1.2.3