Do not join flag parameters in iptables module (#36658) (#40501)

* Do not join flag parameters

This put a comma between every character of the tcp flag parameters, resulting in a bad iptables command.

Fixes #36490

* Use suboptions to ensure tcp_flags options are lists

* Add unit tests for tcp_flags

* Add example of how to use tcp_flags

(cherry picked from commit c9d3bb59a4)

changelogs/fragments/iptables-tcp-flags.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+  - iptables - use suboptions to properly join tcp_flags options (https://github.com/ansible/ansible/issues/36490)

lib/ansible/modules/system/iptables.py

@@ -106,11 +106,15 @@ options:
     description:
       - TCP flags specification.
       - C(tcp_flags) expects a dict with the two keys C(flags) and C(flags_set).
-        - The C(flags) list is the mask, a list of flags you want to examine.
-        - The C(flags_set) list tells which one(s) should be set.
-        If one of the two values is missing, the --tcp-flags option will be ignored.
     default: {}
     version_added: "2.4"
+    suboptions:
+        flags:
+            description:
+                - List of flags you want to examine.
+        flags_set:
+            description:
+                - Flags to be set.
   match:
     description:
       - Specifies a match to use, that is, an extension module that tests for
@@ -340,6 +344,19 @@ EXAMPLES = '''
     protocol: tcp
     reject_with: tcp-reset
     ip_version: ipv4
+
+# Set tcp flags
+- iptables:
+    chain: OUTPUT
+    jump: DROP
+    protocol: tcp
+    tcp_flags:
+      flags: ALL
+      flags_set:
+        - ACK
+        - RST
+        - SYN
+        - FIN
 '''
 
 import re
@@ -518,7 +535,11 @@ def main():
             destination=dict(type='str'),
             to_destination=dict(type='str'),
             match=dict(type='list', default=[]),
-            tcp_flags=dict(type='dict', default={}),
+            tcp_flags=dict(type='dict',
+                           options=dict(
+                                flags=dict(type='list'),
+                                flags_set=dict(type='list'))
+                           ),
             jump=dict(type='str'),
             log_prefix=dict(type='str'),
             goto=dict(type='str'),
@@ -605,5 +626,6 @@ def main():
 
     module.exit_json(**args)
 
+
 if __name__ == '__main__':
     main()

test/units/modules/system/test_iptables.py

@@ -1,4 +1,3 @@
-from ansible.compat.tests import unittest
 from ansible.compat.tests.mock import patch
 from ansible.module_utils import basic
 from ansible.modules.system import iptables
@@ -578,3 +577,62 @@ class TestIptables(ModuleTestCase):
             '--reject-with',
             'tcp-reset',
         ])
+
+    def test_tcp_flags(self):
+        """ Test various ways of inputting tcp_flags """
+        args = [
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': 'flags=ALL flags_set="ACK,RST,SYN,FIN"'
+            },
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': {
+                    'flags': 'ALL',
+                    'flags_set': 'ACK,RST,SYN,FIN'
+                }
+            },
+            {
+                'chain': 'OUTPUT',
+                'protocol': 'tcp',
+                'jump': 'DROP',
+                'tcp_flags': {
+                    'flags': ['ALL'],
+                    'flags_set': ['ACK', 'RST', 'SYN', 'FIN']
+                }
+            },
+
+        ]
+
+        for item in args:
+            set_module_args(item)
+
+            commands_results = [
+                (0, '', ''),
+            ]
+
+            with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+                run_command.side_effect = commands_results
+                with self.assertRaises(AnsibleExitJson) as result:
+                    iptables.main()
+                    self.assertTrue(result.exception.args[0]['changed'])
+
+            self.assertEqual(run_command.call_count, 1)
+            self.assertEqual(run_command.call_args_list[0][0][0], [
+                '/sbin/iptables',
+                '-t',
+                'filter',
+                '-C',
+                'OUTPUT',
+                '-p',
+                'tcp',
+                '--tcp-flags',
+                'ALL',
+                'ACK,RST,SYN,FIN',
+                '-j',
+                'DROP'
+            ])