[BUG] modules/ipset.py new_set Syntax error

[litnialex]

Description
Running
salt-call -l debug ipset.new_set name=whitelist set_type="hash:net" comment=True counters=True
Gives error:
[ERROR ] salt.loaded.int.module.cmdmod:1305 output: ipset v7.5: Syntax error: unknown inet family ['/usr/sbin/ipset', 'create', 'whitelist', 'hash:net', 'counters', 'family', [...], 'inet', 'comment']

Setup
Default setup, no special configuration.

Please be as specific as possible and give set-up details.

• on-prem machine
• VM (Virtualbox, KVM, etc. please specify)
• VM running on a cloud service, please be explicit and add details
• container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD

Steps to Reproduce the behavior

# salt-call -l debug ipset.new_set name=whitelist set_type="hash:net"  comment=True counters=True family=ipv4
[DEBUG   ] Reading configuration from /etc/salt/minion
[DEBUG   ] Including configuration from '/etc/salt/minion.d/_schedule.conf'
[DEBUG   ] Reading configuration from /etc/salt/minion.d/_schedule.conf
[DEBUG   ] Including configuration from '/etc/salt/minion.d/odoopbx.conf'
[DEBUG   ] Reading configuration from /etc/salt/minion.d/odoopbx.conf
[DEBUG   ] Including configuration from '/etc/salt/minion_local.conf'
[DEBUG   ] Reading configuration from /etc/salt/minion_local.conf
[DEBUG   ] Using cached minion ID from /etc/salt/minion_id: baba4741-4bcd-44a9-a4af-3682d8224631
[DEBUG   ] Using importlib_metadata to load entry points
[DEBUG   ] Override  __grains__: <module 'salt.loaded.int.log_handlers.sentry_mod' from '/usr/local/lib/python3.8/dist-packages/salt/log/handlers/sentry_mod.py'>
14:16:19 [DEBUG   ] salt.utils.parsers:220 Configuration file path: /etc/salt/minion
14:16:19 [WARNING ] salt.utils.verify:590 Insecure logging configuration detected! Sensitive data may be logged.
14:16:19 [DEBUG   ] salt.loader:868 Grains refresh requested. Refreshing grains.
14:16:19 [DEBUG   ] salt.config:1917 Reading configuration from /etc/salt/minion
14:16:19 [DEBUG   ] salt.config:2080 Including configuration from '/etc/salt/minion.d/_schedule.conf'
14:16:19 [DEBUG   ] salt.config:1917 Reading configuration from /etc/salt/minion.d/_schedule.conf
14:16:19 [DEBUG   ] salt.config:2080 Including configuration from '/etc/salt/minion.d/odoopbx.conf'
14:16:19 [DEBUG   ] salt.config:1917 Reading configuration from /etc/salt/minion.d/odoopbx.conf
14:16:19 [DEBUG   ] salt.config:2080 Including configuration from '/etc/salt/minion_local.conf'
14:16:19 [DEBUG   ] salt.config:1917 Reading configuration from /etc/salt/minion_local.conf
14:16:19 [DEBUG   ] salt.loader.lazy:857 Override  __utils__: <module 'salt.loaded.int.grains.zfs' from '/usr/local/lib/python3.8/dist-packages/salt/grains/zfs.py'>
14:16:19 [DEBUG   ] salt.modules.network:2132 Elapsed time getting FQDNs: 0.009395122528076172 seconds
14:16:19 [DEBUG   ] salt.loaded.int.grains.extra:56 Loading static grains from /etc/salt/grains
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded zfs.is_supported
14:16:20 [DEBUG   ] salt.minion:798 Connecting to master. Attempt 1 of 1
14:16:20 [DEBUG   ] salt.utils.network:2268 "localhost" Not an IP address? Assuming it is a hostname.
14:16:20 [DEBUG   ] salt.minion:238 Master URI: tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.crypt:505 Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'baba4741-4bcd-44a9-a4af-3682d8224631', 'tcp://127.0.0.1:4506')
14:16:20 [DEBUG   ] salt.transport.zeromq:394 Generated random reconnect delay between '1000ms' and '11000ms' (10505)
14:16:20 [DEBUG   ] salt.transport.zeromq:401 Setting zmq_reconnect_ivl to '10505ms'
14:16:20 [DEBUG   ] salt.transport.zeromq:405 Setting zmq_reconnect_ivl_max to '11000ms'
14:16:20 [DEBUG   ] salt.transport.zeromq:156 Connecting the Minion to the Master URI (for the return server): tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.transport.zeromq:1180 Trying to connect to: tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.crypt:219 salt.crypt.get_rsa_pub_key: Loading public key
14:16:20 [DEBUG   ] salt.crypt:942 Decrypting the current master AES key
14:16:20 [DEBUG   ] salt.crypt:211 salt.crypt.get_rsa_key: Loading private key
14:16:20 [DEBUG   ] salt.crypt:191 salt.crypt._get_key_with_evict: Loading private key
14:16:20 [DEBUG   ] salt.crypt:869 Loaded minion key: /etc/salt/pki/minion/minion.pem
14:16:20 [DEBUG   ] salt.crypt:219 salt.crypt.get_rsa_pub_key: Loading public key
14:16:20 [DEBUG   ] salt.transport.zeromq:177 Closing AsyncZeroMQReqChannel instance
14:16:20 [DEBUG   ] salt.transport.zeromq:462 Connecting the Minion to the Master publish port, using the URI: tcp://127.0.0.1:4505
14:16:20 [DEBUG   ] salt.crypt:211 salt.crypt.get_rsa_key: Loading private key
14:16:20 [DEBUG   ] salt.crypt:869 Loaded minion key: /etc/salt/pki/minion/minion.pem
14:16:20 [DEBUG   ] salt.pillar:69 Determining pillar cache
14:16:20 [DEBUG   ] salt.crypt:505 Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'baba4741-4bcd-44a9-a4af-3682d8224631', 'tcp://127.0.0.1:4506')
14:16:20 [DEBUG   ] salt.transport.zeromq:156 Connecting the Minion to the Master URI (for the return server): tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.transport.zeromq:1180 Trying to connect to: tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.crypt:211 salt.crypt.get_rsa_key: Loading private key
14:16:20 [DEBUG   ] salt.crypt:869 Loaded minion key: /etc/salt/pki/minion/minion.pem
14:16:20 [DEBUG   ] salt.transport.zeromq:177 Closing AsyncZeroMQReqChannel instance
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded jinja.render
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded yaml.render
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded ipset.new_set
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded direct_call.execute
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded cmd.run
14:16:20 [INFO    ] salt.loaded.int.module.cmdmod:417 Executing command /usr/sbin/ipset in directory '/root'
14:16:20 [ERROR   ] salt.loaded.int.module.cmdmod:877 Command '/usr/sbin/ipset' failed with return code: 1
14:16:20 [ERROR   ] salt.loaded.int.module.cmdmod:879 stdout: ipset v7.5: Syntax error: unknown inet family ['/usr/sbin/ipset', 'create', 'whitelist', 'hash:net', 'counters', 'family', [...], 'inet', 'comment']
14:16:20 [ERROR   ] salt.loaded.int.module.cmdmod:883 retcode: 1
14:16:20 [ERROR   ] salt.loaded.int.module.cmdmod:1300 Command '/usr/sbin/ipset' failed with return code: 1
14:16:20 [ERROR   ] salt.loaded.int.module.cmdmod:1305 output: ipset v7.5: Syntax error: unknown inet family ['/usr/sbin/ipset', 'create', 'whitelist', 'hash:net', 'counters', 'family', [...], 'inet', 'comment']
14:16:20 [DEBUG   ] salt.crypt:505 Initializing new AsyncAuth for ('/etc/salt/pki/minion', 'baba4741-4bcd-44a9-a4af-3682d8224631', 'tcp://127.0.0.1:4506')
14:16:20 [DEBUG   ] salt.transport.zeromq:156 Connecting the Minion to the Master URI (for the return server): tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.transport.zeromq:1180 Trying to connect to: tcp://127.0.0.1:4506
14:16:20 [DEBUG   ] salt.transport.zeromq:177 Closing AsyncZeroMQReqChannel instance
14:16:20 [DEBUG   ] salt.utils.lazy:99 LazyLoaded nested.output
local:
    ipset v7.5: Syntax error: unknown inet family ['/usr/sbin/ipset', 'create', 'whitelist', 'hash:net', 'counters', 'family', [...], 'inet', 'comment']

Versions Report

# salt-call  --versions-report
Salt Version:
          Salt: 3004
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: 18.6.1
      dateutil: 2.7.3
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.2
       libgit2: Not Installed
      M2Crypto: 0.31.0
          Mako: 1.0.7
       msgpack: 1.0.3
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.14.1
        pygit2: Not Installed
        Python: 3.8.10 (default, Nov 26 2021, 20:14:08)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 21.0.2
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.3
 
System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-99-generic
        system: Linux
       version: Ubuntu 20.04 focal

[zloyded]

[remingtonc]

The above (closed) PR resolved this issue for me. #61621

[jasonborden]

The backport for this bug fix the into 3006.x branch evidently did not merge.

[dmurphy18]

@jasonborden I am able to find the fix in current 3007.x branch, suggest you look closer. Given the original date of this, 2023, it is definitively in 3007.x branch, and appears to be in the 3006.x branch too

[jasonborden]

@dmurphy18 Can you please show me where?
All I know is that I can reproduce the exact behavior of this bug on 3006.8

Also when I compare the commit history at of the 3006.x branch at https://github.com/saltstack/salt/commits/3006.x/salt/modules/ipset.py to the 3007.x branch at https://github.com/saltstack/salt/commits/3007.x/salt/modules/ipset.py I can only see the commit in the 3007.x branch.

The 3006.x branch has some of the changes made in the patch, but the change at line 330 of salt/modules/ipset.py does not appear to be there.

[dmurphy18]

@jasonborden Checkout these in the 3006.x branch

salt/salt/modules/ipset.py

Line 317 in 3618966

return f"Error: {item} is a required argument"

salt/salt/modules/ipset.py

Line 330 in 3618966

cmd.extend(["family", cmd, ipset_family])

salt/salt/modules/ipset.py

Line 480 in 3618966

return f"Error: Set {name} does not exist"

salt/salt/modules/ipset.py

Lines 486 to 517 in 3618966

	if "timeout" in kwargs:
	if "timeout" not in setinfo["Header"]:
	return f"Error: Set {name} not created with timeout support"
	if "packets" in kwargs or "bytes" in kwargs:
	if "counters" not in setinfo["Header"]:
	return f"Error: Set {name} not created with counters support"
	if "comment" in kwargs:
	if "comment" not in setinfo["Header"]:
	return f"Error: Set {name} not created with comment support"
	if "comment" not in entry:
	cmd = cmd + ["comment", f"{kwargs['comment']}"]
	if {"skbmark", "skbprio", "skbqueue"} & set(kwargs.keys()):
	if "skbinfo" not in setinfo["Header"]:
	return f"Error: Set {name} not created with skbinfo support"
	for item in _ADD_OPTIONS[settype]:
	if item in kwargs:
	cmd.extend([item, kwargs[item]])
	current_members = _find_set_members(name)
	if entry in current_members:
	return f"Warn: Entry {entry} already exists in set {name}"
	# Using -exist to ensure entries are updated if the comment changes
	out = __salt__["cmd.run"](cmd, python_shell=False)
	if not out:
	return "Success"
	return f"Error: {out}"

salt/salt/modules/ipset.py

Lines 544 to 546 in 3618966

	if not out:
	return "Success"
	return f"Error: {out}"

salt/salt/modules/ipset.py

Lines 582 to 583 in 3618966

	if not settype:
	return f"Error: Set {name} does not exist"

salt/salt/modules/ipset.py

Lines 622 to 623 in 3618966

	if not settype:
	return f"Error: Set {name} does not exist"

The above lines appear to correspond with PR https://github.com/saltstack/salt/pull/65044/files changes.

[jasonborden]

@dmurphy18
Ok, I see what you're talking about now. Most of those changes were originally supposed to occur with the PR, but did not because the PR was not applied to 3006.x . Instead most of the changes were applied with a much later commit: "Update code to be Py3.7+ to reduce merge forward conflicts" (03ad4c6). The all important change on line 330 was never done as part that commit and so the bug still exists in 3006.x

[dmurphy18]

@jasonborden The lines look correct to me , even line 330, so solution is to open a new issue since this is closed, almost a year ago, explaining exactly the problem, showing lines etc.

That would be better than continuing this conversation here on an old closed issue.