Merge ~xiaofengw/cloud-init:xiaofengw-add-custom-script-option into cloud-init:master

[Xiaofeng Wang]

VMWware: add option into VMTools config to enable/disable custom script.

VMWware customization already has support to run a custom script during
the VM customization. Adding this option allows a VM administrator to
disable the execution of customization scripts. If set the script
will not execute and the customization status is set to
GUESTCUST_ERROR_SCRIPT_DISABLED.

c516af5... by Xiaofeng Wang on 2019-09-09

VMWware: add option into VMTools configuration to enable/disable custom script.

VMWware customization has already support to run a custom script during the VM
customization. We add this option so that VM administrator could prevent custom
 script to run, then the customization will fail and customization status is
set to GUESTCUST_ERROR_SCRIPT_DISABLED.

[Ryan Harper (raharper)]

Thanks for resubmitting. This looks much better. A couple of inline comments below.

review: Needs Fixing

68f7158... by Xiaofeng Wang on 2019-09-10

VMWware: add option into VMTools configuration to enable/disable custom script.

Fix the comments from Ryan.

[Xiaofeng Wang (xiaofengw)]

> Thanks for resubmitting. This looks much better. A couple of inline comments
> below.

Thanks, all comments are fixed, and also add a new test case to cover "key=Bar=Wark" case. Please review. Thanks.

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

No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want jenkins to rebuild you need to trigger it yourself):
https://code.launchpad.net/~xiaofengw/cloud-init/+git/cloud-init/+merge/372469/+edit-commit-message
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1117/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:68f7158757001d0ecfb676109b72342c818ec59f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1119/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

I've updated the commit message. Everything looks good now.

Thanks!

review: Approve

diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index dd941d2..b156189 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -40,11 +40,15 @@ from cloudinit.sources.helpers.vmware.imc.guestcust_state \
 from cloudinit.sources.helpers.vmware.imc.guestcust_util import (
     enable_nics,
     get_nics_to_enable,
-    set_customization_status
+    set_customization_status,
+    get_tools_config
 )
 
 LOG = logging.getLogger(__name__)
 
+CONFGROUPNAME_GUESTCUSTOMIZATION = "deployPkg"
+GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS = "enable-custom-scripts"
+
 
 class DataSourceOVF(sources.DataSource):
 
@@ -148,6 +152,21 @@ class DataSourceOVF(sources.DataSource):
                     product_marker, os.path.join(self.paths.cloud_dir, 'data'))
                 special_customization = product_marker and not hasmarkerfile
                 customscript = self._vmware_cust_conf.custom_script_name
+                custScriptConfig = get_tools_config(
+                    CONFGROUPNAME_GUESTCUSTOMIZATION,
+                    GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS,
+                    "true")
+                if custScriptConfig.lower() == "false":
+                    # Update the customization status if there is a
+                    # custom script is disabled
+                    if special_customization and customscript:
+                        msg = "Custom script is disabled by VM Administrator"
+                        LOG.debug(msg)
+                        set_customization_status(
+                            GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
+                            GuestCustErrorEnum.GUESTCUST_ERROR_SCRIPT_DISABLED)
+                        raise RuntimeError(msg)
+
                 ccScriptsDir = os.path.join(
                     self.paths.get_cpath("scripts"),
                     "per-instance")
diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_error.py b/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
index db5a00d..65ae739 100644
--- a/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
+++ b/cloudinit/sources/helpers/vmware/imc/guestcust_error.py
@@ -10,5 +10,6 @@ class GuestCustErrorEnum(object):
     """Specifies different errors of Guest Customization engine"""
 
     GUESTCUST_ERROR_SUCCESS = 0
+    GUESTCUST_ERROR_SCRIPT_DISABLED = 6
 
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
index a590f32..eb78172 100644
--- a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
+++ b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py
@@ -7,6 +7,7 @@
 
 import logging
 import os
+import re
 import time
 
 from cloudinit import util
@@ -117,4 +118,40 @@ def enable_nics(nics):
     logger.warning("Can't connect network interfaces after %d attempts",
                    enableNicsWaitRetries)
 
+
+def get_tools_config(section, key, defaultVal):
+    """ Return the value of [section] key from VMTools configuration.
+
+        @param section: String of section to read from VMTools config
+        @returns: String value from key in [section] or defaultVal if
+                  [section] is not present or vmware-toolbox-cmd is
+                  not installed.
+    """
+
+    if not util.which('vmware-toolbox-cmd'):
+        logger.debug(
+            'vmware-toolbox-cmd not installed, returning default value')
+        return defaultVal
+
+    retValue = defaultVal
+    cmd = ['vmware-toolbox-cmd', 'config', 'get', section, key]
+
+    try:
+        (outText, _) = util.subp(cmd)
+        m = re.match(r'([a-zA-Z0-9 ]+)=(.*)', outText)
+        if m:
+            retValue = m.group(2).strip()
+            logger.debug("Get tools config: [%s] %s = %s",
+                         section, key, retValue)
+        else:
+            logger.debug(
+                "Tools config: [%s] %s is not found, return default value: %s",
+                section, key, retValue)
+    except util.ProcessExecutionError as e:
+        logger.error("Failed running %s[%s]", cmd, e.exit_code)
+        logger.exception(e)
+
+    return retValue
+
+
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py
index 349d54c..a615470 100644
--- a/tests/unittests/test_datasource/test_ovf.py
+++ b/tests/unittests/test_datasource/test_ovf.py
@@ -169,19 +169,56 @@ class TestDatasourceOVF(CiTestCase):
             MARKER-ID = 12345345
             """)
         util.write_file(conf_file, conf_content)
-        with self.assertRaises(CustomScriptNotFound) as context:
-            wrap_and_call(
-                'cloudinit.sources.DataSourceOVF',
-                {'util.read_dmi_data': 'vmware',
-                 'util.del_dir': True,
-                 'search_file': self.tdir,
-                 'wait_for_imc_cfg_file': conf_file,
-                 'get_nics_to_enable': ''},
-                ds.get_data)
+        with mock.patch(MPATH + 'get_tools_config', return_value='true'):
+            with self.assertRaises(CustomScriptNotFound) as context:
+                wrap_and_call(
+                    'cloudinit.sources.DataSourceOVF',
+                    {'util.read_dmi_data': 'vmware',
+                     'util.del_dir': True,
+                     'search_file': self.tdir,
+                     'wait_for_imc_cfg_file': conf_file,
+                     'get_nics_to_enable': ''},
+                    ds.get_data)
         customscript = self.tmp_path('test-script', self.tdir)
         self.assertIn('Script %s not found!!' % customscript,
                       str(context.exception))
 
+    def test_get_data_cust_script_disabled(self):
+        """If custom script is disabled by VMware tools configuration,
+        raise a RuntimeError.
+        """
+        paths = Paths({'cloud_dir': self.tdir})
+        ds = self.datasource(
+            sys_cfg={'disable_vmware_customization': False}, distro={},
+            paths=paths)
+        # Prepare the conf file
+        conf_file = self.tmp_path('test-cust', self.tdir)
+        conf_content = dedent("""\
+            [CUSTOM-SCRIPT]
+            SCRIPT-NAME = test-script
+            [MISC]
+            MARKER-ID = 12345346
+            """)
+        util.write_file(conf_file, conf_content)
+        # Prepare the custom sript
+        customscript = self.tmp_path('test-script', self.tdir)
+        util.write_file(customscript, "This is the post cust script")
+
+        with mock.patch(MPATH + 'get_tools_config', return_value='false'):
+            with mock.patch(MPATH + 'set_customization_status',
+                            return_value=('msg', b'')):
+                with self.assertRaises(RuntimeError) as context:
+                    wrap_and_call(
+                        'cloudinit.sources.DataSourceOVF',
+                        {'util.read_dmi_data': 'vmware',
+                         'util.del_dir': True,
+                         'search_file': self.tdir,
+                         'wait_for_imc_cfg_file': conf_file,
+                         'get_nics_to_enable': ''},
+                        ds.get_data)
+        self.assertIn('Custom script is disabled by VM Administrator',
+                      str(context.exception))
+
     def test_get_data_non_vmware_seed_platform_info(self):
         """Platform info properly reports when on non-vmware platforms."""
         paths = Paths({'cloud_dir': self.tdir, 'run_dir': self.tdir})
diff --git a/tests/unittests/test_vmware/test_guestcust_util.py b/tests/unittests/test_vmware/test_guestcust_util.py
new file mode 100644
index 0000000..b8fa994
--- /dev/null
+++ b/tests/unittests/test_vmware/test_guestcust_util.py
@@ -0,0 +1,65 @@
+# Copyright (C) 2019 Canonical Ltd.
+# Copyright (C) 2019 VMware INC.
+#
+# Author: Xiaofeng Wang <[e-mail]>
+#
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from cloudinit import util
+from cloudinit.sources.helpers.vmware.imc.guestcust_util import (
+    get_tools_config,
+)
+from cloudinit.tests.helpers import CiTestCase, mock
+
+
+class TestGuestCustUtil(CiTestCase):
+    def test_get_tools_config_not_installed(self):
+        """
+        This test is designed to verify the behavior if vmware-toolbox-cmd
+        is not installed.
+        """
+        with mock.patch.object(util, 'which', return_value=None):
+            self.assertEqual(
+                get_tools_config('section', 'key', 'defaultVal'), 'defaultVal')
+
+    def test_get_tools_config_internal_exception(self):
+        """
+        This test is designed to verify the behavior if internal exception
+        is raised.
+        """
+        with mock.patch.object(util, 'which', return_value='/dummy/path'):
+            with mock.patch.object(util, 'subp',
+                                   return_value=('key=value', b''),
+                                   side_effect=util.ProcessExecutionError(
+                                       "subp failed", exit_code=99)):
+                # verify return value is 'defaultVal', not 'value'.
+                self.assertEqual(
+                    get_tools_config('section', 'key', 'defaultVal'),
+                    'defaultVal')
+
+    def test_get_tools_config_normal(self):
+        """
+        This test is designed to verify the value could be parsed from
+        key = value of the given [section]
+        """
+        with mock.patch.object(util, 'which', return_value='/dummy/path'):
+            # value is not blank
+            with mock.patch.object(util, 'subp',
+                                   return_value=('key =   value  ', b'')):
+                self.assertEqual(
+                    get_tools_config('section', 'key', 'defaultVal'),
+                    'value')
+            # value is blank
+            with mock.patch.object(util, 'subp',
+                                   return_value=('key = ', b'')):
+                self.assertEqual(
+                    get_tools_config('section', 'key', 'defaultVal'),
+                    '')
+            # value contains =
+            with mock.patch.object(util, 'subp',
+                                   return_value=('key=Bar=Wark', b'')):
+                self.assertEqual(
+                    get_tools_config('section', 'key', 'defaultVal'),
+                    'Bar=Wark')
+
+# vi: ts=4 expandtab