Skip to content

ipset with comment is broken in 3004 #61122

@cod88

Description

@cod88

Description
ipset with comment is broken in 3004

Setup

Linux CentOS 7.9

Steps to Reproduce the behavior
salt-call state.single ipset.present name=salt-3004 set_name=ssh_allow_ip entry='1.1.1.1' comment='cloudflare test'

Expected behavior

local:
----------
          ID: salt-3004
    Function: ipset.present
      Result: True
     Comment: entry 1.1.1.1 comment "cloudflare test" added to set ssh_allow_ip for family ipv4
     Started: 13:08:17.972071
    Duration: 45.026 ms
     Changes:   
              ----------
              locale:
                  salt-3004

Summary for local
------------
Succeeded: 1 (changed=1)

Actual behavior

salt-call state.single ipset.present name=salt-3004 set_name=ssh_allow_ip entry='1.1.1.1' comment='cloudflare test'
local:
----------
          ID: salt-3004
    Function: ipset.present
      Result: False
     Comment: Failed to add to entry 1.1.1.1 comment "cloudflare test" to set ssh_allow_ip for family ipv4.
              Error: ipset v7.1: Syntax error: cannot parse 1.1.1.1 comment "cloudflare test": resolving to IPv4 address failed
     Started: 13:09:48.364796
    Duration: 31.464 ms
     Changes:   

Summary for local
------------
Succeeded: 0
Failed:    1

same for ipset.set_present:

# salt-call state.single ipset.set_present name=salt-3004 set_type=hash:net comment=True

local:
----------
          ID: salt-3004
    Function: ipset.set_present
      Result: False
     Comment: Failed to create set salt-3004 for ipv4: ipset v7.1: Syntax error: unknown inet family ['/usr/sbin/ipset', 'create', 'salt-3004', 'hash:net', 'family', [...], 'inet', 'comment']
     Started: 07:35:08.999186
    Duration: 25.849 ms
     Changes:

Versions Report

salt --versions-report

Salt Version:
Salt: 3004

Dependency Versions:
cffi: Not Installed
cherrypy: Not Installed
dateutil: 2.4.2
docker-py: Not Installed
gitdb: Not Installed
gitpython: Not Installed
Jinja2: 2.11.1
libgit2: Not Installed
M2Crypto: 0.35.2
Mako: Not Installed
msgpack: 0.6.2
msgpack-pure: Not Installed
mysql-python: Not Installed
pycparser: Not Installed
pycrypto: Not Installed
pycryptodome: Not Installed
pygit2: Not Installed
Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
python-gnupg: Not Installed
PyYAML: 3.13
PyZMQ: 17.0.0
smmap: Not Installed
timelib: Not Installed
Tornado: 4.5.3
ZMQ: 4.1.4

System Versions:
dist: centos 7 Core
locale: UTF-8
machine: x86_64
release: 3.10.0-1127.el7.x86_64
system: Linux
version: CentOS Linux 7 Core

Additional context
I am pretty sure it was broken in commit 0e46c14.

cmd is list now, not string and ipset binary doesn't work

commit 0e46c14907f1c63c9bd7206ed3a7dd59e59e0800
Author: Pedro Algarvio <***@***.**>
Date:   Mon Sep 13 14:59:47 2021 +0100

    Fix and migrate ``tests/unit/modules/test_ipset.py`` to PyTest

diff --git a/salt/modules/ipset.py b/salt/modules/ipset.py
index 4191932aa1..c93c28f487 100644
--- a/salt/modules/ipset.py
+++ b/salt/modules/ipset.py
.
.
.
.
@@ -483,7 +483,7 @@ def add(name=None, entry=None, family="ipv4", **kwargs):
 
     settype = setinfo["Type"]
 
-    cmd = "{}".format(entry)
+    cmd = [_ipset_cmd(), "add", "-exist", name, entry]
 
     if "timeout" in kwargs:
         if "timeout" not in setinfo["Header"]:
@@ -505,14 +505,13 @@ def add(name=None, entry=None, family="ipv4", **kwargs):
 
     for item in _ADD_OPTIONS[settype]:
         if item in kwargs:
-            cmd = "{} {} {}".format(cmd, item, kwargs[item])
+            cmd.extend([item, kwargs[item]])
 
     current_members = _find_set_members(name)
-    if cmd in current_members:
-        return "Warn: Entry {} already exists in set {}".format(cmd, name)
+    if entry in current_members:
+        return "Warn: Entry {} already exists in set {}".format(entry, name)
 
     # Using -exist to ensure entries are updated if the comment changes
-    cmd = "{} add -exist {} {}".format(_ipset_cmd(), name, cmd)
     out = __salt__["cmd.run"](cmd, python_shell=False)
 
     if not out:

Metadata

Metadata

Assignees

Labels

RegressionThe issue is a bug that breaks functionality known to work in previous releases.bugbroken, incorrect, or confusing behaviorseverity-medium3rd level, incorrect or bad functionality, confusing and lacks a work around

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions