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

[BUG] salt.utils.url.create() produces bad URI under python 3.12.7 #66950

Closed
1 of 9 tasks
yangskyboxlabs opened this issue Oct 8, 2024 · 2 comments
Closed
1 of 9 tasks
Labels
Bug broken, incorrect, or confusing behavior needs-triage

Comments

@yangskyboxlabs
Copy link

yangskyboxlabs commented Oct 8, 2024

Description
urllib.parse.urlunparse() may or may not expand file schema with an empty netloc to file:/// or file: due to some contention around schema-based handling for security reasons. Apparently the implemented behaviour changed between python 3.12.5 and 3.12.7 (I've not tried to reproduce with 3.12.6)

This inconsistency breaks salt.utils.url.create() which assumes it would always unparse to file:///path.

(More in Additional context)

Setup
(Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

  • on-prem machine
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD
  • classic packaging
  • onedir packaging
  • used bootstrap to install

Steps to Reproduce the behavior
Run salt-call state.show_sls '*' with any populated state tree with salt installed under python 3.12.7.

States will fail due to errors like:

- Specified SLS top in saltenv base is not available on the salt master or through
  a configured fileserver

Increasing salt.fileclient verbosity gives errors like:

2024-10-08 10:40:20,154 (salt.fileclient) DEBUG: Could not find file 'salt://s' in saltenv 'base'

Expected behavior
state.show_sls '*' should work.

Versions Report

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

Python Version:
        Python: 3.12.7 (tags/v3.12.7:0b05ead, Oct  1 2024, 03:06:41) [MSC v.1941 64 bit (AMD64)]

Dependency Versions:
          cffi: 1.16.0
      cherrypy: 18.8.0
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: 4.0.10
     gitpython: 3.1.43
        Jinja2: 3.1.4
       libgit2: 1.7.2
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 23.1
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: 1.14.1
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: Not Installed
         smmap: 5.0.1
       timelib: Not Installed
       Tornado: 6.3.3
           ZMQ: 4.3.4

Salt Package Information:
  Package Type: Not Installed

System Versions:
          dist:
        locale: utf-8
       machine: AMD64
       release: 10
        system: Windows
       version: 10 10.0.19045 SP0 Multiprocessor Free

Additional context

This is how salt.utils.url.create is currently implemented:

def create(path, saltenv=None):
    """
    join `path` and `saltenv` into a 'salt://' URL.
    """
    path = path.replace("\\", "/")
    if salt.utils.platform.is_windows():
        path = salt.utils.path.sanitize_win_path(path)
    path = salt.utils.data.decode(path)

    query = f"saltenv={saltenv}" if saltenv else ""
    url = salt.utils.data.decode(urlunparse(("file", "", path, "", query, "")))
    return "salt://{}".format(url[len("file:///") :])

urlunparse() does not consistently produce the expected file:/// prefix under some versions of python:

PS D:\salt\> py -3.12
Python 3.12.7 (tags/v3.12.7:0b05ead, Oct  1 2024, 03:06:41) [MSC v.1941 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from urllib.parse import urlunparse
>>> urlunparse(("file", "", "path/to/thing", "", "q=v", ""))
'file:path/to/thing?q=v'
PS D:\salt\> py -3.11
Python 3.11.9 (tags/v3.11.9:de54cf5, Apr  2 2024, 10:12:12) [MSC v.1938 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from urllib.parse import urlunparse
>>> urlunparse(("file", "", "path/to/thing", "", "q=v", ""))
'file:///path/to/thing?q=v'
PS D:\salt\> py -3.10
Python 3.10.11 (tags/v3.10.11:7d4cc5a, Apr  5 2023, 00:38:17) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from urllib.parse import urlunparse
>>>  urlunparse(("file", "", "path/to/thing", "", "q=v", ""))
  File "<stdin>", line 1
    urlunparse(("file", "", "path/to/thing", "", "q=v", ""))
IndentationError: unexpected indent
>>> urlunparse(("file", "", "path/to/thing", "", "q=v", ""))
'file:///path/to/thing?q=v'

Which ends up producing broken URIs like this:

>>> import salt.utils.url
>>> salt.utils.url.create("top.sls", saltenv="test")
'salt://.sls?saltenv=test'

I have monkeypatched my local (embedded) salt to workaround this. I've not been able to submit a PR as pre-commits hooks are not working in my local environments, and I'm not willing to spend much more hours digging into this specific problem.

def _create_url(path: str, saltenv: str | None = None) -> str:
    path = path.replace("\\", "/")
    if salt.utils.platform.is_windows():
        path = salt.utils.path.sanitize_win_path(path)  # pyright: ignore[reportUnknownMemberType]
    path = salt.utils.data.decode(path)  # pyright: ignore[reportAssignmentType,reportUnknownMemberType]
    query = f"saltenv={salt.utils.data.decode(saltenv)}" if saltenv else ""  # pyright: ignore[reportUnknownMemberType]
    return f"salt://{urlunsplit(("", "", path, query, ""))}"


salt.utils.url.create = _create_url
import salt.state  # pyright: ignore[reportMissingTypeStubs]  # noqa: E402
@yangskyboxlabs yangskyboxlabs added Bug broken, incorrect, or confusing behavior needs-triage labels Oct 8, 2024
@lkubb
Copy link
Contributor

lkubb commented Oct 8, 2024

This is a duplicate of #66898

@yangskyboxlabs
Copy link
Author

Closing as duplicate of #66898

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage
Projects
None yet
Development

No branches or pull requests

2 participants