Merge pull request #34010 from terminalmage/issue33831

Do not cache remote files if they are already cached

salt/modules/file.py

@@ -3496,13 +3496,14 @@ def get_managed(
     # Copy the file to the minion and templatize it
     sfn = ''
     source_sum = {}
+    remote_protos = ('http', 'https', 'ftp', 'swift', 's3')
 
     def _get_local_file_source_sum(path):
         '''
         DRY helper for getting the source_sum value from a locally cached
         path.
         '''
-        return {'hsum': get_hash(path), 'hash_type': 'sha256'}
+        return {'hsum': get_hash(path, form='sha256'), 'hash_type': 'sha256'}
 
     # If we have a source defined, let's figure out what the hash is
     if source:
@@ -3518,8 +3519,7 @@ def _get_local_file_source_sum(path):
         else:
             if not skip_verify:
                 if source_hash:
-                    protos = ('salt', 'http', 'https', 'ftp', 'swift',
-                              's3', 'file')
+                    protos = ('salt', 'file') + remote_protos
 
                     def _invalid_source_hash_format():
                         '''
@@ -3551,75 +3551,78 @@ def _invalid_source_hash_format():
 
                     else:
                         # The source_hash is a hash string
-                        comps = source_hash.split('=')
+                        comps = source_hash.split('=', 1)
                         if len(comps) < 2:
                             return _invalid_source_hash_format()
                         source_sum['hsum'] = comps[1].strip()
                         source_sum['hash_type'] = comps[0].strip()
                 else:
-                    return '', {}, ('Unable to determine upstream hash of '
-                                    'source file {0}'.format(source))
-
-    # if the file is a template we need to actually template the file to get
-    # a checksum, but we can cache the template itself, but only if there is
-    # a template source (it could be a templated contents)
-    if template and source:
-        # Check if we have the template cached
-        template_dest = __salt__['cp.is_cached'](source, saltenv)
-        if template_dest and source_hash:
-            comps = source_hash.split('=')
-            cached_template_sum = get_hash(template_dest, form=source_sum['hash_type'])
-            if cached_template_sum == source_sum['hsum']:
-                sfn = template_dest
-
-        # If we didn't have the template file, let's get it
+                    msg = (
+                        'Unable to verify upstream hash of source file {0}, '
+                        'please set source_hash or set skip_verify to True'
+                        .format(source)
+                    )
+                    return '', {}, msg
+
+    if source and (template or urlparsed_source.scheme in remote_protos):
+        # Check if we have the template or remote file cached
+        cached_dest = __salt__['cp.is_cached'](source, saltenv)
+        if cached_dest and (source_hash or skip_verify):
+            htype = source_sum.get('hash_type', 'sha256')
+            cached_sum = get_hash(cached_dest, form=htype)
+            if skip_verify or cached_sum == source_sum['hsum']:
+                sfn = cached_dest
+                source_sum = {'hsum': cached_sum, 'hash_type': htype}
+
+        # If we didn't have the template or remote file, let's get it
         if not sfn:
             try:
                 sfn = __salt__['cp.cache_file'](source, saltenv)
             except Exception as exc:
                 # A 404 or other error code may raise an exception, catch it
                 # and return a comment that will fail the calling state.
-                return '', {}, ('Failed to cache template file {0}: {1}'
-                                .format(source, exc))
+                return '', {}, 'Failed to cache {0}: {1}'.format(source, exc)
 
-        # exists doesn't play nice with sfn as bool
-        # but if cache failed, sfn == False
+        # If cache failed, sfn will be False, so do a truth check on sfn first
+        # as invoking os.path.exists() on a bool raises a TypeError.
         if not sfn or not os.path.exists(sfn):
             return sfn, {}, 'Source file \'{0}\' not found'.format(source)
         if sfn == name:
             raise SaltInvocationError(
                 'Source file cannot be the same as destination'
             )
-        if template in salt.utils.templates.TEMPLATE_REGISTRY:
-            context_dict = defaults if defaults else {}
-            if context:
-                context_dict.update(context)
-            data = salt.utils.templates.TEMPLATE_REGISTRY[template](
-                sfn,
-                name=name,
-                source=source,
-                user=user,
-                group=group,
-                mode=mode,
-                saltenv=saltenv,
-                context=context_dict,
-                salt=__salt__,
-                pillar=__pillar__,
-                grains=__grains__,
-                opts=__opts__,
-                **kwargs)
-        else:
-            return sfn, {}, ('Specified template format {0} is not supported'
-                             ).format(template)
-
-        if data['result']:
-            sfn = data['data']
-            hsum = get_hash(sfn)
-            source_sum = {'hash_type': 'sha256',
-                          'hsum': hsum}
-        else:
-            __clean_tmp(sfn)
-            return sfn, {}, data['data']
+
+        if template:
+            if template in salt.utils.templates.TEMPLATE_REGISTRY:
+                context_dict = defaults if defaults else {}
+                if context:
+                    context_dict.update(context)
+                data = salt.utils.templates.TEMPLATE_REGISTRY[template](
+                    sfn,
+                    name=name,
+                    source=source,
+                    user=user,
+                    group=group,
+                    mode=mode,
+                    saltenv=saltenv,
+                    context=context_dict,
+                    salt=__salt__,
+                    pillar=__pillar__,
+                    grains=__grains__,
+                    opts=__opts__,
+                    **kwargs)
+            else:
+                return sfn, {}, ('Specified template format {0} is not supported'
+                                 ).format(template)
+
+            if data['result']:
+                sfn = data['data']
+                hsum = get_hash(sfn, form='sha256')
+                source_sum = {'hash_type': 'sha256',
+                              'hsum': hsum}
+            else:
+                __clean_tmp(sfn)
+                return sfn, {}, data['data']
 
     return sfn, source_sum, ''
 
@@ -4260,7 +4263,7 @@ def manage_file(name,
                         'Specified {0} checksum for {1} ({2}) does not match '
                         'actual checksum ({3})'.format(
                             source_sum['hash_type'],
-                            name,
+                            source,
                             source_sum['hsum'],
                             dl_sum
                         )