search

sosreport / sos

A unified tool for collecting system logs and other debug information
http://sos.rtfd.org
GNU General Public License v2.0
511 stars 543 forks source link

[policies] review pkg_by_name() wildcard behaviour #1817

Closed bmr-cymru closed 2 years ago

bmr-cymru commented 5 years ago

It was noticed while reviewing #1816 that the PackageManager.pkg_by_name() method applies shell-style wildcards to the package names it returns by applying fnmatch.filter() to the results of all_pkgs_by_name(). This can lead to confusing results when several packages exist which match a supplied pattern, since the function arbitrarily returns the last entry of the list of matching packages:

>>> p.package_manager.pkg_by_name("glibc")
u'glibc'
>>> p.package_manager.pkg_by_name("glibc*")
u'glibc-langpack-en'
>>> p.package_manager.all_pkgs_by_name("glibc*")
[u'glibc-devel', u'glibc-all-langpacks', u'glibc', u'glibc-common', u'glibc-headers', u'glibc-langpack-en']

Review this behaviour to see if anything relies on it currently and consider making the pkg_by_name() method an exact match.

TurboTurtle commented 2 years ago

Reviewing this, there are currently 10 plugins that use wildcards in their enablement triggers for packages - drbd, java, mysql, nss, openssl, openvswitch, ovn_central, ovn_host, pam, and perl.

Some of these are old, and some are relatively newer. So, we have a couple options here:

  1. Do nothing, and keep allowing pkg_by_name() to accept wildcards
  2. Switch is_installed() to directly leverage all_pkgs_by_name() and have pkg_by_name() only do exact matches. There are uses of pkg_by_name() outside plugin enablement, such as cluster profiles and presets loading/enablement that today do not have any wildcards in use
  3. Switch pkg_by_name() to be an exact match, and update these plugins to not use wildcards in their package triggers.

I'm personally leaning towards option 2. While not a very common use case, allowing wildcards in packages tuples for plugins is handy when the package has version information baked into the name (for some forsaken reason), e.g. - openvswitch with packages = ('openvswitch', 'openvswitch2.*', [...]

pmoravec commented 2 years ago

+1 for the option 2. Plugin enablement by package* makes sense from a user perspective, while we should prevent the confusion that Bryn wrote.