Merge lp:~rvb/maas/sudo-mipf into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Fix the call to maas-import-pxe-files, this script needs root access.

This fix goes hand in hand with the packaging fix in https://code.launchpad.net/~rvb/maas/packaging.sudo-mipf/+merge/132874.

[Jeroen T. Vermeulen (jtv)]

Yup, that needs root! I wasn't aware of mock.ANY -- it solves a problem that I've been wondering about for some time.

One question: can you think of any situations where the difference between "maas-import-pxe-files" (in the script invocation) and "/usr/sbin/maas-import-pxe-files" (in the sudoers file) might cause trouble?

review: Approve

[Raphaël Badin (rvb)]

> Yup, that needs root! I wasn't aware of mock.ANY -- it solves a problem that
> I've been wondering about for some time.
>
> One question: can you think of any situations where the difference between
> "maas-import-pxe-files" (in the script invocation) and "/usr/sbin/maas-import-
> pxe-files" (in the sudoers file) might cause trouble?

Not really. I think this is the right thing to do:
- the sudo rule allows one to run /usr/sbin/maas-import-pxe-files, which is the script that the package installs.
- the code simply calls 'maas-import-pxe-files', thus allowing the user to override the script if he wants to.

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-10-30 11:19:02 +0000
+++ src/provisioningserver/tasks.py	2012-11-05 11:58:36 +0000
@@ -381,4 +381,4 @@
     if http_proxy is not None:
         env['http_proxy'] = http_proxy
         env['https_proxy'] = http_proxy
-    check_call(['maas-import-pxe-files'], env=env)
+    check_call(['sudo', '-n', 'maas-import-pxe-files'], env=env)

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-10-30 11:19:02 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-11-05 11:58:36 +0000
@@ -543,14 +543,15 @@
     def test_import_pxe_files(self):
         recorder = self.patch(tasks, 'check_call', Mock())
         import_pxe_files()
-        recorder.assert_called_once_with(['maas-import-pxe-files'], env=ANY)
+        recorder.assert_called_once_with(
+            ['sudo', '-n', 'maas-import-pxe-files'], env=ANY)
         self.assertIsInstance(import_pxe_files, Task)
 
     def test_import_pxe_files_preserves_environment(self):
         recorder = self.patch(tasks, 'check_call', Mock())
         import_pxe_files()
         recorder.assert_called_once_with(
-            ['maas-import-pxe-files'], env=os.environ)
+            ['sudo', '-n', 'maas-import-pxe-files'], env=os.environ)
 
     def test_import_pxe_files_sets_proxy(self):
         recorder = self.patch(tasks, 'check_call', Mock())
@@ -558,4 +559,4 @@
         import_pxe_files(http_proxy=proxy)
         expected_env = dict(os.environ, http_proxy=proxy, https_proxy=proxy)
         recorder.assert_called_once_with(
-            ['maas-import-pxe-files'], env=expected_env)
+            ['sudo', '-n', 'maas-import-pxe-files'], env=expected_env)