Firewall_rules diff mode support (#407)HEADmain

* fw_rules diff init

* comment support

* sanity fix

* remove redundant

* integration tests for fw_rules diff mode

* 1.3- integration tests for firewall_rule diff

* 1.4+ long string linter fix

* 1.3- long string linter fix

* long str rework

* typo

* Remove commented-out diff block in YAML file

removed comments

---------

Co-authored-by: Daniil Baturin <daniil@baturin.org>

5 files changed, 416 insertions, 26 deletions

diff --git a/changelogs/fragments/firewall_rules_diff.yml b/changelogs/fragments/firewall_rules_diff.yml
new file mode 100644
index 00000000..9e62a515
--- /dev/null
+++ b/changelogs/fragments/firewall_rules_diff.yml
@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - vyos_firewall_rules - Add support for diff mode for rulesets
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 2942b191..5c2ef6ca 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
@@ -30,10 +30,13 @@ from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.facts.facts
 from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.utils import (
     list_diff_want_only,
 )
-
-from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import get_os_version
-
-from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import LooseVersion
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import (
+    LooseVersion,
+)
+from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import (
+    get_os_version,
+    load_config,
+)
 
 
 class Firewall_rules(ConfigBase):
@@ -78,6 +81,14 @@ class Firewall_rules(ConfigBase):
         result = {"changed": False}
         warnings = list()
         commands = list()
+        diff = None
+
+        try:
+            self._module.params["comment"]
+        except KeyError:
+            comment = []
+        else:
+            comment = self._module.params["comment"]
 
         if self.state in self.ACTION_STATES:
             existing_firewall_rules_facts = self.get_firewall_rules_facts()
@@ -87,6 +98,12 @@ class Firewall_rules(ConfigBase):
         if self.state in self.ACTION_STATES or self.state == "rendered":
             commands.extend(self.set_config(deepcopy(existing_firewall_rules_facts)))
 
+        if commands and self._module._diff:
+            commit = not self._module.check_mode
+            diff = load_config(self._module, commands, commit=commit, comment=comment)
+            if diff:
+                result["diff"] = {"prepared": str(diff)}
+
         if commands and self.state in self.ACTION_STATES:
             if not self._module.check_mode:
                 self._connection.edit_config(commands)
@@ -213,13 +230,17 @@ class Firewall_rules(ConfigBase):
                         commands.append(self._compute_command(rs_id, remove=True))
                         # Blank out the only rule set that it is removed.
                         for entry in have:
-                            if entry['afi'] == rs_id['afi'] and rs_id['name']:
+                            if entry["afi"] == rs_id["afi"] and rs_id["name"]:
                                 entry["rule_sets"] = [
-                                    rule_set for rule_set in entry["rule_sets"] if rule_set.get("name") != rs_id['name']
+                                    rule_set
+                                    for rule_set in entry["rule_sets"]
+                                    if rule_set.get("name") != rs_id["name"]
                                 ]
-                            elif entry['afi'] == rs_id['afi'] and rs_id['filter']:
+                            elif entry["afi"] == rs_id["afi"] and rs_id["filter"]:
                                 entry["rule_sets"] = [
-                                    rule_set for rule_set in entry["rule_sets"] if rule_set.get("filter") != rs_id['filter']
+                                    rule_set
+                                    for rule_set in entry["rule_sets"]
+                                    if rule_set.get("filter") != rs_id["filter"]
                                 ]
             commands.extend(self._state_merged(want, have))
         return commands
@@ -264,7 +285,7 @@ class Firewall_rules(ConfigBase):
                     for h in have:
                         if h["afi"] == w["afi"]:
                             commands.append(
-                                self._compute_command(self._rs_id(None, w["afi"]), remove=True)
+                                self._compute_command(self._rs_id(None, w["afi"]), remove=True),
                             )
         elif have:
             for h in have:
@@ -452,7 +473,9 @@ class Firewall_rules(ConfigBase):
                     if LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"):
                         commands.append(cmd + (" " + attr + " " + item))
                     else:
-                        commands.append(cmd + (" " + attr + " " + item + " " + self._bool_to_str(val)))
+                        commands.append(
+                            cmd + (" " + attr + " " + item + " " + self._bool_to_str(val)),
+                        )
                 elif not opr and item in l_set and not self._in_target(h_state, item):
                     commands.append(cmd + (" " + attr + " " + item))
         return commands
@@ -555,7 +578,9 @@ class Firewall_rules(ConfigBase):
                         else:
                             commands.append(cmd + (" " + attr + " " + item + " " + str(val)))
                 elif not opr and item in l_set and not self._in_target(h_icmp, item):
-                    commands.append(cmd + (" " + attr + " " + item.replace("_", "-") + " " + str(val)))
+                    commands.append(
+                        cmd + (" " + attr + " " + item.replace("_", "-") + " " + str(val)),
+                    )
         return commands
 
     def _add_interface(self, attr, w, h, cmd, opr):
@@ -577,11 +602,11 @@ class Firewall_rules(ConfigBase):
                 if opr and item in l_set and not (h_if and self._is_w_same(w[attr], h_if, item)):
                     commands.append(
                         cmd
-                        + (" " + attr.replace("_", "-") + " " + item.replace("_", "-") + " " + val)
+                        + (" " + attr.replace("_", "-") + " " + item.replace("_", "-") + " " + val),
                     )
                 elif not opr and item in l_set and not (h_if and self._in_target(h_if, item)):
                     commands.append(
-                        cmd + (" " + attr.replace("_", "-") + " " + item.replace("_", "-"))
+                        cmd + (" " + attr.replace("_", "-") + " " + item.replace("_", "-")),
                     )
         return commands
 
@@ -654,14 +679,14 @@ class Firewall_rules(ConfigBase):
                 for flag in flags:
                     invert = flag.get("invert", False)
                     commands.append(
-                        cmd + (" " + attr + " flags " + ("not " if invert else "") + flag["flag"])
+                        cmd + (" " + attr + " flags " + ("not " if invert else "") + flag["flag"]),
                     )
             elif not opr:
                 flags = list_diff_want_only(want, have)
                 for flag in flags:
                     invert = flag.get("invert", False)
                     commands.append(
-                        cmd + (" " + attr + " flags " + ("not " if invert else "") + flag["flag"])
+                        cmd + (" " + attr + " flags " + ("not " if invert else "") + flag["flag"]),
                     )
         return commands
 
@@ -969,7 +994,10 @@ class Firewall_rules(ConfigBase):
         if number:
             cmd += " rule " + str(number)
         if attrib:
-            if LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4") and attrib == "enable_default_log":
+            if (
+                LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4")
+                and attrib == "enable_default_log"
+            ):
                 cmd += " " + "default-log"
             else:
                 cmd += " " + attrib.replace("_", "-")
@@ -1107,15 +1135,23 @@ class Firewall_rules(ConfigBase):
             for item in rs:
                 self._prune_stubs(item)
         elif isinstance(rs, dict):
-            keys_to_remove = [key for key, value in rs.items()
-                              if (
-                                  (key == "disable" and value is False)
-                                  or
-                                  (key == "log" and value == "disable" and
-                                   LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"))
-                                  or
-                                  (key in ["new", "invalid", "related", "established"] and value is False and
-                                   LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4")))]
+            keys_to_remove = [
+                key
+                for key, value in rs.items()
+                if (
+                    (key == "disable" and value is False)
+                    or (
+                        key == "log"
+                        and value == "disable"
+                        and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4")
+                    )
+                    or (
+                        key in ["new", "invalid", "related", "established"]
+                        and value is False
+                        and LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4")
+                    )
+                )
+            ]
             for key in keys_to_remove:
                 del rs[key]
             for key in rs:
@@ -1132,7 +1168,10 @@ class Firewall_rules(ConfigBase):
         elif isinstance(w, list) and isinstance(rs, list):
             try:
                 sorted_list1 = sorted(w, key=lambda x: str(x))  # pylint: disable=unnecessary-lambda
-                sorted_list2 = sorted(rs, key=lambda x: str(x))  # pylint: disable=unnecessary-lambda
+                sorted_list2 = sorted(
+                    rs,
+                    key=lambda x: str(x),  # pylint: disable=unnecessary-lambda
+                )
             except TypeError:
                 return False
             return all(self._is_same_rs(x, y) for x, y in zip(sorted_list1, sorted_list2))
diff --git a/tests/integration/targets/vyos_firewall_rules/tests/cli/diff_mode.yaml b/tests/integration/targets/vyos_firewall_rules/tests/cli/diff_mode.yaml
new file mode 100644
index 00000000..43f3c88b
--- /dev/null
+++ b/tests/integration/targets/vyos_firewall_rules/tests/cli/diff_mode.yaml
@@ -0,0 +1,61 @@
+---
+- debug:
+    msg: START vyos_firewall_rules diff mode integration tests on connection={{ ansible_connection }}
+
+- include_tasks: _remove_config.yaml
+
+- include_tasks: _populate.yaml
+
+- block:
+    - name: Replace device configurations - No Diff
+      register: result
+      diff: true
+      vyos.vyos.vyos_firewall_rules:
+        config: "{{ populate }}"
+
+    - name: Assert No Diff
+      assert:
+        that:
+          - result['changed'] == false
+          - result.diff is not defined
+
+    - name: Replace single rule's attribute and register Diff
+      register: result
+      diff: true
+      vyos.vyos.vyos_firewall_rules:
+        config: "{{ replaced_diff_01.config }}"
+        state: replaced
+
+    - name: Assert - Diff for a single rule and attribute
+      assert:
+        that:
+          - result['changed'] == true
+          - result.diff.prepared == "{{ replaced_diff_01.diff.rstrip() }}"
+
+    - name: Replace single rule's multiple attributes and register Diff
+      register: result
+      diff: true
+      vyos.vyos.vyos_firewall_rules:
+        config: "{{ replaced_diff_02.config }}"
+        state: replaced
+
+    - name: Assert - Diff for a single rule and multiple attributes
+      assert:
+        that:
+          - result['changed'] == true
+          - result.diff.prepared == "{{ replaced_diff_02.diff.rstrip() }}"
+
+    - name: Replace attributes in multiple rules and register Diff
+      register: result
+      diff: true
+      vyos.vyos.vyos_firewall_rules:
+        config: "{{ replaced_diff_03.config }}"
+        state: replaced
+
+    - name: Assert - Diff for a single rule and multiple attributes
+      assert:
+        that:
+          - result['changed'] == true
+          - result.diff.prepared == "{{ replaced_diff_03.diff.rstrip() }}"
+  always:
+    - include_tasks: _remove_config.yaml
diff --git a/tests/integration/targets/vyos_firewall_rules/vars/pre-v1_4.yaml b/tests/integration/targets/vyos_firewall_rules/vars/pre-v1_4.yaml
index c7d7398b..825afe67 100644
--- a/tests/integration/targets/vyos_firewall_rules/vars/pre-v1_4.yaml
+++ b/tests/integration/targets/vyos_firewall_rules/vars/pre-v1_4.yaml
@@ -128,3 +128,142 @@ state_dict:
   new: false
   invalid: false
   related: true
+
+replaced_diff_01:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: accept
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: tcp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: reject
+              description: Rule 102 is configured by Ansible
+              protocol: tcp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [edit firewall name INBOUND rule 101]
+    >action reject
+
+    [edit]
+
+replaced_diff_02:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: accept
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: tcp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: accept
+              description: Rule 102 is configured by Ansible
+              protocol: udp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [edit firewall name INBOUND rule 102]
+    >action accept
+    >protocol udp
+
+    [edit]
+
+replaced_diff_03:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: reject
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: udp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: accept
+              description: Rule 102 is configured by Ansible
+              protocol: tcp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [edit firewall ipv6-name UPLINK rule 1]
+    >action reject
+    >protocol udp
+    [edit firewall name INBOUND rule 102]
+    >protocol tcp
+
+    [edit]
diff --git a/tests/integration/targets/vyos_firewall_rules/vars/v1_4.yaml b/tests/integration/targets/vyos_firewall_rules/vars/v1_4.yaml
index 08675983..20ee461b 100644
--- a/tests/integration/targets/vyos_firewall_rules/vars/v1_4.yaml
+++ b/tests/integration/targets/vyos_firewall_rules/vars/v1_4.yaml
@@ -120,3 +120,151 @@ deleted_afi_all:
 state_dict:
   established: true
   related: true
+
+replaced_diff_01:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: accept
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: tcp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: reject
+              description: Rule 102 is configured by Ansible
+              protocol: tcp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [firewall ipv4 name INBOUND rule 101]
+    - action \"accept\"
+    + action \"reject\"
+
+
+    [edit]
+
+replaced_diff_02:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: accept
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: tcp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: accept
+              description: Rule 102 is configured by Ansible
+              protocol: udp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [firewall ipv4 name INBOUND rule 102]
+    - action \"reject\"
+    + action \"accept\"
+    - protocol \"tcp\"
+    + protocol \"udp\"
+
+
+    [edit]
+
+replaced_diff_03:
+  config:
+    - afi: ipv6
+      rule_sets:
+        - name: UPLINK
+          description: This is ipv6 specific rule-set
+          default_action: accept
+          rules:
+            - number: 1
+              action: reject
+              description: Fwipv6-Rule 1 is configured by Ansible
+              protocol: udp
+            - number: 2
+              action: accept
+              description: Fwipv6-Rule 2 is configured by Ansible
+              protocol: tcp
+    - afi: ipv4
+      rule_sets:
+        - name: INBOUND
+          description: IPv4 INBOUND rule set
+          default_action: accept
+          rules:
+            - number: 101
+              action: reject
+              description: Rule 101 is configured by Ansible
+              protocol: tcp
+            - number: 102
+              action: accept
+              description: Rule 102 is configured by Ansible
+              protocol: tcp
+            - number: 103
+              action: accept
+              description: Rule 103 is configured by Ansible
+              destination:
+                group:
+                  address_group: inbound
+              source:
+                address: 192.0.2.0
+              state: "{{ state_dict }}"
+  diff: |-
+    [firewall ipv4 name INBOUND rule 102]
+    - protocol \"udp\"
+    + protocol \"tcp\"
+    [firewall ipv6 name UPLINK rule 1]
+    - action \"accept\"
+    + action \"reject\"
+    - protocol \"tcp\"
+    + protocol \"udp\"
+
+
+    [edit]