Merge ~pappacena/launchpad:ui-manage-ocirecipe-for-projects into launchpad:master

[Thiago F. Pappacena]

Adjustments to make OCIRecipe work also with OCIProjects based on project, rather then distributions.

95f67e6... by Thiago F. Pappacena on 2020-06-01

Adding feature flag to control distribution for project-based OCI recipes

[Thiago F. Pappacena (pappacena)]

twom, added the feature flag to control which distribution should be used for OCIRecipes of OCIProjects based on IProduct. We fallback gracefully to Ubuntu if the feature flag is not set.

[Tom Wardill (twom)]

review: Approve

diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 6633966..e25d463 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -91,8 +91,22 @@ class TestOCIRecipeNavigation(TestCaseWithFactory):
             "http://launchpad.test/~person/distro/+oci/oci-project/"
             "+recipe/recipe", canonical_url(recipe))
 
-    def test_recipe(self):
-        recipe = self.factory.makeOCIRecipe()
+    def test_recipe_traverse_distribution(self):
+        # Make sure we can reach recipe of distro-based OCI projects.
+        distro = self.factory.makeDistribution()
+        oci_project = self.factory.makeOCIProject(pillar=distro)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/~%s/%s/+oci/%s/+recipe/%s" % (
+                recipe.owner.name, recipe.oci_project.pillar.name,
+                recipe.oci_project.name, recipe.name))
+        self.assertEqual(recipe, obj)
+
+    def test_recipe_traverse_project(self):
+        # Make sure we can reach recipe of project-based OCI projects.
+        project = self.factory.makeProduct()
+        oci_project = self.factory.makeOCIProject(pillar=project)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
         obj, _, _ = test_traverse(
             "http://launchpad.test/~%s/%s/+oci/%s/+recipe/%s" % (
                 recipe.owner.name, recipe.oci_project.pillar.name,
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 4b2b06d..3628d71 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -21,6 +21,7 @@ __all__ = [
     'OCIRecipeFeatureDisabled',
     'OCIRecipeNotOwner',
     'OCI_RECIPE_ALLOW_CREATE',
+    'OCI_RECIPE_BUILD_DISTRIBUTION',
     'OCI_RECIPE_WEBHOOKS_FEATURE_FLAG',
     ]
 
@@ -80,6 +81,7 @@ from lp.services.webhooks.interfaces import IWebhookTarget
 
 OCI_RECIPE_WEBHOOKS_FEATURE_FLAG = "oci.recipe.webhooks.enabled"
 OCI_RECIPE_ALLOW_CREATE = 'oci.recipe.create.enabled'
+OCI_RECIPE_BUILD_DISTRIBUTION = 'oci.default_build_distribution'
 
 
 @error_status(http_client.UNAUTHORIZED)
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 8562778..f5ecbdf 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -36,6 +36,7 @@ from zope.interface import implementer
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.security import IAuthorization
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
@@ -56,6 +57,7 @@ from lp.oci.interfaces.ocirecipe import (
     NoSourceForOCIRecipe,
     NoSuchOCIRecipe,
     OCI_RECIPE_ALLOW_CREATE,
+    OCI_RECIPE_BUILD_DISTRIBUTION,
     OCIRecipeBuildAlreadyPending,
     OCIRecipeFeatureDisabled,
     OCIRecipeNotOwner,
@@ -67,6 +69,7 @@ from lp.oci.interfaces.ociregistrycredentials import (
     )
 from lp.oci.model.ocipushrule import OCIPushRule
 from lp.oci.model.ocirecipebuild import OCIRecipeBuild
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.role import IPersonRoles
 from lp.registry.model.distribution import Distribution
@@ -216,9 +219,20 @@ class OCIRecipe(Storm, WebhookTargetMixin):
 
     @property
     def distribution(self):
-        # XXX twom 2019-12-05 This may need to change when an OCIProject
-        # pillar isn't just a distribution
-        return self.oci_project.distribution
+        if self.oci_project.distribution:
+            return self.oci_project.distribution
+        # For OCI projects that are not based on distribution, we use the
+        # default distribution set by the following feature flag (or
+        # defaults to Ubuntu, if none is set).
+        distro_name = getFeatureFlag(OCI_RECIPE_BUILD_DISTRIBUTION)
+        if not distro_name:
+            return getUtility(ILaunchpadCelebrities).ubuntu
+        distro = getUtility(IDistributionSet).getByName(distro_name)
+        if not distro:
+            raise ValueError(
+                "'%s' is not a valid value for feature flag '%s'" %
+                (distro_name, OCI_RECIPE_BUILD_DISTRIBUTION))
+        return distro
 
     @property
     def distro_series(self):
@@ -237,9 +251,10 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         """See `IOCIRecipe`."""
         clauses = [Processor.id == DistroArchSeries.processor_id]
         if self.distro_series is not None:
+            enabled_archs_resultset = removeSecurityProxy(
+                self.distro_series.enabled_architectures)
             clauses.append(DistroArchSeries.id.is_in(
-                self.distro_series.enabled_architectures.get_select_expr(
-                    DistroArchSeries.id)))
+                enabled_archs_resultset.get_select_expr(DistroArchSeries.id)))
         else:
             # We might not know the series if the OCI project's distribution
             # has no series at all, which can happen in tests.  Fall back to
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 1c69638..d728cf7 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -43,6 +43,7 @@ from lp.oci.interfaces.ocirecipe import (
     NoSourceForOCIRecipe,
     NoSuchOCIRecipe,
     OCI_RECIPE_ALLOW_CREATE,
+    OCI_RECIPE_BUILD_DISTRIBUTION,
     OCI_RECIPE_WEBHOOKS_FEATURE_FLAG,
     OCIRecipeBuildAlreadyPending,
     OCIRecipeBuildRequestStatus,
@@ -94,6 +95,45 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
         with admin_logged_in():
             self.assertProvides(target, IOCIRecipe)
 
+    def test_default_distribution_on_project_pillar(self):
+        # If OCI_RECIPE_BUILD_DISTRIBUTION flag is not set, we use Ubuntu.
+        project = self.factory.makeProduct()
+        oci_project = self.factory.makeOCIProject(pillar=project)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        self.assertEqual('ubuntu', recipe.distribution.name)
+
+    def test_feature_flag_distribution_on_project_pillar(self):
+        # With the OCI_RECIPE_BUILD_DISTRIBUTION feature flag set, we should
+        # use the distribution with the given name.
+        distro_name = "mydistro"
+        distribution = self.factory.makeDistribution(name=distro_name)
+        project = self.factory.makeProduct()
+        oci_project = self.factory.makeOCIProject(pillar=project)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        with FeatureFixture({OCI_RECIPE_BUILD_DISTRIBUTION: distro_name}):
+            self.assertEqual(distribution, recipe.distribution)
+
+    def test_feature_flag_inexisting_distribution_on_project_pillar(self):
+        # If we mistakenly set the flag to a non-existing distribution,
+        # things should break explicitly.
+        project = self.factory.makeProduct()
+        oci_project = self.factory.makeOCIProject(pillar=project)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        with FeatureFixture({OCI_RECIPE_BUILD_DISTRIBUTION: "banana-distro"}):
+            expected_msg = (
+                "'banana-distro' is not a valid value for feature flag '%s'"
+                % OCI_RECIPE_BUILD_DISTRIBUTION)
+            self.assertRaisesWithContent(
+                ValueError, expected_msg, getattr, recipe, 'distribution')
+
+    def test_distribution_for_distro_based_oci_project(self):
+        # For distribution-based OCI projects, we should use OCIProject's
+        # distribution as the recipe distribution.
+        distribution = self.factory.makeDistribution()
+        oci_project = self.factory.makeOCIProject(pillar=distribution)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        self.assertEqual(distribution, recipe.distribution)
+
     def test_initial_date_last_modified(self):
         # The initial value of date_last_modified is date_created.
         recipe = self.factory.makeOCIRecipe(date_created=ONE_DAY_AGO)
diff --git a/lib/lp/registry/browser/personproduct.py b/lib/lp/registry/browser/personproduct.py
index 90f8b55..a249b27 100644
--- a/lib/lp/registry/browser/personproduct.py
+++ b/lib/lp/registry/browser/personproduct.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Views, menus and traversal related to PersonProducts."""
@@ -10,19 +10,23 @@ __all__ = [
     'PersonProductNavigation',
     ]
 
-
-from zope.component import queryAdapter
+from zope.component import (
+    getUtility,
+    queryAdapter,
+    )
 from zope.interface import implementer
 from zope.traversing.interfaces import IPathAdapter
 
 from lp.app.errors import NotFoundError
 from lp.code.browser.vcslisting import PersonTargetDefaultVCSNavigationMixin
 from lp.code.interfaces.branchnamespace import get_branch_namespace
+from lp.registry.interfaces.personociproject import IPersonOCIProjectFactory
 from lp.registry.interfaces.personproduct import IPersonProduct
 from lp.services.webapp import (
     canonical_url,
     Navigation,
     StandardLaunchpadFacets,
+    stepthrough,
     )
 from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
@@ -33,6 +37,12 @@ class PersonProductNavigation(PersonTargetDefaultVCSNavigationMixin,
     """Navigation to branches for this person/product."""
     usedfor = IPersonProduct
 
+    @stepthrough('+oci')
+    def traverse_oci(self, name):
+        oci_project = self.context.product.getOCIProject(name)
+        return getUtility(IPersonOCIProjectFactory).create(
+            self.context.person, oci_project)
+
     def traverse(self, branch_name):
         """Look for a branch in the person/product namespace."""
         namespace = get_branch_namespace(