salt icon indicating copy to clipboard operation
salt copied to clipboard

[TECH DEBT] Salt fails to load top.sls on Python3.13rc2

Open marmarek opened this issue 1 year ago • 4 comments

Description of the tech debt to be addressed, include links and screenshots

A recent change in Python 3.13 causes salt to fails on loading top.sls. With debugging enabled, I see now:

[DEBUG   ] Could not find file 'salt://.sls' in saltenv 'base'
[DEBUG   ] No contents loaded for saltenv 'base'

This seems to be directly caused by the change in Python: https://github.com/python/cpython/issues/85110

Old behavior:

>>> urllib.parse.urlunparse(("file", "", "top.sls", "", "", ""))
'file:///top.sls'

New behavior:

>>> urllib.parse.urlunparse(("file", "", "top.sls", "", "", ""))
'file:top.sls'

Technically, I think the latter might be more correct. But it still breaks salt's expectations. Specifically here: https://github.com/saltstack/salt/blob/246d0664577ef72da8bd1f0c4dff0d18b4428b23/salt/utils/url.py#L49-L50

With file:top.sls, looking for length of file:/// is wrong now.

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
          Salt: 3006.9
 
Python Version:
        Python: 3.13.0rc2 (main, Sep  7 2024, 00:00:00) [GCC 14.2.1 20240801 (Red Hat 14.2.1-1)]
 
Dependency Versions:
          cffi: 1.17.0
      cherrypy: Not Installed
  cryptography: Not Installed
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.4
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.6
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 24.1
     pycparser: 2.20
      pycrypto: Not Installed
  pycryptodome: 3.20.0
        pygit2: Not Installed
  python-gnupg: Not Installed
        PyYAML: 6.0.1
         PyZMQ: 25.1.1
        relenv: Not Installed
         smmap: Not Installed
       timelib: Not Installed
       Tornado: Not Installed
           ZMQ: 4.3.5
 
System Versions:
          dist: qubes 4.3 R4.3
        locale: utf-8
       machine: x86_64
       release: 6.6.48-1.qubes.fc41.x86_64
        system: Linux
       version: Qubes OS 4.3 R4.3

The same issue applies to Fedora 41 (in beta now). And while I tested using Salt 3006.9, the relevant code is the same in master.

marmarek avatar Sep 18 '24 02:09 marmarek

Take care that as if is related to cython it's independent from python3.13.

I have the same bug (and same fix working) for python 3.12.6/cython 3.0.11

ptitdoc avatar Sep 18 '24 08:09 ptitdoc

Indeed Python 3.12.6 is affected too, and so is Fedora 40 and 39.

marmarek avatar Sep 18 '24 16:09 marmarek

This is the only bug I have seen so far running on python 3.12.

jfindlay avatar Oct 10 '24 12:10 jfindlay

This seems to be directly caused by the change in Python: https://github.com/python/cpython/issues/85110

This seems to be the effective point of code change for the upstream issue.

jfindlay avatar Oct 10 '24 12:10 jfindlay

@marmarek thanks for your report and candidate pull request #66899, it proved to be of great help already at https://github.com/saltstack/salt/issues/66588#issuecomment-2552038027 :pray:

What I don't understand is the label "tech debt". It seems like a plain bug (and a big one that made salt-ssh completely unusable to me without the fix), and "tech debt" may send the wrong signal. Could it be dropped from the title? What was the idea?

hartwork avatar Dec 18 '24 19:12 hartwork

What I don't understand is the label "tech debt". It seems like a plain bug (and a big one that made salt-ssh completely unusable to me without the fix).

(+1 ...maybe) It can also cause incorrect states to be executed.

# cat /srv/salt/k/init.sls
always-passes-with-any-kwarg k:
  test.nop:
    - name: foo
    - something: {{ tplfile }}
    - foo: bar

A show_sls then gives:

root@ryz1:/srv/salt# salt-call --local state.show_sls uuuuuuk -l debug
local:
    ----------
    always-passes-with-any-kwarg k:
        ----------
        test:
            |_
              ----------
              name:
                  foo
            |_
              ----------
              something:
                  /var/cache/salt/minion/files/base/k/init.sls
            |_
              ----------
              foo:
                  bar
            - nop
            |_
              ----------
              order:
                  10000
        __sls__:
            uuuuuuk
        __env__:
            base

Every preceeding 'u' in the state specification is ignored, and instead state 'k' is/would be applied.

Regarding "Tech Debt"-label; it has no describing text so I can only guess from it's name the bug is related to salt-stack falling behind the newest and greatest technology. I don't think it says or send any signal regarding it's severity, it's just a clue to getting it fixed.

Mutisk avatar Dec 29 '24 00:12 Mutisk

FYI this is still an issue with 3006.14 and Python 3.12.10 (probably 3.12.6 and later).

pjcreath avatar Aug 19 '25 20:08 pjcreath