Merge ~jugmac00/lpci:add-additional-apt-repositories into lpci:main

[Jürgen Gmach]

Add new configuration option to provide additional package repositories

[Jürgen Gmach (jugmac00)]

The auth placeholder feature will be added together with "Pass credentials from Launchpad via launchpad-buildd to lpcraft"
https://warthogs.atlassian.net/browse/LP-812

[Colin Watson (cjwatson)]

Adding apt archives commonly also involves adding the corresponding GPG key that signs the archive, which is going to be a bit difficult to retrofit to this syntax, so I think we should consider some slightly different syntaxes before committing to this.

Have you considered the approach taken by https://snapcraft.io/docs/package-repositories? Rather than supplying the whole `sources.list` line as a string, it instead assembles it out of a YAML object that specifies the URL, components, and architectures separately, and that can also provide a key ID and possibly a key server to fetch the GPG key. This approach also makes it easy for it to have shortcuts for using Launchpad PPAs, which can be a bit simpler because the signing key data can be fetched from Launchpad (that wouldn't be initially necessary for this branch since we probably wouldn't be doing that sort of thing for Artifactory, but I'd like to have the option to add that in future for broader Launchpad CI needs).

I would definitely like to have something like this that can be a little more easily extended, even if we don't adopt literally the same syntax. But if our needs are compatible with snapcraft's, then there seems no reason not to adopt the same syntax, given that lpcraft is at least somewhat inspired by snapcraft in other areas.

review: Needs Information

[Jürgen Gmach (jugmac00)]

Thanks for the review - I really like your ideas.

I have updated my MP and it is ready for review again.

[Colin Watson (cjwatson)]

review: Approve

[Jürgen Gmach (jugmac00)]

Thanks for the review - I applied most suggestions :-)

diff --git a/NEWS.rst b/NEWS.rst
index 72603d6..1d6b28c 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -9,6 +9,8 @@ Version history
 
 - Hide the internal ``run-one`` command from ``--help`` output.
 
+- Add new configuration option to provide additional package repositories.
+
 0.0.17 (2022-06-17)
 ===================
 
diff --git a/docs/configuration.rst b/docs/configuration.rst
index ecaa727..b453270 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -50,6 +50,11 @@ Job definitions
 ``packages`` (optional)
     Packages to install using ``apt`` as dependencies of this job.
 
+``package-repositories`` (optional)
+    Repositories which will be added to the already existing ones in
+    `/etc/apt/sources.list`.
+    Also see the :ref:`package-repositories` section below.
+
 ``snaps`` (optional)
     Snaps to install as dependencies of this job.
 
@@ -132,3 +137,35 @@ Output properties
     This value is parsed using `pydantic's standard timedelta parsing
     <https://pydantic-docs.helpmanual.io/usage/types/#datetime-types>`_,
     restricted to non-negative timedeltas.
+
+
+.. _package-repositories:
+
+Package-repositories properties
+-------------------------------
+
+The properties are inspired by the properties of `Snapcraft
+<https://snapcraft.io/docs/package-repositories>`_.
+Only a subset of them is currently implemented.
+
+More properties can be implemented on demand.
+
+``type`` (required)
+    Specifies the type of package-repository.
+    Currently only `apt` is supported.
+
+``formats`` (required)
+    Specifies the format of the package-repository.
+    Supported values: ``deb`` and ``deb-src``.
+
+``components`` (required)
+    Specifies the component of the package-repository,
+    One or several of ``main``, ``restricted``, ``universe``, ``multiverse``.
+
+``suites`` (required)
+    Specifies the suite of the package-repository.
+    One or several of ``bionic``, ``focal``, ``jammy``.
+
+``url`` (required)
+    Specifies the URL of the package-repository.
+    e.g. ``http://ppa.launchpad.net/snappy-dev/snapcraft-daily/ubuntu``
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index dd39c1c..757dfe0 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -9,6 +9,7 @@ import os
 import shlex
 from argparse import ArgumentParser, Namespace
 from pathlib import Path, PurePath
+from tempfile import NamedTemporaryFile
 from typing import Dict, List, Optional, Set
 
 from craft_cli import BaseCommand, EmitterMode, emit
@@ -18,7 +19,7 @@ from dotenv import dotenv_values
 from pluggy import PluginManager
 
 from lpcraft import env
-from lpcraft.config import Config, Job, Output
+from lpcraft.config import Config, Job, Output, PackageRepository
 from lpcraft.errors import CommandError
 from lpcraft.plugin.manager import get_plugin_manager
 from lpcraft.plugins import PLUGINS
@@ -226,19 +227,38 @@ def _install_apt_packages(
     host_architecture: str,
     remote_cwd: Path,
     apt_replacement_repositories: Optional[List[str]],
+    additional_apt_repositories: Optional[List[PackageRepository]],
     environment: Optional[Dict[str, Optional[str]]],
 ) -> None:
-    if apt_replacement_repositories:
-        # replace sources.list
-        lines = "\n".join(apt_replacement_repositories) + "\n"
+    if apt_replacement_repositories or additional_apt_repositories:
+        sources_list_path = "/etc/apt/sources.list"
+
+        with NamedTemporaryFile(mode="w+") as tmpfile:
+            try:
+                instance.pull_file(
+                    source=Path(sources_list_path),
+                    destination=Path(tmpfile.name),
+                )
+            except Exception as e:
+                raise CommandError(str(e), retcode=1)
+            sources = tmpfile.read()
+
+        if apt_replacement_repositories:
+            sources = "\n".join(apt_replacement_repositories) + "\n"
+        if additional_apt_repositories:
+            for repository in additional_apt_repositories:
+                sources += (
+                    "\n" + "\n".join(repository.sources_list_lines()) + "\n"
+                )
         with emit.open_stream("Replacing /etc/apt/sources.list") as stream:
             instance.push_file_io(
-                destination=PurePath("/etc/apt/sources.list"),
-                content=io.BytesIO(lines.encode()),
+                destination=PurePath(sources_list_path),
+                content=io.BytesIO(sources.encode()),
                 file_mode="0644",
                 group="root",
                 user="root",
             )
+
     # update local repository information
     apt_update = ["apt", "update"]
     with emit.open_stream(f"Running {apt_update}") as stream:
@@ -316,6 +336,7 @@ def _run_job(
     provider: Provider,
     output: Optional[Path],
     apt_replacement_repositories: Optional[List[str]] = None,
+    additional_apt_repositories: Optional[List[PackageRepository]] = None,
     env_from_cli: Optional[List[str]] = None,
     plugin_settings: Optional[List[str]] = None,
 ) -> None:
@@ -406,6 +427,7 @@ def _run_job(
                 host_architecture=host_architecture,
                 remote_cwd=remote_cwd,
                 apt_replacement_repositories=apt_replacement_repositories,
+                additional_apt_repositories=additional_apt_repositories,
                 environment=environment,
             )
         for cmd in (pre_run_command, run_command, post_run_command):
@@ -518,6 +540,7 @@ class RunCommand(BaseCommand):
                                 apt_replacement_repositories=(
                                     args.apt_replace_repositories
                                 ),
+                                additional_apt_repositories=job.package_repositories,  # noqa: E501
                                 env_from_cli=args.set_env,
                                 plugin_settings=args.plugin_setting,
                             )
@@ -625,6 +648,7 @@ class RunOneCommand(BaseCommand):
                 provider,
                 args.output_directory,
                 apt_replacement_repositories=args.apt_replace_repositories,
+                additional_apt_repositories=job.package_repositories,
                 env_from_cli=args.set_env,
                 plugin_settings=args.plugin_setting,
             )
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 165d435..0da675a 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -1995,6 +1995,67 @@ class TestRun(RunBaseTestCase):
             execute_run.call_args_list,
         )
 
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_run_with_additional_package_repositories(
+        self, mock_get_host_architecture, mock_get_provider
+    ):
+        existing_repositories = [
+            "deb http://archive.ubuntu.com/ubuntu/ focal main restricted",
+            "deb-src http://archive.ubuntu.com/ubuntu/ focal main restricted",
+        ]
+
+        def fake_pull_file(source: Path, destination: Path) -> None:
+            destination.write_text("\n".join(existing_repositories))
+
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = launcher.return_value.execute_run
+        execute_run.return_value = subprocess.CompletedProcess([], 0)
+        launcher.return_value.pull_file.side_effect = fake_pull_file
+
+        config = dedent(
+            """
+            pipeline:
+                - test
+            jobs:
+                test:
+                    series: focal
+                    architectures: amd64
+                    run: ls -la
+                    packages: [git]
+                    package-repositories:
+                        - type: apt
+                          formats: [deb]
+                          components: [main, universe]
+                          suites: [focal]
+                          url: https://canonical.example.org/artifactory/jammy-golang-backport
+            """  # noqa: E501
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("run")
+
+        self.assertEqual(0, result.exit_code)
+
+        mock_info = launcher.return_value.push_file_io.call_args_list
+        self.assertEqual(
+            Path("/etc/apt/sources.list"), mock_info[0][1]["destination"]
+        )
+
+        file_contents = mock_info[0][1]["content"].read().decode()
+        self.assertEqual(
+            dedent(
+                """\
+            deb http://archive.ubuntu.com/ubuntu/ focal main restricted
+            deb-src http://archive.ubuntu.com/ubuntu/ focal main restricted
+            deb https://canonical.example.org/artifactory/jammy-golang-backport focal main universe
+            """  # noqa: E501
+            ),
+            file_contents,
+        )
+
 
 class TestRunOne(RunBaseTestCase):
     def test_config_file_not_under_project_directory(self):
@@ -2637,3 +2698,109 @@ class TestRunOne(RunBaseTestCase):
             ],
             execute_run.call_args_list,
         )
+
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_run_with_additional_package_repositories(
+        self, mock_get_host_architecture, mock_get_provider
+    ):
+        existing_repositories = [
+            "deb http://archive.ubuntu.com/ubuntu/ focal main restricted",
+            "deb-src http://archive.ubuntu.com/ubuntu/ focal main restricted",
+        ]
+
+        def fake_pull_file(source: Path, destination: Path) -> None:
+            destination.write_text("\n".join(existing_repositories))
+
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = launcher.return_value.execute_run
+        execute_run.return_value = subprocess.CompletedProcess([], 0)
+        launcher.return_value.pull_file.side_effect = fake_pull_file
+        config = dedent(
+            """
+            pipeline:
+                - test
+            jobs:
+                test:
+                    series: focal
+                    architectures: amd64
+                    run: ls -la
+                    packages: [git]
+                    package-repositories:
+                        - type: apt
+                          formats: [deb]
+                          components: [main, universe]
+                          suites: [focal]
+                          url: https://canonical.example.org/artifactory/jammy-golang-backport
+            """  # noqa: E501
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("run-one", "test", "0")
+
+        self.assertEqual(0, result.exit_code)
+
+        mock_info = launcher.return_value.push_file_io.call_args_list
+        self.assertEqual(
+            Path("/etc/apt/sources.list"), mock_info[0][1]["destination"]
+        )
+
+        file_contents = mock_info[0][1]["content"].read().decode()
+        self.assertEqual(
+            dedent(
+                """\
+            deb http://archive.ubuntu.com/ubuntu/ focal main restricted
+            deb-src http://archive.ubuntu.com/ubuntu/ focal main restricted
+            deb https://canonical.example.org/artifactory/jammy-golang-backport focal main universe
+            """  # noqa: E501
+            ),
+            file_contents,
+        )
+
+    @patch("lpcraft.env.get_managed_environment_project_path")
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_fails_pulling_sources_list(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = LocalExecuteRun(self.tmp_project_path)
+        launcher.return_value.execute_run = execute_run
+        launcher.return_value.pull_file.side_effect = FileNotFoundError(
+            "File not found"
+        )
+        config = dedent(
+            """
+            pipeline:
+                - test
+            jobs:
+                test:
+                    series: focal
+                    architectures: amd64
+                    run: ls -la
+                    packages: [git]
+                    package-repositories:
+                        - type: apt
+                          formats: [deb]
+                          components: [main, universe]
+                          suites: [focal]
+                          url: https://canonical.example.org/repodir
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("run-one", "test", "0")
+
+        self.assertThat(
+            result,
+            MatchesStructure.byEquality(
+                exit_code=1, errors=[CommandError("File not found", retcode=1)]
+            ),
+        )
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 0a4120d..0e835e2 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -5,10 +5,10 @@ import re
 from datetime import timedelta
 from enum import Enum
 from pathlib import Path
-from typing import Any, Dict, List, Optional, Type, Union
+from typing import Any, Dict, Iterator, List, Optional, Type, Union
 
 import pydantic
-from pydantic import StrictStr
+from pydantic import AnyHttpUrl, StrictStr
 
 from lpcraft.errors import ConfigurationError
 from lpcraft.plugins import PLUGINS
@@ -76,6 +76,64 @@ def _validate_plugin_config(
     return values
 
 
+class PackageType(str, Enum):
+    """Specifies the type of the package repository.
+
+    Currently only supports apt.
+    """
+
+    apt = "apt"
+
+
+class PackageFormat(str, Enum):
+    """Specifies the format of the package repository."""
+
+    deb = "deb"
+    deb_src = "deb-src"
+
+
+class PackageComponent(str, Enum):
+    """Specifies the component of the package repository."""
+
+    main = "main"
+    restricted = "restricted"
+    universe = "universe"
+    multiverse = "multiverse"
+
+
+class PackageSuite(str, Enum):
+    """Specifies the suite of the package repository.
+
+    e.g. xenial, focal, ...
+    """
+
+    bionic = "bionic"  # 18.04
+    focal = "focal"  # 20.04
+    jammy = "jammy"  # 22.04
+
+
+class PackageRepository(ModelConfigDefaults):
+    """A representation of a package repository.
+
+    inspired by https://snapcraft.io/docs/package-repositories
+    """
+
+    type: PackageType  # e.g. `apt``
+    formats: List[PackageFormat]  # e.g. `[deb, deb-src]`
+    components: List[PackageComponent]  # e.g. `[main, universe]`
+    suites: List[PackageSuite]  # e.g. `[bionic, focal]`
+    url: AnyHttpUrl
+
+    def sources_list_lines(self) -> Iterator[str]:
+        """Yield repository lines as strings.
+
+        e.g. 'deb https://canonical.example.org/artifactory/jammy-golang-backport focal main'
+        """  # noqa: E501
+        for format in self.formats:
+            for suite in self.suites:
+                yield f"{format} {self.url!s} {suite} {' '.join(self.components)}"  # noqa: E501
+
+
 class Job(ModelConfigDefaults):
     """A job definition."""
 
@@ -92,6 +150,7 @@ class Job(ModelConfigDefaults):
     output: Optional[Output]
     snaps: Optional[List[StrictStr]]
     packages: Optional[List[StrictStr]]
+    package_repositories: Optional[List[PackageRepository]]
     plugin: Optional[StrictStr]
     plugin_config: Optional[BaseConfig]
 
diff --git a/lpcraft/main.py b/lpcraft/main.py
index 83fe767..f6263cf 100644
--- a/lpcraft/main.py
+++ b/lpcraft/main.py
@@ -63,6 +63,21 @@ def main(argv: Optional[List[str]] = None) -> int:
         )
     ]
 
+    # dispatcher = Dispatcher(
+    #     "lpcraft",
+    #     command_groups,
+    #     summary=summary,
+    #     extra_global_args=extra_global_args,
+    #     default_command=RunCommand,
+    # )
+    # global_args = dispatcher.pre_parse_args(argv)
+    # if global_args["version"]:
+    #     emit.message(lpcraft_version)
+    #     emit.ended_ok()
+    #     return 0
+    # dispatcher.load_command(None)
+    # ret = dispatcher.run() or 0
+
     try:
         dispatcher = Dispatcher(
             "lpcraft",
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index d34b718..3daf342 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -7,7 +7,7 @@ from pathlib import Path
 from textwrap import dedent
 
 from fixtures import TempDir
-from pydantic import ValidationError
+from pydantic import AnyHttpUrl, ValidationError
 from testtools import TestCase
 from testtools.matchers import (
     Equals,
@@ -16,7 +16,7 @@ from testtools.matchers import (
     MatchesStructure,
 )
 
-from lpcraft.config import Config, OutputDistributeEnum
+from lpcraft.config import Config, OutputDistributeEnum, PackageRepository
 from lpcraft.errors import CommandError
 
 
@@ -403,3 +403,81 @@ class TestConfig(TestCase):
         config = Config.load(path)
 
         self.assertEqual("tox", config.jobs["test"][0].plugin)
+
+    def test_package_repositories(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [nginx, apache2]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb]
+                              components: [main]
+                              suites: [focal]
+                              url: https://canonical.example.org/artifactory/jammy-golang-backport
+                """  # noqa: E501
+            )
+        )
+
+        config = Config.load(path)
+
+        self.assertEqual(
+            [
+                PackageRepository(
+                    type="apt",
+                    formats=["deb"],
+                    components=["main"],
+                    suites=["focal"],
+                    url=AnyHttpUrl(
+                        "https://canonical.example.org/artifactory/jammy-golang-backport",  # noqa: E501
+                        scheme="https",
+                        host="canonical.example.org",
+                        tld="org",
+                        host_type="domain",
+                        path="/artifactory/jammy-golang-backport",
+                    ),
+                )
+            ],
+            config.jobs["test"][0].package_repositories,
+        )
+
+    def test_package_repositories_as_string(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [nginx, apache2]
+                        package-repositories:
+                            - type: apt
+                              formats: [deb, deb-src]
+                              components: [main]
+                              suites: [focal, bionic]
+                              url: https://canonical.example.org/artifactory/jammy-golang-backport
+                """  # noqa: E501
+            )
+        )
+        config = Config.load(path)
+        expected = [
+            "deb https://canonical.example.org/artifactory/jammy-golang-backport focal main",  # noqa: E501
+            "deb https://canonical.example.org/artifactory/jammy-golang-backport bionic main",  # noqa: E501
+            "deb-src https://canonical.example.org/artifactory/jammy-golang-backport focal main",  # noqa: E501
+            "deb-src https://canonical.example.org/artifactory/jammy-golang-backport bionic main",  # noqa: E501
+        ]
+        repositories = config.jobs["test"][0].package_repositories
+        assert repositories is not None  # workaround necessary to please mypy
+        self.assertEqual(
+            expected, (list(repositories[0].sources_list_lines()))
+        )