Merge pull request #40464 from terminalmage/userdata-renderer

salt-cloud: Do not pass userdata_file through yaml renderer

doc/ref/configuration/master.rst

@@ -1351,6 +1351,23 @@ The renderer to use on the minions to render the state data.
 
     renderer: yaml_jinja
 
+.. conf_master:: userdata_template
+
+``userdata_template``
+---------------------
+
+.. versionadded:: 2016.11.4
+
+Default: ``None``
+
+The renderer to use for templating userdata files in salt-cloud, if the
+``userdata_template`` is not set in the cloud profile. If no value is set in
+the cloud profile or master config file, no templating will be performed.
+
+.. code-block:: yaml
+
+    userdata_template: jinja
+
 .. conf_master:: jinja_trim_blocks
 
 ``jinja_trim_blocks``

doc/topics/cloud/aws.rst

@@ -354,6 +354,35 @@ functionality was added to Salt in the 2015.5.0 release.
       # Pass userdata to the instance to be created
       userdata_file: /etc/salt/my-userdata-file
 
+.. note::
+    From versions 2016.11.0 and 2016.11.3, this file was passed through the
+    master's :conf_master:`renderer` to template it. However, this caused
+    issues with non-YAML data, so templating is no longer performed by default.
+    To template the userdata_file, add a ``userdata_template`` option to the
+    cloud profile:
+
+    .. code-block:: yaml
+
+        my-ec2-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/my-userdata-file
+          userdata_template: jinja
+
+    If no ``userdata_template`` is set in the cloud profile, then the master
+    configuration will be checked for a :conf_master:`userdata_template` value.
+    If this is not set, then no templating will be performed on the
+    userdata_file.
+
+    To disable templating in a cloud profile when a
+    :conf_master:`userdata_template` has been set in the master configuration
+    file, simply set ``userdata_template`` to ``False`` in the cloud profile:
+
+    .. code-block:: yaml
+
+        my-ec2-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/my-userdata-file
+          userdata_template: False
 
 EC2 allows a location to be set for servers to be deployed in. Availability
 zones exist inside regions, and may be added to increase specificity.

doc/topics/cloud/openstack.rst

@@ -153,4 +153,33 @@ cloud-init if available.
 
 .. code-block:: yaml
 
-    userdata_file: /etc/salt/cloud-init/packages.yml
+    my-openstack-config:
+      # Pass userdata to the instance to be created
+      userdata_file: /etc/salt/cloud-init/packages.yml
+
+.. note::
+    As of the 2016.11.4 release, this file can be templated. To use templating,
+    simply specify a ``userdata_template`` option in the cloud profile:
+
+    .. code-block:: yaml
+
+        my-openstack-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/cloud-init/packages.yml
+          userdata_template: jinja
+
+    If no ``userdata_template`` is set in the cloud profile, then the master
+    configuration will be checked for a :conf_master:`userdata_template` value.
+    If this is not set, then no templating will be performed on the
+    userdata_file.
+
+    To disable templating in a cloud profile when a
+    :conf_master:`userdata_template` has been set in the master configuration
+    file, simply set ``userdata_template`` to ``False`` in the cloud profile:
+
+    .. code-block:: yaml
+
+        my-openstack-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/cloud-init/packages.yml
+          userdata_template: False

doc/topics/cloud/windows.rst

@@ -73,12 +73,45 @@ profile configuration as `userdata_file`. For instance:
 
 .. code-block:: yaml
 
-    userdata_file: /etc/salt/windows-firewall.ps1
+    my-ec2-config:
+      # Pass userdata to the instance to be created
+      userdata_file: /etc/salt/windows-firewall.ps1
 
-If you are using WinRM on EC2 the HTTPS port for the WinRM service must also be enabled
-in your userdata. By default EC2 Windows images only have insecure HTTP enabled. To
-enable HTTPS and basic authentication required by pywinrm consider the following
-userdata example:
+.. note::
+    From versions 2016.11.0 and 2016.11.3, this file was passed through the
+    master's :conf_master:`renderer` to template it. However, this caused
+    issues with non-YAML data, so templating is no longer performed by default.
+    To template the userdata_file, add a ``userdata_template`` option to the
+    cloud profile:
+
+    .. code-block:: yaml
+
+        my-ec2-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/windows-firewall.ps1
+          userdata_template: jinja
+
+    If no ``userdata_template`` is set in the cloud profile, then the master
+    configuration will be checked for a :conf_master:`userdata_template` value.
+    If this is not set, then no templating will be performed on the
+    userdata_file.
+
+    To disable templating in a cloud profile when a
+    :conf_master:`userdata_template` has been set in the master configuration
+    file, simply set ``userdata_template`` to ``False`` in the cloud profile:
+
+    .. code-block:: yaml
+
+        my-ec2-config:
+          # Pass userdata to the instance to be created
+          userdata_file: /etc/salt/windows-firewall.ps1
+          userdata_template: False
+
+
+If you are using WinRM on EC2 the HTTPS port for the WinRM service must also be
+enabled in your userdata. By default EC2 Windows images only have insecure HTTP
+enabled. To enable HTTPS and basic authentication required by pywinrm consider
+the following userdata example:
 
 .. code-block:: powershell
 

doc/topics/releases/2016.11.4.rst

@@ -25,3 +25,38 @@ makes cache operations faster. It doesn't make much sence for the ``localfs``
 cache driver but helps for more complex drivers like ``consul``.
 For more details see ``memcache_expire_seconds`` and other ``memcache_*``
 options in the master config reverence.
+
+Salt-Cloud Fixes
+================
+
+2016.11.0 added support for templating userdata files for the :mod:`ec2
+<salt.cloud.clouds.ec2>` driver, using the :conf_master:`renderer` option from
+the master config file. However, as the default renderer first evaluates jinja
+templating, followed by loading the data as a YAML dictionary, this results in
+unpredictable results when userdata files are comprised of non-YAML data (which
+they generally are).
+
+2016.11.4 fixes this by only templating the userdata_file when it is explicitly
+configured to do so. This is done by adding a new optional parameter to the
+cloud profile called ``userdata_template``. This option is used in the same way
+as the ``template`` argument in :py:func:`file.managed
+<salt.states.file.managed>` states, it is simply set to the desired templating
+renderer:
+
+.. code-block:: yaml
+
+    my-ec2-config:
+      # Pass userdata to the instance to be created
+      userdata_file: /etc/salt/my-userdata-file
+      userdata_template: jinja
+
+If no ``userdata_template``  option is set in the cloud profile, then
+salt-cloud will check for the presence of the master configuration parameter
+:conf_master:`userdata_renderer`. If this is also not set, then no templating
+will be performed on the userdata_file.
+
+In addition, the other cloud drivers which support setting a ``userdata_file``
+(:mod:`azurearm <salt.cloud.clouds.azurearm>`, :mod:`nova
+<salt.cloud.clouds.nova>`, and :mod:`openstack <salt.cloud.clouds.openstack>`)
+have had templating support added to bring them to feature parity with the ec2
+driver's implementation of the ``userdata_file`` option.

salt/cloud/clouds/azurearm.py

@@ -881,8 +881,13 @@ def request_instance(call=None, kwargs=None):  # pylint: disable=unused-argument
             with salt.utils.fopen(userdata_file, 'r') as fh_:
                 userdata = fh_.read()
 
+    userdata = salt.utils.cloud.userdata_template(__opts__, vm_, userdata)
+
     if userdata is not None:
-        os_kwargs['custom_data'] = base64.b64encode(userdata)
+        try:
+            os_kwargs['custom_data'] = base64.b64encode(userdata)
+        except Exception as exc:
+            log.exception('Failed to encode userdata: %s', exc)
 
     iface_data = create_interface(kwargs=vm_)
     vm_['iface_id'] = iface_data['id']

salt/cloud/clouds/ec2.py

@@ -93,8 +93,6 @@ import salt.utils
 from salt._compat import ElementTree as ET
 import salt.utils.http as http
 import salt.utils.aws as aws
-import salt.loader
-from salt.template import compile_template
 
 # Import salt.cloud libs
 import salt.utils.cloud
@@ -1691,18 +1689,13 @@ def request_instance(vm_=None, call=None):
             with salt.utils.fopen(userdata_file, 'r') as fh_:
                 userdata = fh_.read()
 
-    if userdata is not None:
-        render_opts = __opts__.copy()
-        render_opts.update(vm_)
-        renderer = __opts__.get('renderer', 'yaml_jinja')
-        rend = salt.loader.render(render_opts, {})
-        blacklist = __opts__['renderer_blacklist']
-        whitelist = __opts__['renderer_whitelist']
-        userdata = compile_template(
-            ':string:', rend, renderer, blacklist, whitelist, input_data=userdata,
-        )
+    userdata = salt.utils.cloud.userdata_template(__opts__, vm_, userdata)
 
-        params[spot_prefix + 'UserData'] = base64.b64encode(userdata)
+    if userdata is not None:
+        try:
+            params[spot_prefix + 'UserData'] = base64.b64encode(userdata)
+        except Exception as exc:
+            log.exception('Failed to encode userdata: %s', exc)
 
     vm_size = config.get_cloud_config_value(
         'size', vm_, __opts__, search_global=False

salt/cloud/clouds/nova.py

@@ -645,12 +645,17 @@ def request_instance(vm_=None, call=None):
                 kwargs['files'][src_path] = files[src_path]
 
     userdata_file = config.get_cloud_config_value(
-        'userdata_file', vm_, __opts__, search_global=False
+        'userdata_file', vm_, __opts__, search_global=False, default=None
     )
-
     if userdata_file is not None:
-        with salt.utils.fopen(userdata_file, 'r') as fp:
-            kwargs['userdata'] = fp.read()
+        try:
+            with salt.utils.fopen(userdata_file, 'r') as fp_:
+                kwargs['userdata'] = salt.utils.cloud.userdata_template(
+                    __opts__, vm_, fp_.read()
+                )
+        except Exception as exc:
+            log.exception(
+                'Failed to read userdata from %s: %s', userdata_file, exc)
 
     kwargs['config_drive'] = config.get_cloud_config_value(
         'config_drive', vm_, __opts__, search_global=False

salt/cloud/clouds/openstack.py

@@ -526,12 +526,17 @@ def request_instance(vm_=None, call=None):
                 kwargs['ex_files'][src_path] = fp_.read()
 
     userdata_file = config.get_cloud_config_value(
-        'userdata_file', vm_, __opts__, search_global=False
+        'userdata_file', vm_, __opts__, search_global=False, default=None
     )
-
     if userdata_file is not None:
-        with salt.utils.fopen(userdata_file, 'r') as fp:
-            kwargs['ex_userdata'] = fp.read()
+        try:
+            with salt.utils.fopen(userdata_file, 'r') as fp_:
+                kwargs['ex_userdata'] = salt.utils.cloud.userdata_template(
+                    __opts__, vm_, fp_.read()
+                )
+        except Exception as exc:
+            log.exception(
+                'Failed to read userdata from %s: %s', userdata_file, exc)
 
     config_drive = config.get_cloud_config_value(
         'config_drive', vm_, __opts__, default=None, search_global=False

salt/template.py

@@ -13,9 +13,10 @@ import logging
 
 # Import salt libs
 import salt.utils
+import salt.utils.stringio
 from salt.utils.odict import OrderedDict
-from salt._compat import string_io
-from salt.ext.six import string_types
+from salt.ext import six
+from salt.ext.six.moves import StringIO
 
 log = logging.getLogger(__name__)
 
@@ -57,7 +58,7 @@ def compile_template(template,
 
     if template != ':string:':
         # Template was specified incorrectly
-        if not isinstance(template, string_types):
+        if not isinstance(template, six.string_types):
             log.error('Template was specified incorrectly: {0}'.format(template))
             return ret
         # Template does not exist
@@ -80,7 +81,9 @@ def compile_template(template,
     # Get the list of render funcs in the render pipe line.
     render_pipe = template_shebang(template, renderers, default, blacklist, whitelist, input_data)
 
-    input_data = string_io(input_data)
+    windows_newline = '\r\n' in input_data
+
+    input_data = StringIO(input_data)
     for render, argline in render_pipe:
         # For GPG renderer, input_data can be an OrderedDict (from YAML) or dict (from py renderer).
         # Repress the error.
@@ -120,6 +123,23 @@ def compile_template(template,
                 # structure. We don't want to log this, so ignore this
                 # exception.
                 pass
+
+    # Preserve newlines from original template
+    if windows_newline:
+        if salt.utils.stringio.is_readable(ret):
+            is_stringio = True
+            contents = ret.read()
+        else:
+            is_stringio = False
+            contents = ret
+
+        if isinstance(contents, six.string_types):
+            if '\r\n' not in contents:
+                contents = contents.replace('\n', '\r\n')
+                ret = StringIO(contents) if is_stringio else contents
+            else:
+                if is_stringio:
+                    ret.seek(0)
     return ret
 
 

salt/utils/cloud.py

@@ -55,6 +55,8 @@ except ImportError:
 import salt.crypt
 import salt.client
 import salt.config
+import salt.loader
+import salt.template
 import salt.utils
 import salt.utils.event
 from salt.utils import vt
@@ -3207,3 +3209,51 @@ def check_key_path_and_mode(provider, key_path):
         return False
 
     return True
+
+
+def userdata_template(opts, vm_, userdata):
+    '''
+    Use the configured templating engine to template the userdata file
+    '''
+    # No userdata, no need to template anything
+    if userdata is None:
+        return userdata
+
+    userdata_template = salt.config.get_cloud_config_value(
+        'userdata_template', vm_, opts, search_global=False, default=None
+    )
+    if userdata_template is False:
+        return userdata
+    # Use the cloud profile's userdata_template, otherwise get it from the
+    # master configuration file.
+    renderer = opts.get('userdata_template') \
+        if userdata_template is None \
+        else userdata_template
+    if renderer is None:
+        return userdata
+    else:
+        render_opts = opts.copy()
+        render_opts.update(vm_)
+        rend = salt.loader.render(render_opts, {})
+        blacklist = opts['renderer_blacklist']
+        whitelist = opts['renderer_whitelist']
+        templated = salt.template.compile_template(
+            ':string:',
+            rend,
+            renderer,
+            blacklist,
+            whitelist,
+            input_data=userdata,
+        )
+        if not isinstance(templated, six.string_types):
+            # template renderers like "jinja" should return a StringIO
+            try:
+                templated = ''.join(templated.readlines())
+            except AttributeError:
+                log.warning(
+                    'Templated userdata resulted in non-string result (%s), '
+                    'converting to string', templated
+                )
+                templated = str(templated)
+
+        return templated

salt/utils/stringio.py

@@ -0,0 +1,39 @@
+# -*- coding: utf-8 -*-
+'''
+Functions for StringIO objects
+'''
+
+from __future__ import absolute_import
+
+# Import 3rd-party libs
+from salt.ext import six
+
+# Not using six's fake cStringIO since we need to be able to tell if the object
+# is readable, and this can't be done via what six exposes.
+if six.PY2:
+    import StringIO
+    import cStringIO
+    readable_types = (StringIO.StringIO, cStringIO.InputType)
+    writable_types = (StringIO.StringIO, cStringIO.OutputType)
+else:
+    import io
+    readable_types = (io.StringIO,)
+    writable_types = (io.StringIO,)
+
+
+def is_stringio(obj):
+    return isinstance(obj, readable_types)
+
+
+def is_readable(obj):
+    if six.PY2:
+        return isinstance(obj, readable_types)
+    else:
+        return isinstance(obj, readable_types) and obj.readable()
+
+
+def is_writable(obj):
+    if six.PY2:
+        return isinstance(obj, writable_types)
+    else:
+        return isinstance(obj, writable_types) and obj.writable()

tests/unit/template_test.py

@@ -7,13 +7,15 @@
 from __future__ import absolute_import
 
 # Import Salt Testing libs
-from salttesting import TestCase
+from salttesting import skipIf, TestCase
 from salttesting.helpers import ensure_in_syspath
+from salttesting.mock import NO_MOCK, NO_MOCK_REASON, MagicMock
 
 ensure_in_syspath('../')
 
 # Import Salt libs
 from salt import template
+from salt.ext.six.moves import StringIO
 
 
 class TemplateTestCase(TestCase):
@@ -29,6 +31,57 @@ class TemplateTestCase(TestCase):
         ret = template.compile_template(['1', '2', '3'], None, None, None, None)
         self.assertDictEqual(ret, {})
 
+    @skipIf(NO_MOCK, NO_MOCK_REASON)
+    def test_compile_template_preserves_windows_newlines(self):
+        '''
+        Test to ensure that a file with Windows newlines, when rendered by a
+        template renderer, does not eat the CR character.
+        '''
+        def _get_rend(renderer, value):
+            '''
+            We need a new MagicMock each time since we're dealing with StringIO
+            objects which are read like files.
+            '''
+            return {renderer: MagicMock(return_value=StringIO(value))}
+
+        input_data_windows = 'foo\r\nbar\r\nbaz\r\n'
+        input_data_non_windows = input_data_windows.replace('\r\n', '\n')
+        renderer = 'test'
+        blacklist = whitelist = []
+
+        ret = template.compile_template(
+            ':string:',
+            _get_rend(renderer, input_data_non_windows),
+            renderer,
+            blacklist,
+            whitelist,
+            input_data=input_data_windows).read()
+        # Even though the mocked renderer returned a string without the windows
+        # newlines, the compiled template should still have them.
+        self.assertEqual(ret, input_data_windows)
+
+        # Now test that we aren't adding them in unnecessarily.
+        ret = template.compile_template(
+            ':string:',
+            _get_rend(renderer, input_data_non_windows),
+            renderer,
+            blacklist,
+            whitelist,
+            input_data=input_data_non_windows).read()
+        self.assertEqual(ret, input_data_non_windows)
+
+        # Finally, ensure that we're not unnecessarily replacing the \n with
+        # \r\n in the event that the renderer returned a string with the
+        # windows newlines intact.
+        ret = template.compile_template(
+            ':string:',
+            _get_rend(renderer, input_data_windows),
+            renderer,
+            blacklist,
+            whitelist,
+            input_data=input_data_windows).read()
+        self.assertEqual(ret, input_data_windows)
+
     def test_check_render_pipe_str(self):
         '''
         Check that all renderers specified in the pipe string are available.