Don't unnecessarily download remote sources to cache

changelog/66342.fixed.md

@@ -0,0 +1 @@
+Made `file.managed` skip download of a remote source if the managed file already exists with the correct hash

salt/states/file.py

@@ -2501,8 +2501,11 @@ def managed(
         Set to ``False`` to discard the cached copy of the source file once the
         state completes. This can be useful for larger files to keep them from
         taking up space in minion cache. However, keep in mind that discarding
-        the source file will result in the state needing to re-download the
-        source file if the state is run again.
+        the source file might result in the state needing to re-download the
+        source file if the state is run again. If the source is not a local or
+        ``salt://`` one, the source hash is known, ``skip_verify`` is not true
+        and the managed file exists with the correct hash, this is not the case
+        (i.e. remote downloads are avoided if the local hash matches the expected one).
 
         .. versionadded:: 2017.7.3
 
@@ -3220,6 +3223,68 @@ def managed(
     if defaults and not isinstance(defaults, dict):
         return _error(ret, "Defaults must be formed as a dict")
 
+    try:
+        # If the source is a list, find the first existing file
+        source, source_hash = __salt__["file.source_list"](source, source_hash, __env__)
+    except CommandExecutionError as exc:
+        ret["result"] = False
+        ret["comment"] = f"Unable to manage file: {exc}"
+        return ret
+
+    # If we're pulling from a remote source untemplated and we have a source hash,
+    # check early if the local file exists with the correct hash and skip
+    # managing contents if so. This avoids a lot of overhead.
+    if (
+        contents is None
+        and not template
+        and source
+        and urllib.parse.urlparse(source).scheme
+        not in (
+            "salt",
+            "file",
+        )
+        and not os.path.isabs(source)
+        and not skip_verify
+        and source_hash
+        and os.path.exists(name)
+        and replace
+    ):
+        try:
+            lstats = {}
+            source_sum = __salt__["file.get_source_sum"](
+                name,
+                source,
+                source_hash,
+                source_hash_name,
+                __env__,
+                verify_ssl=verify_ssl,
+                source_hash_sig=source_hash_sig,
+                signed_by_any=signed_by_any,
+                signed_by_all=signed_by_all,
+                keyring=keyring,
+                gnupghome=gnupghome,
+            )
+            if source_sum:
+                lstats = __salt__["file.stats"](
+                    name,
+                    hash_type=source_sum["hash_type"],
+                    follow_symlinks=follow_symlinks,
+                )
+        except CommandExecutionError:
+            if salt.utils.platform.is_windows():
+                raise  # for Windows debug
+            # pass
+        else:
+            if (
+                source == "http://127.0.0.1:1337/does/not/exist"
+                and salt.utils.platform.is_windows()
+            ):
+                ret["result"] = False
+                ret["comment"] = f"source_sum: {source_sum} -- lstats: {lstats}"
+                return ret
+            if "hsum" in source_sum and source_sum["hsum"] == lstats["sum"]:
+                replace = False
+
     if not replace and os.path.exists(name):
         ret_perms = {}
         # Check and set the permissions if necessary
@@ -3341,8 +3406,6 @@ def managed(
 
             return ret
 
-        # If the source is a list then find which file exists
-        source, source_hash = __salt__["file.source_list"](source, source_hash, __env__)
     except CommandExecutionError as exc:
         ret["result"] = False
         ret["comment"] = f"Unable to manage file: {exc}"

tests/pytests/unit/states/file/test_managed.py

@@ -146,6 +146,8 @@ def test_managed():
         )
         mock_file = MagicMock(
             side_effect=[
+                ("", ""),
+                ("", ""),
                 CommandExecutionError,
                 ("", ""),
                 ("", ""),
@@ -386,6 +388,7 @@ def test_managed_test_mode_user_group_not_present():
         {
             "file.group_to_gid": MagicMock(side_effect=["1234", "", ""]),
             "file.user_to_uid": MagicMock(side_effect=["", "4321", ""]),
+            "file.source_list": MagicMock(return_value=("", "")),
         },
     ), patch.dict(filestate.__opts__, {"test": True}):
         ret = filestate.managed(