Community.general.listen_ports_facts is broken for Netstat. Should at least default to SS.

[magical-inference]

Summary

The parsing-implementation for Netstat is broken to a degree where the feature should either be removed entirely or the code should default to using SS whenever possible. The parsing of the netstat-output must be rewritten from scratch: no regexes are used, lines are just split, the pid-name split function is broken, the whole module reads like someone wanted to have a decent implementation for SS and left the rest to die. This cost me and colleagues a couple hours today and probably does damage elsewhere, too, as I'm writing this.

Resulting behavior: service-names in the result returned to ansible sometimes end with colons. Other broken behaviour is likely, too -- the line-parsing looks almost maliciously bad. The last col in netstats outputs contains formats like "123/name: /path/to..", but the line is just split(). This needs to be either implemented correctly or removed for good.

The reason this is so terrible: depending on the usage of Netstat or SS, the output either looks fine (alias: it works) or it doesn't. When you use this module, you're likely to combine the results within jinja. Data manipulation in jinja/ansible is complicated enough as it is (!), broken modules with alternating behaviours cost more energy and attention than they save.

Regards

Issue Type

Bug Report

Component Name

community.general.listen_ports_facts

Ansible Version

ansible [core 2.12.6]

Community.general Version

community.general 4.8.2

Configuration

No response

OS / Environment

Linux

Steps to Reproduce

Use the module on a system that has netstat-output which contains spaces in the last column. But this is definitely broken beyond just this one example

Expected Results

pid-names are the same for ss and netstat output

Actual Results

pid-names from netstat output are broken

Code of Conduct

• [X] I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/system/listen_ports_facts.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @ndavison click here for bot help

[magical-inference]

A dirty hack to remove the bug that exposes the underlying issue:

def split_pid_name(pid_name):
    """
    Split the entry PID/Program name into the PID (int) and the name (str)
    :param pid_name:  PID/Program String seperated with a dash. E.g 51/sshd: returns pid = 51 and name = sshd
    :return: PID (int) and the program name (str)
    """
    try:
        pid, name = pid_name.split("/", 1)
    except ValueError:
        # likely unprivileged user, so add empty name & pid
        return 0, ""
    else:
        name = name.split()[0].rstrip(":")
        return int(pid), name

See the second-last line, replacing name = name.rstrip(":") (as it is now). The current state fails because the name-variable doesn't end on a colon. name, at this point, looks something like 'sshd: /usr/sbi', so rstrip fails (because the string doesn't end on :).

I don't want to push this as a PR because the underlying code is still a hack. The entire parsing function for netstat just isn't decent code. Someone should do this properly (look at the SS-implementation! Good code!), I don't want to commit hacks to something as fundamental as service-port-listing in ansible.

[ansibullbot]

Files identified in the description:

• plugins/modules/listen_ports_facts.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help