iptables chain creation does not populate with a rule (#80257)

* iptables chain creation does not populate with a rule

fixes #80256

* Add changelog fragment

* Add rules and flush chain during integration tests

* Check chain rule on comment

* Update test/integration/targets/iptables/tasks/chain_management.yml

changelogs/fragments/80257-iptables-chain-creation-does-not-populate-a-rule.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - iptables - remove default rule creation when creating iptables chain to be more similar to the command line utility (https://github.com/ansible/ansible/issues/80256).

lib/ansible/modules/iptables.py

@@ -895,33 +895,38 @@ def main():
             delete_chain(iptables_path, module, module.params)
 
     else:
-        insert = (module.params['action'] == 'insert')
-        rule_is_present = check_rule_present(
-            iptables_path, module, module.params
-        )
-        chain_is_present = rule_is_present or check_chain_present(
-            iptables_path, module, module.params
-        )
-        should_be_present = (args['state'] == 'present')
-
-        # Check if target is up to date
-        args['changed'] = (rule_is_present != should_be_present)
-        if args['changed'] is False:
-            # Target is already up to date
-            module.exit_json(**args)
-
-        # Check only; don't modify
-        if not module.check_mode:
-            if should_be_present:
-                if not chain_is_present and args['chain_management']:
-                    create_chain(iptables_path, module, module.params)
-
-                if insert:
-                    insert_rule(iptables_path, module, module.params)
+        # Create the chain if there are no rule arguments
+        if (args['state'] == 'present') and not args['rule']:
+            chain_is_present = check_chain_present(
+                iptables_path, module, module.params
+            )
+            args['changed'] = not chain_is_present
+
+            if (not chain_is_present and args['chain_management'] and not module.check_mode):
+                create_chain(iptables_path, module, module.params)
+
+        else:
+            insert = (module.params['action'] == 'insert')
+            rule_is_present = check_rule_present(
+                iptables_path, module, module.params
+            )
+
+            should_be_present = (args['state'] == 'present')
+            # Check if target is up to date
+            args['changed'] = (rule_is_present != should_be_present)
+            if args['changed'] is False:
+                # Target is already up to date
+                module.exit_json(**args)
+
+            # Modify if not check_mode
+            if not module.check_mode:
+                if should_be_present:
+                    if insert:
+                        insert_rule(iptables_path, module, module.params)
+                    else:
+                        append_rule(iptables_path, module, module.params)
                 else:
-                    append_rule(iptables_path, module, module.params)
-            else:
-                remove_rule(iptables_path, module, module.params)
+                    remove_rule(iptables_path, module, module.params)
 
     module.exit_json(**args)
 

test/integration/targets/iptables/tasks/chain_management.yml

@@ -45,6 +45,26 @@
       - result is not failed
       - '"FOOBAR-CHAIN" in result.stdout'
 
+- name: add rule to foobar chain
+  become: true
+  iptables:
+    chain: FOOBAR-CHAIN
+    source: 0.0.0.0
+    destination: 0.0.0.0
+    jump: DROP
+    comment: "FOOBAR-CHAIN RULE"
+
+- name: get the state of the iptable rules after rule is added to foobar chain
+  become: true
+  shell: "{{ iptables_bin }} -L"
+  register: result
+
+- name: assert rule is present in foobar chain
+  assert:
+    that:
+      - result is not failed
+      - '"FOOBAR-CHAIN RULE" in result.stdout'
+
 - name: flush the foobar chain
   become: true
   iptables:
@@ -68,4 +88,3 @@
     that:
       - result is not failed
       - '"FOOBAR-CHAIN" not in result.stdout'
-      - '"FOOBAR-RULE" not in result.stdout'

test/units/modules/test_iptables.py

@@ -181,7 +181,7 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 2)
+        self.assertEqual(run_command.call_count, 1)
         self.assertEqual(run_command.call_args_list[0][0][0], [
             '/sbin/iptables',
             '-t',
@@ -208,7 +208,6 @@ class TestIptables(ModuleTestCase):
 
         commands_results = [
             (1, '', ''),  # check_rule_present
-            (0, '', ''),  # check_chain_present
             (0, '', ''),
         ]
 
@@ -218,7 +217,7 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 3)
+        self.assertEqual(run_command.call_count, 2)
         self.assertEqual(run_command.call_args_list[0][0][0], [
             '/sbin/iptables',
             '-t',
@@ -232,7 +231,7 @@ class TestIptables(ModuleTestCase):
             '-j',
             'ACCEPT'
         ])
-        self.assertEqual(run_command.call_args_list[2][0][0], [
+        self.assertEqual(run_command.call_args_list[1][0][0], [
             '/sbin/iptables',
             '-t',
             'filter',
@@ -272,7 +271,7 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 2)
+        self.assertEqual(run_command.call_count, 1)
         self.assertEqual(run_command.call_args_list[0][0][0], [
             '/sbin/iptables',
             '-t',
@@ -321,7 +320,7 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 3)
+        self.assertEqual(run_command.call_count, 2)
         self.assertEqual(run_command.call_args_list[0][0][0], [
             '/sbin/iptables',
             '-t',
@@ -343,7 +342,7 @@ class TestIptables(ModuleTestCase):
             '--to-ports',
             '8600'
         ])
-        self.assertEqual(run_command.call_args_list[2][0][0], [
+        self.assertEqual(run_command.call_args_list[1][0][0], [
             '/sbin/iptables',
             '-t',
             'nat',
@@ -1019,10 +1018,8 @@ class TestIptables(ModuleTestCase):
         })
 
         commands_results = [
-            (1, '', ''),  # check_rule_present
             (1, '', ''),  # check_chain_present
             (0, '', ''),  # create_chain
-            (0, '', ''),  # append_rule
         ]
 
         with patch.object(basic.AnsibleModule, 'run_command') as run_command:
@@ -1031,32 +1028,20 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 4)
+        self.assertEqual(run_command.call_count, 2)
 
         self.assertEqual(run_command.call_args_list[0][0][0], [
-            '/sbin/iptables',
-            '-t', 'filter',
-            '-C', 'FOOBAR',
-        ])
-
-        self.assertEqual(run_command.call_args_list[1][0][0], [
             '/sbin/iptables',
             '-t', 'filter',
             '-L', 'FOOBAR',
         ])
 
-        self.assertEqual(run_command.call_args_list[2][0][0], [
+        self.assertEqual(run_command.call_args_list[1][0][0], [
             '/sbin/iptables',
             '-t', 'filter',
             '-N', 'FOOBAR',
         ])
 
-        self.assertEqual(run_command.call_args_list[3][0][0], [
-            '/sbin/iptables',
-            '-t', 'filter',
-            '-A', 'FOOBAR',
-        ])
-
         commands_results = [
             (0, '', ''),  # check_rule_present
         ]
@@ -1078,7 +1063,6 @@ class TestIptables(ModuleTestCase):
 
         commands_results = [
             (1, '', ''),  # check_rule_present
-            (1, '', ''),  # check_chain_present
         ]
 
         with patch.object(basic.AnsibleModule, 'run_command') as run_command:
@@ -1087,15 +1071,9 @@ class TestIptables(ModuleTestCase):
                 iptables.main()
                 self.assertTrue(result.exception.args[0]['changed'])
 
-        self.assertEqual(run_command.call_count, 2)
+        self.assertEqual(run_command.call_count, 1)
 
         self.assertEqual(run_command.call_args_list[0][0][0], [
-            '/sbin/iptables',
-            '-t', 'filter',
-            '-C', 'FOOBAR',
-        ])
-
-        self.assertEqual(run_command.call_args_list[1][0][0], [
             '/sbin/iptables',
             '-t', 'filter',
             '-L', 'FOOBAR',