search

saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.19k stars 5.48k forks source link

The source_hash parameter for file.managed doesn't support files of arbitrary (utf16) encoding #51354

Open arizvisa opened 5 years ago

arizvisa commented 5 years ago

So, Lol. I wouldn't have thought this as being too common for it to be too big of a deal, but it turns out that the hash file for y'alls windows installer (https://repo.saltstack.com/windows/Salt-Minion-2018.3.3-Py2-x86-Setup.exe.md5) is a utf16 encoded md5. Due to it being utf16, the file.managed fails.

Here's a statefile:

Download Salt-Minion installer for windows:
    file.managed:
        - source: https://repo.saltstack.com/windows/Salt-Minion-2018.3.3-Py2-x86-Setup.exe
        - name: /path/to/wherever
        - source_hash: https://repo.saltstack.com/windows/Salt-Minion-2018.3.3-Py2-x86-Setup.exe.md5
        - mode: 0664

This results in:

vmware-iso: ----------
vmware-iso:           ID: Download Salt-Minion installer for windows
vmware-iso:     Function: file.managed
vmware-iso:         Name: /path/to/wherever
vmware-iso:       Result: False
vmware-iso:      Comment: Unable to manage file: 'ascii' codec can't decode byte 0xff in position 0: ordinal not in range(128)
vmware-iso:      Started: 10:52:51.086256
vmware-iso:     Duration: 326.789 ms
vmware-iso:      Changes:
vmware-iso:

This is in the 2019.2 branch. If you look in salt.modules.file, you can see that get_managed() will call salt.utils.hashutils.get_hash() in order to process the source_hash file. The salt.utils.hashutils.get_hash() function, which doesn't take any wildarg or kwarg parameters, then calls salt.utils.files.fopen() to open the file. Although salt.utils.files.fopen() function can take an encoding= parameter, the get_hash() function doesn't even bother since it only has three positional parameters and none of them are an encoding.

Ch3LL commented 5 years ago

looks like i'm able to replicate this on 2018.3.3 as well. We will need to get this fixed thanks.

arizvisa commented 5 years ago

Any word on this? If I take the time to write up a PR will you guys merge it? I have a number of PRs waiting since like the beginning of the year which is why I ask..

arizvisa commented 5 years ago

(just for my record, it seems like #52944 fixes a similar issue in the same module)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

arizvisa commented 4 years ago

Was this fixed at all? Or just abandoned?

stale[bot] commented 4 years ago

Thank you for updating this issue. It is no longer marked as stale.