From 0a448dd034883c07f85091dbfc9117de7227eb8d Mon Sep 17 00:00:00 2001
From: Chad Smith <chad.smith@canonical.com>
Date: Thu, 25 May 2017 11:04:55 -0600
Subject: ntp: Add schema definition and passive schema validation.

cloud-config files are very flexible and permissive.
This adds a jsonsschema definition to the cc_ntp module and validation
functions in cloudinit/config/schema which will log warnings about
invalid configuration values in the ntp section.

A cmdline tools/cloudconfig-schema is added which can be used in our dev
environments to quickly attempt to exercise the ntp schema.
It is also exposed as a main in cloudinit.config.schema.
(python3 -m cloudinit.config.schema)

LP: #1692916
---
 tests/unittests/test_handler/test_handler_ntp.py | 109 +++++++++++
 tests/unittests/test_handler/test_schema.py      | 220 +++++++++++++++++++++++
 2 files changed, 329 insertions(+)
 create mode 100644 tests/unittests/test_handler/test_schema.py

(limited to 'tests/unittests/test_handler')

diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index bc4277b7..6cafa63d 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -212,4 +212,113 @@ class TestNtp(FilesystemMockingTestCase):
             'Skipping module named cc_ntp, not present or disabled by cfg\n',
             self.logs.getvalue())
 
+    def test_ntp_handler_schema_validation_allows_empty_ntp_config(self):
+        """Ntp schema validation allows for an empty ntp: configuration."""
+        invalid_config = {'ntp': {}}
+        distro = 'ubuntu'
+        cc = self._get_cloud(distro)
+        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
+        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+            stream.write(NTP_TEMPLATE)
+        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        self.assertNotIn('Invalid config:', self.logs.getvalue())
+        with open(ntp_conf) as stream:
+            content = stream.read()
+        default_pools = [
+            "{0}.{1}.pool.ntp.org".format(x, distro)
+            for x in range(0, cc_ntp.NR_POOL_SERVERS)]
+        self.assertEqual(
+            "servers []\npools {0}\n".format(default_pools),
+            content)
+
+    def test_ntp_handler_schema_validation_warns_non_string_item_type(self):
+        """Ntp schema validation warns of non-strings in pools or servers.
+
+        Schema validation is not strict, so ntp config is still be rendered.
+        """
+        invalid_config = {'ntp': {'pools': [123], 'servers': ['valid', None]}}
+        cc = self._get_cloud('ubuntu')
+        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
+        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+            stream.write(NTP_TEMPLATE)
+        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        self.assertIn(
+            "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n"
+            "ntp.servers.1: None is not of type 'string'",
+            self.logs.getvalue())
+        with open(ntp_conf) as stream:
+            content = stream.read()
+        self.assertEqual("servers ['valid', None]\npools [123]\n", content)
+
+    def test_ntp_handler_schema_validation_warns_of_non_array_type(self):
+        """Ntp schema validation warns of non-array pools or servers types.
+
+        Schema validation is not strict, so ntp config is still be rendered.
+        """
+        invalid_config = {'ntp': {'pools': 123, 'servers': 'non-array'}}
+        cc = self._get_cloud('ubuntu')
+        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
+        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+            stream.write(NTP_TEMPLATE)
+        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        self.assertIn(
+            "Invalid config:\nntp.pools: 123 is not of type 'array'\n"
+            "ntp.servers: 'non-array' is not of type 'array'",
+            self.logs.getvalue())
+        with open(ntp_conf) as stream:
+            content = stream.read()
+        self.assertEqual("servers non-array\npools 123\n", content)
+
+    def test_ntp_handler_schema_validation_warns_invalid_key_present(self):
+        """Ntp schema validation warns of invalid keys present in ntp config.
+
+        Schema validation is not strict, so ntp config is still be rendered.
+        """
+        invalid_config = {
+            'ntp': {'invalidkey': 1, 'pools': ['0.mycompany.pool.ntp.org']}}
+        cc = self._get_cloud('ubuntu')
+        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
+        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+            stream.write(NTP_TEMPLATE)
+        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        self.assertIn(
+            "Invalid config:\nntp: Additional properties are not allowed "
+            "('invalidkey' was unexpected)",
+            self.logs.getvalue())
+        with open(ntp_conf) as stream:
+            content = stream.read()
+        self.assertEqual(
+            "servers []\npools ['0.mycompany.pool.ntp.org']\n",
+            content)
+
+    def test_ntp_handler_schema_validation_warns_of_duplicates(self):
+        """Ntp schema validation warns of duplicates in servers or pools.
+
+        Schema validation is not strict, so ntp config is still be rendered.
+        """
+        invalid_config = {
+            'ntp': {'pools': ['0.mypool.org', '0.mypool.org'],
+                    'servers': ['10.0.0.1', '10.0.0.1']}}
+        cc = self._get_cloud('ubuntu')
+        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
+        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
+            stream.write(NTP_TEMPLATE)
+        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
+            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
+        self.assertIn(
+            "Invalid config:\nntp.pools: ['0.mypool.org', '0.mypool.org'] has "
+            "non-unique elements\nntp.servers: ['10.0.0.1', '10.0.0.1'] has "
+            "non-unique elements",
+            self.logs.getvalue())
+        with open(ntp_conf) as stream:
+            content = stream.read()
+        self.assertEqual(
+            "servers ['10.0.0.1', '10.0.0.1']\n"
+            "pools ['0.mypool.org', '0.mypool.org']\n",
+            content)
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
new file mode 100644
index 00000000..3239e326
--- /dev/null
+++ b/tests/unittests/test_handler/test_schema.py
@@ -0,0 +1,220 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from cloudinit.config.schema import (
+    CLOUD_CONFIG_HEADER, SchemaValidationError, get_schema_doc,
+    validate_cloudconfig_file, validate_cloudconfig_schema,
+    main)
+from cloudinit.util import write_file
+
+from ..helpers import CiTestCase, mock
+
+from copy import copy
+from six import StringIO
+from textwrap import dedent
+
+
+class SchemaValidationErrorTest(CiTestCase):
+    """Test validate_cloudconfig_schema"""
+
+    def test_schema_validation_error_expects_schema_errors(self):
+        """SchemaValidationError is initialized from schema_errors."""
+        errors = (('key.path', 'unexpected key "junk"'),
+                  ('key2.path', '"-123" is not a valid "hostname" format'))
+        exception = SchemaValidationError(schema_errors=errors)
+        self.assertIsInstance(exception, Exception)
+        self.assertEqual(exception.schema_errors, errors)
+        self.assertEqual(
+            'Cloud config schema errors: key.path: unexpected key "junk", '
+            'key2.path: "-123" is not a valid "hostname" format',
+            str(exception))
+        self.assertTrue(isinstance(exception, ValueError))
+
+
+class ValidateCloudConfigSchemaTest(CiTestCase):
+    """Tests for validate_cloudconfig_schema."""
+
+    with_logs = True
+
+    def test_validateconfig_schema_non_strict_emits_warnings(self):
+        """When strict is False validate_cloudconfig_schema emits warnings."""
+        schema = {'properties': {'p1': {'type': 'string'}}}
+        validate_cloudconfig_schema({'p1': -1}, schema, strict=False)
+        self.assertIn(
+            "Invalid config:\np1: -1 is not of type 'string'\n",
+            self.logs.getvalue())
+
+    def test_validateconfig_schema_emits_warning_on_missing_jsonschema(self):
+        """Warning from validate_cloudconfig_schema when missing jsonschema."""
+        schema = {'properties': {'p1': {'type': 'string'}}}
+        with mock.patch.dict('sys.modules', **{'jsonschema': ImportError()}):
+            validate_cloudconfig_schema({'p1': -1}, schema, strict=True)
+        self.assertIn(
+            'Ignoring schema validation. python-jsonschema is not present',
+            self.logs.getvalue())
+
+    def test_validateconfig_schema_strict_raises_errors(self):
+        """When strict is True validate_cloudconfig_schema raises errors."""
+        schema = {'properties': {'p1': {'type': 'string'}}}
+        with self.assertRaises(SchemaValidationError) as context_mgr:
+            validate_cloudconfig_schema({'p1': -1}, schema, strict=True)
+        self.assertEqual(
+            "Cloud config schema errors: p1: -1 is not of type 'string'",
+            str(context_mgr.exception))
+
+    def test_validateconfig_schema_honors_formats(self):
+        """When strict is True validate_cloudconfig_schema raises errors."""
+        schema = {
+            'properties': {'p1': {'type': 'string', 'format': 'hostname'}}}
+        with self.assertRaises(SchemaValidationError) as context_mgr:
+            validate_cloudconfig_schema({'p1': '-1'}, schema, strict=True)
+        self.assertEqual(
+            "Cloud config schema errors: p1: '-1' is not a 'hostname'",
+            str(context_mgr.exception))
+
+
+class ValidateCloudConfigFileTest(CiTestCase):
+    """Tests for validate_cloudconfig_file."""
+
+    def setUp(self):
+        super(ValidateCloudConfigFileTest, self).setUp()
+        self.config_file = self.tmp_path('cloudcfg.yaml')
+
+    def test_validateconfig_file_error_on_absent_file(self):
+        """On absent config_path, validate_cloudconfig_file errors."""
+        with self.assertRaises(RuntimeError) as context_mgr:
+            validate_cloudconfig_file('/not/here', {})
+        self.assertEqual(
+            'Configfile /not/here does not exist',
+            str(context_mgr.exception))
+
+    def test_validateconfig_file_error_on_invalid_header(self):
+        """On invalid header, validate_cloudconfig_file errors.
+
+        A SchemaValidationError is raised when the file doesn't begin with
+        CLOUD_CONFIG_HEADER.
+        """
+        write_file(self.config_file, '#junk')
+        with self.assertRaises(SchemaValidationError) as context_mgr:
+            validate_cloudconfig_file(self.config_file, {})
+        self.assertEqual(
+            'Cloud config schema errors: header: File {0} needs to begin with '
+            '"{1}"'.format(self.config_file, CLOUD_CONFIG_HEADER.decode()),
+            str(context_mgr.exception))
+
+    def test_validateconfig_file_error_on_non_yaml_format(self):
+        """On non-yaml format, validate_cloudconfig_file errors."""
+        write_file(self.config_file, '#cloud-config\n{}}')
+        with self.assertRaises(SchemaValidationError) as context_mgr:
+            validate_cloudconfig_file(self.config_file, {})
+        self.assertIn(
+            'schema errors: format: File {0} is not valid yaml.'.format(
+                self.config_file),
+            str(context_mgr.exception))
+
+    def test_validateconfig_file_sctricty_validates_schema(self):
+        """validate_cloudconfig_file raises errors on invalid schema."""
+        schema = {
+            'properties': {'p1': {'type': 'string', 'format': 'hostname'}}}
+        write_file(self.config_file, '#cloud-config\np1: "-1"')
+        with self.assertRaises(SchemaValidationError) as context_mgr:
+            validate_cloudconfig_file(self.config_file, schema)
+        self.assertEqual(
+            "Cloud config schema errors: p1: '-1' is not a 'hostname'",
+            str(context_mgr.exception))
+
+
+class GetSchemaDocTest(CiTestCase):
+    """Tests for get_schema_doc."""
+
+    def setUp(self):
+        super(GetSchemaDocTest, self).setUp()
+        self.required_schema = {
+            'title': 'title', 'description': 'description', 'id': 'id',
+            'name': 'name', 'frequency': 'frequency',
+            'distros': ['debian', 'rhel']}
+
+    def test_get_schema_doc_returns_restructured_text(self):
+        """get_schema_doc returns restructured text for a cloudinit schema."""
+        full_schema = copy(self.required_schema)
+        full_schema.update(
+            {'properties': {
+                'prop1': {'type': 'array', 'description': 'prop-description',
+                          'items': {'type': 'int'}}}})
+        self.assertEqual(
+            dedent("""
+                name
+                ---
+                **Summary:** title
+
+                description
+
+                **Internal name:** ``id``
+
+                **Module frequency:** frequency
+
+                **Supported distros:** debian, rhel
+
+                **Config schema**:
+                    **prop1:** (array of int) prop-description\n\n"""),
+            get_schema_doc(full_schema))
+
+    def test_get_schema_doc_returns_restructured_text_with_examples(self):
+        """get_schema_doc returns indented examples when present in schema."""
+        full_schema = copy(self.required_schema)
+        full_schema.update(
+            {'examples': {'ex1': [1, 2, 3]},
+             'properties': {
+                'prop1': {'type': 'array', 'description': 'prop-description',
+                          'items': {'type': 'int'}}}})
+        self.assertIn(
+            dedent("""
+                **Config schema**:
+                    **prop1:** (array of int) prop-description
+
+                **Examples**::
+
+                    ex1"""),
+            get_schema_doc(full_schema))
+
+    def test_get_schema_doc_raises_key_errors(self):
+        """get_schema_doc raises KeyErrors on missing keys."""
+        for key in self.required_schema:
+            invalid_schema = copy(self.required_schema)
+            invalid_schema.pop(key)
+            with self.assertRaises(KeyError) as context_mgr:
+                get_schema_doc(invalid_schema)
+            self.assertEqual("'{0}'".format(key), str(context_mgr.exception))
+
+
+class MainTest(CiTestCase):
+
+    def test_main_missing_args(self):
+        """Main exits non-zero and reports an error on missing parameters."""
+        with mock.patch('sys.argv', ['mycmd']):
+            with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr:
+                self.assertEqual(1, main(), 'Expected non-zero exit code')
+        self.assertEqual(
+            'Expected either --config-file argument or --doc\n',
+            m_stderr.getvalue())
+
+    def test_main_prints_docs(self):
+        """When --doc parameter is provided, main generates documentation."""
+        myargs = ['mycmd', '--doc']
+        with mock.patch('sys.argv', myargs):
+            with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
+                self.assertEqual(0, main(), 'Expected 0 exit code')
+        self.assertIn('\nNTP\n---\n', m_stdout.getvalue())
+
+    def test_main_validates_config_file(self):
+        """When --config-file parameter is provided, main validates schema."""
+        myyaml = self.tmp_path('my.yaml')
+        myargs = ['mycmd', '--config-file', myyaml]
+        with open(myyaml, 'wb') as stream:
+            stream.write(b'#cloud-config\nntp:')   # shortest ntp schema
+        with mock.patch('sys.argv', myargs):
+            with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout:
+                self.assertEqual(0, main(), 'Expected 0 exit code')
+        self.assertIn(
+            'Valid cloud-config file {0}'.format(myyaml), m_stdout.getvalue())
+
+# vi: ts=4 expandtab syntax=python
-- 
cgit v1.2.3


From a62a94b4edd7c61a268350c84e43b0aa8f68b0c2 Mon Sep 17 00:00:00 2001
From: Chad Smith <chad.smith@canonical.com>
Date: Fri, 2 Jun 2017 14:42:26 -0600
Subject: Tests: Skip jsonschema related unit tests when dependency is absent.

On some build environments we don't have python-jsonschema installed.
Since this dependency is an optional runtime dependency, we can also make
it an optional unit test dependency. Add a skip of related unittests when
jsonschema is not present.

Also, KeyError messages on CentOs don't have single quotes around the
missing 'key-name'. Make our KeyError assertion a bit more flexible with
the assertIn call.

LP: #1695318
---
 tests/unittests/test_handler/test_handler_ntp.py | 12 +++++++++++-
 tests/unittests/test_handler/test_schema.py      | 17 ++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

(limited to 'tests/unittests/test_handler')

diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 6cafa63d..25d7365a 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -3,7 +3,7 @@
 from cloudinit.config import cc_ntp
 from cloudinit.sources import DataSourceNone
 from cloudinit import (distros, helpers, cloud, util)
-from ..helpers import FilesystemMockingTestCase, mock
+from ..helpers import FilesystemMockingTestCase, mock, skipIf
 
 
 import os
@@ -16,6 +16,12 @@ servers {{servers}}
 pools {{pools}}
 """
 
+try:
+    import jsonschema  # NOQA
+    _missing_jsonschema_dep = False
+except ImportError:
+    _missing_jsonschema_dep = True
+
 
 class TestNtp(FilesystemMockingTestCase):
 
@@ -232,6 +238,7 @@ class TestNtp(FilesystemMockingTestCase):
             "servers []\npools {0}\n".format(default_pools),
             content)
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_non_string_item_type(self):
         """Ntp schema validation warns of non-strings in pools or servers.
 
@@ -252,6 +259,7 @@ class TestNtp(FilesystemMockingTestCase):
             content = stream.read()
         self.assertEqual("servers ['valid', None]\npools [123]\n", content)
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_of_non_array_type(self):
         """Ntp schema validation warns of non-array pools or servers types.
 
@@ -272,6 +280,7 @@ class TestNtp(FilesystemMockingTestCase):
             content = stream.read()
         self.assertEqual("servers non-array\npools 123\n", content)
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_invalid_key_present(self):
         """Ntp schema validation warns of invalid keys present in ntp config.
 
@@ -295,6 +304,7 @@ class TestNtp(FilesystemMockingTestCase):
             "servers []\npools ['0.mycompany.pool.ntp.org']\n",
             content)
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_of_duplicates(self):
         """Ntp schema validation warns of duplicates in servers or pools.
 
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index 3239e326..fd0e4f5e 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -6,12 +6,18 @@ from cloudinit.config.schema import (
     main)
 from cloudinit.util import write_file
 
-from ..helpers import CiTestCase, mock
+from ..helpers import CiTestCase, mock, skipIf
 
 from copy import copy
 from six import StringIO
 from textwrap import dedent
 
+try:
+    import jsonschema  # NOQA
+    _missing_jsonschema_dep = False
+except ImportError:
+    _missing_jsonschema_dep = True
+
 
 class SchemaValidationErrorTest(CiTestCase):
     """Test validate_cloudconfig_schema"""
@@ -35,6 +41,7 @@ class ValidateCloudConfigSchemaTest(CiTestCase):
 
     with_logs = True
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_validateconfig_schema_non_strict_emits_warnings(self):
         """When strict is False validate_cloudconfig_schema emits warnings."""
         schema = {'properties': {'p1': {'type': 'string'}}}
@@ -43,6 +50,7 @@ class ValidateCloudConfigSchemaTest(CiTestCase):
             "Invalid config:\np1: -1 is not of type 'string'\n",
             self.logs.getvalue())
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_validateconfig_schema_emits_warning_on_missing_jsonschema(self):
         """Warning from validate_cloudconfig_schema when missing jsonschema."""
         schema = {'properties': {'p1': {'type': 'string'}}}
@@ -52,6 +60,7 @@ class ValidateCloudConfigSchemaTest(CiTestCase):
             'Ignoring schema validation. python-jsonschema is not present',
             self.logs.getvalue())
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_validateconfig_schema_strict_raises_errors(self):
         """When strict is True validate_cloudconfig_schema raises errors."""
         schema = {'properties': {'p1': {'type': 'string'}}}
@@ -61,8 +70,9 @@ class ValidateCloudConfigSchemaTest(CiTestCase):
             "Cloud config schema errors: p1: -1 is not of type 'string'",
             str(context_mgr.exception))
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_validateconfig_schema_honors_formats(self):
-        """When strict is True validate_cloudconfig_schema raises errors."""
+        """With strict True, validate_cloudconfig_schema errors on format."""
         schema = {
             'properties': {'p1': {'type': 'string', 'format': 'hostname'}}}
         with self.assertRaises(SchemaValidationError) as context_mgr:
@@ -111,6 +121,7 @@ class ValidateCloudConfigFileTest(CiTestCase):
                 self.config_file),
             str(context_mgr.exception))
 
+    @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_validateconfig_file_sctricty_validates_schema(self):
         """validate_cloudconfig_file raises errors on invalid schema."""
         schema = {
@@ -183,7 +194,7 @@ class GetSchemaDocTest(CiTestCase):
             invalid_schema.pop(key)
             with self.assertRaises(KeyError) as context_mgr:
                 get_schema_doc(invalid_schema)
-            self.assertEqual("'{0}'".format(key), str(context_mgr.exception))
+            self.assertIn(key, str(context_mgr.exception))
 
 
 class MainTest(CiTestCase):
-- 
cgit v1.2.3


From 8d58f12248c0bd9e8f88296f29935fd3dc33b415 Mon Sep 17 00:00:00 2001
From: Joshua Powers <josh.powers@canonical.com>
Date: Wed, 7 Jun 2017 14:39:18 -0600
Subject: test: Fix pyflakes complaint of unused import.

The jsonschema package is used only when available, but the lint
check thinks the import is unused across pyflakes and flake8. In
order to avoid having exceptions for both assert that the import
works right after and the import is considered used.

The '# NOQA' doesn't affect pyflakes (only flake8).

LP: #1695918
---
 tests/unittests/test_handler/test_handler_ntp.py | 3 ++-
 tests/unittests/test_handler/test_schema.py      | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

(limited to 'tests/unittests/test_handler')

diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 25d7365a..3a9f7f7e 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -17,7 +17,8 @@ pools {{pools}}
 """
 
 try:
-    import jsonschema  # NOQA
+    import jsonschema
+    assert jsonschema  # avoid pyflakes error F401: import unused
     _missing_jsonschema_dep = False
 except ImportError:
     _missing_jsonschema_dep = True
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index fd0e4f5e..eda4802a 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -13,7 +13,8 @@ from six import StringIO
 from textwrap import dedent
 
 try:
-    import jsonschema  # NOQA
+    import jsonschema
+    assert jsonschema  # avoid pyflakes error F401: import unused
     _missing_jsonschema_dep = False
 except ImportError:
     _missing_jsonschema_dep = True
-- 
cgit v1.2.3


From 55a006afca73633c607c537dee62097e85011443 Mon Sep 17 00:00:00 2001
From: Scott Moser <smoser@brickies.net>
Date: Wed, 14 Jun 2017 09:33:54 -0400
Subject: tools/run-centos: cleanups and move to using read-dependencies

These changes are all in an effort to get tools/run-centos using
read-dependencies rather than the 'setup-centos' script with a separate
set of dependencies listed.

 - tools/read-dependencies: support taking multiple --requirements
   options.  This allows run-centos to get both test and build
   dependencies.  Ultimately, I think it might be nicer for
   read-dependencies to take a list of "goals" (build, test, run or
   test-tox) rather than having the caller need to know to provide
   multiple --requirements.

 - packages/pkg-deps.json: drop the version on the sudo package.
   centos 6 has newer (1.8.6p3) version than listed, so its not a problem.

 - test_handler_disk_setup.py: a test case here was using assertLogs
   which is not present in the version of unittest2 that is available in
   centos 6 epel. We just adjust it to use with_logs = True.

 - tools/run-cents:
   - improve usage with example
   - add 'inside_as_cd' to provide the dir you want to cd first to.
   - avoid the intermediate tarball on disk in the container.
   - add 'prep' subcommand and use it to install pre-dependencies.
   - use read-dependencies.
---
 packages/pkg-deps.json                             |   2 +-
 .../test_handler/test_handler_disk_setup.py        |  32 ++---
 tools/read-dependencies                            |  32 +++--
 tools/run-centos                                   | 142 ++++++++++++++-------
 tools/setup-centos                                 |  49 -------
 5 files changed, 139 insertions(+), 118 deletions(-)
 delete mode 100755 tools/setup-centos

(limited to 'tests/unittests/test_handler')

diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json
index 8b8f3c37..822d29d9 100644
--- a/packages/pkg-deps.json
+++ b/packages/pkg-deps.json
@@ -62,7 +62,7 @@
          "procps",
          "rsyslog",
          "shadow-utils",
-         "sudo >= 1.7.2p2-3"
+         "sudo"
       ]
    },
    "suse" : {
diff --git a/tests/unittests/test_handler/test_handler_disk_setup.py b/tests/unittests/test_handler/test_handler_disk_setup.py
index 916a0d7a..8a6d49ed 100644
--- a/tests/unittests/test_handler/test_handler_disk_setup.py
+++ b/tests/unittests/test_handler/test_handler_disk_setup.py
@@ -3,7 +3,7 @@
 import random
 
 from cloudinit.config import cc_disk_setup
-from ..helpers import ExitStack, mock, TestCase
+from ..helpers import CiTestCase, ExitStack, mock, TestCase
 
 
 class TestIsDiskUsed(TestCase):
@@ -174,32 +174,32 @@ class TestUpdateFsSetupDevices(TestCase):
             return_value=('/dev/xdb1', False))
 @mock.patch('cloudinit.config.cc_disk_setup.device_type', return_value=None)
 @mock.patch('cloudinit.config.cc_disk_setup.util.subp', return_value=('', ''))
-class TestMkfsCommandHandling(TestCase):
+class TestMkfsCommandHandling(CiTestCase):
+
+    with_logs = True
 
     def test_with_cmd(self, subp, *args):
         """mkfs honors cmd and logs warnings when extra_opts or overwrite are
         provided."""
-        with self.assertLogs(
-                'cloudinit.config.cc_disk_setup') as logs:
-            cc_disk_setup.mkfs({
-                'cmd': 'mkfs -t %(filesystem)s -L %(label)s %(device)s',
-                'filesystem': 'ext4',
-                'device': '/dev/xdb1',
-                'label': 'with_cmd',
-                'extra_opts': ['should', 'generate', 'warning'],
-                'overwrite': 'should generate warning too'
-            })
+        cc_disk_setup.mkfs({
+            'cmd': 'mkfs -t %(filesystem)s -L %(label)s %(device)s',
+            'filesystem': 'ext4',
+            'device': '/dev/xdb1',
+            'label': 'with_cmd',
+            'extra_opts': ['should', 'generate', 'warning'],
+            'overwrite': 'should generate warning too'
+        })
 
         self.assertIn(
-            'WARNING:cloudinit.config.cc_disk_setup:fs_setup:extra_opts ' +
+            'extra_opts ' +
             'ignored because cmd was specified: mkfs -t ext4 -L with_cmd ' +
             '/dev/xdb1',
-            logs.output)
+            self.logs.getvalue())
         self.assertIn(
-            'WARNING:cloudinit.config.cc_disk_setup:fs_setup:overwrite ' +
+            'overwrite ' +
             'ignored because cmd was specified: mkfs -t ext4 -L with_cmd ' +
             '/dev/xdb1',
-            logs.output)
+            self.logs.getvalue())
 
         subp.assert_called_once_with(
             'mkfs -t ext4 -L with_cmd /dev/xdb1', shell=True)
diff --git a/tools/read-dependencies b/tools/read-dependencies
index 4ba2c1bc..8a585343 100755
--- a/tools/read-dependencies
+++ b/tools/read-dependencies
@@ -18,6 +18,7 @@ import re
 import subprocess
 import sys
 
+DEFAULT_REQUIREMENTS = 'requirements.txt'
 
 # Map the appropriate package dir needed for each distro choice
 DISTRO_PKG_TYPE_MAP = {
@@ -51,8 +52,9 @@ def get_parser():
     """Return an argument parser for this command."""
     parser = ArgumentParser(description=__doc__)
     parser.add_argument(
-        '-r', '--requirements-file', type=str, dest='req_file',
-        default='requirements.txt', help='The pip-style requirements file')
+        '-r', '--requirements-file', type=str, dest='req_files',
+        action='append', default=None,
+        help='pip-style requirements file [default=%s]' % DEFAULT_REQUIREMENTS)
     parser.add_argument(
         '-d', '--distro', type=str, choices=DISTRO_PKG_TYPE_MAP.keys(),
         help='The name of the distro to generate package deps for.')
@@ -144,12 +146,24 @@ def main(distro):
         topd = os.path.realpath(os.environ.get('CLOUD_INIT_TOP_D'))
     else:
         topd = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
-    req_path = os.path.join(topd, args.req_file)
-    if not os.path.isfile(req_path):
-        sys.stderr.write("Unable to locate '%s' file that should "
-                         "exist in cloud-init root directory." % req_path)
-        return 1
-    pip_pkg_names = parse_pip_requirements(req_path)
+
+    if args.req_files is None:
+        args.req_files = [os.path.join(topd, DEFAULT_REQUIREMENTS)]
+        if not os.path.isfile(args.req_files[0]):
+            sys.stderr.write("Unable to locate '%s' file that should "
+                             "exist in cloud-init root directory." %
+                             args.req_files[0])
+            sys.exit(1)
+
+    bad_files = [r for r in args.req_files if not os.path.isfile(r)]
+    if bad_files:
+        sys.stderr.write(
+            "Unable to find requirements files: %s\n" % ','.join(bad_files))
+        sys.exit(1)
+
+    pip_pkg_names = set()
+    for req_path in args.req_files:
+        pip_pkg_names.update(set(parse_pip_requirements(req_path)))
     deps_from_json = get_package_deps_from_json(topd, args.distro)
     renames = deps_from_json.get('renames', {})
     translated_pip_names = translate_pip_to_system_pkg(
@@ -174,7 +188,7 @@ def pkg_install(pkg_list, distro, dry_run=False):
     """Install a list of packages using the DISTRO_INSTALL_PKG_CMD."""
     print('Installing deps: {0}{1}'.format(
           '(dryrun)' if dry_run else '', ' '.join(pkg_list)))
-    pkg_list.extend(EXTRA_SYSTEM_BASE_PKGS)
+    pkg_list = list(pkg_list) + EXTRA_SYSTEM_BASE_PKGS
     install_cmd = []
     if dry_run:
        install_cmd.append('echo')
diff --git a/tools/run-centos b/tools/run-centos
index de21d756..99ba6be0 100755
--- a/tools/run-centos
+++ b/tools/run-centos
@@ -14,17 +14,22 @@ errorrc() { local r=$?; error "$@" "ret=$r"; return $r; }
 
 Usage() {
     cat <<EOF
-Usage: ${0##*/} [ options ] CentOS version
+Usage: ${0##*/} [ options ] version
 
     This utility can makes it easier to run tests, build rpm and source rpm
         generation inside a LXC of the specified version of CentOS.
 
+    version is major release number (6 or 7)
+
     options:
       -a | --artifact keep .rpm artifacts
       -k | --keep     keep container after tests
       -r | --rpm      build .rpm
       -s | --srpm     build .src.rpm
       -u | --unittest run unit tests
+
+    Example:
+      * ${0##*/} --rpm --srpm --unittest 6
 EOF
 }
 
@@ -48,6 +53,10 @@ inside_as() {
     # executes cmd with args inside container as user in users home dir.
     local name="$1" user="$2"
     shift 2
+    if [ "$user" = "root" ]; then
+        inside "$name" "$@"
+        return
+    fi
     local stuffed="" b64=""
     stuffed=$(getopt --shell sh --options "" -- -- "$@")
     stuffed=${stuffed# -- }
@@ -56,6 +65,12 @@ inside_as() {
         'cd; eval set -- "$(echo '$b64' | base64 --decode)" && exec "$@"'
 }
 
+inside_as_cd() {
+    local name="$1" user="$2" dir="$3"
+    shift 3
+    inside_as "$name" "$user" sh -c 'cd "$0" && exec "$@"' "$dir" "$@"
+}
+
 inside() {
     local name="$1"
     shift
@@ -63,26 +78,52 @@ inside() {
 }
 
 inject_cloud_init(){
-    local name="$1"
-    tarball_name='cloud-init.tar.gz'
-    top_d=$(git rev-parse --show-toplevel) ||
-        fail "failed to get top level"
-    cd "$top_d" ||
-        fail "failed to cd to git top dir"
-    tar_folder=${PWD##*/}
-    cd ..
-    tar -czf "$TEMP_D/$tarball_name" "$tar_folder" ||
-        fail "failed: creating tarball_name"
-    cd "$tar_folder" ||
-        fail "failed: changing directory"
-
-    user='centos'
-    tarball="/home/$user/$tarball_name"
-    inside "$name" useradd "$user"
-    lxc file push "$TEMP_D/$tarball_name" "$name/home/$user"/
-    inside "$name" chown "$user:$user" "$tarball"
-    inside_as "$name" "$user" tar -C "/home/$user" -xzf "$tarball" ||
-        fail "failed: extracting tarball"
+    # take current cloud-init git dir and put it inside $name at
+    # ~$user/cloud-init.
+    local name="$1" user="$2" top_d="" dname="" pstat=""
+    top_d=$(git rev-parse --show-toplevel) || {
+        errorrc "Failed to get git top level in $PWD";
+        return
+    }
+    dname=$(basename "${top_d}") || return
+    debug 1 "collecting ${top_d} ($dname) into user $user in $name."
+    tar -C "${top_d}/.." -cpf - "$dname" |
+        inside_as "$name" "$user" sh -ec '
+            dname=$1
+            rm -Rf "$dname"
+            tar -xpf -
+            [ "$dname" = "cloud-init" ] || mv "$dname" cloud-init' \
+            extract "$dname"
+    [ "${PIPESTATUS[*]}" = "0 0" ] || {
+        error "Failed to push tarball of '$top_d' into $name" \
+            " for user $user (dname=$dname)"
+        return 1
+    }
+    return 0
+}
+
+prep() {
+    # we need some very basic things not present in the container.
+    #  - git
+    #  - tar (CentOS 6 lxc container does not have it)
+    #  - python-argparse (or python3)
+    local needed="" pair="" pkg="" cmd="" needed=""
+    for pair in tar:tar git:git; do
+        pkg=${pair#*:}
+        cmd=${pair%%:*}
+        command -v $cmd >/dev/null 2>&1 || needed="${needed} $pkg"
+    done
+    if ! command -v python3; then
+        python -c "import argparse" >/dev/null 2>&1 ||
+            needed="${needed} python-argparse"
+    fi
+    needed=${needed# }
+    if [ -z "$needed" ]; then
+        error "No prep packages needed"
+        return 0
+    fi
+    error "Installing prep packages: ${needed}"
+    yum install --assumeyes ${needed}
 }
 
 start_container() {
@@ -121,8 +162,8 @@ delete_container() {
 }
 
 main() {
-    local short_opts="ahkrsuv:"
-    local long_opts="artifact,help,keep,rpm,srpm,unittest,verbose:"
+    local short_opts="ahkrsuv"
+    local long_opts="artifact,help,keep,rpm,srpm,unittest,verbose"
     local getopt_out=""
     getopt_out=$(getopt --name "${0##*/}" \
         --options "${short_opts}" --long "${long_opts}" -- "$@") &&
@@ -149,60 +190,70 @@ main() {
 
     [ $# -eq 1 ] || { bad_Usage "ERROR: Must provide version!"; return; }
     version="$1"
+    case "$version" in
+        6|7) :;;
+        *) error "Expected version of 6 or 7, not '$version'"; return;;
+    esac
 
     TEMP_D=$(mktemp -d "${TMPDIR:-/tmp}/${0##*/}.XXXXXX") ||
         fail "failed to make tempdir"
     trap cleanup EXIT
 
     # program starts here
-    local uuid="" name=""
+    local uuid="" name="" user="ci-test" cdir=""
+    cdir="/home/$user/cloud-init"
     uuid=$(uuidgen -t) || { error "no uuidgen"; return 1; }
     name="cloud-init-centos-${uuid%%-*}"
 
     start_container "images:centos/$version" "$name"
-    # CentOS 6 does not come with tar
-    if [ "$version" = "6" ]; then
-        inside "$name" yum install --assumeyes tar || {
-            errorrc "FAIL: yum install tar failed";
-        }
-    fi
+
+    # prep the container (install very basic dependencies)
+    inside "$name" bash -s prep <"$0" ||
+        { errorrc "Failed to prep container $name"; return; }
+
+    # add the user
+    inside "$name" useradd "$user"
 
     debug 1 "inserting cloud-init"
-    inject_cloud_init "$name" || {
+    inject_cloud_init "$name" "$user" || {
         errorrc "FAIL: injecting cloud-init into $name failed."
         return
     }
 
-    # install dependencies
-    debug 1 "installing dependencies"
-    inside "$name" /bin/sh <tools/setup-centos ||
-        fail "failed: setting up container $name"
+    inside_as_cd "$name" root "$cdir" \
+        ./tools/read-dependencies \
+            --requirements-file=requirements.txt \
+            --requirements-file=test-requirements.txt \
+            --distro=centos --install || {
+        errorrc "FAIL: failed to install dependencies with read-dependencies"
+        return
+    }
 
-    local errors=0 do_cd="cd $tar_folder"
-    inside_as "$name" "$user" sh -ec "$do_cd; git checkout .; git status" ||
+    local errors=0
+    inside_as_cd "$name" "$user" "$cdir" \
+        sh -ec "git checkout .; git status" ||
             { errorrc "git checkout failed."; errors=$(($errors+1)); }
 
     if [ -n "$unittest" ]; then
         debug 1 "running unit tests."
-        inside_as "$name" "$user" sh -ec "$do_cd; nosetests tests/unittests" ||
+        inside_as_cd "$name" "$user" "$cdir" nosetests tests/unittests ||
             { errorrc "nosetests failed."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$srpm" ]; then
         debug 1 "building srpm."
-        inside_as "$name" "$user" sh -ec "$do_cd; ./packages/brpm --srpm" ||
+        inside_as_cd "$name" "$user" "$cdir" ./packages/brpm --srpm ||
             { errorrc "brpm --srpm."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$rpm" ]; then
         debug 1 "building rpm."
-        inside_as "$name" "$user" sh -ec "$do_cd; ./packages/brpm" ||
+        inside_as_cd "$name" "$user" "$cdir" ./packages/brpm ||
             { errorrc "brpm failed."; errors=$(($errors+1)); }
     fi
 
     if [ -n "$artifact" ]; then
-        cmd="ls /home/$user/$tar_folder/*.rpm"
-        for built_rpm in $(lxc exec "$name" -- sh -c "$cmd"); do
+        for built_rpm in $(inside "$name" sh -c "echo $cdir/*.rpm"); do
             lxc file pull "$name/$built_rpm" .
         done
     fi
@@ -214,5 +265,10 @@ main() {
     return 0
 }
 
-main "$@"
+if [ "$1" = "prep" ]; then
+    shift
+    prep "$@"
+else
+    main "$@"
+fi
 # vi: ts=4 expandtab
diff --git a/tools/setup-centos b/tools/setup-centos
deleted file mode 100755
index bc5da8a7..00000000
--- a/tools/setup-centos
+++ /dev/null
@@ -1,49 +0,0 @@
-#!/bin/sh
-# This file is part of cloud-init. See LICENSE file for license information.
-set -fux
-export LANG=C
-
-packages="
-    file
-    git
-    pyserial
-    python-argparse
-    python-cheetah
-    python-configobj
-    python-devel
-    python-jinja2
-    python-jsonpatch
-    python-oauthlib
-    python-pip
-    python-prettytable
-    python-requests
-    python-six
-    PyYAML
-    rpm-build
-"
-
-pips="
-    contextlib2
-    httpretty
-    mock
-    nose
-    pep8
-    unittest2
-"
-
-error() { echo "$@" 1>&2; }
-fail() { [ $# -eq 0 ] || error "$@"; exit 1; }
-info() { echo "$@"; }
-
-pips=$(for p in $pips; do echo "$p"; done | sort -u)
-packages=$(for p in $packages; do echo "$p"; done | sort -u)
-
-if ! rpm -q epel-release >/dev/null; then
-    yum install --assumeyes epel-release ||
-        fail "failed: yum install epel-release"
-fi
-yum install --assumeyes $packages ||
-        fail "failed: yum install" "$packages"
-
-pip install --upgrade $pips ||
-    fail "failed: pip install $pips"
-- 
cgit v1.2.3


From 0fe6a0607408d387f4b0d4482b95afbc5d3f3909 Mon Sep 17 00:00:00 2001
From: Andrew Jorgensen <ajorgens@amazon.com>
Date: Fri, 5 Dec 2014 14:34:10 -0800
Subject: write_file(s): Print permissions as octal, not decimal

Unix file modes are usually represented as octal, but they were being
interpreted as decimal, for example 0o644 would be printed as '420'.

Reviewed-by: Tom Kirchner <tjk@amazon.com>
---
 cloudinit/config/cc_write_files.py                 | 10 ++++++-
 cloudinit/util.py                                  |  6 ++++-
 tests/unittests/helpers.py                         |  8 +++---
 tests/unittests/test_datasource/test_azure.py      |  3 ++-
 tests/unittests/test_handler/test_handler_ntp.py   |  3 ++-
 .../test_handler/test_handler_write_files.py       | 31 ++++++++++++++++++++--
 6 files changed, 52 insertions(+), 9 deletions(-)

(limited to 'tests/unittests/test_handler')

diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py
index 72e1cdd6..1835a31b 100644
--- a/cloudinit/config/cc_write_files.py
+++ b/cloudinit/config/cc_write_files.py
@@ -53,6 +53,7 @@ import six
 from cloudinit.settings import PER_INSTANCE
 from cloudinit import util
 
+
 frequency = PER_INSTANCE
 
 DEFAULT_OWNER = "root:root"
@@ -119,7 +120,14 @@ def decode_perms(perm, default, log):
             # Force to string and try octal conversion
             return int(str(perm), 8)
     except (TypeError, ValueError):
-        log.warn("Undecodable permissions %s, assuming %s", perm, default)
+        reps = []
+        for r in (perm, default):
+            try:
+                reps.append("%o" % r)
+            except TypeError:
+                reps.append("%r" % r)
+        log.warning(
+            "Undecodable permissions {0}, returning default {1}".format(*reps))
         return default
 
 
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 415ca374..ec68925e 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1751,8 +1751,12 @@ def write_file(filename, content, mode=0o644, omode="wb", copy_mode=False):
     else:
         content = decode_binary(content)
         write_type = 'characters'
+    try:
+        mode_r = "%o" % mode
+    except TypeError:
+        mode_r = "%r" % mode
     LOG.debug("Writing to %s - %s: [%s] %s %s",
-              filename, omode, mode, len(content), write_type)
+              filename, omode, mode_r, len(content), write_type)
     with SeLinuxGuard(path=filename):
         with open(filename, omode) as fh:
             fh.write(content)
diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py
index e78abce2..569f1aef 100644
--- a/tests/unittests/helpers.py
+++ b/tests/unittests/helpers.py
@@ -97,11 +97,13 @@ class CiTestCase(TestCase):
         super(CiTestCase, self).setUp()
         if self.with_logs:
             # Create a log handler so unit tests can search expected logs.
-            logger = logging.getLogger()
+            self.logger = logging.getLogger()
             self.logs = six.StringIO()
+            formatter = logging.Formatter('%(levelname)s: %(message)s')
             handler = logging.StreamHandler(self.logs)
-            self.old_handlers = logger.handlers
-            logger.handlers = [handler]
+            handler.setFormatter(formatter)
+            self.old_handlers = self.logger.handlers
+            self.logger.handlers = [handler]
 
     def tearDown(self):
         if self.with_logs:
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 42f49e06..b17f389c 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -263,7 +263,8 @@ fdescfs            /dev/fd          fdescfs rw              0 0
             {}, distro=None, paths=self.paths)
         self.assertFalse(dsrc.get_data())
         self.assertEqual(
-            "Non-Azure DMI asset tag '{0}' discovered.\n".format(nonazure_tag),
+            "DEBUG: Non-Azure DMI asset tag '{0}' discovered.\n".format(
+                nonazure_tag),
             self.logs.getvalue())
 
     def test_basic_seed_dir(self):
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 3a9f7f7e..c4299d94 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -216,7 +216,8 @@ class TestNtp(FilesystemMockingTestCase):
         """When no ntp section is defined handler logs a warning and noops."""
         cc_ntp.handle('cc_ntp', {}, None, None, [])
         self.assertEqual(
-            'Skipping module named cc_ntp, not present or disabled by cfg\n',
+            'DEBUG: Skipping module named cc_ntp, '
+            'not present or disabled by cfg\n',
             self.logs.getvalue())
 
     def test_ntp_handler_schema_validation_allows_empty_ntp_config(self):
diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py
index fb252d1d..88a4742b 100644
--- a/tests/unittests/test_handler/test_handler_write_files.py
+++ b/tests/unittests/test_handler/test_handler_write_files.py
@@ -1,10 +1,10 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.config.cc_write_files import write_files
+from cloudinit.config.cc_write_files import write_files, decode_perms
 from cloudinit import log as logging
 from cloudinit import util
 
-from ..helpers import FilesystemMockingTestCase
+from ..helpers import CiTestCase, FilesystemMockingTestCase
 
 import base64
 import gzip
@@ -98,6 +98,33 @@ class TestWriteFiles(FilesystemMockingTestCase):
         self.assertEqual(len(expected), flen_expected)
 
 
+class TestDecodePerms(CiTestCase):
+
+    with_logs = True
+
+    def test_none_returns_default(self):
+        """If None is passed as perms, then default should be returned."""
+        default = object()
+        found = decode_perms(None, default, self.logger)
+        self.assertEqual(default, found)
+
+    def test_integer(self):
+        """A valid integer should return itself."""
+        found = decode_perms(0o755, None, self.logger)
+        self.assertEqual(0o755, found)
+
+    def test_valid_octal_string(self):
+        """A string should be read as octal."""
+        found = decode_perms("644", None, self.logger)
+        self.assertEqual(0o644, found)
+
+    def test_invalid_octal_string_returns_default_and_warns(self):
+        """A string with invalid octal should warn and return default."""
+        found = decode_perms("999", None, self.logger)
+        self.assertIsNone(found)
+        self.assertIn("WARNING: Undecodable", self.logs.getvalue())
+
+
 def _gzip_bytes(data):
     buf = six.BytesIO()
     fp = None
-- 
cgit v1.2.3


From ecb408afa1104fe49ce6eb1dc5708be56abd5cb2 Mon Sep 17 00:00:00 2001
From: Scott Moser <smoser@brickies.net>
Date: Thu, 15 Jun 2017 10:03:45 -0400
Subject: FreeBSD: Make freebsd a variant, fix unittests and
 tools/build-on-freebsd.

 - Simplify the logic of 'variant' in util.system_info
   much of the data from
   https://github.com/hpcugent/easybuild/wiki/OS_flavor_name_version
 - fix get_resource_disk_on_freebsd when running on a system without
   an Azure resource disk.
 - fix tools/build-on-freebsd to replace oauth with oauthlib and add
   bash which is a dependency for tests.
 - update a fiew places that were checking for freebsd but not using
   the util.is_FreeBSD()
---
 cloudinit/config/cc_growpart.py                  |  2 +-
 cloudinit/config/cc_power_state_change.py        |  2 +-
 cloudinit/sources/DataSourceAzure.py             |  2 +-
 cloudinit/util.py                                | 46 ++++++++++--------------
 config/cloud.cfg.tmpl                            | 20 +++++------
 tests/unittests/test_handler/test_handler_ntp.py |  2 +-
 tests/unittests/test_util.py                     |  9 +++--
 tools/build-on-freebsd                           |  6 ++--
 8 files changed, 40 insertions(+), 49 deletions(-)

(limited to 'tests/unittests/test_handler')

diff --git a/cloudinit/config/cc_growpart.py b/cloudinit/config/cc_growpart.py
index d2bc6e6c..bafca9d8 100644
--- a/cloudinit/config/cc_growpart.py
+++ b/cloudinit/config/cc_growpart.py
@@ -214,7 +214,7 @@ def device_part_info(devpath):
 
     # FreeBSD doesn't know of sysfs so just get everything we need from
     # the device, like /dev/vtbd0p2.
-    if util.system_info()["platform"].startswith('FreeBSD'):
+    if util.is_FreeBSD():
         m = re.search('^(/dev/.+)p([0-9])$', devpath)
         return (m.group(1), m.group(2))
 
diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py
index c1c6fe7e..eba58b02 100644
--- a/cloudinit/config/cc_power_state_change.py
+++ b/cloudinit/config/cc_power_state_change.py
@@ -71,7 +71,7 @@ def givecmdline(pid):
         # Example output from procstat -c 1
         #   PID COMM             ARGS
         #     1 init             /bin/init --
-        if util.system_info()["platform"].startswith('FreeBSD'):
+        if util.is_FreeBSD():
             (output, _err) = util.subp(['procstat', '-c', str(pid)])
             line = output.splitlines()[1]
             m = re.search('\d+ (\w|\.|-)+\s+(/\w.+)', line)
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 71e7c55c..4fe0d635 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -101,7 +101,7 @@ def get_dev_storvsc_sysctl():
         sysctl_out, err = util.subp(['sysctl', 'dev.storvsc'])
     except util.ProcessExecutionError:
         LOG.debug("Fail to execute sysctl dev.storvsc")
-        return None
+        sysctl_out = ""
     return sysctl_out
 
 
diff --git a/cloudinit/util.py b/cloudinit/util.py
index ec68925e..c93b6d7e 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -573,7 +573,7 @@ def is_ipv4(instr):
 
 
 def is_FreeBSD():
-    return system_info()['platform'].startswith('FreeBSD')
+    return system_info()['variant'] == "freebsd"
 
 
 def get_cfg_option_bool(yobj, key, default=False):
@@ -598,37 +598,29 @@ def get_cfg_option_int(yobj, key, default=0):
 def system_info():
     info = {
         'platform': platform.platform(),
+        'system': platform.system(),
         'release': platform.release(),
         'python': platform.python_version(),
         'uname': platform.uname(),
-        'dist': platform.linux_distribution(),  # pylint: disable=W1505
+        'dist': platform.dist(),  # pylint: disable=W1505
     }
-    plat = info['platform'].lower()
-    # Try to get more info about what it actually is, in a format
-    # that we can easily use across linux and variants...
-    if plat.startswith('darwin'):
-        info['variant'] = 'darwin'
-    elif plat.endswith("bsd"):
-        info['variant'] = 'bsd'
-    elif plat.startswith('win'):
-        info['variant'] = 'windows'
-    elif 'linux' in plat:
-        # Try to get a single string out of these...
-        linux_dist, _version, _id = info['dist']
-        linux_dist = linux_dist.lower()
-        if linux_dist in ('ubuntu', 'linuxmint', 'mint'):
-            info['variant'] = 'ubuntu'
+    system = info['system'].lower()
+    var = 'unknown'
+    if system == "linux":
+        linux_dist = info['dist'][0].lower()
+        if linux_dist in ('centos', 'fedora', 'debian'):
+            var = linux_dist
+        elif linux_dist in ('ubuntu', 'linuxmint', 'mint'):
+            var = 'ubuntu'
+        elif linux_dist == 'redhat':
+            var = 'rhel'
         else:
-            for prefix, variant in [('redhat', 'rhel'),
-                                    ('centos', 'centos'),
-                                    ('fedora', 'fedora'),
-                                    ('debian', 'debian')]:
-                if linux_dist.startswith(prefix):
-                    info['variant'] = variant
-        if 'variant' not in info:
-            info['variant'] = 'linux'
-    if 'variant' not in info:
-        info['variant'] = 'unknown'
+            var = 'linux'
+    elif system in ('windows', 'darwin', "freebsd"):
+        var = system
+
+    info['variant'] = var
+
     return info
 
 
diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
index 5af2a88f..f4b9069b 100644
--- a/config/cloud.cfg.tmpl
+++ b/config/cloud.cfg.tmpl
@@ -2,7 +2,7 @@
 # The top level settings are used as module
 # and system configuration.
 
-{% if variant in ["bsd"] %}
+{% if variant in ["freebsd"] %}
 syslog_fix_perms: root:wheel
 {% endif %}
 # A set of users which may be applied and/or used by various modules
@@ -13,7 +13,7 @@ users:
 
 # If this is set, 'root' will not be able to ssh in and they
 # will get a message to login instead as the default $user
-{% if variant in ["bsd"] %}
+{% if variant in ["freebsd"] %}
 disable_root: false
 {% else %}
 disable_root: true
@@ -30,7 +30,7 @@ ssh_pwauth:   0
 # This will cause the set+update hostname module to not operate (if true)
 preserve_hostname: false
 
-{% if variant in ["bsd"] %}
+{% if variant in ["freebsd"] %}
 # This should not be required, but leave it in place until the real cause of
 # not beeing able to find -any- datasources is resolved.
 datasource_list: ['ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
@@ -53,13 +53,13 @@ cloud_init_modules:
  - write-files
  - growpart
  - resizefs
-{% if variant not in ["bsd"] %}
+{% if variant not in ["freebsd"] %}
  - disk_setup
  - mounts
 {% endif %}
  - set_hostname
  - update_hostname
-{% if variant not in ["bsd"] %}
+{% if variant not in ["freebsd"] %}
  - update_etc_hosts
  - ca-certs
  - rsyslog
@@ -87,7 +87,7 @@ cloud_config_modules:
  - apt-pipelining
  - apt-configure
 {% endif %}
-{% if variant not in ["bsd"] %}
+{% if variant not in ["freebsd"] %}
  - ntp
 {% endif %}
  - timezone
@@ -108,7 +108,7 @@ cloud_final_modules:
  - landscape
  - lxd
 {% endif %}
-{% if variant not in ["bsd"] %}
+{% if variant not in ["freebsd"] %}
  - puppet
  - chef
  - salt-minion
@@ -130,10 +130,8 @@ cloud_final_modules:
 # (not accessible to handlers/transforms)
 system_info:
    # This will affect which distro class gets used
-{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu"] %}
+{% if variant in ["centos", "debian", "fedora", "rhel", "ubuntu", "freebsd"] %}
    distro: {{ variant }}
-{% elif variant in ["bsd"] %}
-   distro: freebsd
 {% else %}
    # Unknown/fallback distro.
    distro: ubuntu
@@ -182,7 +180,7 @@ system_info:
       cloud_dir: /var/lib/cloud/
       templates_dir: /etc/cloud/templates/
    ssh_svcname: sshd
-{% elif variant in ["bsd"] %}
+{% elif variant in ["freebsd"] %}
    # Default user name + that default users groups (if added/used)
    default_user:
      name: freebsd
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index c4299d94..7f278646 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -62,7 +62,7 @@ class TestNtp(FilesystemMockingTestCase):
     def test_ntp_rename_ntp_conf(self):
         """When NTP_CONF exists, rename_ntp moves it."""
         ntpconf = self.tmp_path("ntp.conf", self.new_root)
-        os.mknod(ntpconf)
+        util.write_file(ntpconf, "")
         with mock.patch("cloudinit.config.cc_ntp.NTP_CONF", ntpconf):
             cc_ntp.rename_ntp_conf()
         self.assertFalse(os.path.exists(ntpconf))
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 014aa6a3..a73fd26a 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -20,6 +20,9 @@ except ImportError:
     import mock
 
 
+BASH = util.which('bash')
+
+
 class FakeSelinux(object):
 
     def __init__(self, match_what):
@@ -544,17 +547,17 @@ class TestReadSeeded(helpers.TestCase):
 
 class TestSubp(helpers.TestCase):
 
-    stdin2err = ['bash', '-c', 'cat >&2']
+    stdin2err = [BASH, '-c', 'cat >&2']
     stdin2out = ['cat']
     utf8_invalid = b'ab\xaadef'
     utf8_valid = b'start \xc3\xa9 end'
     utf8_valid_2 = b'd\xc3\xa9j\xc8\xa7'
-    printenv = ['bash', '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--']
+    printenv = [BASH, '-c', 'for n in "$@"; do echo "$n=${!n}"; done', '--']
 
     def printf_cmd(self, *args):
         # bash's printf supports \xaa.  So does /usr/bin/printf
         # but by using bash, we remove dependency on another program.
-        return(['bash', '-c', 'printf "$@"', 'printf'] + list(args))
+        return([BASH, '-c', 'printf "$@"', 'printf'] + list(args))
 
     def test_subp_handles_utf8(self):
         # The given bytes contain utf-8 accented characters as seen in e.g.
diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd
index ccc10b40..ff9153ad 100755
--- a/tools/build-on-freebsd
+++ b/tools/build-on-freebsd
@@ -8,6 +8,7 @@ fail() { echo "FAILED:" "$@" 1>&2; exit 1; }
 # Check dependencies:
 depschecked=/tmp/c-i.dependencieschecked
 pkgs="
+   bash
    dmidecode
    e2fsprogs
    py27-Jinja2
@@ -16,7 +17,7 @@ pkgs="
    py27-configobj
    py27-jsonpatch
    py27-jsonpointer
-   py27-oauth
+   py27-oauthlib
    py27-prettytable
    py27-requests
    py27-serial
@@ -35,9 +36,6 @@ touch $depschecked
 python setup.py build
 python setup.py install -O1 --skip-build --prefix /usr/local/ --init-system sysvinit_freebsd
 
-# Install the correct config file:
-cp config/cloud.cfg-freebsd /etc/cloud/cloud.cfg
-
 # Enable cloud-init in /etc/rc.conf:
 sed -i.bak -e "/cloudinit_enable=.*/d" /etc/rc.conf
 echo 'cloudinit_enable="YES"' >> /etc/rc.conf
-- 
cgit v1.2.3


From 4330a98161a5c514c7f85a76a58d057c85b00174 Mon Sep 17 00:00:00 2001
From: Andrew Jorgensen <ajorgens@amazon.com>
Date: Fri, 16 Jun 2017 18:17:36 +0000
Subject: write_files: Remove log from helper function signatures.

Instead of passing around a 'log' reference to functions, just import
logging and use that.  This is the pattern that is now more common in
cloud-init.
---
 cloudinit/config/cc_write_files.py                 | 27 ++++++++++++----------
 .../test_handler/test_handler_write_files.py       | 14 +++++------
 2 files changed, 22 insertions(+), 19 deletions(-)

(limited to 'tests/unittests/test_handler')

diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py
index 1835a31b..54ae3a68 100644
--- a/cloudinit/config/cc_write_files.py
+++ b/cloudinit/config/cc_write_files.py
@@ -50,6 +50,7 @@ import base64
 import os
 import six
 
+from cloudinit import log as logging
 from cloudinit.settings import PER_INSTANCE
 from cloudinit import util
 
@@ -60,6 +61,8 @@ DEFAULT_OWNER = "root:root"
 DEFAULT_PERMS = 0o644
 UNKNOWN_ENC = 'text/plain'
 
+LOG = logging.getLogger(__name__)
+
 
 def handle(name, cfg, _cloud, log, _args):
     files = cfg.get('write_files')
@@ -67,10 +70,10 @@ def handle(name, cfg, _cloud, log, _args):
         log.debug(("Skipping module named %s,"
                    " no/empty 'write_files' key in configuration"), name)
         return
-    write_files(name, files, log)
+    write_files(name, files)
 
 
-def canonicalize_extraction(encoding_type, log):
+def canonicalize_extraction(encoding_type):
     if not encoding_type:
         encoding_type = ''
     encoding_type = encoding_type.lower().strip()
@@ -85,31 +88,31 @@ def canonicalize_extraction(encoding_type, log):
     if encoding_type in ['b64', 'base64']:
         return ['application/base64']
     if encoding_type:
-        log.warn("Unknown encoding type %s, assuming %s",
-                 encoding_type, UNKNOWN_ENC)
+        LOG.warning("Unknown encoding type %s, assuming %s",
+                    encoding_type, UNKNOWN_ENC)
     return [UNKNOWN_ENC]
 
 
-def write_files(name, files, log):
+def write_files(name, files):
     if not files:
         return
 
     for (i, f_info) in enumerate(files):
         path = f_info.get('path')
         if not path:
-            log.warn("No path provided to write for entry %s in module %s",
-                     i + 1, name)
+            LOG.warning("No path provided to write for entry %s in module %s",
+                        i + 1, name)
             continue
         path = os.path.abspath(path)
-        extractions = canonicalize_extraction(f_info.get('encoding'), log)
+        extractions = canonicalize_extraction(f_info.get('encoding'))
         contents = extract_contents(f_info.get('content', ''), extractions)
         (u, g) = util.extract_usergroup(f_info.get('owner', DEFAULT_OWNER))
-        perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS, log)
+        perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS)
         util.write_file(path, contents, mode=perms)
         util.chownbyname(path, u, g)
 
 
-def decode_perms(perm, default, log):
+def decode_perms(perm, default):
     if perm is None:
         return default
     try:
@@ -126,8 +129,8 @@ def decode_perms(perm, default, log):
                 reps.append("%o" % r)
             except TypeError:
                 reps.append("%r" % r)
-        log.warning(
-            "Undecodable permissions {0}, returning default {1}".format(*reps))
+        LOG.warning(
+            "Undecodable permissions %s, returning default %s", *reps)
         return default
 
 
diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py
index 88a4742b..1129e77d 100644
--- a/tests/unittests/test_handler/test_handler_write_files.py
+++ b/tests/unittests/test_handler/test_handler_write_files.py
@@ -49,13 +49,13 @@ class TestWriteFiles(FilesystemMockingTestCase):
         expected = "hello world\n"
         filename = "/tmp/my.file"
         write_files(
-            "test_simple", [{"content": expected, "path": filename}], LOG)
+            "test_simple", [{"content": expected, "path": filename}])
         self.assertEqual(util.load_file(filename), expected)
 
     def test_yaml_binary(self):
         self.patchUtils(self.tmp)
         data = util.load_yaml(YAML_TEXT)
-        write_files("testname", data['write_files'], LOG)
+        write_files("testname", data['write_files'])
         for path, content in YAML_CONTENT_EXPECTED.items():
             self.assertEqual(util.load_file(path), content)
 
@@ -87,7 +87,7 @@ class TestWriteFiles(FilesystemMockingTestCase):
                     files.append(cur)
                     expected.append((cur['path'], data))
 
-        write_files("test_decoding", files, LOG)
+        write_files("test_decoding", files)
 
         for path, content in expected:
             self.assertEqual(util.load_file(path, decode=False), content)
@@ -105,22 +105,22 @@ class TestDecodePerms(CiTestCase):
     def test_none_returns_default(self):
         """If None is passed as perms, then default should be returned."""
         default = object()
-        found = decode_perms(None, default, self.logger)
+        found = decode_perms(None, default)
         self.assertEqual(default, found)
 
     def test_integer(self):
         """A valid integer should return itself."""
-        found = decode_perms(0o755, None, self.logger)
+        found = decode_perms(0o755, None)
         self.assertEqual(0o755, found)
 
     def test_valid_octal_string(self):
         """A string should be read as octal."""
-        found = decode_perms("644", None, self.logger)
+        found = decode_perms("644", None)
         self.assertEqual(0o644, found)
 
     def test_invalid_octal_string_returns_default_and_warns(self):
         """A string with invalid octal should warn and return default."""
-        found = decode_perms("999", None, self.logger)
+        found = decode_perms("999", None)
         self.assertIsNone(found)
         self.assertIn("WARNING: Undecodable", self.logs.getvalue())
 
-- 
cgit v1.2.3