launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26913
[Merge] ~cjwatson/launchpad:issue-private-archive-macaroons into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:issue-private-archive-macaroons into launchpad:master with ~cjwatson/launchpad:get-sources-list-accept-behaviour as a prerequisite.
Commit message:
Dispatch sources.list entries for private archives using macaroons
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/401323
This allows snap base archive dependencies on private archives to work (for 16.04 ESM), and also takes a significant step towards abolishing `Archive.buildd_secret`. These tokens are only valid while the build is running, and so are much less vulnerable to accidental leakage.
The old buildd secret is still used in the file map passed to builders to fetch the source package for binary package builds in private archives, so it can't yet be removed entirely.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:issue-private-archive-macaroons into launchpad:master.
diff --git a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
index 4d768d8..d210d64 100644
--- a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
@@ -46,6 +46,15 @@ class IBuildFarmJobBehaviour(Interface):
'password' (optional): password to authenticate with
"""
+ def issueMacaroon():
+ """Issue a macaroon to access private resources for this build.
+
+ :raises NotImplementedError: if the build type does not support
+ accessing private resources.
+ :return: A Deferred that calls back with a serialized macaroon or a
+ fault.
+ """
+
def extraBuildArgs(logger=None):
"""Return extra arguments required by the builder for this build.
diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index f8b5034..e0d5380 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -89,6 +89,10 @@ class BuildFarmJobBehaviourBase:
"""The default behaviour is to send no files."""
return {}
+ def issueMacaroon(self):
+ raise NotImplementedError(
+ "This build type does not support accessing private resources.")
+
def extraBuildArgs(self, logger=None):
"""The default behaviour is to send only common extra arguments."""
args = {}
diff --git a/lib/lp/oci/model/ocirecipebuildbehaviour.py b/lib/lp/oci/model/ocirecipebuildbehaviour.py
index e8a60f5..0ca766d 100644
--- a/lib/lp/oci/model/ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/model/ocirecipebuildbehaviour.py
@@ -80,6 +80,14 @@ class OCIRecipeBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
raise CannotBuild(
"Missing chroot for %s" % build.distro_arch_series.displayname)
+ def issueMacaroon(self):
+ """See `IBuildFarmJobBehaviour`."""
+ return cancel_on_timeout(
+ self._authserver.callRemote(
+ "issueMacaroon",
+ "oci-recipe-build", "OCIRecipeBuild", self.build.id),
+ config.builddmaster.authentication_timeout)
+
def _getBuildInfoArgs(self):
def format_user(user):
if user is None:
@@ -142,11 +150,7 @@ class OCIRecipeBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
if build.recipe.git_ref is not None:
if build.recipe.git_repository.private:
- macaroon_raw = yield cancel_on_timeout(
- self._authserver.callRemote(
- "issueMacaroon",
- "oci-recipe-build", "OCIRecipeBuild", build.id),
- config.builddmaster.authentication_timeout)
+ macaroon_raw = yield self.issueMacaroon()
url = build.recipe.git_repository.getCodebrowseUrl(
username=None, password=macaroon_raw)
args["git_repository"] = url
diff --git a/lib/lp/snappy/model/snapbuildbehaviour.py b/lib/lp/snappy/model/snapbuildbehaviour.py
index dda949b..0057670 100644
--- a/lib/lp/snappy/model/snapbuildbehaviour.py
+++ b/lib/lp/snappy/model/snapbuildbehaviour.py
@@ -145,6 +145,13 @@ class SnapBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
raise CannotBuild(
"Missing chroot for %s" % build.distro_arch_series.displayname)
+ def issueMacaroon(self):
+ """See `IBuildFarmJobBehaviour`."""
+ return cancel_on_timeout(
+ self._authserver.callRemote(
+ "issueMacaroon", "snap-build", "SnapBuild", self.build.id),
+ config.builddmaster.authentication_timeout)
+
@defer.inlineCallbacks
def extraBuildArgs(self, logger=None):
"""
@@ -186,10 +193,7 @@ class SnapBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
if build.snap.git_ref.repository_url is not None:
args["git_repository"] = build.snap.git_ref.repository_url
elif build.snap.git_repository.private:
- macaroon_raw = yield cancel_on_timeout(
- self._authserver.callRemote(
- "issueMacaroon", "snap-build", "SnapBuild", build.id),
- config.builddmaster.authentication_timeout)
+ macaroon_raw = yield self.issueMacaroon()
url = build.snap.git_repository.getCodebrowseUrl(
username=None, password=macaroon_raw)
args["git_repository"] = url
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 1d80326..0deebed 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -14,6 +14,7 @@ from textwrap import dedent
import time
import uuid
+from aptsources.sourceslist import SourceEntry
import fixtures
from pymacaroons import Macaroon
import pytz
@@ -77,6 +78,7 @@ from lp.services.log.logger import (
BufferLogger,
DevNullLogger,
)
+from lp.services.macaroons.testing import MacaroonVerifies
from lp.services.statsd.tests import StatsMixin
from lp.services.webapp import canonical_url
from lp.snappy.interfaces.snap import (
@@ -790,6 +792,59 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
self.assertEqual(expected_archives, args["archives"])
@defer.inlineCallbacks
+ def test_extraBuildArgs_snap_base_with_private_archive_dependencies(self):
+ # If the build is using a snap base that has archive dependencies on
+ # private PPAs, extraBuildArgs sends them.
+ self.useFixture(InProcessAuthServerFixture())
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
+ snap_base = self.factory.makeSnapBase()
+ job = self.makeJob(snap_base=snap_base)
+ dependency = self.factory.makeArchive(
+ distribution=job.archive.distribution, private=True)
+ snap_base.addArchiveDependency(
+ dependency, PackagePublishingPocket.RELEASE,
+ getUtility(IComponentSet)["main"])
+ self.factory.makeBinaryPackagePublishingHistory(
+ archive=dependency, distroarchseries=job.build.distro_arch_series,
+ pocket=PackagePublishingPocket.RELEASE,
+ status=PackagePublishingStatus.PUBLISHED)
+ with dbuser(config.builddmaster.dbuser):
+ args = yield job.extraBuildArgs()
+ job.build.updateStatus(BuildStatus.BUILDING)
+ self.assertThat(
+ [SourceEntry(item) for item in args["archives"]],
+ MatchesListwise([
+ MatchesStructure(
+ type=Equals("deb"),
+ uri=AfterPreprocessing(urlsplit, MatchesStructure(
+ scheme=Equals("http"),
+ username=Equals("buildd"),
+ password=MacaroonVerifies("snap-build", dependency),
+ hostname=Equals("private-ppa.launchpad.test"),
+ path=Equals("/%s/%s/%s" % (
+ dependency.owner.name, dependency.name,
+ dependency.distribution.name)))),
+ dist=Equals(job.build.distro_series.name),
+ comps=Equals(["main"])),
+ MatchesStructure.byEquality(
+ type="deb",
+ uri=job.archive.archive_url,
+ dist=job.build.distro_series.name,
+ comps=["main", "universe"]),
+ MatchesStructure.byEquality(
+ type="deb",
+ uri=job.archive.archive_url,
+ dist="%s-security" % job.build.distro_series.name,
+ comps=["main", "universe"]),
+ MatchesStructure.byEquality(
+ type="deb",
+ uri=job.archive.archive_url,
+ dist="%s-updates" % job.build.distro_series.name,
+ comps=["main", "universe"]),
+ ]))
+
+ @defer.inlineCallbacks
def test_extraBuildArgs_ppa_and_snap_base_with_archive_dependencies(self):
# If the build is using a PPA and a snap base that both have archive
# dependencies, extraBuildArgs sends them all.
diff --git a/lib/lp/soyuz/adapters/archivedependencies.py b/lib/lp/soyuz/adapters/archivedependencies.py
index ff1d117..685a82e 100644
--- a/lib/lp/soyuz/adapters/archivedependencies.py
+++ b/lib/lp/soyuz/adapters/archivedependencies.py
@@ -290,7 +290,8 @@ def get_sources_list_for_building(behaviour, distroarchseries,
tools_source=tools_source, tools_fingerprint=tools_fingerprint,
logger=logger)
sources_list_lines, trusted_keys = (
- yield _get_sources_list_for_dependencies(deps, logger=logger))
+ yield _get_sources_list_for_dependencies(
+ behaviour, deps, logger=logger))
external_dep_lines = []
# Append external sources.list lines for this build if specified. No
@@ -343,7 +344,8 @@ def _has_published_binaries(archive, distroarchseries, pocket):
return not published_binaries.is_empty()
-def _get_binary_sources_list_line(archive, distroarchseries, pocket,
+@defer.inlineCallbacks
+def _get_binary_sources_list_line(behaviour, archive, distroarchseries, pocket,
components):
"""Return the correponding binary sources_list line."""
# Encode the private PPA repository password in the
@@ -351,23 +353,24 @@ def _get_binary_sources_list_line(archive, distroarchseries, pocket,
# sanitized to not expose it.
if archive.private:
uri = URI(archive.archive_url)
- uri = uri.replace(
- userinfo="buildd:%s" % archive.buildd_secret)
+ macaroon_raw = yield behaviour.issueMacaroon()
+ uri = uri.replace(userinfo="buildd:%s" % macaroon_raw)
url = str(uri)
else:
url = archive.archive_url
suite = distroarchseries.distroseries.name + pocketsuffix[pocket]
- return 'deb %s %s %s' % (url, suite, ' '.join(components))
+ defer.returnValue('deb %s %s %s' % (url, suite, ' '.join(components)))
@defer.inlineCallbacks
-def _get_sources_list_for_dependencies(dependencies, logger=None):
+def _get_sources_list_for_dependencies(behaviour, dependencies, logger=None):
"""Return sources.list entries and keys.
Process the given list of dependency tuples for the given
`DistroArchseries`.
+ :param behaviour: the build's `IBuildFarmJobBehaviour`.
:param dependencies: list of 3 elements tuples as:
(`IArchive`, `IDistroArchSeries`, `PackagePublishingPocket`,
list of `IComponent` names)
@@ -399,8 +402,8 @@ def _get_sources_list_for_dependencies(dependencies, logger=None):
components = [
component for component in components
if component in archive_components]
- sources_list_line = _get_binary_sources_list_line(
- archive, distro_arch_series, pocket, components)
+ sources_list_line = yield _get_binary_sources_list_line(
+ behaviour, archive, distro_arch_series, pocket, components)
sources_list_lines.append(sources_list_line)
fingerprint = archive.signing_key_fingerprint
if fingerprint is not None and fingerprint not in trusted_keys:
diff --git a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
index 6b74f46..2876ac0 100644
--- a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
+++ b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
@@ -22,6 +22,8 @@ from lp.buildmaster.model.buildfarmjobbehaviour import (
BuildFarmJobBehaviourBase,
)
from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.config import config
+from lp.services.twistedsupport import cancel_on_timeout
from lp.services.webapp import urlappend
from lp.soyuz.adapters.archivedependencies import (
get_primary_current_component,
@@ -131,6 +133,14 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase):
(build.title, build.id, build.pocket.name,
build.distro_series.name))
+ def issueMacaroon(self):
+ """See `IBuildFarmJobBehaviour`."""
+ return cancel_on_timeout(
+ self._authserver.callRemote(
+ "issueMacaroon", "binary-package-build", "BinaryPackageBuild",
+ self.build.id),
+ config.builddmaster.authentication_timeout)
+
@defer.inlineCallbacks
def extraBuildArgs(self, logger=None):
"""
diff --git a/lib/lp/soyuz/model/livefsbuildbehaviour.py b/lib/lp/soyuz/model/livefsbuildbehaviour.py
index c439559..0f93d39 100644
--- a/lib/lp/soyuz/model/livefsbuildbehaviour.py
+++ b/lib/lp/soyuz/model/livefsbuildbehaviour.py
@@ -25,6 +25,8 @@ from lp.buildmaster.model.buildfarmjobbehaviour import (
BuildFarmJobBehaviourBase,
)
from lp.registry.interfaces.series import SeriesStatus
+from lp.services.config import config
+from lp.services.twistedsupport import cancel_on_timeout
from lp.soyuz.adapters.archivedependencies import (
get_sources_list_for_building,
)
@@ -78,6 +80,13 @@ class LiveFSBuildBehaviour(BuildFarmJobBehaviourBase):
raise CannotBuild(
"Missing chroot for %s" % build.distro_arch_series.displayname)
+ def issueMacaroon(self):
+ """See `IBuildFarmJobBehaviour`."""
+ return cancel_on_timeout(
+ self._authserver.callRemote(
+ "issueMacaroon", "livefs-build", "LiveFSBuild", self.build.id),
+ config.builddmaster.authentication_timeout)
+
@defer.inlineCallbacks
def extraBuildArgs(self, logger=None):
"""
diff --git a/lib/lp/soyuz/tests/test_archive.py b/lib/lp/soyuz/tests/test_archive.py
index a49ccd3..2d53207 100644
--- a/lib/lp/soyuz/tests/test_archive.py
+++ b/lib/lp/soyuz/tests/test_archive.py
@@ -13,22 +13,28 @@ from datetime import (
import doctest
import os.path
+from aptsources.sourceslist import SourceEntry
from pytz import UTC
import responses
import six
from six.moves import http_client
+from six.moves.urllib.parse import urlsplit
from storm.store import Store
from testtools.matchers import (
+ AfterPreprocessing,
AllMatch,
DocTestMatches,
+ Equals,
LessThan,
MatchesListwise,
MatchesPredicate,
- MatchesRegex,
MatchesStructure,
)
from testtools.testcase import ExpectedException
-from testtools.twistedsupport import AsynchronousDeferredRunTest
+from testtools.twistedsupport import (
+ AsynchronousDeferredRunTest,
+ AsynchronousDeferredRunTestForBrokenTwisted,
+ )
import transaction
from twisted.internet import defer
from zope.component import getUtility
@@ -57,6 +63,7 @@ from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.registry.interfaces.teammembership import TeamMembershipStatus
+from lp.services.authserver.testing import InProcessAuthServerFixture
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import sqlvalues
from lp.services.features import getFeatureFlag
@@ -67,6 +74,7 @@ from lp.services.gpg.interfaces import (
IGPGHandler,
)
from lp.services.job.interfaces.job import JobStatus
+from lp.services.macaroons.testing import MacaroonVerifies
from lp.services.propertycache import (
clear_property_cache,
get_property_cache,
@@ -1866,11 +1874,15 @@ class TestAddArchiveDependencies(TestCaseWithFactory):
class TestArchiveDependencies(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
- run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=30)
+ run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
+ timeout=30)
@defer.inlineCallbacks
def test_private_sources_list(self):
"""Entries for private dependencies include credentials."""
+ self.useFixture(InProcessAuthServerFixture())
+ self.pushConfig(
+ "launchpad", internal_macaroon_secret_key="some-secret")
p3a = self.factory.makeArchive(name='p3a', private=True)
yield self.useFixture(InProcessKeyServerFixture()).start()
key_path = os.path.join(gpgkeysdir, 'ppa-sample@xxxxxxxxxxxxxxxxx')
@@ -1890,10 +1902,20 @@ class TestArchiveDependencies(TestCaseWithFactory):
sources_list, trusted_keys = yield get_sources_list_for_building(
behaviour, build.distro_arch_series,
build.source_package_release.name)
- matches = MatchesRegex(
- "deb http://buildd:sekrit@xxxxxxxxxxxxxxxxxxxxxxxxxx/";
- "person-name-.*/dependency/ubuntu distroseries-.* main")
- self.assertThat(sources_list[0], matches)
+ # Mark the build as building so that we can verify its macaroons.
+ build.updateStatus(BuildStatus.BUILDING)
+ self.assertThat(SourceEntry(sources_list[0]), MatchesStructure(
+ type=Equals("deb"),
+ uri=AfterPreprocessing(urlsplit, MatchesStructure(
+ scheme=Equals("http"),
+ username=Equals("buildd"),
+ password=MacaroonVerifies("binary-package-build", p3a),
+ hostname=Equals("private-ppa.launchpad.test"),
+ path=Equals("/%s/dependency/ubuntu" % p3a.owner.name),
+ )),
+ dist=Equals(build.distro_series.getSuite(build.pocket)),
+ comps=Equals(["main"]),
+ ))
self.assertThat(trusted_keys, MatchesListwise([
Base64KeyMatches("0D57E99656BEFB0897606EE9A022DD1F5001B46D"),
]))
diff --git a/system-packages.txt b/system-packages.txt
index 84a8548..56a1291 100644
--- a/system-packages.txt
+++ b/system-packages.txt
@@ -19,6 +19,9 @@ apt
apt_inst
apt_pkg
+# Used by tests to parse sources.list entries.
+aptsources
+
# utilities/js-deps
convoy
