launchpad-reviewers team mailing list archive

[Andres Rodriguez <andreserl@xxxxxxxxxxxxx>]

Andres Rodriguez has proposed merging lp:~andreserl/maas/maas_set_correct_file_permissions into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1042865 in MAAS: "maas-import-pxe-files sets incorrect permissions for commissioning dir"
  https://bugs.launchpad.net/maas/+bug/1042865

For more details, see:
https://code.launchpad.net/~andreserl/maas/maas_set_correct_file_permissions/+merge/121923
-- 
https://code.launchpad.net/~andreserl/maas/maas_set_correct_file_permissions/+merge/121923
Your team MAAS Maintainers is requested to review the proposed merge of lp:~andreserl/maas/maas_set_correct_file_permissions into lp:maas.

=== modified file 'src/provisioningserver/pxe/install_image.py'
--- src/provisioningserver/pxe/install_image.py	2012-07-23 10:24:39 +0000
+++ src/provisioningserver/pxe/install_image.py	2012-08-29 18:49:18 +0000
@@ -108,6 +108,7 @@
     if os.path.isdir(old):
         os.rename(old, '%s.old' % old)
     os.rename('%s.new' % old, old)
+    os.chmod(old, 0755)
     # End of critical window.
 
     # Now delete the old image directory at leisure.

=== modified file 'src/provisioningserver/pxe/tests/test_install_image.py'
--- src/provisioningserver/pxe/tests/test_install_image.py	2012-07-23 13:25:15 +0000
+++ src/provisioningserver/pxe/tests/test_install_image.py	2012-08-29 18:49:18 +0000
@@ -85,7 +85,7 @@
         tftproot = self.make_dir()
         arch, subarch, release, purpose = make_arch_subarch_release_purpose()
         self.assertEqual(
-            os.path.join(tftproot, 'maas', arch, subarch, release, purpose),
+            os.path.join(tftproot, arch, subarch, release, purpose),
             make_destination(tftproot, arch, subarch, release, purpose))
 
     def test_make_destination_creates_directory_if_not_present(self):

=== modified file 'src/provisioningserver/pxe/tests/test_tftppath.py'
--- src/provisioningserver/pxe/tests/test_tftppath.py	2012-08-17 14:11:20 +0000
+++ src/provisioningserver/pxe/tests/test_tftppath.py	2012-08-29 18:49:18 +0000
@@ -41,7 +41,7 @@
     def test_compose_config_path_follows_maas_pxe_directory_layout(self):
         name = factory.make_name('config')
         self.assertEqual(
-            'maas/pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),
+            'pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),
             compose_config_path(name))
 
     def test_compose_config_path_does_not_include_tftp_root(self):
@@ -56,7 +56,7 @@
         release = factory.make_name('release')
         purpose = factory.make_name('purpose')
         self.assertEqual(
-            'maas/%s/%s/%s/%s' % (arch, subarch, release, purpose),
+            '%s/%s/%s/%s' % (arch, subarch, release, purpose),
             compose_image_path(arch, subarch, release, purpose))
 
     def test_compose_image_path_does_not_include_tftp_root(self):
@@ -69,7 +69,7 @@
             Not(StartsWith(self.tftproot)))
 
     def test_compose_bootloader_path_follows_maas_pxe_directory_layout(self):
-        self.assertEqual('maas/pxelinux.0', compose_bootloader_path())
+        self.assertEqual('pxelinux.0', compose_bootloader_path())
 
     def test_compose_bootloader_path_does_not_include_tftp_root(self):
         self.assertThat(

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2012-08-27 12:00:44 +0000
+++ src/provisioningserver/tests/test_tftp.py	2012-08-29 18:49:18 +0000
@@ -71,7 +71,7 @@
         the expected groups from a match.
         """
         components = {
-            "bootpath": b"maas",  # Static.
+            "bootpath": None,  # Static.
             "mac": factory.getRandomMACAddress(b"-"),
             }
         config_path = compose_config_path(components["mac"])
@@ -125,7 +125,7 @@
 
     def test_re_config_file_does_not_match_non_config_file(self):
         self.assertIsNone(
-            TFTPBackend.re_config_file.match('maas/pxelinux.cfg/kernel'))
+            TFTPBackend.re_config_file.match('pxelinux.cfg/kernel'))
 
     def test_re_config_file_does_not_match_file_in_root(self):
         self.assertIsNone(
@@ -202,7 +202,7 @@
         output = reader.read(10000)
         # The expected parameters include bootpath; this is extracted from the
         # file path by re_config_file.
-        expected_params = dict(mac=mac, bootpath="maas")
+        expected_params = dict(mac=mac, bootpath=None)
         observed_params = json.loads(output)
         self.assertEqual(expected_params, observed_params)
 
@@ -213,7 +213,7 @@
         backend = TFTPBackend(self.make_dir(), b"http://example.com/";)
         # Fake configuration parameters, as discovered from the file path.
         fake_params = {
-            "bootpath": "maas",
+            "bootpath": None,
             "mac": factory.getRandomMACAddress(b"-"),
             }
         # Fake kernel configuration parameters, as returned from the API call.

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-08-24 09:21:34 +0000
+++ src/provisioningserver/utils.py	2012-08-29 18:49:18 +0000
@@ -75,6 +75,9 @@
             if not os.path.isfile(filename):
                 temp_file = _write_temp_file(content, filename)
                 os.rename(temp_file, filename)
+                # Setting 0755 as we assume `filename` is not a file
+                # but rather it is a directory.
+                os.chmod(filename, 0755)
         finally:
             # Release the lock.
             lock.release()
@@ -83,6 +86,7 @@
         # POSIX systems.
         temp_file = _write_temp_file(content, filename)
         os.rename(temp_file, filename)
+        os.chmod(filename, 0644)
 
 
 def incremental_write(content, filename):