Merge pull request #64609 from s0undt3ch/hotfix/merge-forward

[master] 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 -r requirements/static/ci/py${{ steps.get-python-version.outputs.version }}/tools.txt
+        (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) != ""
 
 
@@ -490,14 +490,14 @@ def _get_reg_software(include_components=True, include_updates=True):
             return False
         if __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="SystemComponent",
             use_32bit_registry=use_32bit_registry,
         ):
             if (
                 __utils__["reg.read_value"](
                     hive=hive,
-                    key="{}\\{}".format(key, sub_key),
+                    key=f"{key}\\{sub_key}",
                     vname="SystemComponent",
                     use_32bit_registry=use_32bit_registry,
                 )["vdata"]
@@ -519,14 +519,14 @@ def _get_reg_software(include_components=True, include_updates=True):
         products_key = "Software\\Classes\\Installer\\Products\\{0}"
         if __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="WindowsInstaller",
             use_32bit_registry=use_32bit_registry,
         ):
             if (
                 __utils__["reg.read_value"](
                     hive=hive,
-                    key="{}\\{}".format(key, sub_key),
+                    key=f"{key}\\{sub_key}",
                     vname="WindowsInstaller",
                     use_32bit_registry=use_32bit_registry,
                 )["vdata"]
@@ -557,14 +557,14 @@ def _get_reg_software(include_components=True, include_updates=True):
         # https://docs.microsoft.com/en-us/windows/win32/msi/arpnoremove
         if __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="NoRemove",
             use_32bit_registry=use_32bit_registry,
         ):
             if (
                 __utils__["reg.read_value"](
                     hive=hive,
-                    key="{}\\{}".format(key, sub_key),
+                    key=f"{key}\\{sub_key}",
                     vname="NoRemove",
                     use_32bit_registry=use_32bit_registry,
                 )["vdata"]
@@ -573,7 +573,7 @@ def _get_reg_software(include_components=True, include_updates=True):
                 return False
         if not __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="UninstallString",
             use_32bit_registry=use_32bit_registry,
         ):
@@ -594,14 +594,14 @@ def _get_reg_software(include_components=True, include_updates=True):
         skip_types = ["Hotfix", "Security Update", "Update Rollup"]
         if __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="ReleaseType",
             use_32bit_registry=use_32bit_registry,
         ):
             if (
                 __utils__["reg.read_value"](
                     hive=hive,
-                    key="{}\\{}".format(key, sub_key),
+                    key=f"{key}\\{sub_key}",
                     vname="ReleaseType",
                     use_32bit_registry=use_32bit_registry,
                 )["vdata"]
@@ -620,7 +620,7 @@ def _get_reg_software(include_components=True, include_updates=True):
         """
         if __utils__["reg.value_exists"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="ParentKeyName",
             use_32bit_registry=use_32bit_registry,
         ):
@@ -637,7 +637,7 @@ def _get_reg_software(include_components=True, include_updates=True):
         """
         d_name_regdata = __utils__["reg.read_value"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="DisplayName",
             use_32bit_registry=use_32bit_registry,
         )
@@ -656,7 +656,7 @@ def _get_reg_software(include_components=True, include_updates=True):
 
         d_vers_regdata = __utils__["reg.read_value"](
             hive=hive,
-            key="{}\\{}".format(key, sub_key),
+            key=f"{key}\\{sub_key}",
             vname="DisplayVersion",
             use_32bit_registry=use_32bit_registry,
         )
@@ -727,7 +727,7 @@ def _get_reg_software(include_components=True, include_updates=True):
     for sub_key in __utils__["reg.list_keys"](**kwargs):
         # If the key does not exist in userdata, skip it
         if not __utils__["reg.key_exists"](
-            hive=kwargs["hive"], key="{}\\{}".format(userdata_key, sub_key)
+            hive=kwargs["hive"], key=f"{userdata_key}\\{sub_key}"
         ):
             continue
         kwargs["sub_key"] = sub_key
@@ -1041,7 +1041,7 @@ def _get_repo_details(saltenv):
     """
     Return repo details for the specified saltenv as a namedtuple
     """
-    contextkey = "winrepo._get_repo_details.{}".format(saltenv)
+    contextkey = f"winrepo._get_repo_details.{saltenv}"
 
     if contextkey in __context__:
         (winrepo_source_dir, local_dest, winrepo_file) = __context__[contextkey]
@@ -1086,9 +1086,7 @@ def _get_repo_details(saltenv):
         os.makedirs(local_dest)
     except OSError as exc:
         if exc.errno != errno.EEXIST:
-            raise CommandExecutionError(
+            raise CommandExecutionError(f"Failed to create {local_dest}: {exc}")
-                "Failed to create {}: {}".format(local_dest, exc)
-            )
 
     winrepo_age = -1
     try:
@@ -1097,9 +1095,7 @@ def _get_repo_details(saltenv):
         winrepo_age = time.time() - mtime
     except OSError as exc:
         if exc.errno != errno.ENOENT:
-            raise CommandExecutionError(
+            raise CommandExecutionError(f"Failed to get age of {winrepo_file}: {exc}")
-                "Failed to get age of {}: {}".format(winrepo_file, exc)
-            )
     except AttributeError:
         # Shouldn't happen but log if it does
         log.warning("st_mtime missing from stat result %s", stat_result)
@@ -1222,9 +1218,7 @@ def _repo_process_pkg_sls(filename, short_path_name, ret, successful_verbose):
 
     def _failed_compile(prefix_msg, error_msg):
         log.error("%s '%s': %s", prefix_msg, short_path_name, error_msg)
-        ret.setdefault("errors", {})[short_path_name] = [
+        ret.setdefault("errors", {})[short_path_name] = [f"{prefix_msg}, {error_msg} "]
-            "{}, {} ".format(prefix_msg, error_msg)
-        ]
         return False
 
     try:
@@ -1250,7 +1244,7 @@ def _repo_process_pkg_sls(filename, short_path_name, ret, successful_verbose):
                     pkgname,
                     short_path_name,
                 )
-                errors.append("package '{}' already defined".format(pkgname))
+                errors.append(f"package '{pkgname}' already defined")
                 break
             for version_str, repodata in version_list.items():
                 # Ensure version is a string/unicode
@@ -1315,16 +1309,14 @@ 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)
             raise
 
         if not cached_hash_file:
-            raise CommandExecutionError(
+            raise CommandExecutionError(f"Source hash file {source_hash} not found")
-                "Source hash file {} not found".format(source_hash)
-            )
 
         ret = __salt__["file.extract_hash"](cached_hash_file, "", file_path)
         if ret is None:
@@ -1582,7 +1574,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
         # Make sure pkginfo was found
         if not pkginfo:
             log.error("Unable to locate package %s", pkg_name)
-            ret[pkg_name] = "Unable to locate package {}".format(pkg_name)
+            ret[pkg_name] = f"Unable to locate package {pkg_name}"
             continue
 
         version_num = options.get("version")
@@ -1640,6 +1632,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 +1648,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,
@@ -1658,62 +1658,56 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
                         exclude_pat="E@init.sls$",
                     )
                 except MinionError as exc:
-                    msg = "Failed to cache {}".format(path)
+                    msg = f"Failed to cache {path}"
                     log.exception(msg, exc_info=exc)
-                    return "{}\n{}".format(msg, exc)
+                    return f"{msg}\n{exc}"
 
             # Check to see if the cache_file is cached... if passed
             if cache_file and cache_file.startswith("salt:"):
+                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:
+                    msg = f"Failed to cache {cache_file}"
+                    log.exception(msg, exc_info=exc)
+                    return f"{msg}\n{exc}"
 
-                # Check to see if the file is cached
+                # Check if the cache_file was cached successfully
-                cached_file = __salt__["cp.is_cached"](cache_file, saltenv)
                 if not cached_file:
-                    try:
+                    log.error("Unable to cache %s", cache_file)
-                        cached_file = __salt__["cp.cache_file"](
+                    ret[pkg_name] = {"failed to cache cache_file": cache_file}
-                            cache_file,
+                    continue
-                            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 version is "latest" we always cache because "cp.is_cached" only
-                if __salt__["cp.hash_file"](cache_file, saltenv) != __salt__[
+            # checks that the file exists, not that is has changed
-                    "cp.hash_file"
+            cached_pkg = False
-                ](cached_file):
+            if version_num != "latest" and not installer.startswith("salt:"):
-                    try:
+                cached_pkg = __salt__["cp.is_cached"](installer, saltenv)
-                        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)
 
-                    # Check if the cache_file was cached successfully
-                    if not cached_file:
-                        log.error("Unable to cache %s", cache_file)
-                        ret[pkg_name] = {"failed to cache cache_file": cache_file}
-                        continue
-
-            # Check to see if the installer is cached
-            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)
+                    msg = f"Failed to cache {installer}"
                     log.exception(msg, exc_info=exc)
-                    return "{}\n{}".format(msg, exc)
+                    return f"{msg}\n{exc}"
 
                 # Check if the installer was cached successfully
                 if not cached_pkg:
@@ -1722,29 +1716,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 +1757,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(
@@ -1802,14 +1772,14 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
             "ComSpec", "{}\\system32\\cmd.exe".format(os.getenv("WINDIR"))
         )
         if use_msiexec:
-            arguments = '"{}" /I "{}"'.format(msiexec, cached_pkg)
+            arguments = f'"{msiexec}" /I "{cached_pkg}"'
             if pkginfo[version_num].get("allusers", True):
-                arguments = "{} ALLUSERS=1".format(arguments)
+                arguments = f"{arguments} ALLUSERS=1"
         else:
-            arguments = '"{}"'.format(cached_pkg)
+            arguments = f'"{cached_pkg}"'
 
         if install_flags:
-            arguments = "{} {}".format(arguments, install_flags)
+            arguments = f"{arguments} {install_flags}"
 
         # Install the software
         # Check Use Scheduler Option
@@ -1823,7 +1793,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
                 force=True,
                 action_type="Execute",
                 cmd=cmd_shell,
-                arguments='/c "{}"'.format(arguments),
+                arguments=f'/c "{arguments}"',
                 start_in=cache_path,
                 trigger_type="Once",
                 start_date="1975-01-01",
@@ -1875,7 +1845,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
         else:
             # Launch the command
             result = __salt__["cmd.run_all"](
-                '"{}" /c "{}"'.format(cmd_shell, arguments),
+                f'"{cmd_shell}" /c "{arguments}"',
                 cache_path,
                 output_loglevel="trace",
                 python_shell=False,
@@ -2028,7 +1998,7 @@ def remove(name=None, pkgs=None, **kwargs):
 
         # Make sure pkginfo was found
         if not pkginfo:
-            msg = "Unable to locate package {}".format(pkgname)
+            msg = f"Unable to locate package {pkgname}"
             log.error(msg)
             ret[pkgname] = msg
             continue
@@ -2063,12 +2033,12 @@ 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:
                     log.debug("%s %s not installed", pkgname, version_num)
-                    ret[pkgname] = {"current": "{} not installed".format(version_num)}
+                    ret[pkgname] = {"current": f"{version_num} not installed"}
                     continue
             elif "latest" in pkginfo:
                 # we do not have version entry, assume software can self upgrade and use latest
@@ -2083,9 +2053,7 @@ def remove(name=None, pkgs=None, **kwargs):
             log.error(
                 "%s %s no definition to remove this version", pkgname, version_num
             )
-            ret[pkgname] = {
+            ret[pkgname] = {"current": f"{version_num} no definition, cannot removed"}
-                "current": "{} no definition, cannot removed".format(version_num)
-            }
             continue
 
         for target in removal_targets:
@@ -2107,8 +2075,15 @@ def remove(name=None, pkgs=None, **kwargs):
                 ret[pkgname] = {"no uninstaller defined": target}
                 continue
 
-            # Where is the uninstaller
+            # Hash the uninstaller source after verifying it was defined
-            if uninstaller.startswith(("salt:", "http:", "https:", "ftp:")):
+            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,29 +2092,43 @@ 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)
+                        msg = f"Failed to cache {path}"
                         log.exception(msg, exc_info=exc)
-                        return "{}\n{}".format(msg, exc)
+                        return f"{msg}\n{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)
+                        msg = f"Failed to cache {uninstaller}"
                         log.exception(msg, exc_info=exc)
-                        return "{}\n{}".format(msg, exc)
+                        return f"{msg}\n{exc}"
 
                     # Check if the uninstaller was cached successfully
                     if not cached_pkg:
@@ -2147,32 +2136,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
+                # Run the uninstaller directly (not hosted on salt:, https:, etc.)
-                # (not hosted on salt:, https:, etc.)
                 cached_pkg = os.path.expandvars(uninstaller)
 
             # Fix non-windows slashes
@@ -2197,12 +2162,12 @@ def remove(name=None, pkgs=None, **kwargs):
             if use_msiexec:
                 # Check if uninstaller is set to {guid}, if not we assume its a remote msi file.
                 # which has already been downloaded.
-                arguments = '"{}" /X "{}"'.format(msiexec, cached_pkg)
+                arguments = f'"{msiexec}" /X "{cached_pkg}"'
             else:
-                arguments = '"{}"'.format(cached_pkg)
+                arguments = f'"{cached_pkg}"'
 
             if uninstall_flags:
-                arguments = "{} {}".format(arguments, uninstall_flags)
+                arguments = f"{arguments} {uninstall_flags}"
 
             # Uninstall the software
             changed.append(pkgname)
@@ -2217,7 +2182,7 @@ def remove(name=None, pkgs=None, **kwargs):
                     force=True,
                     action_type="Execute",
                     cmd=cmd_shell,
-                    arguments='/c "{}"'.format(arguments),
+                    arguments=f'/c "{arguments}"',
                     start_in=cache_path,
                     trigger_type="Once",
                     start_date="1975-01-01",
@@ -2234,7 +2199,7 @@ def remove(name=None, pkgs=None, **kwargs):
             else:
                 # Launch the command
                 result = __salt__["cmd.run_all"](
-                    '"{}" /c "{}"'.format(cmd_shell, arguments),
+                    f'"{cmd_shell}" /c "{arguments}"',
                     output_loglevel="trace",
                     python_shell=False,
                     redirect_stderr=True,

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,
@@ -78,7 +92,7 @@ def test_pkg__get_reg_software():
 
 def test_pkg__get_reg_software_noremove():
     search = "test_pkg_noremove"
-    key = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{}".format(search)
+    key = f"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{search}"
     win_reg.set_value(hive="HKLM", key=key, vname="DisplayName", vdata=search)
     win_reg.set_value(hive="HKLM", key=key, vname="DisplayVersion", vdata="1.0.0")
     win_reg.set_value(
@@ -100,7 +114,7 @@ def test_pkg__get_reg_software_noremove():
 
 def test_pkg__get_reg_software_noremove_not_present():
     search = "test_pkg_noremove_not_present"
-    key = "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{}".format(search)
+    key = f"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\{search}"
     win_reg.set_value(hive="HKLM", key=key, vname="DisplayName", vdata=search)
     win_reg.set_value(hive="HKLM", key=key, vname="DisplayVersion", vdata="1.0.0")
     try:
@@ -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(
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
-        win_pkg.__salt__, {"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(
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
-        win_pkg.__salt__, {"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)}
+        win_pkg.__salt__,
-    ), patch.dict(
+        {
-        win_pkg.__salt__, {"cp.cache_file": mock_cp}
+            "cmd.run_all": MagicMock(return_value={"retcode": 0}),
-    ), patch.dict(
+            "cp.cache_file": mock_cache_file,
-        win_pkg.__salt__, {"cmd.run_all": MagicMock(return_value={"retcode": 0})}
+            "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(