Optimize template (#28044)

* Optimize template

* In fixing template to handle diff correctly #24477, I introduced more
  round trips to the remote end which slowed things down  The new code now
  uses one fewer round trips than the old code.
* Reimplement a large part of template by calling the copy action plugin
  instead of doing it in template's code.  This reduces the code in
  template and gives us one place to fix bugs and optimize.
* Add a follow parameter to template that mirrors the follow parameters
  for file and copy.
* Fix copy's diff handling (probably broken in my rewrite for in 2.4
  development)
* Adjusted when copy creates tmp dirs to rduce round trips in copy and
  template.

Fixes #27956

1 files changed, 30 insertions, 76 deletions

diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py
index 33aa580dce..e0674b1569 100644
--- a/lib/ansible/plugins/action/template.py
+++ b/lib/ansible/plugins/action/template.py
@@ -18,10 +18,12 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import os
+import shutil
+import tempfile
 
 from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleFileNotFound
-from ansible.module_utils._text import to_text
+from ansible.module_utils._text import to_bytes, to_text
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.plugins.action import ActionBase
 from ansible.template import generate_ansible_template_vars
@@ -58,6 +60,7 @@ class ActionModule(ActionBase):
         source = self._task.args.get('src', None)
         dest = self._task.args.get('dest', None)
         force = boolean(self._task.args.get('force', True), strict=False)
+        follow = boolean(self._task.args.get('follow', False), strict=False)
         state = self._task.args.get('state', None)
         newline_sequence = self._task.args.get('newline_sequence', self.DEFAULT_NEWLINE_SEQUENCE)
         variable_start_string = self._task.args.get('variable_start_string', None)
@@ -92,20 +95,6 @@ class ActionModule(ActionBase):
         if 'failed' in result:
             return result
 
-        # Expand any user home dir specification
-        dest = self._remote_expand_user(dest)
-
-        directory_prepended = False
-        if dest.endswith(os.sep):
-            # Optimization.  trailing slash means we know it's a directory
-            directory_prepended = True
-            dest = self._connection._shell.join_path(dest, os.path.basename(source))
-        else:
-            # Find out if it's a directory
-            dest_stat = self._execute_remote_stat(dest, task_vars, True, tmp=tmp)
-            if dest_stat['exists'] and dest_stat['isdir']:
-                dest = self._connection._shell.join_path(dest, os.path.basename(source))
-
         # Get vault decrypted tmp file
         try:
             tmp_source = self._loader.get_real_file(source)
@@ -160,70 +149,35 @@ class ActionModule(ActionBase):
         finally:
             self._loader.cleanup_tmp_file(tmp_source)
 
-        if not tmp:
-            tmp = self._make_tmp_path()
-
-        local_checksum = checksum_s(resultant)
-        remote_checksum = self.get_checksum(dest, task_vars, not directory_prepended, source=source, tmp=tmp)
-        if isinstance(remote_checksum, dict):
-            # Error from remote_checksum is a dict.  Valid return is a str
-            result.update(remote_checksum)
-            return result
-
-        diff = {}
-        new_module_args = self._task.args.copy()
-
-        # remove newline_sequence from standard arguments
-        new_module_args.pop('newline_sequence', None)
-        new_module_args.pop('block_start_string', None)
-        new_module_args.pop('block_end_string', None)
-        new_module_args.pop('variable_start_string', None)
-        new_module_args.pop('variable_end_string', None)
-        new_module_args.pop('trim_blocks', None)
-
-        if (remote_checksum == '1') or (force and local_checksum != remote_checksum):
-
-            result['changed'] = True
-            # if showing diffs, we need to get the remote value
-            if self._play_context.diff:
-                diff = self._get_diff_data(dest, resultant, task_vars, source_file=False)
-
-            if not self._play_context.check_mode:  # do actual work through copy
-                xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant)
-
-                # fix file permissions when the copy is done as a different user
-                self._fixup_perms2((tmp, xfered))
-
-                # run the copy module
-                new_module_args.update(
-                    dict(
-                        src=xfered,
-                        dest=dest,
-                        original_basename=os.path.basename(source),
-                        follow=True,
-                    ),
-                )
-                result.update(self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False))
-
-            if result.get('changed', False) and self._play_context.diff:
-                result['diff'] = diff
+        new_task = self._task.copy()
+        new_task.args.pop('newline_sequence', None)
+        new_task.args.pop('block_start_string', None)
+        new_task.args.pop('block_end_string', None)
+        new_task.args.pop('variable_start_string', None)
+        new_task.args.pop('variable_end_string', None)
+        new_task.args.pop('trim_blocks', None)
+        try:
+            tempdir = tempfile.mkdtemp()
+            result_file = os.path.join(tempdir, os.path.basename(source))
+            with open(result_file, 'wb') as f:
+                f.write(to_bytes(resultant, errors='surrogate_or_strict'))
 
-        else:
-            # when running the file module based on the template data, we do
-            # not want the source filename (the name of the template) to be used,
-            # since this would mess up links, so we clear the src param and tell
-            # the module to follow links.  When doing that, we have to set
-            # original_basename to the template just in case the dest is
-            # a directory.
-            new_module_args.update(
+            new_task.args.update(
                 dict(
-                    src=None,
-                    original_basename=os.path.basename(source),
-                    follow=True,
+                    src=result_file,
+                    dest=dest,
+                    follow=follow,
                 ),
             )
-            result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False))
-
-        self._remove_tmp_path(tmp)
+            copy_action = self._shared_loader_obj.action_loader.get('copy',
+                                                                    task=new_task,
+                                                                    connection=self._connection,
+                                                                    play_context=self._play_context,
+                                                                    loader=self._loader,
+                                                                    templar=self._templar,
+                                                                    shared_loader_obj=self._shared_loader_obj)
+            result.update(copy_action.run(task_vars=task_vars))
+        finally:
+            shutil.rmtree(tempdir)
 
         return result