Windows: Use the correct newline sequence for the platform (#21846)

This change to the template action plugin make template use the
platform's native newline_sequence for Jinja.

We also added the option `newline_sequence` to change the newline
sequence using by Jinja if you need to use another newline sequence than
the platform default.

This was previously discussed in
https://github.com/ansible/ansible/issues/16255#issuecomment-278289414

And also relates to issue #21128

lib/ansible/modules/files/template.py

@@ -20,7 +20,7 @@ ANSIBLE_METADATA = {'metadata_version': '1.0',
                     'supported_by': 'core'}
 
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: template
 version_added: historical
@@ -51,27 +51,55 @@ options:
     description:
       - Create a backup file including the timestamp information so you can get
         the original file back if you somehow clobbered it incorrectly.
-    required: false
     choices: [ "yes", "no" ]
     default: "no"
+  newline_sequence:
+    description:
+      - Specify the newline sequence to use for templating files.
+    choices: [ '\n', '\r', '\r\n' ]
+    default: '\n'
+    version_added: '2.3'
+  block_start_string:
+    description:
+      - The string marking the beginning of a block.
+    default: '{%'
+    version_added: '2.3'
+  block_end_string:
+    description:
+      - The string marking the end of a block.
+    default: '%}'
+    version_added: '2.3'
+  variable_start_string:
+    description:
+      - The string marking the beginning of a print statement.
+    default: '{{'
+    version_added: '2.3'
+  variable_end_string:
+    description:
+      - The string marking the end of a print statement.
+    default: '}}'
+    version_added: '2.3'
+  trim_blocks:
+    description:
+      - If this is set to True the first newline after a block is removed (block, not variable tag!).
+    default: "no"
+    version_added: '2.3'
   force:
     description:
       - the default is C(yes), which will replace the remote file when contents
         are different than the source.  If C(no), the file will only be transferred
         if the destination does not exist.
-    required: false
     choices: [ "yes", "no" ]
     default: "yes"
 notes:
+  - For Windows you can use M(win_template) which uses '\r\n' as C(newline_sequence).
   - Including a string that uses a date in the template will result in the template being marked 'changed' each time
   - "Since Ansible version 0.9, templates are loaded with C(trim_blocks=True)."
   - "Also, you can override jinja2 settings by adding a special header to template file.
-    i.e. C(#jinja2:variable_start_string:'[%' , variable_end_string:'%]', trim_blocks: False)
+    i.e. C(#jinja2:variable_start_string:'[%', variable_end_string:'%]', trim_blocks: False)
     which changes the variable interpolation markers to  [% var %] instead of  {{ var }}.
     This is the best way to prevent evaluation of things that look like, but should not be Jinja2.
     raw/endraw in Jinja2 will not work as you expect because templates in Ansible are recursively evaluated."
-
-
 author:
     - Ansible Core Team
     - Michael DeHaan
@@ -80,7 +108,7 @@ extends_documentation_fragment:
     - validate
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
 # Example from Ansible Playbooks
 - template:
     src: /mytemplates/foo.j2
@@ -97,6 +125,12 @@ EXAMPLES = '''
     group: wheel
     mode: "u=rw,g=r,o=r"
 
+# Create a DOS-style text file from a template
+- template:
+    src: config.ini.j2
+    dest: /share/windows/config.ini
+    newline_sequence: '\r\n'
+
 # Copy a new "sudoers" file into place, after passing validation with visudo
 - template:
     src: /mine/sudoers

lib/ansible/modules/windows/win_template.py

@@ -48,16 +48,57 @@ options:
     description:
       - Location to render the template to on the remote machine.
     required: true
+  newline_sequence:
+    description:
+      - Specify the newline sequence to use for templating files.
+    choices: [ '\n', '\r', '\r\n' ]
+    default: '\r\n'
+    version_added: '2.3'
+  block_start_string:
+    description:
+      - The string marking the beginning of a block.
+    default: '{%'
+    version_added: '2.3'
+  block_end_string:
+    description:
+      - The string marking the end of a block.
+    default: '%}'
+    version_added: '2.3'
+  variable_start_string:
+    description:
+      - The string marking the beginning of a print statement.
+    default: '{{'
+    version_added: '2.3'
+  variable_end_string:
+    description:
+      - The string marking the end of a print statement.
+    default: '}}'
+    version_added: '2.3'
+  trim_blocks:
+    description:
+      - If this is set to True the first newline after a block is removed (block, not variable tag!).
+    default: "no"
+    version_added: '2.3'
+  force:
+    description:
+      - the default is C(yes), which will replace the remote file when contents
+        are different than the source.  If C(no), the file will only be transferred
+        if the destination does not exist.
+    choices: [ "yes", "no" ]
+    default: "yes"
+    version_added: '2.3'
 notes:
-  - "templates are loaded with C(trim_blocks=True)."
-  - By default, windows line endings are not created in the generated file.
-  - "In order to ensure windows line endings are in the generated file, add the following header
-    as the first line of your template: ``#jinja2: newline_sequence:'\\r\\n'`` and ensure each line
-    of the template ends with \\\\r\\\\n"
+  - For other platforms you can use M(template) which uses '\n' as C(newline_sequence).
+  - Templates are loaded with C(trim_blocks=True).
   - Beware fetching files from windows machines when creating templates
     because certain tools, such as Powershell ISE,  and regedit's export facility
     add a Byte Order Mark as the first character of the file, which can cause tracebacks.
-  - Use "od -cx" to examine your templates for Byte Order Marks.
+  - To find Byte Order Marks in files, use C(Format-Hex <file> -Count 16) on Windows, and use C(od -a -t x1 -N 16 <file>) on Linux.
+  - "Also, you can override jinja2 settings by adding a special header to template file.
+    i.e. C(#jinja2:variable_start_string:'[%', variable_end_string:'%]', trim_blocks: False)
+    which changes the variable interpolation markers to  [% var %] instead of  {{ var }}.
+    This is the best way to prevent evaluation of things that look like, but should not be Jinja2.
+    raw/endraw in Jinja2 will not work as you expect because templates in Ansible are recursively evaluated."
 author: "Jon Hawkesworth (@jhawkesworth)"
 '''
 
@@ -66,4 +107,10 @@ EXAMPLES = r'''
   win_template:
     src: /mytemplates/file.conf.j2
     dest: C:\temp\file.conf
+
+- name: Create a Unix-style file from a Jinja2 template
+  win_template:
+    src: unix/config.conf.j2
+    dest: C:\share\unix\config.conf
+    newline_sequence: '\n'
 '''

lib/ansible/plugins/action/template.py

@@ -34,6 +34,7 @@ boolean = C.mk_boolean
 class ActionModule(ActionBase):
 
     TRANSFERS_FILES = True
+    DEFAULT_NEWLINE_SEQUENCE = "\n"
 
     def get_checksum(self, dest, all_vars, try_directory=False, source=None, tmp=None):
         try:
@@ -61,6 +62,19 @@ class ActionModule(ActionBase):
         dest   = self._task.args.get('dest', None)
         force  = boolean(self._task.args.get('force', True))
         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)
+        variable_end_string = self._task.args.get('variable_end_string', None)
+        block_start_string = self._task.args.get('block_start_string', None)
+        block_end_string = self._task.args.get('block_end_string', None)
+        trim_blocks = self._task.args.get('trim_blocks', None)
+
+        wrong_sequences = ["\\n", "\\r", "\\r\\n"]
+        allowed_sequences = ["\n", "\r", "\r\n"]
+
+        # We need to convert unescaped sequences to proper escaped sequences for Jinja2
+        if newline_sequence in wrong_sequences:
+            newline_sequence = allowed_sequences[wrong_sequences.index(newline_sequence)]
 
         if state is not None:
             result['failed'] = True
@@ -68,6 +82,9 @@ class ActionModule(ActionBase):
         elif source is None or dest is None:
             result['failed'] = True
             result['msg'] = "src and dest are required"
+        elif newline_sequence not in allowed_sequences:
+            result['failed'] = True
+            result['msg'] = "newline_sequence needs to be one of: \n, \r or \r\n"
         else:
             try:
                 source = self._find_needle('templates', source)
@@ -117,7 +134,6 @@ class ActionModule(ActionBase):
                 time.localtime(os.path.getmtime(b_source))
             )
 
-
             searchpath = []
             # set jinja2 internal search path for includes
             if 'ansible_search_path' in task_vars:
@@ -135,6 +151,17 @@ class ActionModule(ActionBase):
             searchpath = newsearchpath
 
             self._templar.environment.loader.searchpath = searchpath
+            self._templar.environment.newline_sequence = newline_sequence
+            if block_start_string is not None:
+                self._templar.environment.block_start_string = block_start_string
+            if block_end_string is not None:
+                self._templar.environment.block_end_string = block_end_string
+            if variable_start_string is not None:
+                self._templar.environment.variable_start_string = variable_start_string
+            if variable_end_string is not None:
+                self._templar.environment.variable_end_string = variable_end_string
+            if trim_blocks is not None:
+                self._templar.environment.trim_blocks = bool(trim_blocks)
 
             old_vars = self._templar._available_variables
             self._templar.set_available_variables(temp_vars)
@@ -158,6 +185,14 @@ class ActionModule(ActionBase):
         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

lib/ansible/plugins/action/win_template.py

@@ -26,4 +26,4 @@ from ansible.plugins.action.template import ActionModule as TemplateActionModule
 # Even though TemplateActionModule inherits from ActionBase, we still need to
 # directly inherit from ActionBase to appease the plugin loader.
 class ActionModule(TemplateActionModule, ActionBase):
-    pass
+    DEFAULT_NEWLINE_SEQUENCE = '\r\n'

lib/ansible/template/__init__.py

@@ -223,12 +223,13 @@ class Templar:
 
         self.SINGLE_VAR = re.compile(r"^%s\s*(\w*)\s*%s$" % (self.environment.variable_start_string, self.environment.variable_end_string))
 
-        self.block_start    = self.environment.block_start_string
-        self.block_end      = self.environment.block_end_string
-        self.variable_start = self.environment.variable_start_string
-        self.variable_end   = self.environment.variable_end_string
-        self._clean_regex   = re.compile(r'(?:%s|%s|%s|%s)' % (self.variable_start, self.block_start, self.block_end, self.variable_end))
-        self._no_type_regex = re.compile(r'.*\|\s*(?:%s)\s*(?:%s)?$' % ('|'.join(C.STRING_TYPE_FILTERS), self.variable_end))
+        self._clean_regex   = re.compile(r'(?:%s|%s|%s|%s)' % (
+            self.environment.variable_start_string,
+            self.environment.block_start_string,
+            self.environment.block_end_string,
+            self.environment.variable_end_string
+        ))
+        self._no_type_regex = re.compile(r'.*\|\s*(?:%s)\s*(?:%s)?$' % ('|'.join(C.STRING_TYPE_FILTERS), self.environment.variable_end_string))
 
     def _get_filters(self):
         '''
@@ -294,17 +295,17 @@ class Templar:
                 token = mo.group(0)
                 token_start = mo.start(0)
 
-                if token[0] == self.variable_start[0]:
-                    if token == self.block_start:
+                if token[0] == self.environment.variable_start_string[0]:
+                    if token == self.environment.block_start_string:
                         block_openings.append(token_start)
-                    elif token == self.variable_start:
+                    elif token == self.environment.variable_start_string:
                         print_openings.append(token_start)
 
-                elif token[1] == self.variable_end[1]:
+                elif token[1] == self.environment.variable_end_string[1]:
                     prev_idx = None
-                    if token == self.block_end and block_openings:
+                    if token == self.environment.block_end_string and block_openings:
                         prev_idx = block_openings.pop()
-                    elif token == self.variable_end and print_openings:
+                    elif token == self.environment.variable_end_string and print_openings:
                         prev_idx = print_openings.pop()
 
                     if prev_idx is not None:
@@ -622,7 +623,7 @@ class Templar:
                 # newline here if preserve_newlines is False.
                 res_newlines = _count_newlines_from_end(res)
                 if data_newlines > res_newlines:
-                    res += '\n' * (data_newlines - res_newlines)
+                    res += self.environment.newline_sequence * (data_newlines - res_newlines)
             return res
         except (UndefinedError, AnsibleUndefinedVariable) as e:
             if fail_on_undefined:

test/integration/targets/template/files/foo.dos.txt

@@ -0,0 +1,3 @@
+BEGIN
+templated_var_loaded
+END

test/integration/targets/template/files/foo.unix.txt

@@ -1 +1,3 @@
+BEGIN
 templated_var_loaded
+END

test/integration/targets/template/tasks/main.yml

@@ -62,14 +62,14 @@
   copy: src=foo.txt dest={{output_dir}}/foo.txt
 
 - name: compare templated file to known good
-  shell: diff -w {{output_dir}}/foo.templated {{output_dir}}/foo.txt
+  shell: diff -uw {{output_dir}}/foo.templated {{output_dir}}/foo.txt
   register: diff_result
 
 - name: verify templated file matches known good
-  assert:  
-    that: 
-        - 'diff_result.stdout == ""' 
-        - "diff_result.rc == 0" 
+  assert:
+    that:
+        - 'diff_result.stdout == ""'
+        - "diff_result.rc == 0"
 
 # VERIFY MODE
 
@@ -251,3 +251,121 @@
   assert:
     that:
     - "template_result|changed"
+
+- name: change var for the template
+  set_fact:
+    templated_var: "templated_var_loaded"
+
+# UNIX TEMPLATE
+- name: fill in a basic template (Unix)
+  template:
+    src: foo2.j2
+    dest: '{{ output_dir }}/foo.unix.templated'
+  register: template_result
+
+- name: verify that the file was marked as changed (Unix)
+  assert:
+    that:
+      - 'template_result|changed'
+
+- name: fill in a basic template again (Unix)
+  template:
+    src: foo2.j2
+    dest: '{{ output_dir }}/foo.unix.templated'
+  register: template_result2
+
+- name: verify that the template was not changed (Unix)
+  assert:
+    that:
+      - 'not template_result2|changed'
+
+# VERIFY UNIX CONTENTS
+- name: copy known good into place (Unix)
+  copy:
+    src: foo.unix.txt
+    dest: '{{ output_dir }}/foo.unix.txt'
+
+- name: Dump templated file (Unix)
+  command: hexdump -C {{ output_dir }}/foo.unix.templated
+
+- name: Dump expected file (Unix)
+  command: hexdump -C {{ output_dir }}/foo.unix.txt
+
+- name: compare templated file to known good (Unix)
+  command: diff -u {{ output_dir }}/foo.unix.templated {{ output_dir }}/foo.unix.txt
+  register: diff_result
+
+- name: verify templated file matches known good (Unix)
+  assert:
+    that:
+        - 'diff_result.stdout == ""'
+        - "diff_result.rc == 0"
+
+# DOS TEMPLATE
+- name: fill in a basic template (DOS)
+  template:
+    src: foo2.j2
+    dest: '{{ output_dir }}/foo.dos.templated'
+    newline_sequence: '\r\n'
+  register: template_result
+
+- name: verify that the file was marked as changed (DOS)
+  assert:
+    that:
+      - 'template_result|changed'
+
+- name: fill in a basic template again (DOS)
+  template:
+    src: foo2.j2
+    dest: '{{ output_dir }}/foo.dos.templated'
+    newline_sequence: '\r\n'
+  register: template_result2
+
+- name: verify that the template was not changed (DOS)
+  assert:
+    that:
+      - 'not template_result2|changed'
+
+# VERIFY DOS CONTENTS
+- name: copy known good into place (DOS)
+  copy:
+    src: foo.dos.txt
+    dest: '{{ output_dir }}/foo.dos.txt'
+
+- name: Dump templated file (DOS)
+  command: hexdump -C {{ output_dir }}/foo.dos.templated
+
+- name: Dump expected file (DOS)
+  command: hexdump -C {{ output_dir }}/foo.dos.txt
+
+- name: compare templated file to known good (DOS)
+  command: diff -u {{ output_dir }}/foo.dos.templated {{ output_dir }}/foo.dos.txt
+  register: diff_result
+
+- name: verify templated file matches known good (DOS)
+  assert:
+    that:
+        - 'diff_result.stdout == ""'
+        - "diff_result.rc == 0"
+
+# VERIFY DOS CONTENTS
+- name: copy known good into place (Unix)
+  copy:
+    src: foo.unix.txt
+    dest: '{{ output_dir }}/foo.unix.txt'
+
+- name: Dump templated file (Unix)
+  command: hexdump -C {{ output_dir }}/foo.unix.templated
+
+- name: Dump expected file (Unix)
+  command: hexdump -C {{ output_dir }}/foo.unix.txt
+
+- name: compare templated file to known good (Unix)
+  command: diff -u {{ output_dir }}/foo.unix.templated {{ output_dir }}/foo.unix.txt
+  register: diff_result
+
+- name: verify templated file matches known good (Unix)
+  assert:
+    that:
+        - 'diff_result.stdout == ""'
+        - "diff_result.rc == 0"

test/integration/targets/template/templates/foo2.j2

@@ -0,0 +1,3 @@
+BEGIN
+{{ templated_var }}
+END

test/integration/targets/template/templates/foo3.j2

@@ -0,0 +1,3 @@
+BEGIN
+[% templated_var %]
+END

test/integration/targets/win_template/files/foo.dos.txt

@@ -0,0 +1,3 @@
+BEGIN
+templated_var_loaded
+END

test/integration/targets/win_template/files/foo.unix.txt

@@ -0,0 +1,3 @@
+BEGIN
+templated_var_loaded
+END

test/integration/targets/win_template/tasks/main.yml

@@ -16,58 +16,109 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-- name: fill in a basic template
-#  win_template: src=foo.j2 dest={{win_output_dir}}/foo.templated mode=0644
-  win_template: src=foo.j2 dest={{win_output_dir}}/foo.templated
-  register: template_result
-
-- assert:
-    that:
-        - "'changed' in template_result"
-#        - "'dest' in template_result"
-#        - "'group' in template_result"
-#        - "'gid' in template_result"
-#        - "'checksum' in template_result"
-#        - "'owner' in template_result"
-#        - "'size' in template_result"
-#        - "'src' in template_result"
-#        - "'state' in template_result"
-#        - "'uid' in template_result"
-
-- name: verify that the file was marked as changed
-  assert:
-    that:
-      - "template_result.changed == true"
-
-- name: fill in a basic template again
+# DOS TEMPLATE
+- name: fill in a basic template (DOS)
   win_template:
     src: foo.j2
-    dest: "{{win_output_dir}}/foo.templated"
+    dest: '{{ win_output_dir }}/foo.dos.templated'
+  register: template_result
+
+- name: verify that the file was marked as changed (DOS)
+  assert:
+    that:
+      - 'template_result|changed'
+
+- name: fill in a basic template again (DOS)
+  win_template:
+    src: foo.j2
+    dest: '{{ win_output_dir }}/foo.dos.templated'
   register: template_result2
 
-- name: verify that the template was not changed
+- name: verify that the template was not changed (DOS)
   assert:
     that:
-      - "not template_result2|changed"
+      - 'not template_result2|changed'
 
-# VERIFY CONTENTS
+# VERIFY DOS CONTENTS
+- name: copy known good into place (DOS)
+  win_copy:
+    src: foo.dos.txt
+    dest: '{{ win_output_dir }}\\foo.dos.txt'
 
-- name: copy known good into place
-  win_copy: src=foo.txt dest={{win_output_dir}}\\foo.txt
-
-- name: compare templated file to known good
-  raw: fc.exe {{win_output_dir}}\\foo.templated {{win_output_dir}}\\foo.txt
+- name: compare templated file to known good (DOS)
+  raw: fc.exe {{ win_output_dir }}\\foo.dos.templated {{ win_output_dir }}\\foo.dos.txt
   register: diff_result
 
-- debug: var=diff_result
+- debug:
+    var: diff_result
 
-- name: verify templated file matches known good
+- name: verify templated file matches known good (DOS)
   assert:
     that:
-#        - 'diff_result.stdout == ""'
-        - 'diff_result.stdout_lines[1] == "FC: no differences encountered"'
+        - '"FC: no differences encountered" in diff_result.stdout'
         - "diff_result.rc == 0"
 
+# UNIX TEMPLATE
+- name: fill in a basic template (Unix)
+  win_template:
+    src: foo.j2
+    dest: '{{ win_output_dir }}/foo.unix.templated'
+    newline_sequence: '\n'
+  register: template_result
+
+- name: verify that the file was marked as changed (Unix)
+  assert:
+    that:
+      - 'template_result|changed'
+
+- name: fill in a basic template again (Unix)
+  win_template:
+    src: foo.j2
+    dest: '{{ win_output_dir }}/foo.unix.templated'
+    newline_sequence: '\n'
+  register: template_result2
+
+- name: verify that the template was not changed (Unix)
+  assert:
+    that:
+      - 'not template_result2|changed'
+
+# VERIFY UNIX CONTENTS
+- name: copy known good into place (Unix)
+  win_copy:
+    src: foo.unix.txt
+    dest: '{{ win_output_dir }}\\foo.unix.txt'
+
+- name: compare templated file to known good (Unix)
+  raw: fc.exe {{ win_output_dir }}\\foo.unix.templated {{ win_output_dir }}\\foo.unix.txt
+  register: diff_result
+
+- debug:
+    var: diff_result
+
+- name: verify templated file matches known good (Unix)
+  assert:
+    that:
+        - '"FC: no differences encountered" in diff_result.stdout'
+
+# VERIFY DOS CONTENTS
+- name: copy known good into place (DOS)
+  win_copy:
+    src: foo.dos.txt
+    dest: '{{ win_output_dir }}\\foo.dos.txt'
+
+- name: compare templated file to known good (DOS)
+  raw: fc.exe {{ win_output_dir }}\\foo.dos.templated {{ win_output_dir }}\\foo.dos.txt
+  register: diff_result
+
+- debug:
+    var: diff_result
+
+- name: verify templated file matches known good (DOS)
+  assert:
+    that:
+        - '"FC: no differences encountered" in diff_result.stdout'
+
 # VERIFY MODE
 # can't set file mode on windows so commenting this test out
 #- name: set file mode

test/integration/targets/win_template/templates/foo.j2

@@ -1 +1,3 @@
+BEGIN
 {{ templated_var }}
+END

test/integration/targets/win_template/templates/foo2.j2

@@ -0,0 +1,3 @@
+BEGIN
+[% templated_var %]
+END

test/sanity/code-smell/line-endings.sh

@@ -4,7 +4,9 @@ grep -rIPl '\r' . \
     --exclude-dir .git \
     --exclude-dir .tox \
     | grep -v -F \
-    -e './test/integration/targets/win_regmerge/templates/win_line_ending.j2'
+    -e './test/integration/targets/template/files/foo.dos.txt' \
+    -e './test/integration/targets/win_regmerge/templates/win_line_ending.j2' \
+    -e './test/integration/targets/win_template/files/foo.dos.txt' \
 
 if [ $? -ne 1 ]; then
     printf 'One or more file(s) listed above have invalid line endings.\n'