[BUG] winpkg.latest broken in 3006.3: AttributeError: 'NoneType' object has no attribute 'split'

[darkpixel]

Description
We have been using winrepo-ng for nearly a year. A recent upgrade of minions from 3006.2 to 3006.3 caused our winpkg definitions to stop working.

Calling pkg.latest for a winrepo-ng package results in an error.

Setup
Create a package definition for Chrome:

{% set versions = [
    '116.0.5845.181',
    '116.0.5845.141',
    '116.0.5845.111',
    '116.0.5845.97',
] %}


chrome:
  {% for version in versions %}
  '{{ version }}':
    full_name: 'Google Chrome'
    {%- if grains['cpuarch'] == 'AMD64' %}
    installer: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    uninstaller: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    {%- else %}
    installer: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise.msi'
    uninstaller: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise.msi'
    {%- endif %}
    install_flags: '/qn /norestart'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False
  {% endfor %}

This appears to only work if Chrome needs an update, so check here https://versionhistory.googleapis.com/v1/chrome/platforms/win/channels/stable/versions to see if a newer version is available. I don't believe this error occurs if Chrome isn't installed, but I need to verify that.

Create a state:

chrome:
  pkg.latest: []

Call the state: salt 'minion_id' state.sls chrome.sls

Error output:

----------
          ID: chrome
    Function: pkg.latest
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "C:\Program Files\Salt Project\Salt\lib\site-packages\salt\state.py", line 2381, in call
                  ret = self.states[cdata["full"]](
                File "C:\Program Files\Salt Project\Salt\lib\site-packages\salt\loader\lazy.py", line 159, in __call__
                  ret = self.loader.run(run_func, *args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\lib\site-packages\salt\loader\lazy.py", line 1245, in run
                  return self._last_context.run(self._run_as, _func_or_method, *args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\lib\site-packages\salt\loader\lazy.py", line 1260, in _run_as
                  return _func_or_method(*args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\lib\site-packages\salt\loader\lazy.py", line 1293, in wrapper
                  return f(*args, **kwargs)
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\states\pkg.py", line 2788, in latest
                  failed = [
                File "C:\Program Files\Salt Project\Salt\Lib\site-packages\salt\states\pkg.py", line 2792, in <listcomp>
                  or targets[x] not in changes[x].get("new").split(",")
              AttributeError: 'NoneType' object has no attribute 'split'
     Started: 07:51:53.832412
    Duration: 190710.546 ms
     Changes:   

Minion version:

    Salt Version:
              Salt: 3006.3
     
    Python Version:
            Python: 3.10.13 (heads/main:7ee24e6, Sep  6 2023, 02:19:49) [MSC v.1936 64 bit (AMD64)]
     
    Dependency Versions:
              cffi: 1.14.6
          cherrypy: 18.6.1
          dateutil: 2.8.1
         docker-py: Not Installed
             gitdb: 4.0.7
         gitpython: Not Installed
            Jinja2: 3.1.2
           libgit2: Not Installed
      looseversion: 1.0.2
          M2Crypto: Not Installed
              Mako: Not Installed
           msgpack: 1.0.2
      msgpack-pure: Not Installed
      mysql-python: Not Installed
         packaging: 22.0
         pycparser: 2.21
          pycrypto: Not Installed
      pycryptodome: 3.10.1
            pygit2: Not Installed
      python-gnupg: 0.4.8
            PyYAML: 6.0.1
             PyZMQ: 25.0.2
            relenv: 0.13.10
             smmap: 4.0.0
           timelib: 0.2.4
           Tornado: 4.5.3
               ZMQ: 4.3.4
     
    System Versions:
              dist:   
            locale: cp1252
           machine: AMD64
           release: 10
            system: Windows
           version: 10 10.0.22000 SP0 Multiprocessor Free

[darkpixel]

Actually, this looks like it's a bug in the pkg state, not anything to do with winpkg.

https://github.com/saltstack/salt/blob/8505972418f071f6dddbb41bdee6f2980d7fcf37/salt/states/pkg.py#L2792C21-L2792C21

[damon-atkins]

From memory, When using latest, package state ask winpkg what is installed, ask winpkg what is the latest version, then determines if a newer version is greater than existing version on the system, if so then calls winpkg to install it. Their is some special coding around 'latest' in winpkg but that's for package definitions which use latest instead of a real version number.

[damon-atkins]

@darkpixel I suggest you look at the example of https://github.com/saltstack/salt-winrepo-ng/blob/master/chrome.sls version numbers should really only be used when you can download a specific version. e.g. https://github.com/saltstack/salt-winrepo-ng/blob/master/putty.sls We use latest in the example because Chrome self updates.

[darkpixel]

@damon-atkins I guess I'm just not being clear with my explanation.

Go ahead and create a Chrome package like you see in https://github.com/saltstack/salt-winrepo-ng/blob/master/chrome.sls.

Now go into HKLM\SOFTWARE\Policies\Google\Update and create/set the UpdateDefault REG_DWORD key to 0 to prevent Chrome from automatically updating.

Wait for a new release of Chrome.

Now, use Salt to ensure that everyone is running the latest version of Chrome because...say...a bug was discovered that allowed users to compromise a system through malformed .webp file.

Salt won't update it to the latest version of Chrome....because winpkg.latest looks at the latest stanza in the pkg-ng file and HAS NO IDEA what the real-world latest version of Chrome is. As of this moment it appears to be 117.0.5938.89. So if a box is running an older version of Chrome (say 116.0.5845.188) winpkg.latest looks at the cached installer file to see if it exists. If it doesn't exist in the cache, it downloads and installs it (thereby updating Chrome). If the installer already exists in the cache, it appears to check it against the download source....if the installer is different it updates it...but if the installer hasn't been updated, nothing happens.

Applications that self-update don't work properly with winpkg.managed if the installer never changes.

Salt doesn't appear to hand the case where you have an existing and older version of the program installed, but the installer is already in the cache and hasn't changed and you're using the latest version stanza in winpkg-ng.

[damon-atkins]

Hi @darkpixel if you have a vendor which does not support download a specific version like chrome. Then you need to some how download the version you want. And host on the salt master, s3, http etc your self, and put a version number in the binary.

Something like the following will allow you to chose a specific version you have store on the salt master or latest version. (note I have just cut and pasted this together as an example). Keep in mind latest is design for self update apps. So if you disable self updating, doing version=latest will only work once. i.e. first install. If you want latest to be calculated then leave the latest version tag out.

chrome:
{% for version in ['117.0.5938.89','117.1.4123.11'] %}
  '{{ version }}':
    full_name: 'Google Chrome'
    installer: 'salt:/windows-software/GoogleChromeStandaloneEnterprise64-{{ version }}.msi'
    uninstaller: 'salt:/windows-software/GoogleChromeStandaloneEnterprise64-{{ version }}.msi'
    install_flags: '/qn /norestart'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False
{% endfor %}
  latest:
    full_name: 'Google Chrome'
    installer: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    uninstaller: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    install_flags: '/qn /norestart'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False

If you want to be able to control which version of software you need to have a installer: abc-{{version}}.msi pattern. There is little value in listing lots of versions and pointing them to the same download.

If salt does not download the installer unless its needed, i.e. it will not download the install every time a high-state happens and look inside the msi or exe and try and determine its version. If you think of a high state running on 100 plus servers, all downloading the msi at the same time. Winpkg pulls the version number of the existing software from the registry, and the only other version number it looks at is the version string in the package definition to determine the action taken. version=latest is calculated if the version latest tag is not used, other wise for a specific version use version=120.1.1.1

[darkpixel]

I think that's a bad solution.
I think winpkg should be modified.
You should be able to include version numbers so salt knows when a program is out-of-date, and when it is out-of-date it can download the installer and run it.

I shouldn't have to pre-download the installer, rename it to have a version number in it to get past caching, and then serve it all from my salt master instead of Google's (in this case) CDN.

[damon-atkins]

Hi @darkpixel If you look at winget every version has a separate download path, I believe choc is the same.

The idea of install version=1.1.1.1 is that it downloads that version only and then installs it knowning its that version. Any software download without a version string or a hash etc in its name makes it impossible to download the version of the software the user wants to install. Firefox offers all versions available for individual download for the last 5 years https://github.com/saltstack/salt-winrepo-ng/blob/master/firefox-esr.sls .

[damon-atkins]

See https://github.com/microsoft/winget-pkgs/tree/master/manifests/g/Google/Chrome The community updates this every time the version changes. i.e. delete old version and creates the new version to match.

[darkpixel]

I get it, but I don't think I'm being clear.
I should be able to pkg.install chrome version=latest and Salt should know what the latest version is via the pkg-ng files.

If a workstation isn't running the latest version, it should fire off the installer. I know I can't pkg.install chrome version=86 because the installer always installs the latest version.

In other words, winpkg should have functionality added to specifically handle out-of-date applications that just need the latest version installed.

I can handle this very inefficiently with a cmd.run:

ensure_chrome_up_to_date:
  cmd.run:
    - name: 'msiexec /i https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi /qb'

The problem is that Salt basically conflates pkg.version=latest, caching installers with identical names, and not knowing what the version latest should resolve to.

[damon-atkins]

Instead of

ensure_chrome_up_to_date:
  cmd.run:
    - name: 'msiexec /i https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi /qb'

Then do pkg definition of

chrome:
  '9999999.0':
    full_name: 'Google Chrome'
    installer: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    uninstaller: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    install_flags: '/qn /norestart'
    uninstall_flags: '/qn /norestart'
    msiexec: True
    locale: en_US
    reboot: False

Then version=latest It will always execute the install because the version installed will always be less than '99999999.0'.

[darkpixel]

I get it. I appreciate that you're trying to help...but I don't want to always install Chrome on every highstate. I want it to run the installer when Chrome is actually out-of-date.

Part of the whole idea behind Salt is that you can ensure systems are brought into compliance based on states and pillar data.

When Chrome releases an update, I want to ensure the installer is run if the version number is lower than what's specified in the winrepo package definition...just like you would with any other program that's versioned properly.

(with the obvious understanding that Chrome doesn't let you roll back to a previous version...making that impossible)

Either Google needs to start versioning their installer or Salt needs to fix winpkg...and I'm going to bet that Google isn't going to budge on their installer.

I think the easiest option would be to add a flag to the package definition. Maybe something like ignore_cache: True that causes Salt to re-download the installer and re-run the install.

[damon-atkins]

@darkpixel
If you try to install a version of an msi that's already installed, I assume it will abort.

winpkg like other packages manager take action base on required versions, those version are held in the package definition.

I believe Google polices is to keep moving forward, i.e. the next release will fix security issues, and if you go to an older release you most likely will have vulnerabilities as well, which have been known for longer.
The following is untested example:

/tmp/GoogleChromeStandaloneEnterprise64.msi:
    file.managed:
        - source: https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi
        - use_etag: true
ensure_chrome_up_to_date:
  cmd.run:
    - name: 'msiexec /i /tmp/GoogleChromeStandaloneEnterprise64.msi /qb'
    - onchanges_in:
      - file: '/tmp/GoogleChromeStandaloneEnterprise64.msi'

Might be able to use file.cache instead of file.managed?

In another comment you said "You should be able to include version numbers to salt knows when a program is out-of-date, and when it is out-of-date it can download the installer and run it." What version number would you use, were would it come from? How would you known the version number of https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi without downloading it every time.

[darkpixel]

In another comment you said "You should be able to include version numbers to salt knows when a program is out-of-date, and when it is out-of-date it can download the installer and run it."

Something like this untested pseudo-code code should give you an authoritative set of versions for Chrome:

{% for chrome_versions = __salt__['http.query'] 'https://versionhistory.googleapis.com/v1/chrome/platforms/win/channels/stable/versions')['versions']|from_json %}
chrome:
  '{{ chrome_versions['version'] }}':
    full_name: 'Google Chrome'
    installer: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    uninstaller: 'https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi'
    install_flags: '/qn /norestart'
    uninstall_flags: '/qn /norestart'
    msiexec: True
{% endfor %}

winpkg like other packages manager take action base on required versions, those version are held in the package definition.

So...when you have a state that says chrome: pkg.latest: [] winpkg should look at the package definition to find out what the highest version number is, and download/launch the latest installer. pkg-ng shouldn't just assume every package in the world has a version number as part of the filename and that an installer never changes and therefore cache the installer file once and forever.

Fortunately in the case of Windows Installer files, msiexec really doesn't care that the file ends in .msi, so you can easily tell winpkg to download https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi?break_cache={{ version }} and msiexec will still install it.

This doesn't work so well for executable files as far as I can tell. There doesn't seem to be a way to tell Windows to launch an executable file that is named Setup.exe?break_cache=1.2.3.

How would you known the version number of https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi without downloading it every time.

As mentioned above, you don't look at the installer URL and cache to figure it out. You already have version numbers defined in the pkgng definition and you compare that to the version number reported by Add/Remove Programs in Windows. that tells you to download the installer and run it. Honestly, I think the pkgng software definitions need to have a flag ignore_cache: True or whatever. Ignore whatever file you think you have downloaded, re-download it, and run the setup process when there is a newer version reported in pkgng as compared to Windows Add/Remove Programs.

[darkpixel]

Also, the more I think about it...I'm not sure why Salt even needs to cache Windows Installer files.

You can quite literally run:

msiexec /i https://dl.google.com/edgedl/chrome/install/GoogleChromeStandaloneEnterprise64.msi /qb

msiexec will take care of downloading and running the installer on its own.
Obviously this doesn't work for executables, but it works fine with MSI files.