Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VMware: Support parsing FORCE-RUN-POST-CUST-SCRIPT #426

Conversation

xiaofengw-vmware
Copy link
Contributor

vCloud product wants to run custom script despite of VMware Tools configuration, so we introduce a new flag FORCE-RUN-POST-CUST-SCRIPT. If it is yes, custom script run no matter enable-custom-scripts is true or false.

@blackboxsw blackboxsw self-assigned this Jun 11, 2020
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Thank you for the submission @xiaofengw-vmware.

I would like to see us log a debug message about this behavior override because FORCE-RUN-POST-CUST-SCRIPT seems to conflict with what was desired/intended behavior of the custom script and marker files.

It would aid in triage of 'unexpected' behavior from customers if we leave a small bread crumb about why custom scripts were still run in this case.

here's a proposed diff which adds that log to see what you think.

diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index 90f93b65e..ac8dd25ea 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -29,7 +29,7 @@ from cloudinit.sources.helpers.vmware.imc.config_nic \
 from cloudinit.sources.helpers.vmware.imc.config_passwd \
     import PasswordConfigurator
 from cloudinit.sources.helpers.vmware.imc.guestcust_error \
-    import GuestCustErrorEnum
+    import GuestCustErrorEnum as GuestCustError
 from cloudinit.sources.helpers.vmware.imc.guestcust_event \
     import GuestCustEventEnum as GuestCustEvent
 from cloudinit.sources.helpers.vmware.imc.guestcust_state \
@@ -155,21 +155,28 @@ class DataSourceOVF(sources.DataSource):
 
                 # In case there is a custom script, check whether VMware
                 # Tools configuration allow the custom script to run.
-                if (special_customization and customscript and
-                        not self._vmware_cust_conf.force_run_post_script):
-                    custScriptConfig = get_tools_config(
-                        CONFGROUPNAME_GUESTCUSTOMIZATION,
-                        GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS,
-                        "false")
-                    if custScriptConfig.lower() != "true":
-                        # Update the customization status if custom script is
-                        # disabled
-                        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)
+                if special_customization and customscript:
+                    if self._vmware_cust_conf.force_run_post_script:
+                        LOG.debug(
+                            "Force run of custom script due to"
+                            " FORCE-RUN-POST-CUST-SCRIPT."
+                        )
+                    else:
+                        custScriptConfig = get_tools_config(
+                            CONFGROUPNAME_GUESTCUSTOMIZATION,
+                            GUESTCUSTOMIZATION_ENABLE_CUST_SCRIPTS,
+                            "false")
+                        if custScriptConfig.lower() != "true":
+                            # Update the customization status if custom script
+                            # is disabled
+                            msg = (
+                                "Custom script is disabled by VM Administrator"
+                            )
+                            LOG.debug(msg)
+                            set_customization_status(
+                                GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
+                                GuestCustError.GUESTCUST_ERROR_SCRIPT_DISABLED)
+                            raise RuntimeError(msg)
 
                 ccScriptsDir = os.path.join(
                     self.paths.get_cpath("scripts"),
@@ -265,7 +272,7 @@ class DataSourceOVF(sources.DataSource):
             enable_nics(self._vmware_nics_to_enable)
             set_customization_status(
                 GuestCustStateEnum.GUESTCUST_STATE_DONE,
-                GuestCustErrorEnum.GUESTCUST_ERROR_SUCCESS)
+                GuestCustError.GUESTCUST_ERROR_SUCCESS)
             set_gc_status(self._vmware_cust_conf, "Successful")
 
         else:

@xiaofengw-vmware
Copy link
Contributor Author

set_customization_status

Thanks for your comments. When I follow your comment, I couldn't pass the flake8 check for below lines:
set_customization_status(
GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
GuestCustError.GUESTCUST_ERROR_SCRIPT_DISABLED)
It report that the 3rd lines exceed 79 characters. And it seems I couldn't split it into 2 lines. Do you have any suggestions?

@smoser
Copy link
Collaborator

smoser commented Jun 15, 2020

Can the user disable FORCE-RUN-POST-CUST-SCRIPT in any way?

I can see that some users may be helped by this. Personally, I do not think cloud-init should, by default, allow arbitrary execution of code by the platform in the guest. If the default behavior is to do so, then it needs to allow the user to override that behavior in user-data.

@xiaofengw-vmware
Copy link
Contributor Author

Can the user disable FORCE-RUN-POST-CUST-SCRIPT in any way?

I can see that some users may be helped by this. Personally, I do not think cloud-init should, by default, allow arbitrary execution of code by the platform in the guest. If the default behavior is to do so, then it needs to allow the user to override that behavior in user-data.

Thanks Scott. There is no way to disable FORCE-RUN-POST-CUST-SCRIPT. I agree with your that an arbitrary script should be disabled by default.That's the reason why we introduced enable-custom-scripts into the virtual machine configuration file so that guest user could decide whether a script could be executed by vsphere administartor, by default it is disabled.
After we introduced it, our vCloud product customer complained that it break their regression and they prefer to enable it by default. To resolve this, we made this change and vCloud adminstrator will configure FORCE-RUN-POST-CUST-SCRIPT to yes and skip checking enable-custom-scripts. This new API is only exposed to vCloud administrator, other administrators have no change to configure FORCE-RUN-POST-CUST-SCRIPT. How do you think?Thanks.

@smoser
Copy link
Collaborator

smoser commented Jun 16, 2020

@xiaofengw-vmware

Thanks Scott. There is no way to disable FORCE-RUN-POST-CUST-SCRIPT. I agree with your that an arbitrary script should be disabled by default.That's the reason why we introduced enable-custom-scripts into the virtual machine configuration file so that guest user could decide whether a script could be executed by vsphere administartor, by default it is disabled.
After we introduced it, our vCloud product customer complained that it break their regression and they prefer to enable it by default. To resolve this, we made this change and vCloud adminstrator will configure FORCE-RUN-POST-CUST-SCRIPT to yes and skip checking enable-custom-scripts. This new API is only exposed to vCloud administrator, other administrators have no change to configure FORCE-RUN-POST-CUST-SCRIPT. How do you think?Thanks.

The above isn't clear to me.

Cloud-init supports vendor-data, but user-data overrides vendor-data, and can completely disable it. Thus, if the user (the one who starts the instance) wants to disable the platform (or "vmware administrator"), they can.

If cloud-init is providing a facility for someone other than the user to execute arbitrary code in an instance, then cloud-init needs to provide the user a way to disable that. If that upsets the vmware administrator then so be it.

If I'm about to start an instance on vmware and this pull request is accepted, can I disable vmware administrator access?

@xiaofengw-vmware
Copy link
Contributor Author

FORCE-RUN-POST-CUST-SCRIPT

Thanks Scott, I understood your concern. I will change this flag to DEFAULT-RUN-POST-CUST-SCRIPT. This flag will only change the default behavior if enable-custom-scripts is absent in VM Tools configuration. If enable-custom-scripts is set , then DEFAULT-RUN-POST-CUST-SCRIPT will be ignored so that VM administrator could decide whether to execute the script. Is this solution ok for you? I will draft the codes and send you for review. Thanks.

@smoser
Copy link
Collaborator

smoser commented Jun 18, 2020

@xiaofengw-vmware,

My request is simple:

  • user should be able to disable all execution of code that was not inside the image or provided by the user (user-data)
  • this is documented how to do so.

@xiaofengw-vmware
Copy link
Contributor Author

@xiaofengw-vmware,

My request is simple:

  • user should be able to disable all execution of code that was not inside the image or provided by the user (user-data)
    [Xiaofengw]: Yes, user could disable the code by setting in VM Tools configuration file.
  • this is documented how to do so.
    [Xiaofengw]: We will document this in our VM Tools administration document. Do you think whether we need to document this in cloud-init? Thanks.

@xiaofengw-vmware
Copy link
Contributor Author

xiaofengw-vmware commented Jun 19, 2020

@xiaofengw-vmware,

My request is simple:

  • user should be able to disable all execution of code that was not inside the image or provided by the user (user-data)
  • this is documented how to do so.

Thank you all for the comments. We have changed our design and introduce a flag called "DEFAULT-RUN-POST-CUST-SCRIPT". This flag only change the default value of "enable-custom-scripts" to yes. So "enable-custom-scripts" will be "yes" in case it is absent. If "enable-custom-scripts" is explicitly set in VM Tools configuration, we still follow its value to enable/disable the script. Please review below pull request. Thanks.
#441

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants