From b28632c3e581371f3b0d670d376ab409a4b8fa0e Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson 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 or ipv6-name instead of just . 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 Reviewed-by: None --- .../vyos/fixtures/vyos_firewall_rules_config.cfg | 8 ++++---- .../network/vyos/test_vyos_firewall_rules.py | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) (limited to 'tests/unit/modules') diff --git a/tests/unit/modules/network/vyos/fixtures/vyos_firewall_rules_config.cfg b/tests/unit/modules/network/vyos/fixtures/vyos_firewall_rules_config.cfg index 87263015..32d42941 100644 --- a/tests/unit/modules/network/vyos/fixtures/vyos_firewall_rules_config.cfg +++ b/tests/unit/modules/network/vyos/fixtures/vyos_firewall_rules_config.cfg @@ -9,7 +9,7 @@ set firewall name V4-INGRESS rule 101 set firewall name V4-INGRESS rule 101 disabled set firewall name V4-INGRESS rule 101 action 'accept' set firewall name V4-INGRESS rule 101 ipsec 'match-ipsec' -set firewall name V4-EGRESS default-action 'reject' -set firewall ipv6-name V6-EGRESS default-action 'reject' -set firewall ipv6-name V6-EGRESS rule 20 -set firewall ipv6-name V6-EGRESS rule 20 icmpv6 type 'echo-request' \ No newline at end of file +set firewall name EGRESS default-action 'reject' +set firewall ipv6-name EGRESS default-action 'reject' +set firewall ipv6-name EGRESS rule 20 +set firewall ipv6-name EGRESS rule 20 icmpv6 type 'echo-request' 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 f80157c5..4be8ec9d 100644 --- a/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py +++ b/tests/unit/modules/network/vyos/test_vyos_firewall_rules.py @@ -772,7 +772,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): description="This rule-set is configured by Ansible RM", ), dict( - name="V6-EGRESS", + name="EGRESS", default_action="reject", description="This rule-set is configured by Ansible RM", rules=[ @@ -800,7 +800,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): "set firewall name V4-INGRESS rule 102 description 'Rule 102 is configured by Ansible RM'", "set firewall name V4-INGRESS rule 102", "set firewall ipv6-name V6-INGRESS description 'This rule-set is configured by Ansible RM'", - "set firewall ipv6-name V6-EGRESS description 'This rule-set is configured by Ansible RM'", + "set firewall ipv6-name EGRESS description 'This rule-set is configured by Ansible RM'", ] self.execute_module(changed=True, commands=commands) @@ -838,7 +838,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): default_action="accept", ), dict( - name="V6-EGRESS", + name="EGRESS", default_action="reject", rules=[ dict( @@ -883,7 +883,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): ], ), dict( - name="V4-EGRESS", + name="EGRESS", default_action="reject", ), ], @@ -896,7 +896,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): default_action="accept", ), dict( - name="V6-EGRESS", + name="EGRESS", default_action="reject", rules=[ dict( @@ -970,7 +970,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): ], ), dict( - name="V4-EGRESS", + name="EGRESS", default_action="reject", ), ], @@ -983,7 +983,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): default_action="accept", ), dict( - name="V6-EGRESS", + name="EGRESS", default_action="reject", rules=[ dict( @@ -1062,9 +1062,9 @@ class TestVyosFirewallRulesModule(TestVyosModule): ) commands = [ "delete firewall ipv6-name V6-INGRESS", - "delete firewall ipv6-name V6-EGRESS", + "delete firewall ipv6-name EGRESS", "delete firewall name V4-INGRESS", - "delete firewall name V4-EGRESS", + "delete firewall name EGRESS", "set firewall name V4-IN default-action 'accept'", "set firewall name V4-IN description 'This is IPv4 INGRESS rule set'", "set firewall name V4-IN enable-default-log", @@ -1115,7 +1115,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): ], ), dict( - name="V4-EGRESS", + name="EGRESS", default_action="reject", ), ], @@ -1128,7 +1128,7 @@ class TestVyosFirewallRulesModule(TestVyosModule): default_action="accept", ), dict( - name="V6-EGRESS", + name="EGRESS", default_action="reject", rules=[ dict( -- cgit v1.2.3