launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-private-flag into launchpad:master with ~pappacena/launchpad:ocirecipebuild-private-repos as a prerequisite.

Commit message:
Adding OCIRecipe.private model attribute mapping

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397176
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-private-flag into launchpad:master.

diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 633831f..95ade1d 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -20,9 +20,12 @@ __all__ = [
     'OCIRecipeBuildAlreadyPending',
     'OCIRecipeFeatureDisabled',
     'OCIRecipeNotOwner',
+    'OCIRecipePrivacyMismatch',
+    'OCIRecipePrivateFeatureDisabled',
     'OCI_RECIPE_ALLOW_CREATE',
     'OCI_RECIPE_BUILD_DISTRIBUTION',
-    'OCI_RECIPE_WEBHOOKS_FEATURE_FLAG',
+    'OCI_RECIPE_PRIVATE_FEATURE_FLAG',
+    'OCI_RECIPE_WEBHOOKS_FEATURE_FLAG'
     ]
 
 from lazr.lifecycle.snapshot import doNotSnapshot
@@ -141,6 +144,25 @@ class CannotModifyOCIRecipeProcessor(Exception):
             self._fmt % {'processor': processor.name})
 
 
+@error_status(http_client.BAD_REQUEST)
+class OCIRecipePrivacyMismatch(Exception):
+    """OCI recipe privacy does not match its content."""
+
+    def __init__(self, message=None):
+        super(OCIRecipePrivacyMismatch, self).__init__(
+            message or
+            "OCI recipe contains private information and cannot be public.")
+
+
+@error_status(http_client.UNAUTHORIZED)
+class OCIRecipePrivateFeatureDisabled(Unauthorized):
+    """Only certain users can create private OCI recipe objects."""
+
+    def __init__(self):
+        super(OCIRecipePrivateFeatureDisabled, self).__init__(
+            "You do not have permission to create private snaps")
+
+
 @exported_as_webservice_entry(
     publish_web_link=True, as_of="devel",
     singular_name="oci_recipe_build_request")
@@ -447,6 +469,10 @@ class IOCIRecipeAdminAttributes(Interface):
     These attributes need launchpad.View to see, and launchpad.Admin to change.
     """
 
+    private = exported(Bool(
+        title=_("Private"), required=False, readonly=False,
+        description=_("Whether or not this OCI recipe is private.")))
+
     require_virtualized = Bool(
         title=_("Require virtualized builders"), required=True, readonly=False,
         description=_("Only build this OCI recipe on virtual builders."))
@@ -479,7 +505,7 @@ class IOCIRecipeSet(Interface):
     def new(name, registrant, owner, oci_project, git_ref, build_file,
             description=None, official=False, require_virtualized=True,
             build_daily=False, processors=None, date_created=DEFAULT,
-            allow_internet=True, build_args=None):
+            allow_internet=True, build_args=None, private=False):
         """Create an IOCIRecipe."""
 
     def exists(owner, oci_project, name):
@@ -511,3 +537,6 @@ class IOCIRecipeSet(Interface):
         After this, any OCI recipes that previously used this repository
         will have no source and so cannot dispatch new builds.
         """
+
+    def isValidPrivacy(private, owner, git_ref=None):
+        """Whether or not the privacy context is valid."""
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index fd14762..3e51988 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """A recipe for building Open Container Initiative images."""
@@ -67,9 +67,12 @@ from lp.oci.interfaces.ocirecipe import (
     NoSuchOCIRecipe,
     OCI_RECIPE_ALLOW_CREATE,
     OCI_RECIPE_BUILD_DISTRIBUTION,
+    OCI_RECIPE_PRIVATE_FEATURE_FLAG,
     OCIRecipeBuildAlreadyPending,
     OCIRecipeFeatureDisabled,
     OCIRecipeNotOwner,
+    OCIRecipePrivacyMismatch,
+    OCIRecipePrivateFeatureDisabled,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
@@ -155,6 +158,14 @@ class OCIRecipe(Storm, WebhookTargetMixin):
     build_path = Unicode(name="build_path", allow_none=False)
     _build_args = JSON(name="build_args", allow_none=True)
 
+    def _validate_private(self, attr, value):
+        if not getUtility(IOCIRecipeSet).isValidPrivacy(
+                value, self.owner, self.git_ref):
+            raise OCIRecipePrivacyMismatch
+        return value
+
+    private = Bool(name='private', validator=_validate_private)
+
     require_virtualized = Bool(name="require_virtualized", default=True,
                                allow_none=False)
 
@@ -165,7 +176,8 @@ class OCIRecipe(Storm, WebhookTargetMixin):
     def __init__(self, name, registrant, owner, oci_project, git_ref,
                  description=None, official=False, require_virtualized=True,
                  build_file=None, build_daily=False, date_created=DEFAULT,
-                 allow_internet=True, build_args=None, build_path=None):
+                 allow_internet=True, build_args=None, build_path=None,
+                 private=False):
         if not getFeatureFlag(OCI_RECIPE_ALLOW_CREATE):
             raise OCIRecipeFeatureDisabled()
         super(OCIRecipe, self).__init__()
@@ -184,6 +196,7 @@ class OCIRecipe(Storm, WebhookTargetMixin):
         self.allow_internet = allow_internet
         self.build_args = build_args or {}
         self.build_path = build_path
+        self.private = private
 
     def __repr__(self):
         return "<OCIRecipe ~%s/%s/+oci/%s/+recipe/%s>" % (
@@ -593,7 +606,8 @@ class OCIRecipeSet:
     def new(self, name, registrant, owner, oci_project, git_ref, build_file,
             description=None, official=False, require_virtualized=True,
             build_daily=False, processors=None, date_created=DEFAULT,
-            allow_internet=True, build_args=None, build_path=None):
+            allow_internet=True, build_args=None, build_path=None,
+            private=False):
         """See `IOCIRecipeSet`."""
         if not registrant.inTeam(owner):
             if owner.is_team:
@@ -618,7 +632,7 @@ class OCIRecipeSet:
         oci_recipe = OCIRecipe(
             name, registrant, owner, oci_project, git_ref, description,
             official, require_virtualized, build_file, build_daily,
-            date_created, allow_internet, build_args, build_path)
+            date_created, allow_internet, build_args, build_path, private)
         store.add(oci_recipe)
 
         if processors is None:
@@ -697,6 +711,25 @@ class OCIRecipeSet:
             if git_ref is not None:
                 recipe.git_ref = git_ref
 
+    def isValidPrivacy(self, private, owner, git_ref=None):
+        """See `IOCIRecipeSet`."""
+        # Private OCI recipes may contain anything ...
+        if private:
+            # If appropriately enabled via feature flag.
+            if not getFeatureFlag(OCI_RECIPE_PRIVATE_FEATURE_FLAG):
+                raise OCIRecipePrivateFeatureDisabled
+            return True
+
+        # Public snaps with private sources are not allowed.
+        if git_ref is not None and git_ref.private:
+            return False
+
+        # Public snaps owned by private teams are not allowed.
+        if owner is not None and owner.private:
+            return False
+
+        return True
+
 
 @implementer(IOCIRecipeBuildRequest)
 class OCIRecipeBuildRequest:
diff --git a/lib/lp/oci/tests/helpers.py b/lib/lp/oci/tests/helpers.py
index b656ca6..9042f5b 100644
--- a/lib/lp/oci/tests/helpers.py
+++ b/lib/lp/oci/tests/helpers.py
@@ -37,11 +37,13 @@ class OCIConfigHelperMixin:
             registry_secrets_private_key=base64.b64encode(
                 bytes(self.private_key)).decode("UTF-8"))
         # Default feature flags for our tests
-        feature_flags = feature_flags or {}
-        feature_flags.update({
+        flags = {
             OCI_RECIPE_ALLOW_CREATE: 'on',
-            OCI_RECIPE_PRIVATE_FEATURE_FLAG: 'on'})
-        self.useFixture(FeatureFixture(feature_flags))
+            OCI_RECIPE_PRIVATE_FEATURE_FLAG: 'on'}
+        # Allow users to set their own flags or override our previously set
+        # flags.
+        flags.update(feature_flags or {})
+        self.useFixture(FeatureFixture(flags))
 
 
 class MatchesOCIRegistryCredentials(MatchesAll):
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 1cb69af..bd79fb0 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for OCI image building recipe functionality."""
@@ -30,6 +30,7 @@ from zope.security.interfaces import (
     )
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.enums import InformationType
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.oci.interfaces.ocipushrule import (
@@ -45,10 +46,13 @@ from lp.oci.interfaces.ocirecipe import (
     NoSuchOCIRecipe,
     OCI_RECIPE_ALLOW_CREATE,
     OCI_RECIPE_BUILD_DISTRIBUTION,
+    OCI_RECIPE_PRIVATE_FEATURE_FLAG,
     OCI_RECIPE_WEBHOOKS_FEATURE_FLAG,
     OCIRecipeBuildAlreadyPending,
     OCIRecipeBuildRequestStatus,
     OCIRecipeNotOwner,
+    OCIRecipePrivacyMismatch,
+    OCIRecipePrivateFeatureDisabled,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
@@ -59,6 +63,7 @@ from lp.oci.tests.helpers import (
     MatchesOCIRegistryCredentials,
     OCIConfigHelperMixin,
     )
+from lp.registry.enums import PersonVisibility
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
 from lp.services.database.constants import (
@@ -751,6 +756,50 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
         }, recipe.build_args)
 
 
+class TestOCIRecipePrivacy(OCIConfigHelperMixin, TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestOCIRecipePrivacy, self).setUp()
+        self.setConfig()
+        login_admin()
+
+    def test_create_private(self):
+        recipe = self.factory.makeOCIRecipe(private=True)
+        self.assertTrue(recipe.private)
+
+    def test_create_private_with_feature_disabled(self):
+        self.setConfig(feature_flags={OCI_RECIPE_PRIVATE_FEATURE_FLAG: ''})
+        recipe = self.factory.makeOCIRecipe()
+        self.assertRaises(
+            OCIRecipePrivateFeatureDisabled,
+            setattr, recipe, 'private', True)
+        IStore(recipe).invalidate()
+        self.assertFalse(recipe.private)
+
+    def test_recipe_cannot_be_public_with_private_owner(self):
+        owner = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+        recipe = self.factory.makeOCIRecipe(
+            registrant=owner, owner=owner, private=True)
+        self.assertRaises(
+            OCIRecipePrivacyMismatch,
+            setattr, recipe, 'private', False)
+        IStore(recipe).invalidate()
+        self.assertTrue(recipe.private)
+
+    def test_recipe_cannot_be_public_with_private_repository(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.repository.transitionToInformationType(
+            InformationType.PRIVATESECURITY, git_ref.repository.owner)
+        recipe = self.factory.makeOCIRecipe(git_ref=git_ref, private=True)
+        self.assertRaises(
+            OCIRecipePrivacyMismatch,
+            setattr, recipe, 'private', False)
+        IStore(recipe).invalidate()
+        self.assertTrue(recipe.private)
+
+
 class TestOCIRecipeProcessors(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 0bd8f83..4e4cfeb 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2,7 +2,7 @@
 # NOTE: The first line above must stay first; do not move the copyright
 # notice to the top.  See http://www.python.org/dev/peps/pep-0263/.
 #
-# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Testing infrastructure for the Launchpad application.
@@ -4924,7 +4924,8 @@ class BareLaunchpadObjectFactory(ObjectFactory):
                       oci_project=None, git_ref=None, description=None,
                       official=False, require_virtualized=True,
                       build_file=None, date_created=DEFAULT,
-                      allow_internet=True, build_args=None, build_path=None):
+                      allow_internet=True, build_args=None, build_path=None,
+                      private=False):
         """Make a new OCIRecipe."""
         if name is None:
             name = self.getUniqueString(u"oci-recipe-name")
@@ -4955,7 +4956,8 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             require_virtualized=require_virtualized,
             date_created=date_created,
             allow_internet=allow_internet,
-            build_args=build_args)
+            build_args=build_args,
+            private=private)
 
     def makeOCIRecipeArch(self, recipe=None, processor=None):
         """Make a new OCIRecipeArch."""