Merge ~chad.smith/cloud-init:ubuntu/devel into cloud-init:ubuntu/devel

[Chad Smith]

Sync tip of master to pull in GCE fix for LP: #1752711 and other changes for Bionic

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:964c869024b422c3966975cc6d23981537084b0a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/804/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/804/rebuild

review: Approve (continuous-integration)

diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py
index 28b1d56..57a170f 100644
--- a/cloudinit/config/cc_puppet.py
+++ b/cloudinit/config/cc_puppet.py
@@ -21,6 +21,13 @@ under ``version``, and defaults to ``none``, which selects the latest version
 in the repos. If the ``puppet`` config key exists in the config archive, this
 module will attempt to start puppet even if no installation was performed.
 
+The module also provides keys for configuring the new puppet 4 paths and
+installing the puppet package from the puppetlabs repositories:
+https://docs.puppet.com/puppet/4.2/reference/whered_it_go.html
+The keys are ``package_name``, ``conf_file`` and ``ssl_dir``. If unset, their
+values will default to ones that work with puppet 3.x and with distributions
+that ship modified puppet 4.x that uses the old paths.
+
 Puppet configuration can be specified under the ``conf`` key. The
 configuration is specified as a dictionary containing high-level ``<section>``
 keys and lists of ``<key>=<value>`` pairs within each section. Each section
@@ -44,6 +51,9 @@ in pem format as a multi-line string (using the ``|`` yaml notation).
     puppet:
         install: <true/false>
         version: <version>
+        conf_file: '/etc/puppet/puppet.conf'
+        ssl_dir: '/var/lib/puppet/ssl'
+        package_name: 'puppet'
         conf:
             agent:
                 server: "puppetmaster.example.org"
@@ -63,9 +73,17 @@ from cloudinit import helpers
 from cloudinit import util
 
 PUPPET_CONF_PATH = '/etc/puppet/puppet.conf'
-PUPPET_SSL_CERT_DIR = '/var/lib/puppet/ssl/certs/'
 PUPPET_SSL_DIR = '/var/lib/puppet/ssl'
-PUPPET_SSL_CERT_PATH = '/var/lib/puppet/ssl/certs/ca.pem'
+PUPPET_PACKAGE_NAME = 'puppet'
+
+
+class PuppetConstants(object):
+
+    def __init__(self, puppet_conf_file, puppet_ssl_dir, log):
+        self.conf_path = puppet_conf_file
+        self.ssl_dir = puppet_ssl_dir
+        self.ssl_cert_dir = os.path.join(puppet_ssl_dir, "certs")
+        self.ssl_cert_path = os.path.join(self.ssl_cert_dir, "ca.pem")
 
 
 def _autostart_puppet(log):
@@ -92,22 +110,29 @@ def handle(name, cfg, cloud, log, _args):
         return
 
     puppet_cfg = cfg['puppet']
-
     # Start by installing the puppet package if necessary...
     install = util.get_cfg_option_bool(puppet_cfg, 'install', True)
     version = util.get_cfg_option_str(puppet_cfg, 'version', None)
+    package_name = util.get_cfg_option_str(
+        puppet_cfg, 'package_name', PUPPET_PACKAGE_NAME)
+    conf_file = util.get_cfg_option_str(
+        puppet_cfg, 'conf_file', PUPPET_CONF_PATH)
+    ssl_dir = util.get_cfg_option_str(puppet_cfg, 'ssl_dir', PUPPET_SSL_DIR)
+
+    p_constants = PuppetConstants(conf_file, ssl_dir, log)
     if not install and version:
         log.warn(("Puppet install set false but version supplied,"
                   " doing nothing."))
     elif install:
         log.debug(("Attempting to install puppet %s,"),
                   version if version else 'latest')
-        cloud.distro.install_packages(('puppet', version))
+
+        cloud.distro.install_packages((package_name, version))
 
     # ... and then update the puppet configuration
     if 'conf' in puppet_cfg:
         # Add all sections from the conf object to puppet.conf
-        contents = util.load_file(PUPPET_CONF_PATH)
+        contents = util.load_file(p_constants.conf_path)
         # Create object for reading puppet.conf values
         puppet_config = helpers.DefaultingConfigParser()
         # Read puppet.conf values from original file in order to be able to
@@ -116,19 +141,19 @@ def handle(name, cfg, cloud, log, _args):
         cleaned_lines = [i.lstrip() for i in contents.splitlines()]
         cleaned_contents = '\n'.join(cleaned_lines)
         puppet_config.readfp(StringIO(cleaned_contents),
-                             filename=PUPPET_CONF_PATH)
+                             filename=p_constants.conf_path)
         for (cfg_name, cfg) in puppet_cfg['conf'].items():
             # Cert configuration is a special case
             # Dump the puppet master ca certificate in the correct place
             if cfg_name == 'ca_cert':
                 # Puppet ssl sub-directory isn't created yet
                 # Create it with the proper permissions and ownership
-                util.ensure_dir(PUPPET_SSL_DIR, 0o771)
-                util.chownbyname(PUPPET_SSL_DIR, 'puppet', 'root')
-                util.ensure_dir(PUPPET_SSL_CERT_DIR)
-                util.chownbyname(PUPPET_SSL_CERT_DIR, 'puppet', 'root')
-                util.write_file(PUPPET_SSL_CERT_PATH, cfg)
-                util.chownbyname(PUPPET_SSL_CERT_PATH, 'puppet', 'root')
+                util.ensure_dir(p_constants.ssl_dir, 0o771)
+                util.chownbyname(p_constants.ssl_dir, 'puppet', 'root')
+                util.ensure_dir(p_constants.ssl_cert_dir)
+                util.chownbyname(p_constants.ssl_cert_dir, 'puppet', 'root')
+                util.write_file(p_constants.ssl_cert_path, cfg)
+                util.chownbyname(p_constants.ssl_cert_path, 'puppet', 'root')
             else:
                 # Iterate through the config items, we'll use ConfigParser.set
                 # to overwrite or create new items as needed
@@ -144,8 +169,9 @@ def handle(name, cfg, cloud, log, _args):
                     puppet_config.set(cfg_name, o, v)
             # We got all our config as wanted we'll rename
             # the previous puppet.conf and create our new one
-            util.rename(PUPPET_CONF_PATH, "%s.old" % (PUPPET_CONF_PATH))
-            util.write_file(PUPPET_CONF_PATH, puppet_config.stringify())
+            util.rename(p_constants.conf_path, "%s.old"
+                        % (p_constants.conf_path))
+            util.write_file(p_constants.conf_path, puppet_config.stringify())
 
     # Set it up so it autostarts
     _autostart_puppet(log)
diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py
index 2b38837..5112a34 100644
--- a/cloudinit/config/cc_salt_minion.py
+++ b/cloudinit/config/cc_salt_minion.py
@@ -25,6 +25,9 @@ specified with ``public_key`` and ``private_key`` respectively.
     salt_minion:
         conf:
             master: salt.example.com
+        grains:
+            role:
+                - web
         public_key: |
             ------BEGIN PUBLIC KEY-------
             <key data>
@@ -65,6 +68,12 @@ def handle(name, cfg, cloud, log, _args):
         minion_data = util.yaml_dumps(salt_cfg.get('conf'))
         util.write_file(minion_config, minion_data)
 
+    if 'grains' in salt_cfg:
+        # add grains to /etc/salt/grains
+        grains_config = os.path.join(config_dir, 'grains')
+        grains_data = util.yaml_dumps(salt_cfg.get('grains'))
+        util.write_file(grains_config, grains_data)
+
     # ... copy the key pair if specified
     if 'public_key' in salt_cfg and 'private_key' in salt_cfg:
         if os.path.isdir("/etc/salt/pki/minion"):
diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py
index 2da34a9..bebc991 100644
--- a/cloudinit/sources/DataSourceGCE.py
+++ b/cloudinit/sources/DataSourceGCE.py
@@ -213,16 +213,15 @@ def read_md(address=None, platform_check=True):
     if md['availability-zone']:
         md['availability-zone'] = md['availability-zone'].split('/')[-1]
 
-    encoding = instance_data.get('user-data-encoding')
-    if encoding:
+    if 'user-data' in instance_data:
+        # instance_data was json, so values are all utf-8 strings.
+        ud = instance_data['user-data'].encode("utf-8")
+        encoding = instance_data.get('user-data-encoding')
         if encoding == 'base64':
-            md['user-data'] = b64decode(instance_data.get('user-data'))
-        else:
+            ud = b64decode(ud)
+        elif encoding:
             LOG.warning('unknown user-data-encoding: %s, ignoring', encoding)
-
-    if 'user-data' in md:
-        ret['user-data'] = md['user-data']
-        del md['user-data']
+        ret['user-data'] = ud
 
     ret['meta-data'] = md
     ret['success'] = True
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 338fb97..02dc2ce 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1746,7 +1746,7 @@ def chmod(path, mode):
 def write_file(filename, content, mode=0o644, omode="wb", copy_mode=False):
     """
     Writes a file with the given content and sets the file mode as specified.
-    Resotres the SELinux context if possible.
+    Restores the SELinux context if possible.
 
     @param filename: The full path of the file to write.
     @param content: The content to write to the file.
@@ -1865,8 +1865,13 @@ def subp(args, data=None, rcs=None, env=None, capture=True, shell=False,
         if not isinstance(data, bytes):
             data = data.encode()
 
+    # Popen converts entries in the arguments array from non-bytes to bytes.
+    # When locale is unset it may use ascii for that encoding which can
+    # cause UnicodeDecodeErrors. (LP: #1751051)
+    bytes_args = [x if isinstance(x, six.binary_type) else x.encode("utf-8")
+                  for x in args]
     try:
-        sp = subprocess.Popen(args, stdout=stdout,
+        sp = subprocess.Popen(bytes_args, stdout=stdout,
                               stderr=stderr, stdin=stdin,
                               env=env, shell=shell)
         (out, err) = sp.communicate(data)
diff --git a/debian/changelog b/debian/changelog
index 2552bb6..27dba2c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,16 @@
+cloud-init (18.1-5-g40e77380-0ubuntu1) bionic; urgency=medium
+
+  * New upstream snapshot.
+    - GCE: fix reading of user-data that is not base64 encoded. (LP: #1752711)
+    - doc: fix chef install from apt packages example in RTD.
+    - Implement puppet 4 support [Romanos Skiadas] (LP: #1446804)
+    - subp: Fix subp usage with non-ascii characters when no system locale.
+      (LP: #1751051)
+    - salt: configure grains in grains file rather than in minion config.
+      [Daniel Wallace]
+
+ -- Chad Smith <[e-mail]>  Thu, 01 Mar 2018 15:47:04 -0700
+
 cloud-init (18.1-0ubuntu1) bionic; urgency=medium
 
   * New upstream snapshot.
diff --git a/doc/examples/cloud-config-chef.txt b/doc/examples/cloud-config-chef.txt
index 58d5fdc..defc5a5 100644
--- a/doc/examples/cloud-config-chef.txt
+++ b/doc/examples/cloud-config-chef.txt
@@ -12,8 +12,8 @@
 
 # Key from https://packages.chef.io/chef.asc
 apt:
-  source1:
-    source: "deb http://packages.chef.io/repos/apt/stable $RELEASE main"
+  sources:
+    source1: "deb http://packages.chef.io/repos/apt/stable $RELEASE main"
     key: |
       -----BEGIN PGP PUBLIC KEY BLOCK-----
       Version: GnuPG v1.4.12 (Darwin)
diff --git a/tests/cloud_tests/testcases/modules/salt_minion.py b/tests/cloud_tests/testcases/modules/salt_minion.py
index c697db2..f13b48a 100644
--- a/tests/cloud_tests/testcases/modules/salt_minion.py
+++ b/tests/cloud_tests/testcases/modules/salt_minion.py
@@ -26,4 +26,9 @@ class Test(base.CloudTestCase):
         self.assertIn('<key data>', out)
         self.assertIn('------END PUBLIC KEY-------', out)
 
+    def test_grains(self):
+        """Test master value in config."""
+        out = self.get_data_file('grains')
+        self.assertIn('role: web', out)
+
 # vi: ts=4 expandtab
diff --git a/tests/cloud_tests/testcases/modules/salt_minion.yaml b/tests/cloud_tests/testcases/modules/salt_minion.yaml
index f20d24f..ab0e05b 100644
--- a/tests/cloud_tests/testcases/modules/salt_minion.yaml
+++ b/tests/cloud_tests/testcases/modules/salt_minion.yaml
@@ -17,6 +17,8 @@ cloud_config: |
           ------BEGIN PRIVATE KEY------
           <key data>
           ------END PRIVATE KEY-------
+      grains:
+          role: web
 collect_scripts:
   minion: |
     #!/bin/bash
@@ -30,5 +32,8 @@ collect_scripts:
   minion.pub: |
     #!/bin/bash
     cat /etc/salt/pki/minion/minion.pub
+  grains: |
+    #!/bin/bash
+    cat /etc/salt/grains
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py
index f77c2c4..eb3cec4 100644
--- a/tests/unittests/test_datasource/test_gce.py
+++ b/tests/unittests/test_datasource/test_gce.py
@@ -38,11 +38,20 @@ GCE_META_ENCODING = {
     'instance/hostname': 'server.project-baz.local',
     'instance/zone': 'baz/bang',
     'instance/attributes': {
-        'user-data': b64encode(b'/bin/echo baz\n').decode('utf-8'),
+        'user-data': b64encode(b'#!/bin/echo baz\n').decode('utf-8'),
         'user-data-encoding': 'base64',
     }
 }
 
+GCE_USER_DATA_TEXT = {
+    'instance/id': '12345',
+    'instance/hostname': 'server.project-baz.local',
+    'instance/zone': 'baz/bang',
+    'instance/attributes': {
+        'user-data': '#!/bin/sh\necho hi mom\ntouch /run/up-now\n',
+    }
+}
+
 HEADERS = {'Metadata-Flavor': 'Google'}
 MD_URL_RE = re.compile(
     r'http://metadata.google.internal/computeMetadata/v1/.*')
@@ -135,7 +144,16 @@ class TestDataSourceGCE(test_helpers.HttprettyTestCase):
         shostname = GCE_META_PARTIAL.get('instance/hostname').split('.')[0]
         self.assertEqual(shostname, self.ds.get_hostname())
 
+    def test_userdata_no_encoding(self):
+        """check that user-data is read."""
+        _set_mock_metadata(GCE_USER_DATA_TEXT)
+        self.ds.get_data()
+        self.assertEqual(
+            GCE_USER_DATA_TEXT['instance/attributes']['user-data'].encode(),
+            self.ds.get_userdata_raw())
+
     def test_metadata_encoding(self):
+        """user-data is base64 encoded if user-data-encoding is 'base64'."""
         _set_mock_metadata(GCE_META_ENCODING)
         self.ds.get_data()
 
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 4a92e74..89ae40f 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -8,7 +8,9 @@ import shutil
 import stat
 import tempfile
 
+import json
 import six
+import sys
 import yaml
 
 from cloudinit import importer, util
@@ -733,6 +735,38 @@ class TestSubp(helpers.CiTestCase):
         self.assertEqual("/target/my/path/",
                          util.target_path("/target/", "///my/path/"))
 
+    def test_c_lang_can_take_utf8_args(self):
+        """Independent of system LC_CTYPE, args can contain utf-8 strings.
+
+        When python starts up, its default encoding gets set based on
+        the value of LC_CTYPE.  If no system locale is set, the default
+        encoding for both python2 and python3 in some paths will end up
+        being ascii.
+
+        Attempts to use setlocale or patching (or changing) os.environ
+        in the current environment seem to not be effective.
+
+        This test starts up a python with LC_CTYPE set to C so that
+        the default encoding will be set to ascii.  In such an environment
+        Popen(['command', 'non-ascii-arg']) would cause a UnicodeDecodeError.
+        """
+        python_prog = '\n'.join([
+            'import json, sys',
+            'from cloudinit.util import subp',
+            'data = sys.stdin.read()',
+            'cmd = json.loads(data)',
+            'subp(cmd, capture=False)',
+            ''])
+        cmd = [BASH, '-c', 'echo -n "$@"', '--',
+               self.utf8_valid.decode("utf-8")]
+        python_subp = [sys.executable, '-c', python_prog]
+
+        out, _err = util.subp(
+            python_subp, update_env={'LC_CTYPE': 'C'},
+            data=json.dumps(cmd).encode("utf-8"),
+            decode=False)
+        self.assertEqual(self.utf8_valid, out)
+
 
 class TestEncode(helpers.TestCase):
     """Test the encoding functions"""