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

Proposed by Alberto Donato on 2011-07-05
Status: Merged
Approved by: Thomas Herve on 2011-07-06
Approved revision: 340
Merged at revision: 339
Proposed branch: lp:~ack/landscape-client/sources.list-preserve-old-permissions
Merge into: lp:~landscape/landscape-client/trunk
Diff against target: 66 lines (+28/-2)
2 files modified
landscape/manager/aptsources.py (+6/-2)
landscape/manager/tests/test_aptsources.py (+22/-0)
To merge this branch: bzr merge lp:~ack/landscape-client/sources.list-preserve-old-permissions
Reviewer Review Type Date Requested Status
Thomas Herve (community) 2011-07-05 Approve on 2011-07-06
Kevin McDermott (community) 2011-07-05 Approve on 2011-07-05
Review via email: mp+66866@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Kevin McDermott (bigkevmcd) wrote :

Looks fine to me, +1

review: Approve
Thomas Herve (therve) wrote :

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) wrote :

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) wrote :

Or just check that they are 0644 by default.

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'landscape/manager/aptsources.py'
2--- landscape/manager/aptsources.py 2011-04-15 08:05:21 +0000
3+++ landscape/manager/aptsources.py 2011-07-06 09:18:00 +0000
4@@ -1,5 +1,6 @@
5 import glob
6 import os
7+import shutil
8 import tempfile
9
10 from twisted.internet.defer import succeed
11@@ -130,10 +131,12 @@
12 else:
13 new_sources.write("#%s" % line)
14 new_sources.close()
15- os.rename(path, self.SOURCES_LIST)
16+
17+ os.chmod(path, os.stat(self.SOURCES_LIST).st_mode)
18+ shutil.move(path, self.SOURCES_LIST)
19
20 for filename in glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list")):
21- os.rename(filename, "%s.save" % filename)
22+ shutil.move(filename, "%s.save" % filename)
23
24 for source in sources:
25 filename = os.path.join(self.SOURCES_LIST_D,
26@@ -141,6 +144,7 @@
27 sources_file = file(filename, "w")
28 sources_file.write(source["content"])
29 sources_file.close()
30+ os.chmod(filename, 0644)
31 return self._run_reporter()
32
33 def _run_reporter(self):
34
35=== modified file 'landscape/manager/tests/test_aptsources.py'
36--- landscape/manager/tests/test_aptsources.py 2011-04-15 08:05:21 +0000
37+++ landscape/manager/tests/test_aptsources.py 2011-07-06 09:18:00 +0000
38@@ -55,6 +55,28 @@
39 [{"type": "operation-result",
40 "status": SUCCEEDED, "operation-id": 1}])
41
42+ def test_sources_list_permissions(self):
43+ """
44+ When getting a repository message, L{AptSources} keeps sources.list
45+ permissions.
46+ """
47+ sources = file(self.sourceslist.SOURCES_LIST, "w")
48+ sources.write("oki\n\ndoki\n#comment\n # other comment\n")
49+ sources.close()
50+ sources_stat_orig = os.stat(self.sourceslist.SOURCES_LIST)
51+
52+ self.manager.dispatch_message(
53+ {"type": "apt-sources-replace", "sources": [], "gpg-keys": [],
54+ "operation-id": 1})
55+
56+ service = self.broker_service
57+ self.assertMessages(service.message_store.get_pending_messages(),
58+ [{"type": "operation-result",
59+ "status": SUCCEEDED, "operation-id": 1}])
60+
61+ sources_stat_after = os.stat(self.sourceslist.SOURCES_LIST)
62+ self.assertEqual(sources_stat_orig.st_mode, sources_stat_after.st_mode)
63+
64 def test_random_failures(self):
65 """
66 If a failure happens during the manipulation of sources, the activity

Subscribers

People subscribed via source and target branches

to all changes: