Merge ~sameid/cloud-init:azure-user-locked-on-instance-id-change into cloud-init:master

Proposed by Sam Eiderman
Status: Merged
Approved by: Ryan Harper
Approved revision: 4e418c0f53a5d23ea1b8329aa529c80e10340e17
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~sameid/cloud-init:azure-user-locked-on-instance-id-change
Merge into: cloud-init:master
Diff against target: 44 lines (+19/-2)
2 files modified
cloudinit/sources/DataSourceAzure.py (+3/-2)
tests/unittests/test_datasource/test_azure.py (+16/-0)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Sam Eiderman (community) Needs Resubmitting
Review via email: mp+374740@code.launchpad.net

Commit message

azure: Do not lock user on instance id change

After initial boot ovf-env.xml is copied to agent dir
(/var/lib/waagent/) with REDACTED password.
On subsequent boots DataSourceAzure loads with a configuration where the
user specified in /var/lib/waagent/ovf-env.xml is locked.
If instance id changes, cc_users_groups action will lock the user.

Fix this behavior by not locking the user if its password is REDACTED.

LP: #1849677

To post a comment you must log in.
Revision history for this message
Ryan Harper (raharper) wrote :

Hi,
Thank you for contributing to cloud-init.

To contribute, you must sign the Canonical Contributor License Agreement (CLA) [1].

If you have already signed it as an individual, your Launchpad user will be listed in the contributor-agreement-canonical launchpad group [2]. Unfortunately there is no easy way to check if an organization or company you are doing work for has signed. If you are unsure or have questions, email <email address hidden> or ping powersj in #cloud-init channel via freenode.

For information on how to sign, please see the HACKING document [3].

Thanks again, and please feel free to reach out with any questions.


[1] http://www.canonical.com/contributors
[2] https://launchpad.net/~contributor-agreement-canonical/+members
[3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for providing more background details in the linked bug.

I'd like to add a unittest for this scenario, something like this

% git diff tests/unittests/test_datasource/test_azure.py
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 3547dd9..6251a42 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -769,6 +769,23 @@ scbus-1 on xpt0 bus 0
                          crypt.crypt(odata['UserPassword'],
                                      defuser['passwd'][0:pos]))

+ def test_user_not_locked_if_password_redacted(self):
+ odata = {'HostName': "myhost", 'UserName': "myuser",
+ 'UserPassword': dsaz.DEF_PASSWD_REDACTION}
+ data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
+
+ dsrc = self._get_ds(data)
+ ret = dsrc.get_data()
+ self.assertTrue(ret)
+ self.assertTrue('default_user' in dsrc.cfg['system_info'])
+ defuser = dsrc.cfg['system_info']['default_user']
+
+ # default user should be updated username and should not be locked.
+ self.assertEqual(defuser['name'], odata['UserName'])
+ self.assertIn('lock_passwd', defuser)
+ self.assertFalse(defuser['lock_passwd'])
+
+

When you've signed the CLA and added a unittest, please move this branch back from WIP to Needs Review.

review: Needs Fixing
Revision history for this message
Sam Eiderman (sameid) wrote :

Hi,

Added the unittest, it was spot on - without the change in DataSourceAzure.py it failed, with it - passes.

Also signed the CLA.

review: Needs Resubmitting
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks!

I've pointed our CI at this; I'll follow up and let you know what, if any, failures occur.
Once CI is happy, we can land.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:4e418c0f53a5d23ea1b8329aa529c80e10340e17
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1238/
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/1238//rebuild

review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 4984fa8..cdf49d3 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -1193,9 +1193,10 @@ def read_azure_ovf(contents):
1193 defuser = {}1193 defuser = {}
1194 if username:1194 if username:
1195 defuser['name'] = username1195 defuser['name'] = username
1196 if password and DEF_PASSWD_REDACTION != password:1196 if password:
1197 defuser['passwd'] = encrypt_pass(password)
1198 defuser['lock_passwd'] = False1197 defuser['lock_passwd'] = False
1198 if DEF_PASSWD_REDACTION != password:
1199 defuser['passwd'] = encrypt_pass(password)
11991200
1200 if defuser:1201 if defuser:
1201 cfg['system_info'] = {'default_user': defuser}1202 cfg['system_info'] = {'default_user': defuser}
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 3547dd9..80c6f01 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -769,6 +769,22 @@ scbus-1 on xpt0 bus 0
769 crypt.crypt(odata['UserPassword'],769 crypt.crypt(odata['UserPassword'],
770 defuser['passwd'][0:pos]))770 defuser['passwd'][0:pos]))
771771
772 def test_user_not_locked_if_password_redacted(self):
773 odata = {'HostName': "myhost", 'UserName': "myuser",
774 'UserPassword': dsaz.DEF_PASSWD_REDACTION}
775 data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
776
777 dsrc = self._get_ds(data)
778 ret = dsrc.get_data()
779 self.assertTrue(ret)
780 self.assertTrue('default_user' in dsrc.cfg['system_info'])
781 defuser = dsrc.cfg['system_info']['default_user']
782
783 # default user should be updated username and should not be locked.
784 self.assertEqual(defuser['name'], odata['UserName'])
785 self.assertIn('lock_passwd', defuser)
786 self.assertFalse(defuser['lock_passwd'])
787
772 def test_userdata_plain(self):788 def test_userdata_plain(self):
773 mydata = "FOOBAR"789 mydata = "FOOBAR"
774 odata = {'UserData': {'text': mydata, 'encoding': 'plain'}}790 odata = {'UserData': {'text': mydata, 'encoding': 'plain'}}

Subscribers

People subscribed via source and target branches