Merge ~cjwatson/launchpad:archive-append-mark-suite-dirty into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: ad9c75b7a01c564d6378980c834dc2db45512d5d
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:archive-append-mark-suite-dirty
Merge into: launchpad:master
Diff against target: 106 lines (+48/-22)
2 files modified
lib/lp/soyuz/interfaces/archive.py (+20/-20)
lib/lp/soyuz/tests/test_archive.py (+28/-2)
Reviewer Review Type Date Requested Status
Cristian Gonzalez (community) Approve
Review via email: mp+405379@code.launchpad.net

Commit message

Move markSuiteDirty to IArchiveAppend

Description of the change

`markSuiteDirty` allows forcing a suite in an archive to be republished as if by an upload without actually uploading to it. It was previously in `IArchiveEdit`, which requires the distribution owner in the case of primary archives and the archive owner in all other cases. This turns out to be inconvenient in practice for Ubuntu, where the archive owner is ~ubuntu-archive and handles day-to-day archive administration work, and the distribution owner is ~techboard which doesn't.

There isn't an obvious perfect fit here, but `IArchiveAppend` seems a lot closer: it requires the archive owner or somebody with upload permission in the case of PPAs, and the archive owner in all other cases. The only significant downside is that Launchpad administrators won't be able to use it any more, but I think this is tolerable. If it becomes a problem, we can probably press the `launchpad.Owner` permission into service, since it's currently unused for archives.

To post a comment you must log in.
Revision history for this message
Cristian Gonzalez (cristiangsp) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
2index e4929b7..1752ba3 100644
3--- a/lib/lp/soyuz/interfaces/archive.py
4+++ b/lib/lp/soyuz/interfaces/archive.py
5@@ -1831,6 +1831,26 @@ class IArchiveAppend(Interface):
6 :param job_id: The ID of the `PlainPackageCopyJob` to be removed.
7 """
8
9+ @operation_parameters(
10+ distroseries=Reference(
11+ IDistroSeries, title=_("Distro series"), required=True),
12+ pocket=Choice(
13+ title=_("Pocket"),
14+ vocabulary=PackagePublishingPocket,
15+ required=True),
16+ )
17+ @export_write_operation()
18+ @operation_for_version("devel")
19+ def markSuiteDirty(distroseries, pocket):
20+ """Mark a suite as dirty in this archive.
21+
22+ The next publisher run will publish this suite regardless of whether
23+ it has any pending publications.
24+
25+ :param distroseries: An `IDistroSeries`.
26+ :param pocket: A `PackagePublishingPocket`.
27+ """
28+
29
30 class IArchiveEdit(Interface):
31 """Archive interface for operations restricted by edit privilege."""
32@@ -2219,26 +2239,6 @@ class IArchiveEdit(Interface):
33 :param names: A list of token names.
34 """
35
36- @operation_parameters(
37- distroseries=Reference(
38- IDistroSeries, title=_("Distro series"), required=True),
39- pocket=Choice(
40- title=_("Pocket"),
41- vocabulary=PackagePublishingPocket,
42- required=True),
43- )
44- @export_write_operation()
45- @operation_for_version("devel")
46- def markSuiteDirty(distroseries, pocket):
47- """Mark a suite as dirty in this archive.
48-
49- The next publisher run will publish this suite regardless of whether
50- it has any pending publications.
51-
52- :param distroseries: An `IDistroSeries`.
53- :param pocket: A `PackagePublishingPocket`.
54- """
55-
56
57 class IArchiveDelete(Interface):
58 """Archive interface for operations restricted by delete privilege."""
59diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
60index 425355b..c600cde 100644
61--- a/lib/lp/soyuz/tests/test_archive.py
62+++ b/lib/lp/soyuz/tests/test_archive.py
63@@ -1,4 +1,4 @@
64-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
65+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
66 # GNU Affero General Public License version 3 (see the file LICENSE).
67
68 """Test Archive features."""
69@@ -4584,10 +4584,36 @@ class TestMarkSuiteDirty(TestCaseWithFactory):
70 archive = self.factory.makeArchive()
71 self.assertIsNone(archive.dirty_suites)
72
73- def test_requires_owner(self):
74+ def test_unprivileged_disallowed(self):
75 archive = self.factory.makeArchive()
76 self.assertRaises(Unauthorized, getattr, archive, "markSuiteDirty")
77
78+ def test_primary_archive_uploader_disallowed(self):
79+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
80+ person = self.factory.makePerson()
81+ with person_logged_in(archive.distribution.owner):
82+ archive.newComponentUploader(person, "main")
83+ with person_logged_in(person):
84+ self.assertRaises(Unauthorized, getattr, archive, "markSuiteDirty")
85+
86+ def test_primary_archive_owner_allowed(self):
87+ archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY)
88+ # Simulate Ubuntu's situation where the primary archive is owned by
89+ # an archive admin team while the distribution is owned by the
90+ # technical board, allowing us to tell the difference between
91+ # permissions granted to the archive owner vs. the distribution
92+ # owner.
93+ with person_logged_in(archive.distribution.owner):
94+ archive.distribution.owner = (
95+ getUtility(ILaunchpadCelebrities).ubuntu_techboard)
96+ distroseries = self.factory.makeDistroSeries(
97+ distribution=archive.distribution)
98+ with person_logged_in(archive.owner):
99+ archive.markSuiteDirty(
100+ distroseries, PackagePublishingPocket.UPDATES)
101+ self.assertEqual(
102+ ["%s-updates" % distroseries.name], archive.dirty_suites)
103+
104 def test_first_suite(self):
105 archive = self.factory.makeArchive()
106 distroseries = self.factory.makeDistroSeries(

Subscribers

People subscribed via source and target branches

to status/vote changes: