Merge ~pappacena/launchpad:snap-pillar into launchpad:master

[Thiago F. Pappacena]

Adding Snap.project to be the (optional) pillar of snaps

This adds a project to be the optional pillar of Snaps (mandatory for private Snaps). Once we create the UI to set this, we should start validating and only allowing private Snaps that belongs to a given pillar (so we can use the pillar's sharing options to control Snap's privacy).

Database patch is available here: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397459.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested change.

I've also removed the XXX comment about validating (project + private) attributes at model level. It would break old snaps if we've added that validation in the future.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index bf4b81c..e343a5f 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -302,6 +302,7 @@ public.snapbuild                        = SELECT, INSERT, UPDATE, DELETE
 public.snapbuildjob                     = SELECT, INSERT, UPDATE, DELETE
 public.snapfile                         = SELECT, INSERT, UPDATE, DELETE
 public.snapjob                          = SELECT, INSERT, UPDATE, DELETE
+public.snapsubscription                 = SELECT, INSERT, UPDATE, DELETE
 public.snappydistroseries               = SELECT, INSERT, UPDATE, DELETE
 public.snappyseries                     = SELECT, INSERT, UPDATE, DELETE
 public.sourcepackageformatselection     = SELECT
@@ -2246,6 +2247,7 @@ type=user
 
 [person-merge-job]
 groups=script
+public.accesspolicyartifact             = SELECT
 public.accessartifactgrant              = SELECT, UPDATE, DELETE
 public.accesspolicy                     = SELECT, UPDATE, DELETE
 public.accesspolicygrant                = SELECT, UPDATE, DELETE
@@ -2363,6 +2365,7 @@ public.signedcodeofconduct              = SELECT, UPDATE
 public.snap                             = SELECT, UPDATE
 public.snapbase                         = SELECT, UPDATE
 public.snapbuild                        = SELECT, UPDATE
+public.snapsubscription                 = SELECT, UPDATE, DELETE
 public.snappyseries                     = SELECT, UPDATE
 public.sourcepackagename                = SELECT
 public.sourcepackagepublishinghistory   = SELECT, UPDATE
diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index 3835b23..9b19b1b 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap package interfaces."""
@@ -36,6 +36,7 @@ __all__ = [
     'SnapBuildRequestStatus',
     'SnapNotOwner',
     'SnapPrivacyMismatch',
+    'SnapPrivacyPillarError',
     'SnapPrivateFeatureDisabled',
     ]
 
@@ -88,7 +89,9 @@ from zope.security.interfaces import (
     )
 
 from lp import _
+from lp.app.enums import InformationType
 from lp.app.errors import NameLookupFailed
+from lp.app.interfaces.informationtype import IInformationType
 from lp.app.interfaces.launchpad import IPrivacy
 from lp.app.validators.name import name_validator
 from lp.buildmaster.interfaces.processor import IProcessor
@@ -98,6 +101,7 @@ from lp.code.interfaces.gitrepository import IGitRepository
 from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.person import IPerson
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.role import IHasOwner
 from lp.services.fields import (
     PersonChoice,
@@ -212,7 +216,16 @@ class SnapPrivacyMismatch(Exception):
     def __init__(self, message=None):
         super(SnapPrivacyMismatch, self).__init__(
             message or
-            "Snap contains private information and cannot be public.")
+            "Snap recipe contains private information and cannot be public.")
+
+
+@error_status(http_client.BAD_REQUEST)
+class SnapPrivacyPillarError(Exception):
+    """Private Snaps should be based in a pillar."""
+
+    def __init__(self, message=None):
+        super(SnapPrivacyPillarError, self).__init__(
+            message or "Private Snap recipes should have a pillar.")
 
 
 class BadSnapSearchContext(Exception):
@@ -658,6 +671,11 @@ class ISnapEditableAttributes(IHasOwner):
         vocabulary="AllUserTeamsParticipationPlusSelf",
         description=_("The owner of this snap package.")))
 
+    project = ReferenceChoice(
+        title=_('The project that this Snap is associated with.'),
+        schema=IProduct, vocabulary='Product',
+        required=False, readonly=False)
+
     distro_series = exported(Reference(
         IDistroSeries, title=_("Distro Series"),
         required=False, readonly=False,
@@ -825,6 +843,12 @@ class ISnapAdminAttributes(Interface):
         title=_("Private"), required=False, readonly=False,
         description=_("Whether or not this snap is private.")))
 
+    information_type = exported(Choice(
+        title=_("Information type"), vocabulary=InformationType,
+        required=True, readonly=True, default=InformationType.PUBLIC,
+        description=_(
+            "The type of information contained in this Snap recipe.")))
+
     require_virtualized = exported(Bool(
         title=_("Require virtualized builders"), required=True, readonly=False,
         description=_("Only build this snap package on virtual builders.")))
@@ -850,7 +874,7 @@ class ISnapAdminAttributes(Interface):
 @exported_as_webservice_entry(as_of="beta")
 class ISnap(
     ISnapView, ISnapEdit, ISnapEditableAttributes, ISnapAdminAttributes,
-    IPrivacy):
+    IPrivacy, IInformationType):
     """A buildable snap package."""
 
 
@@ -876,7 +900,8 @@ class ISnapSet(Interface):
             auto_build_archive=None, auto_build_pocket=None,
             require_virtualized=True, processors=None, date_created=None,
             private=False, store_upload=False, store_series=None,
-            store_name=None, store_secrets=None, store_channels=None):
+            store_name=None, store_secrets=None, store_channels=None,
+            project=None):
         """Create an `ISnap`."""
 
     def exists(owner, name):
@@ -885,6 +910,10 @@ class ISnapSet(Interface):
     def isValidPrivacy(private, owner, branch=None, git_ref=None):
         """Whether or not the privacy context is valid."""
 
+    def isValidInformationType(
+            information_type, owner, branch=None, git_ref=None):
+        """Whether or not the information type context is valid."""
+
     @operation_parameters(
         owner=Reference(IPerson, title=_("Owner"), required=True),
         name=TextLine(title=_("Snap name"), required=True))
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 7e3d14c..b298a86 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -57,6 +57,10 @@ from lp.app.browser.tales import (
     ArchiveFormatterAPI,
     DateTimeFormatterAPI,
     )
+from lp.app.enums import (
+    InformationType,
+    PRIVATE_INFORMATION_TYPES,
+    )
 from lp.app.errors import IncompatibleArguments
 from lp.app.interfaces.security import IAuthorization
 from lp.buildmaster.enums import BuildStatus
@@ -298,6 +302,9 @@ class Snap(Storm, WebhookTargetMixin):
     owner_id = Int(name='owner', allow_none=False, validator=_validate_owner)
     owner = Reference(owner_id, 'Person.id')
 
+    project_id = Int(name='project', allow_none=True)
+    project = Reference(project_id, 'Product.id')
+
     distro_series_id = Int(name='distro_series', allow_none=True)
     distro_series = Reference(distro_series_id, 'DistroSeries.id')
 
@@ -352,6 +359,17 @@ class Snap(Storm, WebhookTargetMixin):
 
     private = Bool(name='private', validator=_validate_private)
 
+    def _valid_information_type(self, attr, value):
+        if not getUtility(ISnapSet).isValidInformationType(
+                value, self.owner, self.branch, self.git_ref):
+            raise SnapPrivacyMismatch
+        return value
+
+    _information_type = DBEnum(
+        enum=InformationType, default=InformationType.PUBLIC,
+        name="information_type",
+        validator=_valid_information_type)
+
     allow_internet = Bool(name='allow_internet', allow_none=False)
 
     build_source_tarball = Bool(name='build_source_tarball', allow_none=False)
@@ -374,13 +392,17 @@ class Snap(Storm, WebhookTargetMixin):
                  date_created=DEFAULT, private=False, allow_internet=True,
                  build_source_tarball=False, store_upload=False,
                  store_series=None, store_name=None, store_secrets=None,
-                 store_channels=None):
+                 store_channels=None, project=None):
         """Construct a `Snap`."""
         super(Snap, self).__init__()
 
         # Set the private flag first so that other validators can perform
-        # suitable privacy checks.
+        # suitable privacy checks, but pillar should also be set, since it's
+        # mandatory for private snaps.
+        self.project = project
         self.private = private
+        self.information_type = (InformationType.PROPRIETARY if private else
+                                 InformationType.PUBLIC)
 
         self.registrant = registrant
         self.owner = owner
@@ -408,6 +430,17 @@ class Snap(Storm, WebhookTargetMixin):
         return "<Snap ~%s/+snap/%s>" % (self.owner.name, self.name)
 
     @property
+    def information_type(self):
+        if self._information_type is None:
+            return (InformationType.PROPRIETARY if self.private
+                    else InformationType.PUBLIC)
+        return self._information_type
+
+    @information_type.setter
+    def information_type(self, information_type):
+        self._information_type = information_type
+
+    @property
     def valid_webhook_event_types(self):
         return ["snap:build:0.1"]
 
@@ -461,6 +494,21 @@ class Snap(Storm, WebhookTargetMixin):
             return None
 
     @property
+    def pillar(self):
+        """See `ISnap`."""
+        return self.project
+
+    @pillar.setter
+    def pillar(self, pillar):
+        if pillar is None:
+            self.project = None
+        elif IProduct.providedBy(pillar):
+            self.project = pillar
+        else:
+            raise ValueError(
+                'The pillar of a Snap must be an IProduct instance.')
+
+    @property
     def available_processors(self):
         """See `ISnap`."""
         clauses = [Processor.id == DistroArchSeries.processor_id]
@@ -1098,7 +1146,7 @@ class SnapSet:
             processors=None, date_created=DEFAULT, private=False,
             allow_internet=True, build_source_tarball=False,
             store_upload=False, store_series=None, store_name=None,
-            store_secrets=None, store_channels=None):
+            store_secrets=None, store_channels=None, project=None):
         """See `ISnapSet`."""
         if not registrant.inTeam(owner):
             if owner.is_team:
@@ -1150,7 +1198,7 @@ class SnapSet:
             build_source_tarball=build_source_tarball,
             store_upload=store_upload, store_series=store_series,
             store_name=store_name, store_secrets=store_secrets,
-            store_channels=store_channels)
+            store_channels=store_channels, project=project)
         store.add(snap)
 
         if processors is None:
@@ -1180,6 +1228,11 @@ class SnapSet:
 
         return True
 
+    def isValidInformationType(self, information_type, owner, branch=None,
+                               git_ref=None):
+        private = information_type in PRIVATE_INFORMATION_TYPES
+        return self.isValidPrivacy(private, owner, branch, git_ref)
+
     def _getByName(self, owner, name):
         return IStore(Snap).find(
             Snap, Snap.owner == owner, Snap.name == name).one()
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 706255a..9c2fda4 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap packages."""
@@ -131,6 +131,7 @@ from lp.testing import (
     ANONYMOUS,
     api_url,
     login,
+    login_admin,
     logout,
     person_logged_in,
     record_two_runs,
@@ -1416,11 +1417,24 @@ class TestSnapSet(TestCaseWithFactory):
         self.assertEqual(ref.path, snap.git_path)
         self.assertEqual(ref, snap.git_ref)
 
+    def test_private_snap_information_type_compatibility(self):
+        login_admin()
+        private_snap = getUtility(ISnapSet).new(
+            private=True, **self.makeSnapComponents())
+        self.assertEqual(
+            InformationType.PROPRIETARY, private_snap.information_type)
+
+        public_snap = getUtility(ISnapSet).new(
+            private=False, **self.makeSnapComponents())
+        self.assertEqual(
+            InformationType.PUBLIC, public_snap.information_type)
+
     def test_private_snap_for_public_sources(self):
         # Creating private snaps for public sources is allowed.
         [ref] = self.factory.makeGitRefs()
         components = self.makeSnapComponents(git_ref=ref)
         components['private'] = True
+        components['project'] = self.factory.makeProduct()
         snap = getUtility(ISnapSet).new(**components)
         with person_logged_in(components['owner']):
             self.assertTrue(snap.private)
@@ -2646,7 +2660,7 @@ class TestSnapWebservice(TestCaseWithFactory):
             snap_url, "application/json", json.dumps({"private": False}))
         self.assertEqual(400, response.status)
         self.assertEqual(
-            b"Snap contains private information and cannot be public.",
+            b"Snap recipe contains private information and cannot be public.",
             response.body)
 
     def test_cannot_set_private_components_of_public_snap(self):
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index c833ddf..9914cc8 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.
@@ -4754,7 +4754,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
                  date_created=DEFAULT, private=False, allow_internet=True,
                  build_source_tarball=False, store_upload=False,
                  store_series=None, store_name=None, store_secrets=None,
-                 store_channels=None):
+                 store_channels=None, project=_DEFAULT):
         """Make a new Snap."""
         if registrant is None:
             registrant = self.makePerson()
@@ -4772,6 +4772,12 @@ class BareLaunchpadObjectFactory(ObjectFactory):
                     distribution=distroseries.distribution, owner=owner)
             if auto_build_pocket is None:
                 auto_build_pocket = PackagePublishingPocket.UPDATES
+        if private and project is _DEFAULT:
+            # If we are creating a private snap and didn't explictly set a
+            # pillar for it, we must create a pillar.
+            project = self.makeProduct()
+        if project is _DEFAULT:
+            project = None
         snap = getUtility(ISnapSet).new(
             registrant, owner, distroseries, name,
             require_virtualized=require_virtualized, processors=processors,
@@ -4783,7 +4789,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             build_source_tarball=build_source_tarball,
             store_upload=store_upload, store_series=store_series,
             store_name=store_name, store_secrets=store_secrets,
-            store_channels=store_channels)
+            store_channels=store_channels, project=project)
         if is_stale is not None:
             removeSecurityProxy(snap).is_stale = is_stale
         IStore(snap).flush()