launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/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/~allenap/maas/maas-set-correct-file-permissions/+merge/122310

This changes install_dir() to normalize
-- 
https://code.launchpad.net/~allenap/maas/maas-set-correct-file-permissions/+merge/122310
Your team MAAS Maintainers is requested to review the proposed merge of lp:~allenap/maas/maas-set-correct-file-permissions into lp:maas.

=== modified file 'src/provisioningserver/pxe/install_image.py'
--- src/provisioningserver/pxe/install_image.py	2012-08-30 11:19:16 +0000
+++ src/provisioningserver/pxe/install_image.py	2012-08-31 15:51:31 +0000
@@ -27,6 +27,7 @@
     compose_image_path,
     locate_tftp_path,
     )
+from twisted.python.filepath import FilePath
 
 
 def make_destination(tftproot, arch, subarch, release, purpose):
@@ -110,6 +111,13 @@
     os.rename('%s.new' % old, old)
     # End of critical window.
 
+    # Normalise permissions.
+    for filepath in FilePath(old).walk():
+        if filepath.isdir():
+            filepath.chmod(0755)
+        else:
+            filepath.chmod(0644)
+
     # Now delete the old image directory at leisure.
     rmtree('%s.old' % old, ignore_errors=True)
 

=== modified file 'src/provisioningserver/pxe/tests/test_install_image.py'
--- src/provisioningserver/pxe/tests/test_install_image.py	2012-08-30 11:19:16 +0000
+++ src/provisioningserver/pxe/tests/test_install_image.py	2012-08-31 15:51:31 +0000
@@ -34,6 +34,7 @@
     FileExists,
     Not,
     )
+from twisted.python.filepath import FilePath
 
 
 def make_arch_subarch_release_purpose():
@@ -191,3 +192,20 @@
             FileContains(contents))
         self.assertThat('%s.old' % published_image, Not(DirExists()))
         self.assertThat('%s.new' % published_image, Not(DirExists()))
+
+    def test_install_dir_normalises_permissions(self):
+        # install_dir() normalises directory permissions to 0755 and file
+        # permissions to 0644.
+        target_dir = FilePath(self.make_dir())
+        new_dir = FilePath(self.make_dir())
+        new_dir.chmod(0700)
+        new_image = new_dir.child("image")
+        new_image.touch()
+        new_image.chmod(0600)
+        install_dir(new_dir.path, target_dir.path)
+        self.assertEqual(
+            "rwxr-xr-x",
+            target_dir.getPermissions().shorthand())
+        self.assertEqual(
+            "rw-r--r--",
+            target_dir.child("image").getPermissions().shorthand())