Merge lp:~ack/landscape-client/sources.list-preserve-old-permissions into lp:~landscape/landscape-client/trunk

[Alberto Donato]

This fixes bug #804548, adds a test for it.

[Kevin McDermott (bigkevmcd)]

Looks fine to me, +1

review: Approve

[Thomas Herve (therve)]

This is the wrong approach. The package reporter should be run as the landscape user instead, not run. If you look at the package changer, it already does that before calling the reporter. Instead of doing it in the package changer, this code should be moved in the package reporter, and then the problem will be fixed for the aptsources plugin.

review: Needs Fixing

[Thomas Herve (therve)]

Man I'm tired. Forget that comment.

[1] There is still something to be done for the files in sources.list.d. At least using shutil.move.

review: Approve

[Thomas Herve (therve)]

Or just check that they are 0644 by default.

[lp:~ack/landscape-client/sources.list-preserve-old-permissions]

341. By Alberto Donato on 2011-07-06

Use shutil.move to backup files in /etc/apt/sources.list.d, ensure mode 644 for new files in this dir.

=== modified file 'landscape/manager/aptsources.py'
--- landscape/manager/aptsources.py	2011-04-15 08:05:21 +0000
+++ landscape/manager/aptsources.py	2011-07-06 09:18:00 +0000
@@ -1,5 +1,6 @@
 import glob
 import os
+import shutil
 import tempfile
 
 from twisted.internet.defer import succeed
@@ -130,10 +131,12 @@
             else:
                 new_sources.write("#%s" % line)
         new_sources.close()
-        os.rename(path, self.SOURCES_LIST)
+
+        os.chmod(path, os.stat(self.SOURCES_LIST).st_mode)
+        shutil.move(path, self.SOURCES_LIST)
 
         for filename in glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list")):
-            os.rename(filename, "%s.save" % filename)
+            shutil.move(filename, "%s.save" % filename)
 
         for source in sources:
             filename = os.path.join(self.SOURCES_LIST_D,
@@ -141,6 +144,7 @@
             sources_file = file(filename, "w")
             sources_file.write(source["content"])
             sources_file.close()
+            os.chmod(filename, 0644)
         return self._run_reporter()
 
     def _run_reporter(self):

=== modified file 'landscape/manager/tests/test_aptsources.py'
--- landscape/manager/tests/test_aptsources.py	2011-04-15 08:05:21 +0000
+++ landscape/manager/tests/test_aptsources.py	2011-07-06 09:18:00 +0000
@@ -55,6 +55,28 @@
                             [{"type": "operation-result",
                               "status": SUCCEEDED, "operation-id": 1}])
 
+    def test_sources_list_permissions(self):
+        """
+        When getting a repository message, L{AptSources} keeps sources.list
+        permissions.
+        """
+        sources = file(self.sourceslist.SOURCES_LIST, "w")
+        sources.write("oki\n\ndoki\n#comment\n # other comment\n")
+        sources.close()
+        sources_stat_orig = os.stat(self.sourceslist.SOURCES_LIST)
+
+        self.manager.dispatch_message(
+            {"type": "apt-sources-replace", "sources": [], "gpg-keys": [],
+             "operation-id": 1})
+
+        service = self.broker_service
+        self.assertMessages(service.message_store.get_pending_messages(),
+                            [{"type": "operation-result",
+                              "status": SUCCEEDED, "operation-id": 1}])
+
+        sources_stat_after = os.stat(self.sourceslist.SOURCES_LIST)
+        self.assertEqual(sources_stat_orig.st_mode, sources_stat_after.st_mode)
+
     def test_random_failures(self):
         """
         If a failure happens during the manipulation of sources, the activity