Merge 3006.x into master

.github/actions/setup-python-tools-scripts/action.yml

@@ -33,7 +33,12 @@ runs:
       shell: bash
       working-directory: ${{ inputs.cwd }}
       run: |
+        (python3 -m pip install --help | grep break-system-packages > /dev/null 2>&1) && exitcode=0 || exitcode=1
+        if [ $exitcode -eq 0 ]; then
+          python3 -m pip install --break-system-packages -r requirements/static/ci/py${{ steps.get-python-version.outputs.version }}/tools.txt
+        else
           python3 -m pip install -r requirements/static/ci/py${{ steps.get-python-version.outputs.version }}/tools.txt
+        fi
 
     - name: Get 'python-tools-scripts' Version
       id: get-version

.github/workflows/build-deb-packages.yml

@@ -35,7 +35,7 @@ jobs:
           - src
 
     container:
-      image: ghcr.io/saltstack/salt-ci-containers/packaging:debian-11
+      image: ghcr.io/saltstack/salt-ci-containers/packaging:debian-12
 
     steps:
       # Checkout here so we can easily use custom actions

changelog/64519.fixed.md

@@ -0,0 +1,3 @@
+`win_pkg` Fixes an issue runing `pkg.install` with `version=latest` where the
+new installer would not be cached if there was already an installer present
+with the same name.

changelog/64595.security.md

@@ -0,0 +1,3 @@
+Upgrade to `cryptography==41.0.1`(and therefor `pyopenssl==23.2.0` due to https://github.com/advisories/GHSA-5cpq-8wj7-hf2v
+
+This only really impacts pip installs of Salt and the windows onedir since the linux and macos onedir build every package dependency from source, not from pre-existing wheels.

requirements/darwin.txt

@@ -6,6 +6,7 @@ apache-libcloud>=2.4.0
 backports.ssl_match_hostname>=3.7.0.1; python_version < '3.7'
 cherrypy>=17.4.1
 gitpython>=3.1.30
+cryptography>=41.0.1
 idna>=2.8
 linode-python>=1.1.1
 pyasn1>=0.4.8

requirements/static/ci/py3.10/cloud.txt

@@ -20,7 +20,7 @@ charset-normalizer==2.1.1
     # via
     #   -c requirements/static/ci/py3.10/linux.txt
     #   requests
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/py3.10/linux.txt
     #   pyspnego

requirements/static/ci/py3.10/darwin.txt

@@ -93,10 +93,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.10/darwin.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   etcd3-py
     #   moto
     #   paramiko

requirements/static/ci/py3.10/linux.txt

@@ -103,10 +103,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.10/linux.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   ansible-core
     #   etcd3-py
     #   moto

requirements/static/ci/py3.10/windows.txt

@@ -87,10 +87,11 @@ contextvars==2.4
     # via
     #   -c requirements/static/ci/../pkg/py3.10/windows.txt
     #   -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.10/windows.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   etcd3-py
     #   moto
     #   pyopenssl

requirements/static/ci/py3.11/cloud.txt

@@ -20,7 +20,7 @@ charset-normalizer==2.1.1
     # via
     #   -c requirements/static/ci/py3.11/linux.txt
     #   requests
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/py3.11/linux.txt
     #   pyspnego

requirements/static/ci/py3.11/darwin.txt

@@ -93,10 +93,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.11/darwin.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   etcd3-py
     #   moto
     #   paramiko

requirements/static/ci/py3.11/linux.txt

@@ -103,10 +103,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.11/linux.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   ansible-core
     #   etcd3-py
     #   moto

requirements/static/ci/py3.11/windows.txt

@@ -87,10 +87,11 @@ contextvars==2.4
     # via
     #   -c requirements/static/ci/../pkg/py3.11/windows.txt
     #   -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.11/windows.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   etcd3-py
     #   moto
     #   pyopenssl

requirements/static/ci/py3.8/cloud.txt

@@ -20,7 +20,7 @@ charset-normalizer==2.1.1
     # via
     #   -c requirements/static/ci/py3.8/linux.txt
     #   requests
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/py3.8/linux.txt
     #   pyspnego

requirements/static/ci/py3.8/linux.txt

@@ -103,10 +103,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.8/linux.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   ansible-core
     #   etcd3-py
     #   moto

requirements/static/ci/py3.8/windows.txt

@@ -87,10 +87,11 @@ contextvars==2.4
     # via
     #   -c requirements/static/ci/../pkg/py3.8/windows.txt
     #   -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.8/windows.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   etcd3-py
     #   moto
     #   pyopenssl

requirements/static/ci/py3.9/cloud.txt

@@ -20,7 +20,7 @@ charset-normalizer==2.1.1
     # via
     #   -c requirements/static/ci/py3.9/linux.txt
     #   requests
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/py3.9/linux.txt
     #   pyspnego

requirements/static/ci/py3.9/darwin.txt

@@ -93,10 +93,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.9/darwin.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   etcd3-py
     #   moto
     #   paramiko

requirements/static/ci/py3.9/linux.txt

@@ -103,10 +103,11 @@ contextvars==2.4
     #   -r requirements/base.txt
 croniter==1.3.15 ; sys_platform != "win32"
     # via -r requirements/static/ci/common.in
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.9/linux.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   ansible-core
     #   etcd3-py
     #   moto

requirements/static/ci/py3.9/windows.txt

@@ -87,10 +87,11 @@ contextvars==2.4
     # via
     #   -c requirements/static/ci/../pkg/py3.9/windows.txt
     #   -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -c requirements/static/ci/../pkg/py3.9/windows.txt
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   etcd3-py
     #   moto
     #   pyopenssl

requirements/static/pkg/linux.in

@@ -9,3 +9,4 @@ rpm-vercmp
 setproctitle>=1.2.3
 timelib>=0.2.5
 importlib-metadata>=3.3.0
+cryptography>=41.0.1

requirements/static/pkg/py3.10/darwin.txt

@@ -20,9 +20,10 @@ cherrypy==18.8.0
     # via -r requirements/darwin.txt
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.10/linux.txt

@@ -18,9 +18,10 @@ cherrypy==18.8.0
     # via -r requirements/static/pkg/linux.in
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.10/windows.txt

@@ -25,9 +25,10 @@ clr-loader==0.2.4
     # via pythonnet
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.11/darwin.txt

@@ -20,9 +20,10 @@ cherrypy==18.8.0
     # via -r requirements/darwin.txt
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.11/linux.txt

@@ -18,9 +18,10 @@ cherrypy==18.8.0
     # via -r requirements/static/pkg/linux.in
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.11/windows.txt

@@ -25,9 +25,10 @@ clr-loader==0.2.4
     # via pythonnet
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.8/linux.txt

@@ -18,9 +18,10 @@ cherrypy==18.8.0
     # via -r requirements/static/pkg/linux.in
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.8/windows.txt

@@ -25,9 +25,10 @@ clr-loader==0.2.4
     # via pythonnet
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.9/darwin.txt

@@ -20,9 +20,10 @@ cherrypy==18.8.0
     # via -r requirements/darwin.txt
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/darwin.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.9/linux.txt

@@ -18,9 +18,10 @@ cherrypy==18.8.0
     # via -r requirements/static/pkg/linux.in
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/static/pkg/linux.in
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/static/pkg/py3.9/windows.txt

@@ -25,9 +25,10 @@ clr-loader==0.2.4
     # via pythonnet
 contextvars==2.4
     # via -r requirements/base.txt
-cryptography==40.0.2
+cryptography==41.0.1
     # via
     #   -r requirements/crypto.txt
+    #   -r requirements/windows.txt
     #   pyopenssl
 distro==1.8.0
     # via -r requirements/base.txt

requirements/windows.txt

@@ -11,6 +11,7 @@ certifi>=2022.12.07
 cffi>=1.14.5
 cherrypy>=18.6.1
 gitpython>=3.1.30
+cryptography>=41.0.1
 lxml>=4.6.3
 pyasn1>=0.4.8
 pymssql>=2.2.1

salt/modules/win_pkg.py

@@ -215,7 +215,7 @@ def upgrade_available(name, **kwargs):
     refresh = salt.utils.data.is_true(kwargs.get("refresh", True))
 
     # if latest_version returns blank, the latest version is already installed or
-    # their is no package definition. This is a salt standard which could be improved.
+    # there is no package definition. This is a salt standard which could be improved.
     return latest_version(name, saltenv=saltenv, refresh=refresh) != ""
 
 
@@ -1315,7 +1315,7 @@ def _get_source_sum(source_hash, file_path, saltenv, verify_ssl=True):
         # The source_hash is a file on a server
         try:
             cached_hash_file = __salt__["cp.cache_file"](
-                source_hash, saltenv=saltenv, verify_ssl=verify_ssl
+                source_hash, saltenv=saltenv, verify_ssl=verify_ssl, use_etag=True
             )
         except MinionError as exc:
             log.exception("Failed to cache %s", source_hash, exc_info=exc)
@@ -1640,6 +1640,13 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
             ret[pkg_name] = {"no installer": version_num}
             continue
 
+        # Hash the installer source after verifying it was defined
+        installer_hash = __salt__["cp.hash_file"](installer, saltenv)
+        if isinstance(installer_hash, dict):
+            installer_hash = installer_hash["hsum"]
+        else:
+            installer_hash = None
+
         # Is the installer in a location that requires caching
         if __salt__["config.valid_fileproto"](installer):
 
@@ -1649,6 +1656,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
             # single files
             if cache_dir and installer.startswith("salt:"):
                 path, _ = os.path.split(installer)
+                log.debug(f"PKG: Caching directory: {path}")
                 try:
                     __salt__["cp.cache_dir"](
                         path=path,
@@ -1664,29 +1672,13 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
 
             # Check to see if the cache_file is cached... if passed
             if cache_file and cache_file.startswith("salt:"):
-
-                # Check to see if the file is cached
-                cached_file = __salt__["cp.is_cached"](cache_file, saltenv)
-                if not cached_file:
-                    try:
-                        cached_file = __salt__["cp.cache_file"](
-                            cache_file,
-                            saltenv=saltenv,
-                            verify_ssl=kwargs.get("verify_ssl", True),
-                        )
-                    except MinionError as exc:
-                        msg = "Failed to cache {}".format(cache_file)
-                        log.exception(msg, exc_info=exc)
-                        return "{}\n{}".format(msg, exc)
-
-                # Make sure the cached file is the same as the source
-                if __salt__["cp.hash_file"](cache_file, saltenv) != __salt__[
-                    "cp.hash_file"
-                ](cached_file):
+                cache_file_hash = __salt__["cp.hash_file"](cache_file, saltenv)
+                log.debug(f"PKG: Caching file: {cache_file}")
                 try:
                     cached_file = __salt__["cp.cache_file"](
                         cache_file,
                         saltenv=saltenv,
+                        source_hash=cache_file_hash,
                         verify_ssl=kwargs.get("verify_ssl", True),
                     )
                 except MinionError as exc:
@@ -1700,15 +1692,25 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
                     ret[pkg_name] = {"failed to cache cache_file": cache_file}
                     continue
 
-            # Check to see if the installer is cached
+            # If version is "latest" we always cache because "cp.is_cached" only
+            # checks that the file exists, not that is has changed
+            cached_pkg = False
+            if version_num != "latest" and not installer.startswith("salt:"):
                 cached_pkg = __salt__["cp.is_cached"](installer, saltenv)
+
             if not cached_pkg:
-                # It's not cached. Cache it, mate.
+                # Since we're passing "installer_hash", it should only cache the
+                # file if the source_hash doesn't match, which only works on
+                # files hosted on "salt://". If the http/https url supports
+                # etag, it should also verify that information before caching
+                log.debug(f"PKG: Caching file: {installer}")
                 try:
                     cached_pkg = __salt__["cp.cache_file"](
                         installer,
                         saltenv=saltenv,
+                        source_hash=installer_hash,
                         verify_ssl=kwargs.get("verify_ssl", True),
+                        use_etag=True,
                     )
                 except MinionError as exc:
                     msg = "Failed to cache {}".format(installer)
@@ -1722,29 +1724,6 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
                     )
                     ret[pkg_name] = {"unable to cache": installer}
                     continue
-
-            # Compare the hash of the cached installer to the source only if the
-            # file is hosted on salt:
-            if installer.startswith("salt:"):
-                if __salt__["cp.hash_file"](installer, saltenv) != __salt__[
-                    "cp.hash_file"
-                ](cached_pkg):
-                    try:
-                        cached_pkg = __salt__["cp.cache_file"](
-                            installer,
-                            saltenv=saltenv,
-                            verify_ssl=kwargs.get("verify_ssl", True),
-                        )
-                    except MinionError as exc:
-                        msg = "Failed to cache {}".format(installer)
-                        log.exception(msg, exc_info=exc)
-                        return "{}\n{}".format(msg, exc)
-
-                    # Check if the installer was cached successfully
-                    if not cached_pkg:
-                        log.error("Unable to cache %s", installer)
-                        ret[pkg_name] = {"unable to cache": installer}
-                        continue
         else:
             # Run the installer directly (not hosted on salt:, https:, etc.)
             cached_pkg = installer
@@ -1786,7 +1765,6 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
             log.debug("pkg.install: Source hash matches package hash.")
 
         # Get install flags
-
         install_flags = pkginfo[version_num].get("install_flags", "")
         if options and options.get("extra_install_flags"):
             install_flags = "{} {}".format(
@@ -2063,7 +2041,7 @@ def remove(name=None, pkgs=None, **kwargs):
                     removal_targets.append(ver_install)
         else:
             if version_num in pkginfo:
-                # we known how to remove this version
+                # we know how to remove this version
                 if version_num in old[pkgname]:
                     removal_targets.append(version_num)
                 else:
@@ -2107,8 +2085,15 @@ def remove(name=None, pkgs=None, **kwargs):
                 ret[pkgname] = {"no uninstaller defined": target}
                 continue
 
-            # Where is the uninstaller
-            if uninstaller.startswith(("salt:", "http:", "https:", "ftp:")):
+            # Hash the uninstaller source after verifying it was defined
+            uninstaller_hash = __salt__["cp.hash_file"](uninstaller, saltenv)
+            if isinstance(uninstaller_hash, dict):
+                uninstaller_hash = uninstaller_hash["hsum"]
+            else:
+                uninstaller_hash = None
+
+            # Is the uninstaller in a location that requires caching
+            if __salt__["config.valid_fileproto"](uninstaller):
 
                 # Check for the 'cache_dir' parameter in the .sls file
                 # If true, the entire directory will be cached instead of the
@@ -2117,24 +2102,38 @@ def remove(name=None, pkgs=None, **kwargs):
 
                 if cache_dir and uninstaller.startswith("salt:"):
                     path, _ = os.path.split(uninstaller)
+                    log.debug(f"PKG: Caching dir: {path}")
                     try:
                         __salt__["cp.cache_dir"](
-                            path, saltenv, False, None, "E@init.sls$"
+                            path=path,
+                            saltenv=saltenv,
+                            include_empty=False,
+                            include_pat=None,
+                            exclude_pat="E@init.sls$",
                         )
                     except MinionError as exc:
                         msg = "Failed to cache {}".format(path)
                         log.exception(msg, exc_info=exc)
                         return "{}\n{}".format(msg, exc)
 
-                # Check to see if the uninstaller is cached
+                # Check to see if the uninstaller is cached. We don't want to
+                # check for latest here like we do for "pkg.install" because we
+                # only want to uninstall the version that has been installed
                 cached_pkg = __salt__["cp.is_cached"](uninstaller, saltenv)
                 if not cached_pkg:
-                    # It's not cached. Cache it, mate.
+                    # Since we're passing "uninstaller_hash", it should only
+                    # cache the file if the source_hash doesn't match, which
+                    # only works on files hosted on "salt://". If the http/https
+                    # url supports etag, it should also verify that information
+                    # before caching
+                    log.debug(f"PKG: Caching file: {uninstaller}")
                     try:
                         cached_pkg = __salt__["cp.cache_file"](
                             uninstaller,
                             saltenv=saltenv,
+                            source_hash=uninstaller_hash,
                             verify_ssl=kwargs.get("verify_ssl", True),
+                            use_etag=True,
                         )
                     except MinionError as exc:
                         msg = "Failed to cache {}".format(uninstaller)
@@ -2147,32 +2146,8 @@ def remove(name=None, pkgs=None, **kwargs):
                         ret[pkgname] = {"unable to cache": uninstaller}
                         continue
 
-                # Compare the hash of the cached installer to the source only if
-                # the file is hosted on salt:
-                # TODO cp.cache_file does cache and hash checking? So why do it again?
-                if uninstaller.startswith("salt:"):
-                    if __salt__["cp.hash_file"](uninstaller, saltenv) != __salt__[
-                        "cp.hash_file"
-                    ](cached_pkg):
-                        try:
-                            cached_pkg = __salt__["cp.cache_file"](
-                                uninstaller,
-                                saltenv=saltenv,
-                                verify_ssl=kwargs.get("verify_ssl", True),
-                            )
-                        except MinionError as exc:
-                            msg = "Failed to cache {}".format(uninstaller)
-                            log.exception(msg, exc_info=exc)
-                            return "{}\n{}".format(msg, exc)
-
-                        # Check if the installer was cached successfully
-                        if not cached_pkg:
-                            log.error("Unable to cache %s", uninstaller)
-                            ret[pkgname] = {"unable to cache": uninstaller}
-                            continue
             else:
-                # Run the uninstaller directly
-                # (not hosted on salt:, https:, etc.)
+                # Run the uninstaller directly (not hosted on salt:, https:, etc.)
                 cached_pkg = os.path.expandvars(uninstaller)
 
             # Fix non-windows slashes

tests/pytests/unit/modules/test_win_pkg.py

@@ -6,6 +6,7 @@ import logging
 import pytest
 
 import salt.modules.config as config
+import salt.modules.cp as cp
 import salt.modules.pkg_resource as pkg_resource
 import salt.modules.win_pkg as win_pkg
 import salt.utils.data
@@ -21,8 +22,17 @@ pytestmark = [
 
 
 @pytest.fixture
-def configure_loader_modules():
+def configure_loader_modules(minion_opts):
     pkg_info = {
+        "latest": {
+            "full_name": "Nullsoft Install System",
+            "installer": "http://download.sourceforge.net/project/nsis/nsis-setup.exe",
+            "install_flags": "/S",
+            "uninstaller": "%PROGRAMFILES(x86)%\\NSIS\\uninst-nsis.exe",
+            "uninstall_flags": "/S",
+            "msiexec": False,
+            "reboot": False,
+        },
         "3.03": {
             "full_name": "Nullsoft Install System",
             "installer": "http://download.sourceforge.net/project/nsis/NSIS%203/3.03/nsis-3.03-setup.exe",
@@ -43,16 +53,20 @@ def configure_loader_modules():
         },
     }
 
+    opts = minion_opts
+    opts["master_uri"] = "localhost"
     return {
+        cp: {"__opts__": opts},
         win_pkg: {
             "_get_latest_package_version": MagicMock(return_value="3.03"),
             "_get_package_info": MagicMock(return_value=pkg_info),
             "__salt__": {
+                "config.valid_fileproto": config.valid_fileproto,
+                "cp.hash_file": cp.hash_file,
                 "pkg_resource.add_pkg": pkg_resource.add_pkg,
                 "pkg_resource.parse_targets": pkg_resource.parse_targets,
                 "pkg_resource.sort_pkglist": pkg_resource.sort_pkglist,
                 "pkg_resource.stringify": pkg_resource.stringify,
-                "config.valid_fileproto": config.valid_fileproto,
             },
             "__utils__": {
                 "reg.key_exists": win_reg.key_exists,
@@ -164,19 +178,81 @@ def test_pkg_install_existing():
     se_list_pkgs = {"nsis": ["3.03"]}
     with patch.object(win_pkg, "list_pkgs", return_value=se_list_pkgs), patch.object(
         win_pkg, "_get_reg_software", return_value=ret_reg
-    ), patch.dict(
-        win_pkg.__salt__, {"cp.is_cached": MagicMock(return_value=False)}
     ), patch.dict(
         win_pkg.__salt__,
-        {"cp.cache_file": MagicMock(return_value="C:\\fake\\path.exe")},
-    ), patch.dict(
-        win_pkg.__salt__, {"cmd.run_all": MagicMock(return_value={"retcode": 0})}
+        {
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
+            "cp.cache_file": MagicMock(return_value="C:\\fake\\path.exe"),
+            "cp.is_cached": MagicMock(return_value=True),
+        },
     ):
         expected = {}
         result = win_pkg.install(name="nsis")
         assert expected == result
 
 
+def test_pkg_install_latest():
+    """
+    test pkg.install when the package is already installed
+    no version passed
+    """
+    ret_reg = {"Nullsoft Install System": "3.03"}
+    # The 2nd time it's run, pkg.list_pkgs uses with stringify
+    se_list_pkgs = [{"nsis": ["3.03"]}, {"nsis": "3.04"}]
+    mock_cache_file = MagicMock(return_value="C:\\fake\\path.exe")
+    with patch.object(win_pkg, "list_pkgs", side_effect=se_list_pkgs), patch.object(
+        win_pkg, "_get_reg_software", return_value=ret_reg
+    ), patch.dict(
+        win_pkg.__salt__,
+        {
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
+            "cp.cache_file": mock_cache_file,
+            "cp.is_cached": MagicMock(return_value=False),
+        },
+    ):
+        expected = {"nsis": {"new": "3.04", "old": "3.03"}}
+        result = win_pkg.install(name="nsis", version="latest")
+        assert expected == result
+        mock_cache_file.assert_called_once_with(
+            "http://download.sourceforge.net/project/nsis/nsis-setup.exe",
+            saltenv="base",
+            source_hash=None,
+            verify_ssl=True,
+            use_etag=True,
+        )
+
+
+def test_pkg_install_latest_is_cached():
+    """
+    test pkg.install when the package is already installed
+    no version passed
+    """
+    ret_reg = {"Nullsoft Install System": "3.03"}
+    # The 2nd time it's run, pkg.list_pkgs uses with stringify
+    se_list_pkgs = [{"nsis": ["3.03"]}, {"nsis": "3.04"}]
+    mock_cache_file = MagicMock(return_value="C:\\fake\\path.exe")
+    with patch.object(win_pkg, "list_pkgs", side_effect=se_list_pkgs), patch.object(
+        win_pkg, "_get_reg_software", return_value=ret_reg
+    ), patch.dict(
+        win_pkg.__salt__,
+        {
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
+            "cp.cache_file": mock_cache_file,
+            "cp.is_cached": MagicMock(return_value=True),
+        },
+    ):
+        expected = {"nsis": {"new": "3.04", "old": "3.03"}}
+        result = win_pkg.install(name="nsis", version="latest")
+        assert expected == result
+        mock_cache_file.assert_called_once_with(
+            "http://download.sourceforge.net/project/nsis/nsis-setup.exe",
+            saltenv="base",
+            source_hash=None,
+            verify_ssl=True,
+            use_etag=True,
+        )
+
+
 def test_pkg_install_existing_with_version():
     """
     test pkg.install when the package is already installed
@@ -187,13 +263,13 @@ def test_pkg_install_existing_with_version():
     se_list_pkgs = {"nsis": ["3.03"]}
     with patch.object(win_pkg, "list_pkgs", return_value=se_list_pkgs), patch.object(
         win_pkg, "_get_reg_software", return_value=ret_reg
-    ), patch.dict(
-        win_pkg.__salt__, {"cp.is_cached": MagicMock(return_value=False)}
     ), patch.dict(
         win_pkg.__salt__,
-        {"cp.cache_file": MagicMock(return_value="C:\\fake\\path.exe")},
-    ), patch.dict(
-        win_pkg.__salt__, {"cmd.run_all": MagicMock(return_value={"retcode": 0})}
+        {
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
+            "cp.cache_file": MagicMock(return_value="C:\\fake\\path.exe"),
+            "cp.is_cached": MagicMock(return_value=False),
+        },
     ):
         expected = {}
         result = win_pkg.install(name="nsis", version="3.03")
@@ -233,7 +309,7 @@ def test_pkg_install_name():
             "cmd.run_all": mock_cmd_run_all,
         },
     ):
-        ret = win_pkg.install(
+        win_pkg.install(
             name="firebox",
             version="3.03",
             extra_install_flags="-e True -test_flag True",
@@ -248,22 +324,26 @@ def test_pkg_install_verify_ssl_false():
     ret_reg = {"Nullsoft Install System": "3.03"}
     # The 2nd time it's run, pkg.list_pkgs uses with stringify
     se_list_pkgs = [{"nsis": ["3.03"]}, {"nsis": "3.02"}]
-    mock_cp = MagicMock(return_value="C:\\fake\\path.exe")
+    mock_cache_file = MagicMock(return_value="C:\\fake\\path.exe")
     with patch.object(win_pkg, "list_pkgs", side_effect=se_list_pkgs), patch.object(
         win_pkg, "_get_reg_software", return_value=ret_reg
     ), patch.dict(
-        win_pkg.__salt__, {"cp.is_cached": MagicMock(return_value=False)}
-    ), patch.dict(
-        win_pkg.__salt__, {"cp.cache_file": mock_cp}
-    ), patch.dict(
-        win_pkg.__salt__, {"cmd.run_all": MagicMock(return_value={"retcode": 0})}
+        win_pkg.__salt__,
+        {
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
+            "cp.cache_file": mock_cache_file,
+            "cp.is_cached": MagicMock(return_value=False),
+            "cp.hash_file": MagicMock(return_value={"hsum": "abc123"}),
+        },
     ):
         expected = {"nsis": {"new": "3.02", "old": "3.03"}}
         result = win_pkg.install(name="nsis", version="3.02", verify_ssl=False)
-        mock_cp.assert_called_once_with(
+        mock_cache_file.assert_called_once_with(
             "http://download.sourceforge.net/project/nsis/NSIS%203/3.02/nsis-3.02-setup.exe",
             saltenv="base",
+            source_hash="abc123",
             verify_ssl=False,
+            use_etag=True,
         )
         assert expected == result
 
@@ -300,7 +380,7 @@ def test_pkg_install_single_pkg():
             "cmd.run_all": mock_cmd_run_all,
         },
     ):
-        ret = win_pkg.install(
+        win_pkg.install(
             pkgs=["firebox"],
             version="3.03",
             extra_install_flags="-e True -test_flag True",
@@ -387,7 +467,7 @@ def test_pkg_install_multiple_pkgs():
             "cmd.run_all": mock_cmd_run_all,
         },
     ):
-        ret = win_pkg.install(
+        win_pkg.install(
             pkgs=["firebox", "got"], extra_install_flags="-e True -test_flag True"
         )
         assert "-e True -test_flag True" not in str(mock_cmd_run_all.call_args[0])
@@ -429,7 +509,7 @@ def test_pkg_install_minion_error_https():
             "cp.cache_file": mock_minion_error,
         },
     ):
-        ret = win_pkg.install(
+        result = win_pkg.install(
             name="firebox",
             version="3.03",
         )
@@ -438,7 +518,7 @@ def test_pkg_install_minion_error_https():
             " getaddrinfo failed reading https://repo.test.com/runme.exe"
         )
 
-        assert ret == expected
+        assert result == expected
 
 
 def test_pkg_install_minion_error_salt():
@@ -469,12 +549,13 @@ def test_pkg_install_minion_error_salt():
     ), patch.dict(
         win_pkg.__salt__,
         {
-            "pkg_resource.parse_targets": mock_parse,
-            "cp.is_cached": mock_none,
             "cp.cache_file": mock_minion_error,
+            "cp.is_cached": mock_none,
+            "cp.hash_file": MagicMock(return_value={"hsum": "abc123"}),
+            "pkg_resource.parse_targets": mock_parse,
         },
     ):
-        ret = win_pkg.install(
+        result = win_pkg.install(
             name="firebox",
             version="3.03",
         )
@@ -483,7 +564,7 @@ def test_pkg_install_minion_error_salt():
             "Error: [Errno 1] failed reading salt://software/runme.exe"
         )
 
-        assert ret == expected
+        assert result == expected
 
 
 def test_pkg_install_minion_error_salt_cache_dir():
@@ -505,18 +586,19 @@ def test_pkg_install_minion_error_salt_cache_dir():
     }
 
     err_msg = "Error: [Errno 1] failed reading salt://software"
-    mock_none = MagicMock(return_value=None)
     mock_minion_error = MagicMock(side_effect=MinionError(err_msg))
-    mock_parse = MagicMock(return_value=[{"firebox": "3.03"}, None])
     with patch.object(
         salt.utils.data, "is_true", MagicMock(return_value=True)
     ), patch.object(
         win_pkg, "_get_package_info", MagicMock(return_value=ret__get_package_info)
     ), patch.dict(
         win_pkg.__salt__,
-        {"cp.cache_dir": mock_minion_error},
+        {
+            "cp.cache_dir": mock_minion_error,
+            "cp.hash_file": MagicMock(return_value={"hsum": "abc123"}),
+        },
     ):
-        ret = win_pkg.install(
+        result = win_pkg.install(
             name="firebox",
             version="3.03",
         )
@@ -525,7 +607,7 @@ def test_pkg_install_minion_error_salt_cache_dir():
             "Error: [Errno 1] failed reading salt://software"
         )
 
-        assert ret == expected
+        assert result == expected
 
 
 def test_pkg_remove_log_message(caplog):
@@ -602,17 +684,18 @@ def test_pkg_remove_minion_error_salt_cache_dir():
     ), patch.dict(
         win_pkg.__salt__,
         {
-            "pkg_resource.parse_targets": mock_parse,
             "cp.cache_dir": mock_minion_error,
+            "cp.hash_file": MagicMock(return_value={"hsum": "abc123"}),
+            "pkg_resource.parse_targets": mock_parse,
         },
     ):
-        ret = win_pkg.remove(name="firebox")
+        result = win_pkg.remove(name="firebox")
         expected = (
             "Failed to cache salt://software\n"
             "Error: [Errno 1] failed reading salt://software"
         )
 
-        assert ret == expected
+        assert result == expected
 
 
 def test_pkg_remove_minion_error_salt():
@@ -644,18 +727,19 @@ def test_pkg_remove_minion_error_salt():
     ), patch.dict(
         win_pkg.__salt__,
         {
-            "pkg_resource.parse_targets": mock_parse,
-            "cp.is_cached": mock_none,
             "cp.cache_file": mock_minion_error,
+            "cp.hash_file": MagicMock(return_value={"hsum": "abc123"}),
+            "cp.is_cached": mock_none,
+            "pkg_resource.parse_targets": mock_parse,
         },
     ):
-        ret = win_pkg.remove(name="firebox")
+        result = win_pkg.remove(name="firebox")
         expected = (
             "Failed to cache salt://software/runme.exe\n"
             "Error: [Errno 1] failed reading salt://software/runme.exe"
         )
 
-        assert ret == expected
+        assert result == expected
 
 
 @pytest.mark.parametrize(