Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
vladimir-lu opened this issue Dec 29, 2023 · 27 comments
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: doc C: Fedora C: mgmt diagnosed Technical diagnosis has been performed (see issue comments). P: critical Priority: critical. Between "major" and "blocker" in severity.

Comments

@vladimir-lu
Copy link

Qubes OS release

4.2.0
fedora-39-minimal template

Brief summary

When targeting a fedora 39 template, run into upstream issue saltstack/salt#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 vladimir-lu added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: bug labels Dec 29, 2023
@andrewdavidwong andrewdavidwong added C: mgmt C: Fedora needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. affects-4.2 This issue affects Qubes OS 4.2. updates testing Issue regarding an update that is currently in testing. Triage before migrating update to stable. labels Dec 29, 2023
@marmarek
Copy link
Member

marmarek commented Jan 8, 2024

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

@marmarek
Copy link
Member

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 marmarek added the R: cannot reproduce Resolution: Attempts to replicate the problem have not been reliably successful enough to proceed. label Feb 13, 2024
Copy link

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.

@marmarek marmarek mentioned this issue Feb 13, 2024
6 tasks
@andrewdavidwong andrewdavidwong removed the needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. label Feb 13, 2024
@andrewdavidwong andrewdavidwong closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2024
@boryeumao
Copy link

@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 :(

@DemiMarie
Copy link

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

@DemiMarie DemiMarie reopened this Feb 19, 2024
@boryeumao
Copy link

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

@marmarek
Copy link
Member

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.

@boryeumao
Copy link

boryeumao commented Feb 19, 2024 via email

@andrewdavidwong andrewdavidwong added needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. and removed R: cannot reproduce Resolution: Attempts to replicate the problem have not been reliably successful enough to proceed. updates testing Issue regarding an update that is currently in testing. Triage before migrating update to stable. labels Feb 19, 2024
@qtc-de
Copy link

qtc-de commented Feb 20, 2024

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?

@boryeumao
Copy link

@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
Copy link

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 ?!

@marmarek
Copy link
Member

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.

@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Feb 22, 2024
@Rudd-O
Copy link

Rudd-O commented Feb 23, 2024

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

@Rudd-O
Copy link

Rudd-O commented Feb 23, 2024

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
Copy link

Rudd-O commented Feb 23, 2024

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
Copy link

Rudd-O commented Feb 23, 2024

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.

@ben-grande
Copy link

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.

@DemiMarie
Copy link

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

@andrewdavidwong andrewdavidwong added P: critical Priority: critical. Between "major" and "blocker" in severity. and removed P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. labels Mar 5, 2024
@andrewdavidwong andrewdavidwong pinned this issue Mar 5, 2024
@ben-grande
Copy link

@marmarek The commit QubesOS/qubes-builder-rpm@fa42621 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?

@DemiMarie DemiMarie reopened this Mar 23, 2024
@marmarek
Copy link
Member

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.

@unman
Copy link
Member

unman commented Mar 31, 2024

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

@marmarek marmarek closed this as completed Apr 4, 2024
@andrewdavidwong andrewdavidwong unpinned this issue Apr 10, 2024
marmarek added a commit to QubesOS/qubes-builder-archlinux that referenced this issue May 25, 2024
Make the template salt-able by default

QubesOS/qubes-issues#8806
@aidecoe
Copy link

aidecoe commented Jun 27, 2024

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.

@ben-grande
Copy link

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

@aidecoe
Copy link

aidecoe commented Jun 28, 2024

@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?

@ben-grande
Copy link

@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.

@aidecoe
Copy link

aidecoe commented Jun 28, 2024

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.

@andrewdavidwong
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-4.2 This issue affects Qubes OS 4.2. C: doc C: Fedora C: mgmt diagnosed Technical diagnosis has been performed (see issue comments). P: critical Priority: critical. Between "major" and "blocker" in severity.
Projects
None yet
Development

No branches or pull requests

10 participants