Merge ~lgp171188/lpci:set-sensible-defaults-some-package-repository-fields into lpci:main

[Guruprasad]

Set sensible default values for some package repository fields

Also, simplify the package repository cross-field validation.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/NEWS.rst b/NEWS.rst
index 4defd5a..f351e3f 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -5,8 +5,9 @@ Version history
 0.0.40 (unreleased)
 ===================
 
-- Fix the leakage of package repositories from a job to the next
+- Fix the leakage of package repositories from a job to the next.
 - Add support for Python 3.11.
+- Set sensible default values for some package repository fields.
 
 0.0.39 (2023-01-06)
 ===================
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 180734c..d25c96d 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -190,13 +190,15 @@ Adding a PPA
     Specifies the type of package-repository.
     Currently only ``apt`` is supported.
 
-``formats`` (required)
+``formats`` (optional)
     Specifies the format of the package-repository.
-    Supported values: ``deb`` and ``deb-src``.
+    Supported values: ``deb`` and ``deb-src``. If unspecified,
+    the format is assumed to be ``deb`` , i.e. ``[deb]``
 
-``suites`` (required)
+``suites`` (optional)
     Specifies the suite of the package-repository.
-    One or several of ``bionic``, ``focal``, ``jammy``.
+    One or several of ``bionic``, ``focal``, ``jammy``. If unspecified,
+    the suite is assumed to be the corresponding job's ``series`` value.
 
 ``ppa`` (required)
     Specifies the PPA to be used as the package repository in the short form,
@@ -228,11 +230,13 @@ Adding a deb repository
 
 ``formats`` (required)
     Specifies the format of the package-repository.
-    Supported values: ``deb`` and ``deb-src``.
+    Supported values: ``deb`` and ``deb-src``. If unspecified,
+    the format is assumed to be ``deb``, i.e. ``[deb]``.
 
 ``suites`` (required)
     Specifies the suite of the package-repository.
-    One or several of ``bionic``, ``focal``, ``jammy``.
+    One or several of ``bionic``, ``focal``, ``jammy``. If unspecified,
+    the suite is assumed to be the corresponding job's ``series`` value.
 
 ``components`` (required)
     Specifies the component of the package-repository,
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 7c36713..d228270 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -152,9 +152,9 @@ class PackageRepository(ModelConfigDefaults):
 
     type: PackageType  # e.g. `apt``
     ppa: Optional[PPAShortFormURL]  # e.g. `launchpad/ubuntu/ppa`
-    formats: List[PackageFormat]  # e.g. `[deb, deb-src]`
+    formats: Optional[List[PackageFormat]]  # e.g. `[deb, deb-src]`
     components: Optional[List[PackageComponent]]  # e.g. `[main, universe]`
-    suites: List[PackageSuite]  # e.g. `[bionic, focal]`
+    suites: Optional[List[PackageSuite]]  # e.g. `[bionic, focal]`
     url: Optional[AnyHttpUrl]
     trusted: Optional[bool]
 
@@ -162,26 +162,29 @@ class PackageRepository(ModelConfigDefaults):
     def validate_multiple_fields(
         cls, values: Dict[str, Any]
     ) -> Dict[str, Any]:
-        if "url" not in values and "ppa" not in values:
-            raise ValueError(
-                "One of the following keys is required with an appropriate"
-                " value: 'url', 'ppa'."
-            )
-        elif "url" in values and "ppa" in values:
-            raise ValueError(
-                "Only one of the following keys can be specified:"
-                " 'url', 'ppa'."
-            )
-        elif "ppa" not in values and "components" not in values:
-            raise ValueError(
-                "One of the following keys is required with an appropriate"
-                " value: 'components', 'ppa'."
-            )
-        elif "ppa" in values and "components" in values:
-            raise ValueError(
-                "The 'components' key is not allowed when the 'ppa' key is"
-                " specified. PPAs only support the 'main' component."
-            )
+        if "url" in values:
+            if "ppa" in values:
+                raise ValueError(
+                    "Only one of the following keys can be specified:"
+                    " 'url', 'ppa'."
+                )
+            if "components" not in values:
+                raise ValueError(
+                    "The 'components' key is required when the 'url' key"
+                    " is specified."
+                )
+        else:
+            if "ppa" not in values:
+                raise ValueError(
+                    "One of the following keys is required with an appropriate"
+                    " value: 'url', 'ppa'."
+                )
+            if "components" in values:
+                raise ValueError(
+                    "The 'components' key is not allowed when the 'ppa' key is"
+                    " specified. PPAs only support the 'main' component."
+                )
+
         return values
 
     @validator("components", pre=True, always=True)
@@ -206,6 +209,14 @@ class PackageRepository(ModelConfigDefaults):
             )
         return v
 
+    @validator("formats", pre=True, always=True)
+    def set_formats_default_value(
+        cls, v: List[PackageFormat]
+    ) -> List[PackageFormat]:
+        if not v:
+            v = [PackageFormat.deb]
+        return v
+
     @validator("trusted")
     def convert_trusted(cls, v: bool) -> str:
         # trusted is True or False, but we need `yes` or `no`
@@ -216,6 +227,8 @@ class PackageRepository(ModelConfigDefaults):
 
         e.g. 'deb https://canonical.example.org/artifactory/jammy-golang-backport focal main'
         """  # noqa: E501
+        assert self.formats is not None
+        assert self.suites is not None
         for format in self.formats:
             for suite in self.suites:
                 assert self.components is not None
@@ -254,6 +267,18 @@ class Job(ModelConfigDefaults):
             v = [v]
         return v
 
+    @pydantic.validator("package_repositories")
+    def validate_package_repositories(
+        cls, v: List[PackageRepository], values: Dict[StrictStr, Any]
+    ) -> List[PackageRepository]:
+        package_repositories = None
+        for index, package_repository in enumerate(v):
+            if not package_repository.suites:
+                if not package_repositories:
+                    package_repositories = v.copy()
+                package_repositories[index].suites = [values["series"]]
+        return package_repositories or v
+
     @pydantic.root_validator(pre=True)
     def move_plugin_config_settings(
         cls, values: Dict[StrictStr, Any]
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index b68e20b..6c69eab 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -579,6 +579,32 @@ class TestConfig(TestCase):
             get_ppa_url_parts(PPAShortFormURL("example/debian/bar")),
         )
 
+    def test_default_values_for_package_repository_suites_and_formats(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                        packages: [foo]
+                        package-repositories:
+                            - type: apt
+                              ppa: launchpad/ubuntu/ppa
+                            - type: apt
+                              url: https://canonical.example.org/ubuntu
+                              components: [main]
+                """
+            )
+        )
+        config = Config.load(path)
+
+        for package_repository in config.jobs["test"][0].package_repositories:
+            self.assertEqual(["deb"], package_repository.formats)
+            self.assertEqual(["focal"], package_repository.suites)
+
     def test_missing_ppa_and_url(self):
         path = self.create_config(
             dedent(
@@ -634,7 +660,7 @@ class TestConfig(TestCase):
             path,
         )
 
-    def test_missing_ppa_and_components(self):
+    def test_missing_components_when_url_is_specified(self):
         path = self.create_config(
             dedent(
                 """
@@ -655,8 +681,8 @@ class TestConfig(TestCase):
         )
         self.assertRaisesRegex(
             ValidationError,
-            r"One of the following keys is required with an appropriate"
-            r" value: 'components', 'ppa'.",
+            r"The 'components' key is required when the 'url' key"
+            r" is specified.",
             Config.load,
             path,
         )