Merge ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master

[Maitreyee Saikia]

vcloud directoy: Guest Customization support for passwords

This feature enables the following VMware VCloud Director functionalities:
1. Setting admin password
2. Expire password.
3. Set admin password and expire.
Password configuration is triggered only as part of a full recustomization, that happens either on first power on or when "poweron and full recustomization" is selected. Full customization flow is determined by marker files. Unique marker ids are generated when full recustomization is requested. And marker file based on these marker ids help to determine if we need to execute the above configuration.

1. Retrieve values from configuration file.
2. Parse data and call password customization.
3. Unit tests for retrieved password data

[Maitreyee Saikia (msaikia)]

Thanks for the comments Joshua.
Updated the diff.

[Maitreyee Saikia (msaikia)]

Updated some formatting.

[Scott Moser (smoser)]

Please set a combined commit message ('Set commit message' above.

Can you explain where you're going also?
I'm concerned about adding more "vmware" paths for configuring things that are done elsewhere (or not elsewhere) in cloud-init. We'd like have consistent paths for doing things as much as possible.

I understand that you're trying to have cloud-init take over another more solution, and I'm not entirely opposed to that, but I want to integrate as much as possible rather than having specific paths and function on vmware.

[Maitreyee Saikia (msaikia)]

> Please set a combined commit message ('Set commit message' above.
>
> Can you explain where you're going also?
> I'm concerned about adding more "vmware" paths for configuring things that are
> done elsewhere (or not elsewhere) in cloud-init. We'd like have consistent
> paths for doing things as much as possible.
>
> I understand that you're trying to have cloud-init take over another more
> solution, and I'm not entirely opposed to that, but I want to integrate as
> much as possible rather than having specific paths and function on vmware.

Thanks for your input Scott. I have updated the commit message.
This changeset is just to retrieve data from customization spec, for implementing some other customization functionalities like setting password, running post customization scripts etc. I shall post another changeset with the password configurator, which manipulates the values retrieved from these methods to be used by cloud-init's cc_set_password. We will be using as much cloud-init code as possible. For other functionalities like running pre and post customization specs that will be uploaded by the user, I dont think there is existing support. Please let me know if that sounds ok. Also for functions that are "vmware" specific, shall we still keep it in the helper path that we have created for vmware, or do we want to move them to some common code area?
Thanks
Maitreyee

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

FAILED: Continuous integration, rev:312808680eb5484b112ef874bf13fe2fdac8515e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/87/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/87
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/87/console
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/87
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-amd64/87
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/87

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:7c480f71a2016f9a727479a54a002d03ec24453f
https://jenkins.ubuntu.com/server/job/cloud-init-ci/94/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/94
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/94
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/94
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/94
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/94

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

There is one small nitpick in line.
There isn't anything wrong with this merge proposal, but it
a.) doesn't really do anything
  I realize you're planning on using these things going forward, and thats fine, but there is no reason to accept this MP until we also see that further work. So just add it here. You can rebase/squahs your commits into single function items in a "series" if that helps you.

b.) should add some unit tests.
unit tests will help stop others from breaking your code path inadvertently.

review: Needs Fixing

[Scott Moser (smoser)]

Hi. I'm going to put this in "work in progress".
See the comments above. When youv'e addressed them, move it back to Needs Review.
Thanks
Scott

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

PASSED: Continuous integration, rev:c886370ebec4e2383a017e15d1d54be2c3d18152
https://jenkins.ubuntu.com/server/job/cloud-init-ci/262/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/262
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/262
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/262
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/262
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/262

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

review: Approve (continuous-integration)

[Maitreyee Saikia (msaikia)]

Hi Scott
I have resubmitted my review request after addressing your comments.
https://code.launchpad.net/~msaikia/cloud-init/+git/cloud-init/+merge/322991
Please let me know if you get a chance to review the new changeset.

Thanks
Maitreyee

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Tuesday, March 14, 2017 7:25 AM
To: Maitreyee Saikia <email address hidden>
Subject: Re: [Merge] ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master

Hi. I'm going to put this in "work in progress".
See the comments above. When youv'e addressed them, move it back to Needs Review.
Thanks
Scott
--
https://code.launchpad.net/~msaikia/cloud-init/+git/cloud-init/+merge/305427
You are the owner of ~msaikia/cloud-init:topic-msaikia-vmware.

[Scott Moser (smoser)]

Your commit message is probably out of date. Update that please.
Missing from it is at very least a mention of 'markerfile'. I have a feeling you're doing something that might be already done in util.FileSemaphores.

Thats just a quick review.
There are probably some other things.

Thanks!
Scott

a884268... by Maitreyee Saikia on 2017-06-02

Review comments

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

PASSED: Continuous integration, rev:324eb7e3ad0a22a1a72c322e363d01bffdea2fe6
https://jenkins.ubuntu.com/server/job/cloud-init-ci/451/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/451
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/451
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/451
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/451
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/451

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

review: Approve (continuous-integration)

[Maitreyee Saikia (msaikia)]

> Your commit message is probably out of date. Update that please.
> Missing from it is at very least a mention of 'markerfile'. I have a feeling
> you're doing something that might be already done in util.FileSemaphores.
>
> Thats just a quick review.
> There are probably some other things.
>
> Thanks!
> Scott

Thanks Scott for the reviews. I have updated the changeset and commit message with your comments. As for the Markerfile, it does not have anything to do with semaphores. Here is an explanation of what our marker file is used for. vCloud Director creates a new marker id every time a full customization is requested. And since a new marker id is generated and we do not have the corresponding marker file as yet, we go forward and execute password configurator and then create the marker file in order that this code execution doesnot happen in future unless a new marker id is generated (new marker id is generated only on first power on or when "power on and force recustomization" is selected). So, I do not feel we can use FileSemaphores here.

[Scott Moser (smoser)]

i think i'm fine with this. one tiny comment in line.

a080ca6... by Maitreyee Saikia on 2017-08-03

Addressed review comment to use warning when unable to reset password

[Maitreyee Saikia (msaikia)]

> i think i'm fine with this. one tiny comment in line.
Addressed. Thanks.

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

FAILED: Continuous integration, rev:db962f2e31d9e6acb44828ee27b51a1a9b85a3e3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/115/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:f9a1fa4f56cde70faf7755c6285ccf61a9a176df
https://jenkins.ubuntu.com/server/job/cloud-init-ci/117/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

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

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

I believe you need to rebase to master to resolve this issue. You'll likely need to push --force your branch here to overwrite the history because of the rebase.

0531d33... by Maitreyee Saikia on 2017-08-07

Fix tox errors

2395779... by Maitreyee Saikia on 2017-08-07

Pull from remote

3da515e... by Maitreyee Saikia on 2017-08-07

merge conflicts

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

FAILED: Continuous integration, rev:3da515eeaaf5bd591aeee7b950b37b7553af0fcc
https://jenkins.ubuntu.com/server/job/cloud-init-ci/128/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: CentOS 6 & 7: Build & Test

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

review: Needs Fixing (continuous-integration)

[Chad Smith (chad.smith)]

Thanks for the submit and pinging us. Here's a brief review of your code. Most and nits, feel free to object if you feel strongly one way or another. Again thanks for your time on this. I'll watch for updates to see where we get.

4cec8b6... by Maitreyee Saikia on 2017-08-08

Addressed Chad's review comments

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

FAILED: Continuous integration, rev:4cec8b64ce655496e2df894bc3b742571f116a8d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/129/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: CentOS 6 & 7: Build & Test

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

review: Needs Fixing (continuous-integration)

d9e02b9... by Maitreyee Saikia on 2017-08-09

Fixed CI test in cloud-init for Centos 6/7 tests

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

PASSED: Continuous integration, rev:d9e02b9b1d777445c2a8151f56fa3a1a3ba35e1a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/130/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: CentOS 6 & 7: Build & Test
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

I'm +1 with this changeset, thanks for the good work.

review: Approve

diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index f20c9a6..73d3877 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -25,6 +25,8 @@ from cloudinit.sources.helpers.vmware.imc.config_file \
     import ConfigFile
 from cloudinit.sources.helpers.vmware.imc.config_nic \
     import NicConfigurator
+from cloudinit.sources.helpers.vmware.imc.config_passwd \
+    import PasswordConfigurator
 from cloudinit.sources.helpers.vmware.imc.guestcust_error \
     import GuestCustErrorEnum
 from cloudinit.sources.helpers.vmware.imc.guestcust_event \
@@ -117,6 +119,8 @@ class DataSourceOVF(sources.DataSource):
                 (md, ud, cfg) = read_vmware_imc(conf)
                 dirpath = os.path.dirname(vmwareImcConfigFilePath)
                 nics = get_nics_to_enable(dirpath)
+                markerid = conf.marker_id
+                markerexists = check_marker_exists(markerid)
             except Exception as e:
                 LOG.debug("Error parsing the customization Config File")
                 LOG.exception(e)
@@ -127,7 +131,6 @@ class DataSourceOVF(sources.DataSource):
                 return False
             finally:
                 util.del_dir(os.path.dirname(vmwareImcConfigFilePath))
-
             try:
                 LOG.debug("Applying the Network customization")
                 nicConfigurator = NicConfigurator(conf.nics)
@@ -140,6 +143,35 @@ class DataSourceOVF(sources.DataSource):
                     GuestCustEventEnum.GUESTCUST_EVENT_NETWORK_SETUP_FAILED)
                 enable_nics(nics)
                 return False
+            if markerid and not markerexists:
+                LOG.debug("Applying password customization")
+                pwdConfigurator = PasswordConfigurator()
+                adminpwd = conf.admin_password
+                try:
+                    resetpwd = conf.reset_password
+                    if adminpwd or resetpwd:
+                        pwdConfigurator.configure(adminpwd, resetpwd,
+                                                  self.distro)
+                    else:
+                        LOG.debug("Changing password is not needed")
+                except Exception as e:
+                    LOG.debug("Error applying Password Configuration: %s", e)
+                    set_customization_status(
+                        GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
+                        GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED)
+                    enable_nics(nics)
+                    return False
+            if markerid:
+                LOG.debug("Handle marker creation")
+                try:
+                    setup_marker_files(markerid)
+                except Exception as e:
+                    LOG.debug("Error creating marker files: %s", e)
+                    set_customization_status(
+                        GuestCustStateEnum.GUESTCUST_STATE_RUNNING,
+                        GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED)
+                    enable_nics(nics)
+                    return False
 
             vmwarePlatformFound = True
             set_customization_status(
@@ -445,4 +477,33 @@ datasources = (
 def get_datasource_list(depends):
     return sources.list_from_depends(depends, datasources)
 
+
+# To check if marker file exists
+def check_marker_exists(markerid):
+    """
+    Check the existence of a marker file.
+    Presence of marker file determines whether a certain code path is to be
+    executed. It is needed for partial guest customization in VMware.
+    """
+    if not markerid:
+        return False
+    markerfile = "/.markerfile-" + markerid
+    if os.path.exists(markerfile):
+        return True
+    return False
+
+
+# Create a marker file
+def setup_marker_files(markerid):
+    """
+    Create a new marker file.
+    Marker files are unique to a full customization workflow in VMware
+    environment.
+    """
+    if not markerid:
+        return
+    markerfile = "/.markerfile-" + markerid
+    util.del_file("/.markerfile-*.txt")
+    open(markerfile, 'w').close()
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/helpers/vmware/imc/config.py b/cloudinit/sources/helpers/vmware/imc/config.py
index 9a5e3a8..49d441d 100644
--- a/cloudinit/sources/helpers/vmware/imc/config.py
+++ b/cloudinit/sources/helpers/vmware/imc/config.py
@@ -5,6 +5,7 @@
 #
 # This file is part of cloud-init. See LICENSE file for license information.
 
+
 from .nic import Nic
 
 
@@ -14,13 +15,16 @@ class Config(object):
     Specification file.
     """
 
+    CUSTOM_SCRIPT = 'CUSTOM-SCRIPT|SCRIPT-NAME'
     DNS = 'DNS|NAMESERVER|'
-    SUFFIX = 'DNS|SUFFIX|'
+    DOMAINNAME = 'NETWORK|DOMAINNAME'
+    HOSTNAME = 'NETWORK|HOSTNAME'
+    MARKERID = 'MISC|MARKER-ID'
     PASS = 'PASSWORD|-PASS'
+    RESETPASS = 'PASSWORD|RESET'
+    SUFFIX = 'DNS|SUFFIX|'
     TIMEZONE = 'DATETIME|TIMEZONE'
     UTC = 'DATETIME|UTC'
-    HOSTNAME = 'NETWORK|HOSTNAME'
-    DOMAINNAME = 'NETWORK|DOMAINNAME'
 
     def __init__(self, configFile):
         self._configFile = configFile
@@ -82,4 +86,18 @@ class Config(object):
 
         return res
 
+    @property
+    def reset_password(self):
+        """Retreives if the root password needs to be reset."""
+        resetPass = self._configFile.get(Config.RESETPASS, 'no')
+        resetPass = resetPass.lower()
+        if resetPass not in ('yes', 'no'):
+            raise ValueError('ResetPassword value should be yes/no')
+        return resetPass == 'yes'
+
+    @property
+    def marker_id(self):
+        """Returns marker id."""
+        return self._configFile.get(Config.MARKERID, None)
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/sources/helpers/vmware/imc/config_passwd.py b/cloudinit/sources/helpers/vmware/imc/config_passwd.py
new file mode 100644
index 0000000..75cfbaa
--- /dev/null
+++ b/cloudinit/sources/helpers/vmware/imc/config_passwd.py
@@ -0,0 +1,67 @@
+#    Copyright (C) 2016 Canonical Ltd.
+#    Copyright (C) 2016 VMware INC.
+#
+#    Author: Maitreyee Saikia <[e-mail]>
+#
+#    This file is part of cloud-init. See LICENSE file for license information.
+
+
+import logging
+import os
+
+from cloudinit import util
+
+LOG = logging.getLogger(__name__)
+
+
+class PasswordConfigurator(object):
+    """
+    Class for changing configurations related to passwords in a VM. Includes
+    setting and expiring passwords.
+    """
+    def configure(self, passwd, resetPasswd, distro):
+        """
+        Main method to perform all functionalities based on configuration file
+        inputs.
+        @param passwd: encoded admin password.
+        @param resetPasswd: boolean to determine if password needs to be reset.
+        @return cfg: dict to be used by cloud-init set_passwd code.
+        """
+        LOG.info('Starting password configuration')
+        if passwd:
+            passwd = util.b64d(passwd)
+        allRootUsers = []
+        for line in open('/etc/passwd', 'r'):
+            if line.split(':')[2] == '0':
+                allRootUsers.append(line.split(':')[0])
+        # read shadow file and check for each user, if its uid0 or root.
+        uidUsersList = []
+        for line in open('/etc/shadow', 'r'):
+            user = line.split(':')[0]
+            if user in allRootUsers:
+                uidUsersList.append(user)
+        if passwd:
+            LOG.info('Setting admin password')
+            distro.set_passwd('root', passwd)
+        if resetPasswd:
+            self.reset_password(uidUsersList)
+        LOG.info('Configure Password completed!')
+
+    def reset_password(self, uidUserList):
+        """
+        Method to reset password. Use passwd --expire command. Use chage if
+        not succeeded using passwd command. Log failure message otherwise.
+        @param: list of users for which to expire password.
+        """
+        LOG.info('Expiring password.')
+        for user in uidUserList:
+            try:
+                out, err = util.subp(['passwd', '--expire', user])
+            except util.ProcessExecutionError as e:
+                if os.path.exists('/usr/bin/chage'):
+                    out, e = util.subp(['chage', '-d', '0', user])
+                else:
+                    LOG.warning('Failed to expire password for %s with error: '
+                                '%s', user, e)
+
+# vi: ts=4 expandtab
diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py
index 18475f1..03b36d3 100644
--- a/tests/unittests/test_vmware_config_file.py
+++ b/tests/unittests/test_vmware_config_file.py
@@ -7,8 +7,8 @@
 
 import logging
 import sys
-import unittest
 
+from .helpers import CiTestCase
 from cloudinit.sources.helpers.vmware.imc.boot_proto import BootProtoEnum
 from cloudinit.sources.helpers.vmware.imc.config import Config
 from cloudinit.sources.helpers.vmware.imc.config_file import ConfigFile
@@ -17,7 +17,7 @@ logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
 logger = logging.getLogger(__name__)
 
 
-class TestVmwareConfigFile(unittest.TestCase):
+class TestVmwareConfigFile(CiTestCase):
 
     def test_utility_methods(self):
         cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg")
@@ -90,4 +90,32 @@ class TestVmwareConfigFile(unittest.TestCase):
         self.assertEqual('00:50:56:a6:8c:08', nics[0].mac, "mac0")
         self.assertEqual(BootProtoEnum.DHCP, nics[0].bootProto, "bootproto0")
 
+    def test_config_password(self):
+        cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg")
+
+        cf._insertKey("PASSWORD|-PASS", "test-password")
+        cf._insertKey("PASSWORD|RESET", "no")
+
+        conf = Config(cf)
+        self.assertEqual('test-password', conf.admin_password, "password")
+        self.assertFalse(conf.reset_password, "do not reset password")
+
+    def test_config_reset_passwd(self):
+        cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg")
+
+        cf._insertKey("PASSWORD|-PASS", "test-password")
+        cf._insertKey("PASSWORD|RESET", "random")
+
+        conf = Config(cf)
+        with self.assertRaises(ValueError):
+            conf.reset_password()
+
+        cf.clear()
+        cf._insertKey("PASSWORD|RESET", "yes")
+        self.assertEqual(1, len(cf), "insert size")
+
+        conf = Config(cf)
+        self.assertTrue(conf.reset_password, "reset password")
+
+
 # vi: ts=4 expandtab