From 944f2a8e4db522ad32f547cf350a1268caa6de5a Mon Sep 17 00:00:00 2001 From: Victor Zhestkov <35733135+vzhestkov@users.noreply.github.com> Date: Thu, 24 Jun 2021 13:18:51 +0300 Subject: [PATCH] Fix save for iptables state module (bsc#1185131) (#372) --- salt/states/iptables.py | 86 ++++++++------ tests/unit/states/test_iptables.py | 184 ++++++++++++++++++++++++++++- 2 files changed, 227 insertions(+), 43 deletions(-) diff --git a/salt/states/iptables.py b/salt/states/iptables.py index 61dfc7e665..2e81477f18 100644 --- a/salt/states/iptables.py +++ b/salt/states/iptables.py @@ -401,7 +401,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) @@ -426,7 +426,9 @@ 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: @@ -438,7 +440,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 {} for {}\n" "{}\n{}").format( name, family, command.strip(), out ) @@ -454,16 +456,15 @@ def append(name, table="filter", family="ipv4", **kwargs): ret["comment"] = "Set iptables rule for {} to: {} for {}".format( 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 {} for {}\n" "{}\n{}" - ).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 {} for {}\n" "{}\n{}" + ).format(name, family, command.strip(), out) return ret else: ret["result"] = False @@ -527,7 +528,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) @@ -552,7 +553,9 @@ 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: @@ -564,7 +567,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 {} for {}\n" "{}\n{}").format( name, family, command.strip(), out ) @@ -582,12 +585,15 @@ def insert(name, table="filter", family="ipv4", **kwargs): ret["comment"] = "Set iptables rule for {} to: {} for {}".format( 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 {} for {}\n" "{}\n{}" - ).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 {} for {}\n" "{}\n{}" + ).format(name, family, command.strip(), out) return ret else: ret["result"] = False @@ -646,7 +652,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) @@ -688,12 +694,15 @@ def delete(name, table="filter", family="ipv4", **kwargs): ret["changes"] = {"locale": name} ret["result"] = True ret["comment"] = "Delete iptables rule for {} {}".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 {} for {}\n" "{}\n{}" - ).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 {} for {}\n" "{}\n{}" + ).format(name, family, command.strip(), out) return ret else: ret["result"] = False @@ -751,14 +760,17 @@ def set_policy(name, table="filter", family="ipv4", **kwargs): ret["comment"] = "Set default policy for {} to {} family {}".format( kwargs["chain"], 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 {} to {} family {}".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 {} to {} family {}".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 c49022c962..975ae49c3e 100644 --- a/tests/unit/states/test_iptables.py +++ b/tests/unit/states/test_iptables.py @@ -135,7 +135,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( { @@ -161,7 +161,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): ) 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( { @@ -188,6 +188,65 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): iptables.append("salt", table="", 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 @@ -200,7 +259,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( { @@ -226,7 +285,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): ) 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( { @@ -258,6 +317,67 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): 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 @@ -270,7 +390,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( { @@ -296,7 +416,7 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): ) with patch.dict(iptables.__opts__, {"test": False}): - mock = MagicMock(side_effect=[False, True]) + mock = MagicMock(side_effect=[False, True, False, False]) with patch.dict(iptables.__salt__, {"iptables.delete": mock}): ret.update( { @@ -327,6 +447,58 @@ class IptablesTestCase(TestCase, LoaderModuleMockMixin): 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