diff --git a/changelog/61122.fixed b/changelog/61122.fixed new file mode 100644 index 00000000000..85fc69674ae --- /dev/null +++ b/changelog/61122.fixed @@ -0,0 +1 @@ +Fix ipset state when the comment kwarg is set. diff --git a/salt/modules/ipset.py b/salt/modules/ipset.py index c93c28f4875..25717e7676a 100644 --- a/salt/modules/ipset.py +++ b/salt/modules/ipset.py @@ -302,7 +302,6 @@ def new_set(name=None, set_type=None, family="ipv4", comment=False, **kwargs): IPv6: salt '*' ipset.new_set custom_set list:set family=ipv6 """ - ipset_family = _IPSET_FAMILIES[family] if not name: return "Error: Set Name needs to be specified" @@ -483,7 +482,7 @@ def add(name=None, entry=None, family="ipv4", **kwargs): settype = setinfo["Type"] - cmd = [_ipset_cmd(), "add", "-exist", name, entry] + cmd = [_ipset_cmd(), "add", "-exist", name] + entry.split() if "timeout" in kwargs: if "timeout" not in setinfo["Header"]: @@ -497,7 +496,7 @@ def add(name=None, entry=None, family="ipv4", **kwargs): if "comment" not in setinfo["Header"]: return "Error: Set {} not created with comment support".format(name) if "comment" not in entry: - cmd = '{} comment "{}"'.format(cmd, kwargs["comment"]) + cmd = cmd + ["comment", f"{kwargs['comment']}"] if {"skbmark", "skbprio", "skbqueue"} & set(kwargs.keys()): if "skbinfo" not in setinfo["Header"]: diff --git a/salt/states/ipset.py b/salt/states/ipset.py index f80790c799e..f59e01f80c9 100644 --- a/salt/states/ipset.py +++ b/salt/states/ipset.py @@ -186,7 +186,7 @@ def present(name, entry=None, family="ipv4", **kwargs): if "timeout" in kwargs and "timeout" not in entry_opts: entry_opts = "timeout {} {}".format(kwargs["timeout"], entry_opts) if "comment" in kwargs and "comment" not in entry_opts: - entry_opts = '{} comment "{}"'.format(entry_opts, kwargs["comment"]) + entry_opts = "{} comment {}".format(entry_opts, kwargs["comment"]) _entry = " ".join([entry, entry_opts.lstrip()]).strip() if __salt__["ipset.check"](kwargs["set_name"], _entry, family) is True: @@ -254,7 +254,7 @@ def absent(name, entry=None, entries=None, family="ipv4", **kwargs): if "timeout" in kwargs and "timeout" not in entry_opts: entry_opts = "timeout {} {}".format(kwargs["timeout"], entry_opts) if "comment" in kwargs and "comment" not in entry_opts: - entry_opts = '{} comment "{}"'.format(entry_opts, kwargs["comment"]) + entry_opts = "{} comment {}".format(entry_opts, kwargs["comment"]) _entry = " ".join([entry, entry_opts]).strip() log.debug("_entry %s", _entry) diff --git a/tests/pytests/functional/modules/test_ipset.py b/tests/pytests/functional/modules/test_ipset.py new file mode 100644 index 00000000000..caeb26edae7 --- /dev/null +++ b/tests/pytests/functional/modules/test_ipset.py @@ -0,0 +1,47 @@ +import logging + +import pytest + +pytestmark = [ + pytest.mark.windows_whitelisted, +] + +log = logging.getLogger(__name__) + + +@pytest.fixture(scope="module") +def ipset(modules): + return modules.ipset + + +@pytest.fixture() +def setup_set(ipset): + set_name = "test_name" + kwargs = {"range": "192.168.0.0/16", "comment": "Hello18"} + ipset.new_set(name=set_name, set_type="bitmap:ip", family="ipv4", **kwargs) + yield set_name + ipset.delete_set(set_name) + + +def test_ipset_add(ipset, setup_set): + """ + test ipset.add + """ + # add set first + ret = ipset.add(name=setup_set, entry="192.168.0.3 comment Hello18") + assert ret == "Success" + check_set = ipset.list_sets() + assert any([x for x in check_set if x["Name"] == setup_set]) + + +def test_ipset_add_comment_kwarg(ipset, setup_set): + """ + test ipset.add when comment is set in kwarg + """ + # add set first + kwargs = {"comment": "Hello19"} + entry = "192.168.0.3" + ret = ipset.add(name=setup_set, entry="192.168.0.3", **kwargs) + assert ret == "Success" + check_set = ipset.list_sets() + assert any([x for x in check_set if x["Name"] == setup_set]) diff --git a/tests/pytests/functional/states/test_ipset.py b/tests/pytests/functional/states/test_ipset.py new file mode 100644 index 00000000000..d623a68dd58 --- /dev/null +++ b/tests/pytests/functional/states/test_ipset.py @@ -0,0 +1,35 @@ +import logging + +import pytest + +pytestmark = [ + pytest.mark.windows_whitelisted, +] + +log = logging.getLogger(__name__) + + +@pytest.fixture() +def setup_set(modules): + set_name = "test_name" + kwargs = {"range": "192.168.0.0/16", "comment": "Hello18"} + modules.ipset.new_set(name=set_name, set_type="bitmap:ip", family="ipv4", **kwargs) + yield set_name + modules.ipset.delete_set(set_name) + + +def test_ipset_present(states, setup_set): + """ + test ipset.present + """ + # add set first + entry = "192.168.0.3" + comment = "Hello" + ret = states.ipset.present( + name="setname_entries", set_name=setup_set, entry=entry, comment=comment + ) + assert ret.result + assert ( + ret.comment + == f"entry {entry} comment {comment} added to set {setup_set} for family ipv4\n" + )