qubes-issues icon indicating copy to clipboard operation
qubes-issues copied to clipboard

fedora-39 template vms cannot be targeted by salt because of incorrect import (python 3.12, salt patch required)

Open vladimir-lu opened this issue 1 year ago • 16 comments

Qubes OS release

4.2.0 fedora-39-minimal template

Brief summary

When targeting a fedora 39 template, run into upstream issue https://github.com/saltstack/salt/issues/65360 . I expect it will be fixed in salt 3007 based on the comments on the issue.

Steps to reproduce

  1. Install a fedora 39 template (fedora-39-minimal)
  2. Try to manage packages using salt
  3. Get error about no package called 'backports'

Expected behavior

  1. Install a fedora 39 template
  2. Try to manage packages using salt
  3. Package installation/removal succeeds

Actual behavior

Error message:

       [ERROR   ] An un-handled exception was caught by Salt's global exception handler:
          ModuleNotFoundError: No module named 'backports'
          Traceback (most recent call last):
            File "/var/tmp/.root_dd8a91_salt/salt-call", line 27, in <module>
              salt_call()
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/scripts.py", line 437, in salt_call
              import salt.cli.call
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/cli/call.py", line 3, in <module>
              import salt.cli.caller
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/cli/caller.py", line 12, in <module>
              import salt.channel.client
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/channel/client.py", line 13, in <module>
              import salt.crypt
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/crypt.py", line 26, in <module>
              import salt.payload
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/payload.py", line 12, in <module>
              import salt.loader.context
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/loader/__init__.py", line 15, in <module>
              import salt.config
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/config/__init__.py", line 107, in <module>
              _DFLT_IPC_WBUFFER = int(_gather_buffer_space() * 0.5)
                                      ^^^^^^^^^^^^^^^^^^^^^^
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/config/__init__.py", line 95, in _gather_buffer_space
              import salt.grains.core
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/grains/core.py", line 30, in <module>
              import salt.modules.cmdmod
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/modules/cmdmod.py", line 32, in <module>
              import salt.utils.templates
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/utils/templates.py", line 21, in <module>
              import salt.utils.http
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/utils/http.py", line 27, in <module>
              import salt.ext.tornado.simple_httpclient
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/ext/tornado/simple_httpclient.py", line 9, in <module>
              from salt.ext.tornado.http1connection import HTTP1Connection, HTTP1ConnectionParameters
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/ext/tornado/http1connection.py", line 31, in <module>
              from salt.ext.tornado import iostream
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/ext/tornado/iostream.py", line 41, in <module>
              from salt.ext.tornado.netutil import ssl_wrap_socket, ssl_match_hostname, SSLCertificateError, _client_ssl_defaults, _server_ssl_defaults
            File "/var/tmp/.root_dd8a91_salt/pyall/salt/ext/tornado/netutil.py", line 57, in <module>
              import backports.ssl_match_hostname
          ModuleNotFoundError: No module named 'backports'

Workaround

Do the equivalent of pip3 install backports.ssl_match_hostname (as mentioned in upstream ticket) or patch salt itself.

vladimir-lu avatar Dec 29 '23 10:12 vladimir-lu

Is it still the case if you switch default-mgmt-dvm (or management_dispvm specific to that template) to fedora-39 template?

marmarek avatar Jan 08 '24 13:01 marmarek

Since no response, I assume setting default-mgmt-dvm to fedora-39 fixed the issue. I cannot reproduce the issue when using new template for default-mgmt-dvm.

marmarek avatar Feb 13 '24 00:02 marmarek

This issue has been closed as "cannot reproduce." This means that attempts have been made to replicate the problem, but such attempts have not been reliably successful enough to proceed with fixing the problem.

We respect the time and effort you have taken to file this issue, and we understand that this outcome may be unsatisfying. Please accept our sincere apologies and know that we greatly value your participation and membership in the Qubes community. If the problem becomes reliably reproducible in the future, please let us know in a comment below, and we will reopen this issue.

If anyone reading this believes that this issue was closed in error or that the resolution of "cannot reproduce" is not accurate, please leave a comment below saying so, and we will review this issue again. For more information, see How issues get closed.

github-actions[bot] avatar Feb 13 '24 00:02 github-actions[bot]

@marmarek - For me, switching default-mgmt-dvm to fedora-39 template did not fix the 'ModuleNotFoundError' for 'backports'. Also, neither workaround suggested by @vladimir-lu worked for me. Curiously, using the Qubes OS Updater GUI the update of fedora-39-minimal was successful. Completely at a loss here :(

boryeumao avatar Feb 19 '24 04:02 boryeumao

Updater no longer uses Salt (except for dom0) so it working is expected, but clearly you can reproduce, so reopening.

DemiMarie avatar Feb 19 '24 05:02 DemiMarie

For reference: attached are two files logging the errors from running salt update on fedora-39-minimal template, one with default-mgmt-dvm based on debian-12-minimal as template and the other based on fedora-39.

qubesctl-f39.debian-12-minimal.txt qubesctl-f39.fedora-39.txt

boryeumao avatar Feb 19 '24 08:02 boryeumao

As for the fedora-39 - is that fedora-39-minimal by a chance? I see a different message there:

          ModuleNotFoundError: No module named 'urllib3'

The python3-urllib3 package is installed in non-minimal fedora-39 template.

marmarek avatar Feb 19 '24 11:02 marmarek

It was fedora-39 (fully updated).  Yes I did see that urllib3 module is in the python3 site-packages directory (and could be imported successfully in a dummy python session), so the error message was a mystery to me.  Without sufficient understanding of salt and qubes internal I couldn’t pursue further.

boryeumao avatar Feb 19 '24 11:02 boryeumao

I'm not a salt expert, but the filesystem paths /var/tmp/... suggest that salt creates temporary python scripts within the minion that is being managed and executes them. If this assumption is correct, the problem should only occur on minions that do not have python3-urllib3 installed (like fedora-39-minimal).

And indeed, I can reproduce the issue when operating on a plain fedora-39-minimal template. After installing python3-urllib3 manually in a fresh fedora-39-minimal template, salt is running fine for it without any errors. So the easiest solution is probably to ensure that python3-urllib3 is installed on all officially supported templates?

qtc-de avatar Feb 20 '24 21:02 qtc-de

@qtc-de Confirmed (salt update is successful after installing python-urllib3 in fedora-39-minimal, when default-mgmt-dvm runs on fedora-39 template).

This clues me on why I wasn't able to get the backports.ssl_match_hostname workaround (@vladimir-lu) to work (for debian-12-based default-mgmt-dvm): backports is to be applied to the minion, fedora-39-minimal in my case. After fumbling around a bit, I found that a direct pip install backports.ssl_match_hostname still fails, but downloading the tar file, followed by python setup.py build and then install to /usr/local/lib/, does allow the salt update via qubesctl to finish without errors. During build/install, there were some messages about setup.py being deprecated for newer (and more "gentle") ways to do the installation, which I'm looking into, but at least this is a big step forward. 👍 and Thanks all!

boryeumao avatar Feb 21 '24 16:02 boryeumao

The second workaround mentioned in @vladimir-lu was to patch salt (specifically in this case salt/ext/tornado/netutil.py, to call urllib3 instead of backports). The advantage is that qubesctl won't need to "know" which template default-mgmt-dvm happens to be based on. However, template-39-minimal does not have salt - /var/tmp/.root_xxxxxx-salt/ was left there only after the errors, and it must've been copied there and the question is wherefrom.

Also, for the purpose of better understanding salt (and possibly qubesctl too): narrowly patching netutil.py for update to finish is probably completely fine here - but there could be other cases where it'll be necessary also have backports patched ?!

boryeumao avatar Feb 22 '24 16:02 boryeumao

I'm not a salt expert, but the filesystem paths /var/tmp/... suggest that salt creates temporary python scripts within the minion that is being managed and executes them. If this assumption is correct, the problem should only occur on minions that do not have python3-urllib3 installed (like fedora-39-minimal).

This looks to be correct diagnosis.

marmarek avatar Feb 22 '24 17:02 marmarek

I can repro this too. My machines have python3-urllib3 and this still happens.

Rudd-O avatar Feb 23 '24 17:02 Rudd-O

problematic in ext.tornado

if hasattr(ssl, 'match_hostname') and hasattr(ssl, 'CertificateError'):  # python 3.2+
    ssl_match_hostname = ssl.match_hostname
    SSLCertificateError = ssl.CertificateError
elif ssl is None:
    ssl_match_hostname = SSLCertificateError = None  # type: ignore
else:
    import backports.ssl_match_hostname
    ssl_match_hostname = backports.ssl_match_hostname.match_hostname
    SSLCertificateError = backports.ssl_match_hostname.CertificateError  # type: ignore

is there a match hostname in ssl? no

[user@social ~]$ python3
Python 3.12.1 (main, Dec 18 2023, 00:00:00) [GCC 13.2.1 20231205 (Red Hat 13.2.1-6)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ssl
>>> ssl.match_hostname
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'ssl' has no attribute 'match_hostname'

So this blows up.

Rudd-O avatar Feb 23 '24 17:02 Rudd-O

Replace in salt/ext/tornado/netutil.py line 57 and on:

if hasattr(ssl, 'match_hostname') and hasattr(ssl, 'CertificateError'):  # python 3.2+
    ssl_match_hostname = ssl.match_hostname
    SSLCertificateError = ssl.CertificateError
elif ssl is None:
    ssl_match_hostname = SSLCertificateError = None  # type: ignore
else:
    import backports.ssl_match_hostname
    ssl_match_hostname = backports.ssl_match_hostname.match_hostname
    SSLCertificateError = backports.ssl_match_hostname.CertificateError  # type: ignore

with

if 0:  # patched
    if hasattr(ssl, 'match_hostname') and hasattr(ssl, 'CertificateError'):  # python 3.2+
        ssl_match_hostname = ssl.match_hostname
        SSLCertificateError = ssl.CertificateError
    elif ssl is None:
        ssl_match_hostname = SSLCertificateError = None  # type: ignore
    else:
        import backports.ssl_match_hostname
        ssl_match_hostname = backports.ssl_match_hostname.match_hostname
        SSLCertificateError = backports.ssl_match_hostname.CertificateError  # type: ignore
ssl_match_hostname = SSLCertificateError = None  # type: ignore

and now it works.

Rudd-O avatar Feb 23 '24 17:02 Rudd-O

The quality of the SaltStack code base makes me think it's a miracle this thing even works. I regret betting on this horse years ago.

Rudd-O avatar Feb 23 '24 17:02 Rudd-O

Is it still the case if you switch default-mgmt-dvm (or management_dispvm specific to that template) to fedora-39 template?

I am using dvm-fedora (based on fedora-39) as the management_dispvm.

I can target a fedora-39 with salt (after I did an update and upgrade, I believe this had an impact).

I can't target a fedora-39-minimal due to missing urllib3, even though the management disposable vm is the non-minimal variant. This means that fedora minimal can't be targeted via Salt automatically without user interaction to install python3-urllib3. It can be done with a state targetting dom0 that runs qvm-run -u root fedora-39-minimal dnf install python3-urllib3 (untested), sure, but that is an undesired solution. It also may impact debian-13-minimal if it doesn't have the urllib3 on the next Salt version.

I'm not a salt expert, but the filesystem paths /var/tmp/... suggest that salt creates temporary python scripts within the minion that is being managed and executes them. If this assumption is correct, the problem should only occur on minions that do not have python3-urllib3 installed (like fedora-39-minimal).

This looks to be correct diagnosis.

What is the recommended way for developers using Salt to have it working on minimal templates? As I see, the backports issue can be fixed by switching to the fedora-39 template, while the urllib3 is required in the minion that is minimal and does not have it by default.

ben-grande avatar Mar 05 '24 20:03 ben-grande

@ben-grande I would install python3-urllib3 in the template via your system package manager.

DemiMarie avatar Mar 05 '24 20:03 DemiMarie

@marmarek The commit https://github.com/QubesOS/qubes-builder-rpm/commit/fa42621395a89eceacdcc701c3a672df33791b63 only seems to apply for future template installs. For users that have already installed the template, can this be applied via an update as a dependency to a qubes package?

ben-grande avatar Mar 23 '24 12:03 ben-grande

can this be applied via an update as a dependency to a qubes package?

I don't think this is appropriate. It isn't really "required" by any of the packages installed in minimal template. And if one doesn't use salt, it should be possible to remove python3-urllib3 package without any unintended side effects (which would happen if some package would depend on it). I guess we can add a note to the minimal template documentation about this issue in an older template version.

marmarek avatar Mar 25 '24 02:03 marmarek

I have added a note to the minimal template documentation about this issue.

unman avatar Mar 31 '24 12:03 unman

I'm wondering, is the installation of python3-urllib3 intended to address the original error ModuleNotFoundError: No module named 'backports' or only the problem reported by boryeumao? I'm getting the original error about backports when trying to use Salt on a relatively fresh 4.2 installation. I have installed fedora-40-xfce and removed fedora-39-xfce. The problem is in either of the two.

aidecoe avatar Jun 27 '24 22:06 aidecoe

@aidecoe Change Global prefs management_dispvm to fedora-40-xfce.

ben-grande avatar Jun 28 '24 05:06 ben-grande

@ben-grande the management_dispvm=default-mgmt-dvm. In the manager template for qubes I saw it's debian-12-xfce, as Debian is my default template. I changed it to fedora-40-xfce and that indeed works. Thank you! Salt worked with other Debian-based VMs with default-mgmt-dvm set to debian-12-xfce.

@marmarek is Debian supported in default-mgmt-dvm?

aidecoe avatar Jun 28 '24 06:06 aidecoe

@aidecoe Sorry, I meant the template of management_dispvm, you got it right.

@marmarek is Debian supported in default-mgmt-dvm?

I am not Marek but debian-12 can be a management_dispvm of any Debian qube (qvm-prefs QUBE management_disvpm), but should not be the Global prefs (qubes-prefs management_dispvm) in case you have Fedora qubes and possibly other distros with fast releases that Salt Project can't keep up.

ben-grande avatar Jun 28 '24 06:06 ben-grande

Yes, that makes sense. I chose debian-12-xfce as my default template on the installation. In this case, I suppose, the installer should make an exception and assign fedora template.

aidecoe avatar Jun 28 '24 20:06 aidecoe

Yes, that makes sense. I chose debian-12-xfce as my default template on the installation. In this case, I suppose, the installer should make an exception and assign fedora template.

Please feel free to open a separate issue for that.

andrewdavidwong avatar Jun 28 '24 21:06 andrewdavidwong