Merge ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master
- Git
- lp:~msaikia/cloud-init
- topic-msaikia-vmware
- Merge into master
Status: | Superseded |
---|---|
Proposed branch: | ~msaikia/cloud-init:topic-msaikia-vmware |
Merge into: | cloud-init:master |
Diff against target: |
361 lines (+281/-4) 4 files modified
cloudinit/sources/DataSourceOVF.py (+54/-1) cloudinit/sources/helpers/vmware/imc/config.py (+34/-3) cloudinit/sources/helpers/vmware/imc/config_passwd.py (+140/-0) tests/unittests/test_vmware/test_password_config.py (+53/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Scott Moser | Needs Fixing | ||
Server Team CI bot | continuous-integration | Approve | |
Review via email: mp+305427@code.launchpad.net |
This proposal has been superseded by a proposal from 2017-04-21.
Commit message
Guest Customization support for product vcloud director - Password Configuration.
This feature enables the following VMware VCloud Director functionalities:
1. Setting admin password
2. Expire password.
3. Set admin password and expire.
Description of the change
1. Retrieve values from configuration file.
2. Parse data and call password customization.
3. Unit tests for password configurator
Joshua Harlow (harlowja) : | # |
- 9666992... by Maitreyee Saikia
-
Addressed cloud-init review comments: 1.Changed property names to not have get. 2.No CamelCase for method name.
- 5b39e5a... by Maitreyee Saikia
-
Addressed cloud-init review comments: 1.Changed property names to not have get. 2.No CamelCase for method name.
- 3c065e8... by Maitreyee Saikia
-
Addressed cloud-init review comments: 1.Changed property names to not have get. 2.No CamelCase for method name.
Maitreyee Saikia (msaikia) wrote : | # |
- 5c2f89e... by Maitreyee Saikia
-
Fixed formatting for vmware config file
- fe0e369... by Maitreyee Saikia
-
Fixed formatting for vmware config file
- 9c2b189... by Maitreyee Saikia
-
Fixed formatting for vmware config file
- be30849... by Maitreyee Saikia
-
Merge branch 'topic-
msaikia- vmware' of git.launchpad. net:~msaikia/ cloud-init into topic-msaikia- vmware
Maitreyee Saikia (msaikia) wrote : | # |
Updated some formatting.
Scott Moser (smoser) wrote : | # |
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) wrote : | # |
> 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
Maitreyee Saikia (msaikia) : | # |
- 3128086... by Maitreyee Saikia
-
Merge conflicts
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:312808680eb
https:/
Executed test runs:
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- 7c480f7... by Maitreyee Saikia
-
Merge from main
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:7c480f71a20
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- f71def6... by Maitreyee Saikia
-
Added password configurator
Scott Moser (smoser) wrote : | # |
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.
- f482c10... by Maitreyee Saikia
-
Call to Password Configurator
Scott Moser (smoser) wrote : | # |
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
- 50e9a02... by Maitreyee Saikia
-
merge conflicts
- 4580c7b... by Maitreyee Saikia
-
typo fix
- 1a801a4... by Maitreyee Saikia
-
Return bool for reset_passwd configuration
- c886370... by Maitreyee Saikia
-
remove regex check for post-gc-statusWq
Maitreyee Saikia (msaikia) wrote : | # |
Hi Scott
I have resubmitted my review request after addressing your comments.
https:/
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/
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:/
You are the owner of ~msaikia/
Unmerged commits
- c886370... by Maitreyee Saikia
-
remove regex check for post-gc-statusWq
- 1a801a4... by Maitreyee Saikia
-
Return bool for reset_passwd configuration
- 4580c7b... by Maitreyee Saikia
-
typo fix
- 50e9a02... by Maitreyee Saikia
-
merge conflicts
- f482c10... by Maitreyee Saikia
-
Call to Password Configurator
- f71def6... by Maitreyee Saikia
-
Added password configurator
- 9780fac... by Maitreyee Saikia
-
Added methods to retrieve new fields from vmware customization spec file
- f9dc832... by Maitreyee Saikia
-
Added methods to retrieve new fields from vmware customization spec file
- ee98395... by Maitreyee Saikia
-
Added methods to retrieve new fields from vmware customization spec file
- 9c2b189... by Maitreyee Saikia
-
Fixed formatting for vmware config file
Preview Diff
1 | diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py |
2 | index d70784a..e4decaa 100644 |
3 | --- a/cloudinit/sources/DataSourceOVF.py |
4 | +++ b/cloudinit/sources/DataSourceOVF.py |
5 | @@ -25,6 +25,8 @@ from cloudinit.sources.helpers.vmware.imc.config_file \ |
6 | import ConfigFile |
7 | from cloudinit.sources.helpers.vmware.imc.config_nic \ |
8 | import NicConfigurator |
9 | +from cloudinit.sources.helpers.vmware.imc.config_passwd \ |
10 | + import PasswordConfigurator |
11 | from cloudinit.sources.helpers.vmware.imc.guestcust_error \ |
12 | import GuestCustErrorEnum |
13 | from cloudinit.sources.helpers.vmware.imc.guestcust_event \ |
14 | @@ -117,6 +119,8 @@ class DataSourceOVF(sources.DataSource): |
15 | (md, ud, cfg) = read_vmware_imc(conf) |
16 | dirpath = os.path.dirname(vmwareImcConfigFilePath) |
17 | nics = get_nics_to_enable(dirpath) |
18 | + markerid = conf.marker_id |
19 | + markerexists = check_marker_exists(markerid) |
20 | except Exception as e: |
21 | LOG.debug("Error parsing the customization Config File") |
22 | LOG.exception(e) |
23 | @@ -127,7 +131,6 @@ class DataSourceOVF(sources.DataSource): |
24 | return False |
25 | finally: |
26 | util.del_dir(os.path.dirname(vmwareImcConfigFilePath)) |
27 | - |
28 | try: |
29 | LOG.debug("Applying the Network customization") |
30 | nicConfigurator = NicConfigurator(conf.nics) |
31 | @@ -140,6 +143,37 @@ class DataSourceOVF(sources.DataSource): |
32 | GuestCustEventEnum.GUESTCUST_EVENT_NETWORK_SETUP_FAILED) |
33 | enable_nics(nics) |
34 | return False |
35 | + if markerid and not markerexists: |
36 | + try: |
37 | + LOG.debug("Applying password customization") |
38 | + pwdConfigurator = PasswordConfigurator() |
39 | + adminpwd = conf.admin_password |
40 | + resetpwd = conf.reset_password |
41 | + if adminpwd or resetpwd: |
42 | + cfg_new = pwdConfigurator.configure(adminpwd, resetpwd) |
43 | + cfg = util.mergemanydict([cfg, cfg_new]) |
44 | + else: |
45 | + LOG.debug("Changing password is not needed") |
46 | + except Exception as e: |
47 | + LOG.debug("Error applying the Password Configuration") |
48 | + LOG.exception(e) |
49 | + set_customization_status( |
50 | + GuestCustStateEnum.GUESTCUST_STATE_RUNNING, |
51 | + GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) |
52 | + enable_nics(nics) |
53 | + return False |
54 | + if markerid: |
55 | + LOG.debug("Handle marker creation") |
56 | + try: |
57 | + setup_marker_files(markerid) |
58 | + except Exception as e: |
59 | + LOG.debug("Error creating marker files") |
60 | + LOG.exception(e) |
61 | + set_customization_status( |
62 | + GuestCustStateEnum.GUESTCUST_STATE_RUNNING, |
63 | + GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) |
64 | + enable_nics(nics) |
65 | + return False |
66 | |
67 | vmwarePlatformFound = True |
68 | set_customization_status( |
69 | @@ -445,4 +479,23 @@ datasources = ( |
70 | def get_datasource_list(depends): |
71 | return sources.list_from_depends(depends, datasources) |
72 | |
73 | + |
74 | +# To check if marker file exists |
75 | +def check_marker_exists(markerid): |
76 | + if not markerid: |
77 | + return False |
78 | + markerfile = "/.markerfile-" + markerid |
79 | + if os.path.exists(markerfile): |
80 | + return True |
81 | + return False |
82 | + |
83 | + |
84 | +# Create a marker file |
85 | +def setup_marker_files(markerid): |
86 | + if not markerid: |
87 | + return |
88 | + markerfile = "/.markerfile-" + markerid |
89 | + util.subp(["/bin/rm", "-f", "/.markerfile-*.txt"]) |
90 | + util.subp(["/bin/touch", markerfile]) |
91 | + |
92 | # vi: ts=4 expandtab |
93 | diff --git a/cloudinit/sources/helpers/vmware/imc/config.py b/cloudinit/sources/helpers/vmware/imc/config.py |
94 | index 9a5e3a8..54e26ae 100644 |
95 | --- a/cloudinit/sources/helpers/vmware/imc/config.py |
96 | +++ b/cloudinit/sources/helpers/vmware/imc/config.py |
97 | @@ -5,6 +5,7 @@ |
98 | # |
99 | # This file is part of cloud-init. See LICENSE file for license information. |
100 | |
101 | + |
102 | from .nic import Nic |
103 | |
104 | |
105 | @@ -14,13 +15,17 @@ class Config(object): |
106 | Specification file. |
107 | """ |
108 | |
109 | + CUSTOM_SCRIPT = 'CUSTOM-SCRIPT|SCRIPT-NAME' |
110 | DNS = 'DNS|NAMESERVER|' |
111 | - SUFFIX = 'DNS|SUFFIX|' |
112 | + DOMAINNAME = 'NETWORK|DOMAINNAME' |
113 | + HOSTNAME = 'NETWORK|HOSTNAME' |
114 | + MARKERID = 'MISC|MARKER-ID' |
115 | PASS = 'PASSWORD|-PASS' |
116 | + POST_GC = 'MISC|POST-GC-STATUS' |
117 | + RESETPASS = 'PASSWORD|RESET' |
118 | + SUFFIX = 'DNS|SUFFIX|' |
119 | TIMEZONE = 'DATETIME|TIMEZONE' |
120 | UTC = 'DATETIME|UTC' |
121 | - HOSTNAME = 'NETWORK|HOSTNAME' |
122 | - DOMAINNAME = 'NETWORK|DOMAINNAME' |
123 | |
124 | def __init__(self, configFile): |
125 | self._configFile = configFile |
126 | @@ -82,4 +87,30 @@ class Config(object): |
127 | |
128 | return res |
129 | |
130 | + @property |
131 | + def reset_password(self): |
132 | + """Retreives if the root password needs to be reset.""" |
133 | + resetPass = self._configFile.get(Config.RESETPASS, None) |
134 | + if resetPass and not resetPass.lower() in ('yes', 'no'): |
135 | + raise ValueError('ResetPassword value should be yes/no') |
136 | + return True if resetPass.lower() == 'yes' else False |
137 | + |
138 | + @property |
139 | + def marker_id(self): |
140 | + """Returns marker id.""" |
141 | + return self._configFile.get(Config.MARKERID, None) |
142 | + |
143 | + @property |
144 | + def post_gc_status(self): |
145 | + """Retreives if customization status needs to be posted.""" |
146 | + postGcStatus = self._configFile.get(Config.POST_GC, None) |
147 | + if postGcStatus and not postGcStatus.lower() in ('yes', 'no'): |
148 | + raise ValueError('GC status value should be yes/no') |
149 | + return postGcStatus |
150 | + |
151 | + @property |
152 | + def custom_script_name(self): |
153 | + """Return the name of custom (pre/post) script.""" |
154 | + return self._configFile.get(Config.CUSTOM_SCRIPT, None) |
155 | + |
156 | # vi: ts=4 expandtab |
157 | diff --git a/cloudinit/sources/helpers/vmware/imc/config_passwd.py b/cloudinit/sources/helpers/vmware/imc/config_passwd.py |
158 | new file mode 100644 |
159 | index 0000000..3812eb0 |
160 | --- /dev/null |
161 | +++ b/cloudinit/sources/helpers/vmware/imc/config_passwd.py |
162 | @@ -0,0 +1,140 @@ |
163 | +# vi: ts=4 expandtab |
164 | +# |
165 | +# Copyright (C) 2016 Canonical Ltd. |
166 | +# Copyright (C) 2016 VMware INC. |
167 | +# |
168 | +# Author: Maitreyee Saikia <msaikia@vmware.com> |
169 | +# |
170 | +# This program is free software: you can redistribute it and/or modify |
171 | +# it under the terms of the GNU General Public License version 3, as |
172 | +# published by the Free Software Foundation. |
173 | +# |
174 | +# This program is distributed in the hope that it will be useful, |
175 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
176 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
177 | +# GNU General Public License for more details. |
178 | +# |
179 | +# You should have received a copy of the GNU General Public License |
180 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
181 | + |
182 | +import logging |
183 | +import os |
184 | +import subprocess |
185 | + |
186 | +from cloudinit import util |
187 | + |
188 | +logger = logging.getLogger(__name__) |
189 | + |
190 | + |
191 | +class ProgramNotFound(Exception): |
192 | + pass |
193 | + |
194 | + |
195 | +class Apps(object): |
196 | + """ |
197 | + Stores the path of applications needed to expire or set admin passwords. |
198 | + """ |
199 | + BASE64 = '/usr/bin/base64' |
200 | + CHAGE = '/usr/bin/chage' |
201 | + CHPASSWD = '/usr/sbin/chpasswd' |
202 | + PASSWD = '/usr/bin/passwd' |
203 | + |
204 | + |
205 | +class PasswordConfigurator(object): |
206 | + """ |
207 | + Class for changing configurations related to passwords in a VM. Includes |
208 | + setting and expiring passwords. |
209 | + """ |
210 | + def configure(self, passwd, resetPasswd): |
211 | + """ |
212 | + Main method to perform all functionalities based on configuration file |
213 | + inputs. |
214 | + @param passwd: encoded admin password. |
215 | + @param resetPasswd: boolean to determine if password needs to be reset. |
216 | + @return cfg: dict to be used by cloud-init set_passwd code. |
217 | + """ |
218 | + logger.info('Starting password configuration') |
219 | + if passwd: |
220 | + passwd = self.decode_base64(passwd) |
221 | + allRootUsers = [] |
222 | + for line in open('/etc/passwd', 'r'): |
223 | + if line.split(':')[2] == '0': |
224 | + allRootUsers.append(line.split(':')[0]) |
225 | + # read shadow file and check for each user, if its uid0 or root. |
226 | + uidUsersList = [] |
227 | + rootUsersList = [] |
228 | + for line in open('/etc/shadow', 'r'): |
229 | + user = line.split(':')[0] |
230 | + if user in allRootUsers: |
231 | + uidUsersList.append(user) |
232 | + if user == 'root': |
233 | + rootUsersList.append('%s:%s' % (user, passwd)) |
234 | + rootUsers = '\n'.join(rootUsersList) + '\n' |
235 | + cfg = {} |
236 | + if passwd: |
237 | + cfg = self.set_admin_password(rootUsers, resetPasswd) |
238 | + if not passwd and resetPasswd: |
239 | + self.reset_password(uidUsersList) |
240 | + logger.info('Configure Password completed!') |
241 | + return cfg |
242 | + |
243 | + def decode_base64(self, passwd): |
244 | + """ |
245 | + Method to decode password using base64. |
246 | + Raise error if base64 doesnot exist to decode the password. |
247 | + @param passwd: encoded password. |
248 | + @return: decoded password. |
249 | + """ |
250 | + logger.info('Checking BASE64 to decode password.') |
251 | + if not os.path.exists(Apps.BASE64): |
252 | + logger.debug('Base64 is not found! Exiting!') |
253 | + raise ProgramNotFound('Base64 not found! Cant customize password') |
254 | + decode_passwd = '' |
255 | + if len(passwd) > 0: |
256 | + try: |
257 | + p1 = subprocess.Popen(['/bin/echo', passwd], |
258 | + stdout=subprocess.PIPE) |
259 | + p2 = subprocess.Popen([Apps.BASE64, '-di'], stdin=p1.stdout, |
260 | + stdout=subprocess.PIPE) |
261 | + decode_passwd, _ = p2.communicate() |
262 | + except OSError as e: |
263 | + raise util.ProcessExecutionError(description='Decoding passwd', |
264 | + reason=e, errno=e.errno) |
265 | + logger.info('Decoding password completed') |
266 | + return decode_passwd.decode().strip() |
267 | + |
268 | + def set_admin_password(self, rootUsers, resetPasswd): |
269 | + """ |
270 | + Method to setup a dict to be used by cloud-init code for configuring |
271 | + password. |
272 | + @param rootUsers: 'root:passwd' string to set. |
273 | + resetPasswd: boolean to expire passwd |
274 | + @return: dictionary of dictionary containing information for setting |
275 | + admin password. Format as per cloud-init's set passwd method. |
276 | + """ |
277 | + logger.info('Setting admin password.') |
278 | + cfg = {} |
279 | + if os.path.exists(Apps.CHPASSWD): |
280 | + cfg['chpasswd'] = {} |
281 | + cfg['chpasswd']['list'] = rootUsers |
282 | + cfg['chpasswd']['expire'] = resetPasswd |
283 | + else: |
284 | + logger.debug('Cannot set admin password. chpasswd not present') |
285 | + return cfg |
286 | + |
287 | + def reset_password(self, uidUserList): |
288 | + """ |
289 | + Method to reset password. Use passwd --expire command. Use chage if |
290 | + not succeeded using passwd command. Log failure message otherwise. |
291 | + @param: list of users for which to expire password. |
292 | + """ |
293 | + logger.info('Expiring password.') |
294 | + for user in uidUserList: |
295 | + try: |
296 | + out, err = util.subp([Apps.PASSWD, '--expire', user]) |
297 | + except util.ProcessExecutionError as e: |
298 | + if os.path.exists(Apps.CHAGE): |
299 | + out, e = util.subp([Apps.CHAGE, '-d', '0', user]) |
300 | + else: |
301 | + logger.debug('Failed to expire password for %s with error:\ |
302 | + %s' % (user, e)) |
303 | diff --git a/tests/unittests/test_vmware/test_password_config.py b/tests/unittests/test_vmware/test_password_config.py |
304 | new file mode 100644 |
305 | index 0000000..7418fc8 |
306 | --- /dev/null |
307 | +++ b/tests/unittests/test_vmware/test_password_config.py |
308 | @@ -0,0 +1,53 @@ |
309 | +# vi: ts=4 expandtab |
310 | +# |
311 | +# Copyright (C) 2015 Canonical Ltd. |
312 | +# Copyright (C) 2016 VMware INC. |
313 | +# |
314 | +# Author: Maitreyee Saikia <msaikia@vmware.com> |
315 | +# |
316 | +# This program is free software: you can redistribute it and/or modify |
317 | +# it under the terms of the GNU General Public License version 3, as |
318 | +# published by the Free Software Foundation. |
319 | +# |
320 | +# This program is distributed in the hope that it will be useful, |
321 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of |
322 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
323 | +# GNU General Public License for more details. |
324 | +# |
325 | +# You should have received a copy of the GNU General Public License |
326 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
327 | + |
328 | +import os |
329 | +import subprocess |
330 | +import unittest |
331 | + |
332 | +from cloudinit.sources.helpers.vmware.imc.config_passwd import ( |
333 | + Apps, |
334 | + PasswordConfigurator, |
335 | + ProgramNotFound |
336 | +) |
337 | + |
338 | + |
339 | +class TestVMwarePassword(unittest.TestCase): |
340 | + |
341 | + def test_decode_passwd(self): |
342 | + passwd = 'TestPass' |
343 | + pwConf = PasswordConfigurator() |
344 | + if os.path.exists(Apps.BASE64): |
345 | + p1 = subprocess.Popen(['echo', passwd], stdout=subprocess.PIPE) |
346 | + p2 = subprocess.Popen([Apps.BASE64], stdin=p1.stdout, |
347 | + stdout=subprocess.PIPE) |
348 | + encodedPw, _ = p2.communicate() |
349 | + decodedPw = pwConf.decode_base64(encodedPw.decode().strip()) |
350 | + self.assertEqual(passwd, decodedPw) |
351 | + else: |
352 | + self.assertRaises(ProgramNotFound, pwConf.decode_base64, passwd) |
353 | + |
354 | + def test_set_admin_pass(self): |
355 | + pwConf = PasswordConfigurator() |
356 | + cfg = pwConf.set_admin_password('root:passwd', False) |
357 | + if os.path.exists(Apps.CHPASSWD): |
358 | + self.assertEqual('root:passwd', cfg['chpasswd']['list']) |
359 | + self.assertFalse(cfg['chpasswd']['expire']) |
360 | + else: |
361 | + self.assertEqual({}, cfg) |
Thanks for the comments Joshua.
Updated the diff.