launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/sudo-write-file into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/maas/sudo-write-file/+merge/124607

I'll be needing this for another branch.  Keeping it separate to manage branch size.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/sudo-write-file/+merge/124607
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/sudo-write-file into lp:maas.

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-09-14 11:04:33 +0000
+++ src/provisioningserver/tasks.py	2012-09-17 06:52:21 +0000
@@ -26,8 +26,6 @@
 from subprocess import (
     CalledProcessError,
     check_call,
-    PIPE,
-    Popen,
     )
 
 from celery.task import task
@@ -50,6 +48,7 @@
     PowerAction,
     PowerActionFail,
     )
+from provisioningserver.utils import sudo_write_file
 
 # For each item passed to refresh_secrets, a refresh function to give it to.
 refresh_functions = {
@@ -309,11 +308,9 @@
 
     :param **kwargs: Keyword args passed to dhcp.config.get_config()
     """
-    output = config.get_config(**kwargs).encode("ascii")
-    proc = Popen(
-        ["sudo", "maas-provision", "atomic-write", "--filename",
-        DHCP_CONFIG_FILE, "--mode", "744"], stdin=PIPE)
-    proc.communicate(output)
+    sudo_write_file(
+        DHCP_CONFIG_FILE, config.get_config(**kwargs), encoding='ascii',
+        mode=0744)
     restart_dhcp_server()
 
 

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-09-14 14:16:01 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-09-17 06:52:21 +0000
@@ -38,6 +38,7 @@
     auth,
     cache,
     tasks,
+    utils,
     )
 from provisioningserver.dhcp import (
     config,
@@ -237,7 +238,7 @@
     def test_write_dhcp_config_invokes_script_correctly(self):
         mocked_proc = Mock()
         mocked_popen = self.patch(
-            tasks, "Popen", Mock(return_value=mocked_proc))
+            utils, "Popen", Mock(return_value=mocked_proc))
         mocked_check_call = self.patch(tasks, "check_call")
 
         config_params = self.make_dhcp_config_params()
@@ -246,7 +247,7 @@
         # It should construct Popen with the right parameters.
         mocked_popen.assert_any_call(
             ["sudo", "maas-provision", "atomic-write", "--filename",
-            DHCP_CONFIG_FILE, "--mode", "744"], stdin=PIPE)
+            DHCP_CONFIG_FILE, "--mode", "0744"], stdin=PIPE)
 
         # It should then pass the content to communicate().
         content = config.get_config(**config_params).encode("ascii")

=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py	2012-09-07 07:25:53 +0000
+++ src/provisioningserver/tests/test_utils.py	2012-09-17 06:52:21 +0000
@@ -48,6 +48,7 @@
     pick_new_mtime,
     Safe,
     ShellTemplate,
+    sudo_write_file,
     write_custom_config_section,
     )
 from testtools.matchers import (
@@ -388,8 +389,34 @@
             write_custom_config_section(original, new_custom_section))
 
 
+class SudoWriteFileTest(TestCase):
+    """Testing for `sudo_write_file`."""
+
+    def test_calls_atomic_write(self):
+        self.patch(provisioningserver.utils, 'Popen')
+        path = os.path.join(self.make_dir(), factory.make_name('file'))
+        contents = factory.getRandomString()
+
+        sudo_write_file(path, contents)
+
+        provisioningserver.utils.Popen.assert_called_once_with([
+            'sudo', 'maas-provision', 'atomic-write',
+            '--filename', path, '--mode', '0744',
+            ],
+            stdin=PIPE)
+
+    def test_encodes_contents(self):
+        process = Mock()
+        self.patch(
+            provisioningserver.utils, 'Popen', Mock(return_value=process))
+        contents = factory.getRandomString()
+        encoding = 'utf-16'
+        sudo_write_file(self.make_file(), contents, encoding=encoding)
+        process.communicate.assert_called_once_with(contents.encode(encoding))
+
+
 class ParseConfigTest(TestCase):
-    """Testing for the method `parse_key_value_file`."""
+    """Testing for `parse_key_value_file`."""
 
     def test_parse_key_value_file_parses_config_file(self):
         contents = """

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-09-10 02:45:54 +0000
+++ src/provisioningserver/utils.py	2012-09-17 06:52:21 +0000
@@ -18,6 +18,7 @@
     "MainScript",
     "parse_key_value_file",
     "ShellTemplate",
+    "sudo_write_file",
     "write_custom_config_section",
     ]
 
@@ -28,7 +29,11 @@
 from os import fdopen
 from pipes import quote
 import signal
-from subprocess import CalledProcessError
+from subprocess import (
+    CalledProcessError,
+    PIPE,
+    Popen,
+    )
 import sys
 import tempfile
 from time import time
@@ -262,6 +267,17 @@
     return '\n'.join(lines) + '\n'
 
 
+def sudo_write_file(filename, contents, encoding='utf-8', mode=0744):
+    """Write (or overwrite) file as root.  USE WITH EXTREME CARE."""
+    raw_contents = contents.encode(encoding)
+    proc = Popen([
+        'sudo', 'maas-provision', 'atomic-write',
+        '--filename', filename,
+        '--mode', oct(mode),
+        ], stdin=PIPE)
+    proc.communicate(raw_contents)
+
+
 class Safe:
     """An object that is safe to render as-is."""