Merge lp:~cjwatson/launchpad/fix-release-file-signing into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18595
Proposed branch: lp:~cjwatson/launchpad/fix-release-file-signing
Merge into: lp:launchpad
Diff against target: 208 lines (+59/-18)
5 files modified
lib/lp/archivepublisher/archivesigningkey.py (+7/-8)
lib/lp/archivepublisher/interfaces/archivesigningkey.py (+6/-1)
lib/lp/archivepublisher/publishing.py (+2/-1)
lib/lp/archivepublisher/tests/test_archivesigningkey.py (+27/-7)
lib/lp/archivepublisher/tests/test_publisher.py (+17/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/fix-release-file-signing
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+342252@code.launchpad.net

Commit message

Fix signing of Release files when distsroot is overridden.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/archivepublisher/archivesigningkey.py'
2--- lib/lp/archivepublisher/archivesigningkey.py 2018-01-19 17:38:51 +0000
3+++ lib/lp/archivepublisher/archivesigningkey.py 2018-03-27 23:03:09 +0000
4@@ -62,6 +62,7 @@
5
6 def __init__(self, archive):
7 self.archive = archive
8+ self.pubconf = getPubConfig(self.archive)
9
10 @property
11 def can_sign(self):
12@@ -119,9 +120,11 @@
13 "No signing key available for %s" %
14 self.archive.displayname)
15
16- def signRepository(self, suite, suffix='', log=None):
17+ def signRepository(self, suite, pubconf=None, suffix='', log=None):
18 """See `ISignableArchive`."""
19- suite_path = os.path.join(self._archive_root_path, 'dists', suite)
20+ if pubconf is None:
21+ pubconf = self.pubconf
22+ suite_path = os.path.join(pubconf.distsroot, suite)
23 release_file_path = os.path.join(suite_path, 'Release' + suffix)
24 if not os.path.exists(release_file_path):
25 raise AssertionError(
26@@ -140,12 +143,12 @@
27 def signFile(self, suite, path, log=None):
28 """See `ISignableArchive`."""
29 # Allow the passed path to be relative to the archive root.
30- path = os.path.realpath(os.path.join(self._archive_root_path, path))
31+ path = os.path.realpath(os.path.join(self.pubconf.archiveroot, path))
32
33 # Ensure the resulting path is within the archive root after
34 # normalisation.
35 # NOTE: uses os.sep to prevent /var/tmp/../tmpFOO attacks.
36- archive_root = self._archive_root_path + os.sep
37+ archive_root = self.pubconf.archiveroot + os.sep
38 if not path.startswith(archive_root):
39 raise AssertionError(
40 "Attempting to sign file (%s) outside archive_root for %s" % (
41@@ -159,10 +162,6 @@
42 class ArchiveSigningKey(SignableArchive):
43 """`IArchive` adapter for manipulating its GPG key."""
44
45- @property
46- def _archive_root_path(self):
47- return getPubConfig(self.archive).archiveroot
48-
49 def getPathForSecretKey(self, key):
50 """See `IArchiveSigningKey`."""
51 return os.path.join(
52
53=== modified file 'lib/lp/archivepublisher/interfaces/archivesigningkey.py'
54--- lib/lp/archivepublisher/interfaces/archivesigningkey.py 2018-01-19 15:38:21 +0000
55+++ lib/lp/archivepublisher/interfaces/archivesigningkey.py 2018-03-27 23:03:09 +0000
56@@ -36,10 +36,15 @@
57
58 can_sign = Attribute("True if this archive is set up for signing.")
59
60- def signRepository(suite, log=None):
61+ def signRepository(suite, pubconf=None, suffix='', log=None):
62 """Sign the corresponding repository.
63
64 :param suite: suite name to be signed.
65+ :param pubconf: an optional publisher configuration instance
66+ indicating where files should be written (if not passed, uses
67+ the archive's defaults).
68+ :param suffix: an optional suffix for repository index files (e.g.
69+ ".new" to help with publishing files atomically).
70 :param log: an optional logger.
71 :raises CannotSignArchive: if the context archive is not set up for
72 signing.
73
74=== modified file 'lib/lp/archivepublisher/publishing.py'
75--- lib/lp/archivepublisher/publishing.py 2018-01-19 17:38:51 +0000
76+++ lib/lp/archivepublisher/publishing.py 2018-03-27 23:03:09 +0000
77@@ -1247,7 +1247,8 @@
78 if signable_archive.can_sign:
79 # Sign the repository.
80 self.log.debug("Signing Release file for %s" % suite)
81- signable_archive.signRepository(suite, suffix=".new", log=self.log)
82+ signable_archive.signRepository(
83+ suite, pubconf=self._config, suffix=".new", log=self.log)
84 core_files.add("Release.gpg")
85 core_files.add("InRelease")
86 else:
87
88=== modified file 'lib/lp/archivepublisher/tests/test_archivesigningkey.py'
89--- lib/lp/archivepublisher/tests/test_archivesigningkey.py 2018-03-21 11:52:42 +0000
90+++ lib/lp/archivepublisher/tests/test_archivesigningkey.py 2018-03-27 23:03:09 +0000
91@@ -62,8 +62,7 @@
92 self.assertTrue(signer.can_sign)
93 signer.signFile(self.suite, filename)
94
95- signature = filename + '.gpg'
96- self.assertTrue(os.path.exists(signature))
97+ self.assertTrue(os.path.exists(filename + ".gpg"))
98
99 def test_signFile_absolute_outside_archive(self):
100 filename = os.path.join(self.temp_dir, "signme")
101@@ -72,7 +71,7 @@
102 signer = ISignableArchive(self.archive)
103 self.assertTrue(signer.can_sign)
104 self.assertRaises(
105- AssertionError, lambda: signer.signFile(self.suite, filename))
106+ AssertionError, signer.signFile, self.suite, filename)
107
108 def test_signFile_relative_within_archive(self):
109 filename_relative = "signme"
110@@ -83,8 +82,7 @@
111 self.assertTrue(signer.can_sign)
112 signer.signFile(self.suite, filename_relative)
113
114- signature = filename + '.gpg'
115- self.assertTrue(os.path.exists(signature))
116+ self.assertTrue(os.path.exists(filename + ".gpg"))
117
118 def test_signFile_relative_outside_archive(self):
119 filename_relative = "../signme"
120@@ -94,8 +92,7 @@
121 signer = ISignableArchive(self.archive)
122 self.assertTrue(signer.can_sign)
123 self.assertRaises(
124- AssertionError,
125- lambda: signer.signFile(self.suite, filename_relative))
126+ AssertionError, signer.signFile, self.suite, filename_relative)
127
128
129 class TestSignableArchiveWithRunParts(RunPartsMixin, TestCaseWithFactory):
130@@ -144,6 +141,29 @@
131 "clear signature of %s (%s/%s)\n" %
132 (release_path, self.distro.name, self.suite)))
133
134+ def test_signRepository_honours_pubconf(self):
135+ pubconf = getPubConfig(self.archive)
136+ pubconf.distsroot = self.makeTemporaryDirectory()
137+ suite_dir = os.path.join(pubconf.distsroot, self.suite)
138+ release_path = os.path.join(suite_dir, "Release")
139+ write_file(release_path, "Release contents")
140+
141+ signer = ISignableArchive(self.archive)
142+ self.assertTrue(signer.can_sign)
143+ self.assertRaises(AssertionError, signer.signRepository, self.suite)
144+ signer.signRepository(self.suite, pubconf=pubconf)
145+
146+ self.assertThat(
147+ os.path.join(suite_dir, "Release.gpg"),
148+ FileContains(
149+ "detached signature of %s (%s/%s)\n" %
150+ (release_path, self.distro.name, self.suite)))
151+ self.assertThat(
152+ os.path.join(suite_dir, "InRelease"),
153+ FileContains(
154+ "clear signature of %s (%s/%s)\n" %
155+ (release_path, self.distro.name, self.suite)))
156+
157 def test_signFile_runs_parts(self):
158 filename = os.path.join(self.archive_root, "signme")
159 write_file(filename, "sign this")
160
161=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
162--- lib/lp/archivepublisher/tests/test_publisher.py 2018-03-26 06:51:44 +0000
163+++ lib/lp/archivepublisher/tests/test_publisher.py 2018-03-27 23:03:09 +0000
164@@ -34,6 +34,10 @@
165 except ImportError:
166 from backports import lzma
167 import pytz
168+from testscenarios import (
169+ load_tests_apply_scenarios,
170+ WithScenarios,
171+ )
172 from testtools.matchers import (
173 ContainsAll,
174 DirContains,
175@@ -2928,9 +2932,15 @@
176 os.rename(temporary_dists, original_dists)
177
178
179-class TestPublisherRepositorySignatures(RunPartsMixin, TestPublisherBase):
180+class TestPublisherRepositorySignatures(
181+ WithScenarios, RunPartsMixin, TestPublisherBase):
182 """Testing `Publisher` signature behaviour."""
183
184+ scenarios = [
185+ ('default distsroot', {'override_distsroot': False}),
186+ ('overridden distsroot', {'override_distsroot': True}),
187+ ]
188+
189 run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
190
191 archive_publisher = None
192@@ -2947,6 +2957,9 @@
193 allowed_suites = []
194 self.archive_publisher = getPublisher(
195 archive, allowed_suites, self.logger)
196+ if self.override_distsroot:
197+ self.archive_publisher._config.distsroot = (
198+ self.makeTemporaryDirectory())
199
200 def _publishArchive(self, archive):
201 """Publish a test source in the given archive.
202@@ -3335,3 +3348,6 @@
203 ),
204 }
205 self.assertThat(self.fetchSums(rootdir), MatchesDict(expected))
206+
207+
208+load_tests = load_tests_apply_scenarios