Merge ~cjwatson/launchpad:snap-build-macaroon-snap-base into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 75a90a79f2d9e2f18dcf6dd26905324f7f832f48
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:snap-build-macaroon-snap-base
Merge into: launchpad:master
Prerequisite: ~cjwatson/launchpad:macaroon-verifies-matcher
Diff against target: 90 lines (+21/-15)
3 files modified
lib/lp/snappy/model/snapbase.py (+0/-5)
lib/lp/snappy/model/snapbuild.py (+9/-3)
lib/lp/snappy/tests/test_snapbuild.py (+12/-7)
Reviewer Review Type Date Requested Status
Ioana Lasc (community) Approve
Review via email: mp+401316@code.launchpad.net

Commit message

Allow snap-build macaroons to authorize SnapBase archive dependencies

Description of the change

Technically adding SnapBase archive dependencies on private archives won't quite work yet because nothing issues the relevant macaroons yet, but allowing it at this stage makes it much easier to write tests, and we're almost there anyway.

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/snappy/model/snapbase.py b/lib/lp/snappy/model/snapbase.py
2index 615c783..8916c61 100644
3--- a/lib/lp/snappy/model/snapbase.py
4+++ b/lib/lp/snappy/model/snapbase.py
5@@ -105,11 +105,6 @@ class SnapBase(Storm):
6 if archive_dependency is not None:
7 raise ArchiveDependencyError(
8 "This dependency is already registered.")
9- # XXX cjwatson 2021-03-19: Relax this once we have a way to dispatch
10- # appropriate tokens for snap builds whose base has dependencies on
11- # private archives.
12- if dependency.private:
13- raise ArchiveDependencyError("This dependency is private.")
14 if not dependency.enabled:
15 raise ArchiveDependencyError("Dependencies must not be disabled.")
16
17diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
18index b742fae..76ccc0e 100644
19--- a/lib/lp/snappy/model/snapbuild.py
20+++ b/lib/lp/snappy/model/snapbuild.py
21@@ -93,6 +93,7 @@ from lp.snappy.interfaces.snapbuild import (
22 )
23 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
24 from lp.snappy.mail.snapbuild import SnapBuildMailer
25+from lp.snappy.model.snapbase import SnapBase
26 from lp.snappy.model.snapbuildjob import (
27 SnapBuildJob,
28 SnapBuildJobType,
29@@ -656,9 +657,9 @@ class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
30 """
31 if not ISnapBuild.providedBy(context):
32 raise BadMacaroonContext(context)
33- if not removeSecurityProxy(context).is_private:
34- raise BadMacaroonContext(
35- context, "Refusing to issue macaroon for public build.")
36+ # We allow issuing macaroons for public builds. It's harmless, and
37+ # it allows using SnapBases that have archive dependencies on
38+ # private PPAs.
39 return removeSecurityProxy(context).id
40
41 def checkVerificationContext(self, context, **kwargs):
42@@ -712,6 +713,11 @@ class SnapBuildMacaroonIssuer(MacaroonIssuerBase):
43 Archive.id,
44 where=And(
45 ArchiveDependency.archive == Archive.id,
46+ ArchiveDependency.dependency == context))),
47+ SnapBuild.snap_base_id.is_in(Select(
48+ SnapBase.id,
49+ where=And(
50+ ArchiveDependency.snap_base == SnapBase.id,
51 ArchiveDependency.dependency == context)))))
52 else:
53 return False
54diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
55index 8887249..521cfca 100644
56--- a/lib/lp/snappy/tests/test_snapbuild.py
57+++ b/lib/lp/snappy/tests/test_snapbuild.py
58@@ -902,13 +902,6 @@ class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
59 self.pushConfig(
60 "launchpad", internal_macaroon_secret_key="some-secret")
61
62- def test_issueMacaroon_refuses_public_snap(self):
63- build = self.factory.makeSnapBuild()
64- issuer = getUtility(IMacaroonIssuer, "snap-build")
65- self.assertRaises(
66- BadMacaroonContext, removeSecurityProxy(issuer).issueMacaroon,
67- build)
68-
69 def test_issueMacaroon_good(self):
70 build = self.factory.makeSnapBuild(
71 snap=self.factory.makeSnap(private=True))
72@@ -972,6 +965,18 @@ class TestSnapBuildMacaroonIssuer(MacaroonTestMixin, TestCaseWithFactory):
73 macaroon = issuer.issueMacaroon(build)
74 self.assertMacaroonVerifies(issuer, macaroon, dependency)
75
76+ def test_verifyMacaroon_good_snap_base_archive(self):
77+ snap_base = self.factory.makeSnapBase()
78+ dependency = self.factory.makeArchive(private=True)
79+ snap_base.addArchiveDependency(
80+ dependency, PackagePublishingPocket.RELEASE)
81+ build = self.factory.makeSnapBuild(snap_base=snap_base)
82+ build.updateStatus(BuildStatus.BUILDING)
83+ issuer = removeSecurityProxy(
84+ getUtility(IMacaroonIssuer, "snap-build"))
85+ macaroon = issuer.issueMacaroon(build)
86+ self.assertMacaroonVerifies(issuer, macaroon, dependency)
87+
88 def test_verifyMacaroon_good_no_context(self):
89 [ref] = self.factory.makeGitRefs(
90 information_type=InformationType.USERDATA)

Subscribers

People subscribed via source and target branches

to status/vote changes: