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
=== 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 @@
1import glob1import glob
2import os2import os
3import shutil
3import tempfile4import tempfile
45
5from twisted.internet.defer import succeed6from twisted.internet.defer import succeed
@@ -130,10 +131,12 @@
130 else:131 else:
131 new_sources.write("#%s" % line)132 new_sources.write("#%s" % line)
132 new_sources.close()133 new_sources.close()
133 os.rename(path, self.SOURCES_LIST)134
135 os.chmod(path, os.stat(self.SOURCES_LIST).st_mode)
136 shutil.move(path, self.SOURCES_LIST)
134137
135 for filename in glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list")):138 for filename in glob.glob(os.path.join(self.SOURCES_LIST_D, "*.list")):
136 os.rename(filename, "%s.save" % filename)139 shutil.move(filename, "%s.save" % filename)
137140
138 for source in sources:141 for source in sources:
139 filename = os.path.join(self.SOURCES_LIST_D,142 filename = os.path.join(self.SOURCES_LIST_D,
@@ -141,6 +144,7 @@
141 sources_file = file(filename, "w")144 sources_file = file(filename, "w")
142 sources_file.write(source["content"])145 sources_file.write(source["content"])
143 sources_file.close()146 sources_file.close()
147 os.chmod(filename, 0644)
144 return self._run_reporter()148 return self._run_reporter()
145149
146 def _run_reporter(self):150 def _run_reporter(self):
147151
=== 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 @@
55 [{"type": "operation-result",55 [{"type": "operation-result",
56 "status": SUCCEEDED, "operation-id": 1}])56 "status": SUCCEEDED, "operation-id": 1}])
5757
58 def test_sources_list_permissions(self):
59 """
60 When getting a repository message, L{AptSources} keeps sources.list
61 permissions.
62 """
63 sources = file(self.sourceslist.SOURCES_LIST, "w")
64 sources.write("oki\n\ndoki\n#comment\n # other comment\n")
65 sources.close()
66 sources_stat_orig = os.stat(self.sourceslist.SOURCES_LIST)
67
68 self.manager.dispatch_message(
69 {"type": "apt-sources-replace", "sources": [], "gpg-keys": [],
70 "operation-id": 1})
71
72 service = self.broker_service
73 self.assertMessages(service.message_store.get_pending_messages(),
74 [{"type": "operation-result",
75 "status": SUCCEEDED, "operation-id": 1}])
76
77 sources_stat_after = os.stat(self.sourceslist.SOURCES_LIST)
78 self.assertEqual(sources_stat_orig.st_mode, sources_stat_after.st_mode)
79
58 def test_random_failures(self):80 def test_random_failures(self):
59 """81 """
60 If a failure happens during the manipulation of sources, the activity82 If a failure happens during the manipulation of sources, the activity

Subscribers

People subscribed via source and target branches

to all changes: