Fix ipset state when the comment kwarg is set.

changelog/61122.fixed

@@ -0,0 +1 @@
+Fix ipset state when the comment kwarg is set.

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"]:

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)

tests/pytests/functional/modules/test_ipset.py

@@ -0,0 +1,49 @@
+import logging
+
+import pytest
+
+pytestmark = [
+    pytest.mark.slow_test,
+    pytest.mark.skip_if_binaries_missing("ipset", check_all=False),
+]
+
+
+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])

tests/pytests/functional/states/test_ipset.py

@@ -0,0 +1,40 @@
+import logging
+
+import pytest
+
+pytestmark = [
+    pytest.mark.windows_whitelisted,
+]
+
+log = logging.getLogger(__name__)
+
+pytestmark = [
+    pytest.mark.slow_test,
+    pytest.mark.skip_if_binaries_missing("ipset", check_all=False),
+]
+
+
+@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"
+    )