From 746e50f05a813edf63127fa6b7ed9f602d99c503 Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <35733135+vzhestkov@users.noreply.github.com>
Date: Thu, 24 Jun 2021 13:19:31 +0300
Subject: [PATCH] Fix save for iptables state module (bsc#1185131)
 (#370)

---
 salt/states/iptables.py            | 82 +++++++++++++++-----------
 tests/unit/states/test_iptables.py | 92 ++++++++++++++++++++++++++++--
 2 files changed, 134 insertions(+), 40 deletions(-)

diff --git a/salt/states/iptables.py b/salt/states/iptables.py
index 872862ece2..6c4f4c2389 100644
--- a/salt/states/iptables.py
+++ b/salt/states/iptables.py
@@ -411,7 +411,7 @@ def append(name, table='filter', family='ipv4', **kwargs):
         if save:
             if save_file is True:
                 save_file = None
-            __salt__['iptables.save'](save_file, family=family)
+            __salt__['iptables.save'](filename=save_file, family=family)
         if not ret['changes']['locale']:
             del ret['changes']['locale']
         ret['comment'] = '\n'.join(comments)
@@ -438,7 +438,10 @@ def append(name, table='filter', family='ipv4', **kwargs):
                 filename = kwargs['save']
             else:
                 filename = None
-            saved_rules = __salt__['iptables.get_saved_rules'](family=family)
+            saved_rules = __salt__['iptables.get_saved_rules'](
+                conf_file=filename,
+                family=family
+            )
             _rules = __salt__['iptables.get_rules'](family=family)
             __rules = []
             for table in _rules:
@@ -450,7 +453,7 @@ def append(name, table='filter', family='ipv4', **kwargs):
                     __saved_rules.append(saved_rules[table][chain].get('rules'))
             # Only save if rules in memory are different than saved rules
             if __rules != __saved_rules:
-                out = __salt__['iptables.save'](filename, family=family)
+                out = __salt__['iptables.save'](filename=filename, family=family)
                 ret['comment'] += ('\nSaved iptables rule {0} for {1}\n'
                                    '{2}\n{3}').format(name, family, command.strip(), out)
         return ret
@@ -467,15 +470,14 @@ def append(name, table='filter', family='ipv4', **kwargs):
             name,
             command.strip(),
             family)
-        if 'save' in kwargs:
-            if kwargs['save']:
-                if kwargs['save'] is not True:
-                    filename = kwargs['save']
-                else:
-                    filename = None
-                out = __salt__['iptables.save'](filename, family=family)
-                ret['comment'] = ('Set and saved iptables rule {0} for {1}\n'
-                                  '{2}\n{3}').format(name, family, command.strip(), out)
+        if 'save' in kwargs and kwargs['save']:
+            if kwargs['save'] is not True:
+                filename = kwargs['save']
+            else:
+                filename = None
+            out = __salt__['iptables.save'](filename=filename, family=family)
+            ret['comment'] = ('Set and saved iptables rule {0} for {1}\n'
+                              '{2}\n{3}').format(name, family, command.strip(), out)
         return ret
     else:
         ret['result'] = False
@@ -543,7 +545,7 @@ def insert(name, table='filter', family='ipv4', **kwargs):
         if save:
             if save_file is True:
                 save_file = None
-            __salt__['iptables.save'](save_file, family=family)
+            __salt__['iptables.save'](filename=save_file, family=family)
         if not ret['changes']['locale']:
             del ret['changes']['locale']
         ret['comment'] = '\n'.join(comments)
@@ -570,7 +572,10 @@ def insert(name, table='filter', family='ipv4', **kwargs):
                 filename = kwargs['save']
             else:
                 filename = None
-            saved_rules = __salt__['iptables.get_saved_rules'](family=family)
+            saved_rules = __salt__['iptables.get_saved_rules'](
+                conf_file=filename,
+                family=family
+            )
             _rules = __salt__['iptables.get_rules'](family=family)
             __rules = []
             for table in _rules:
@@ -582,7 +587,7 @@ def insert(name, table='filter', family='ipv4', **kwargs):
                     __saved_rules.append(saved_rules[table][chain].get('rules'))
             # Only save if rules in memory are different than saved rules
             if __rules != __saved_rules:
-                out = __salt__['iptables.save'](filename, family=family)
+                out = __salt__['iptables.save'](filename=filename, family=family)
                 ret['comment'] += ('\nSaved iptables rule {0} for {1}\n'
                                    '{2}\n{3}').format(name, family, command.strip(), out)
         return ret
@@ -599,11 +604,14 @@ def insert(name, table='filter', family='ipv4', **kwargs):
             name,
             command.strip(),
             family)
-        if 'save' in kwargs:
-            if kwargs['save']:
-                out = __salt__['iptables.save'](filename=None, family=family)
-                ret['comment'] = ('Set and saved iptables rule {0} for {1}\n'
-                                  '{2}\n{3}').format(name, family, command.strip(), out)
+        if 'save' in kwargs and kwargs['save']:
+            if kwargs['save'] is not True:
+                filename = kwargs['save']
+            else:
+                filename = None
+            out = __salt__['iptables.save'](filename=filename, family=family)
+            ret['comment'] = ('Set and saved iptables rule {0} for {1}\n'
+                              '{2}\n{3}').format(name, family, command.strip(), out)
         return ret
     else:
         ret['result'] = False
@@ -666,7 +674,7 @@ def delete(name, table='filter', family='ipv4', **kwargs):
         if save:
             if save_file is True:
                 save_file = None
-            __salt__['iptables.save'](save_file, family=family)
+            __salt__['iptables.save'](filename=save_file, family=family)
         if not ret['changes']['locale']:
             del ret['changes']['locale']
         ret['comment'] = '\n'.join(comments)
@@ -717,11 +725,14 @@ def delete(name, table='filter', family='ipv4', **kwargs):
         ret['comment'] = 'Delete iptables rule for {0} {1}'.format(
             name,
             command.strip())
-        if 'save' in kwargs:
-            if kwargs['save']:
-                out = __salt__['iptables.save'](filename=None, family=family)
-                ret['comment'] = ('Deleted and saved iptables rule {0} for {1}\n'
-                                  '{2}\n{3}').format(name, family, command.strip(), out)
+        if 'save' in kwargs and kwargs['save']:
+            if kwargs['save'] is not True:
+                filename = kwargs['save']
+            else:
+                filename = None
+            out = __salt__['iptables.save'](filename=filename, family=family)
+            ret['comment'] = ('Deleted and saved iptables rule {0} for {1}\n'
+                              '{2}\n{3}').format(name, family, command.strip(), out)
         return ret
     else:
         ret['result'] = False
@@ -785,14 +796,17 @@ def set_policy(name, table='filter', family='ipv4', **kwargs):
             kwargs['policy'],
             family
         )
-        if 'save' in kwargs:
-            if kwargs['save']:
-                __salt__['iptables.save'](filename=None, family=family)
-                ret['comment'] = 'Set and saved default policy for {0} to {1} family {2}'.format(
-                    kwargs['chain'],
-                    kwargs['policy'],
-                    family
-                )
+        if 'save' in kwargs and kwargs['save']:
+            if kwargs['save'] is not True:
+                filename = kwargs['save']
+            else:
+                filename = None
+            __salt__['iptables.save'](filename=filename, family=family)
+            ret['comment'] = 'Set and saved default policy for {0} to {1} family {2}'.format(
+                kwargs['chain'],
+                kwargs['policy'],
+                family
+            )
         return ret
     else:
         ret['result'] = False
diff --git a/tests/unit/states/test_iptables.py b/tests/unit/states/test_iptables.py
index 90afa1ea3e..7168f33161 100644
--- a/tests/unit/states/test_iptables.py
+++ b/tests/unit/states/test_iptables.py
@@ -118,7 +118,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
         with patch.object(iptables, '_STATE_INTERNAL_KEYWORDS', mock):
             mock = MagicMock(return_value='a')
             with patch.dict(iptables.__salt__, {'iptables.build_rule': mock}):
-                mock = MagicMock(side_effect=[True, False, False, False])
+                mock = MagicMock(side_effect=[True, False, False, False, False, True])
                 with patch.dict(iptables.__salt__, {'iptables.check': mock}):
                     ret.update({'comment': 'iptables rule for salt'
                                 ' already set (a) for ipv4',
@@ -136,7 +136,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                              chain=''), ret)
 
                     with patch.dict(iptables.__opts__, {'test': False}):
-                        mock = MagicMock(side_effect=[True, False])
+                        mock = MagicMock(side_effect=[True, False, True, True])
                         with patch.dict(iptables.__salt__,
                                         {'iptables.append': mock}):
                             ret.update({'changes': {'locale': 'salt'},
@@ -158,6 +158,33 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                                  chain=''),
                                                  ret)
 
+                            mock_save = MagicMock(side_effect=['Wrote 1 lines to "/tmp/iptables"', ''])
+                            with patch.dict(iptables.__salt__,
+                                        {'iptables.save': mock_save}):
+                                mock_get_saved_rules = MagicMock(side_effect=[''])
+                                with patch.dict(iptables.__salt__,
+                                            {'iptables.get_saved_rules': mock_get_saved_rules}):
+                                    mock = MagicMock(side_effect=[''])
+                                    with patch.dict(iptables.__salt__,
+                                                {'iptables.get_rules': mock}):
+                                        ret.update({'changes': {'locale': 'salt'},
+                                                    'result': True,
+                                                    'comment': 'Set and saved iptables rule'
+                                                    ' salt for ipv4\na\nWrote 1 lines to "/tmp/iptables"'})
+                                        self.assertDictEqual(iptables.append('salt',
+                                                                             table='', chain='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        ret.update({'changes': {},
+                                                    'result': True,
+                                                    'comment': 'iptables rule for salt already set (a) for ipv4'})
+                                        self.assertDictEqual(iptables.append('salt',
+                                                                             table='', chain='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        self.assertEqual(mock_get_saved_rules.mock_calls[0][2]['conf_file'], '/tmp/iptables')
+                                        self.assertEqual(mock_save.mock_calls[0][2]['filename'], '/tmp/iptables')
+
     def test_insert(self):
         '''
             Test to insert a rule into a chain
@@ -173,7 +200,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
         with patch.object(iptables, '_STATE_INTERNAL_KEYWORDS', mock):
             mock = MagicMock(return_value='a')
             with patch.dict(iptables.__salt__, {'iptables.build_rule': mock}):
-                mock = MagicMock(side_effect=[True, False, False, False])
+                mock = MagicMock(side_effect=[True, False, False, False, False, True])
                 with patch.dict(iptables.__salt__, {'iptables.check': mock}):
                     ret.update({'comment': 'iptables rule for salt'
                                 ' already set for ipv4 (a)',
@@ -191,7 +218,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                              chain=''), ret)
 
                     with patch.dict(iptables.__opts__, {'test': False}):
-                        mock = MagicMock(side_effect=[False, True])
+                        mock = MagicMock(side_effect=[False, True, False, True])
                         with patch.dict(iptables.__salt__,
                                         {'iptables.insert': mock}):
                             ret.update({'changes': {'locale': 'salt'},
@@ -215,6 +242,33 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                                  position=''),
                                                  ret)
 
+                            mock_save = MagicMock(side_effect=['Wrote 1 lines to "/tmp/iptables"', ''])
+                            with patch.dict(iptables.__salt__,
+                                        {'iptables.save': mock_save}):
+                                mock_get_saved_rules = MagicMock(side_effect=[''])
+                                with patch.dict(iptables.__salt__,
+                                            {'iptables.get_saved_rules': mock_get_saved_rules}):
+                                    mock = MagicMock(side_effect=[''])
+                                    with patch.dict(iptables.__salt__,
+                                                {'iptables.get_rules': mock}):
+                                        ret.update({'changes': {'locale': 'salt'},
+                                                    'result': True,
+                                                    'comment': 'Set and saved iptables rule'
+                                                    ' salt for ipv4\na\nWrote 1 lines to "/tmp/iptables"'})
+                                        self.assertDictEqual(iptables.insert('salt',
+                                                                             table='', chain='', position='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        ret.update({'changes': {},
+                                                    'result': True,
+                                                    'comment': 'iptables rule for salt already set for ipv4 (a)'})
+                                        self.assertDictEqual(iptables.insert('salt',
+                                                                             table='', chain='', position='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        self.assertEqual(mock_get_saved_rules.mock_calls[0][2]['conf_file'], '/tmp/iptables')
+                                        self.assertEqual(mock_save.mock_calls[0][2]['filename'], '/tmp/iptables')
+
     def test_delete(self):
         '''
             Test to delete a rule to a chain
@@ -230,7 +284,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
         with patch.object(iptables, '_STATE_INTERNAL_KEYWORDS', mock):
             mock = MagicMock(return_value='a')
             with patch.dict(iptables.__salt__, {'iptables.build_rule': mock}):
-                mock = MagicMock(side_effect=[False, True, True, True])
+                mock = MagicMock(side_effect=[False, True, True, True, True, False])
                 with patch.dict(iptables.__salt__, {'iptables.check': mock}):
                     ret.update({'comment': 'iptables rule for salt'
                                 ' already absent for ipv4 (a)',
@@ -248,7 +302,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                              chain=''), ret)
 
                     with patch.dict(iptables.__opts__, {'test': False}):
-                        mock = MagicMock(side_effect=[False, True])
+                        mock = MagicMock(side_effect=[False, True, False, True, False])
                         with patch.dict(iptables.__salt__,
                                         {'iptables.delete': mock}):
                             ret.update({'result': True,
@@ -272,6 +326,32 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin):
                                                                  position=''),
                                                  ret)
 
+                            mock_save = MagicMock(side_effect=['Wrote 1 lines to "/tmp/iptables"', ''])
+                            with patch.dict(iptables.__salt__,
+                                        {'iptables.save': mock_save}):
+                                mock = MagicMock(side_effect=[True, False])
+                                with patch.dict(iptables.__salt__,
+                                            {'iptables.check': mock}):
+                                    mock = MagicMock(side_effect=[''])
+                                    with patch.dict(iptables.__salt__,
+                                                {'iptables.get_rules': mock}):
+                                        ret.update({'changes': {'locale': 'salt'},
+                                                    'result': True,
+                                                    'comment': 'Deleted and saved iptables rule'
+                                                    ' salt for ipv4\na\nWrote 1 lines to "/tmp/iptables"'})
+                                        self.assertDictEqual(iptables.delete('salt',
+                                                                             table='', chain='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        ret.update({'changes': {},
+                                                    'result': True,
+                                                    'comment': 'iptables rule for salt already absent for ipv4 (a)'})
+                                        self.assertDictEqual(iptables.delete('salt',
+                                                                             table='', chain='',
+                                                                             save='/tmp/iptables'),
+                                                                             ret)
+                                        self.assertEqual(mock_save.mock_calls[0][2]['filename'], '/tmp/iptables')
+
     def test_set_policy(self):
         '''
             Test to sets the default policy for iptables firewall tables
-- 
2.32.0


