launchpad-reviewers team mailing list archive

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

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:oci-api-create-recipe into launchpad:master with ~pappacena/launchpad:oci-project-api as a prerequisite.

Commit message:
API operation to create a new OCIRecipe from an existing OCIProject.

The feature is only enabled if we turn on the 'oci.recipe.create.enabled' feature flag.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/381065

This MP includes code from https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/380909. So, it's important to hold merges until the previous one is accepted and merged.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:oci-api-create-recipe into launchpad:master.

diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py
index 46247fc..a8ad5e0 100644
--- a/lib/lp/_schema_circular_imports.py
+++ b/lib/lp/_schema_circular_imports.py
@@ -1097,6 +1097,7 @@ patch_entry_explicit_version(IWikiName, 'beta')
 
 # IOCIProject
 patch_collection_property(IOCIProject, 'series', IOCIProjectSeries)
+patch_entry_return_type(IOCIProject, 'newRecipe', IOCIRecipe)
 
 # IOCIRecipe
 patch_collection_property(IOCIRecipe, 'builds', IOCIRecipeBuild)
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index afc1d43..c68acf0 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -15,6 +15,7 @@ __all__ = [
     'IOCIRecipeView',
     'NoSourceForOCIRecipe',
     'NoSuchOCIRecipe',
+    'OCI_RECIPE_ALLOW_CREATE',
     'OCI_RECIPE_WEBHOOKS_FEATURE_FLAG',
     'OCIRecipeBuildAlreadyPending',
     'OCIRecipeNotOwner',
@@ -58,6 +59,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'
 
 
 @error_status(http_client.UNAUTHORIZED)
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 96cde0e..ac02de7 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -9,6 +9,7 @@ import json
 
 from fixtures import FakeLogger
 from storm.exceptions import LostObjectError
+from storm.store import Store
 from testtools.matchers import (
     ContainsDict,
     Equals,
@@ -27,11 +28,13 @@ from lp.oci.interfaces.ocirecipe import (
     IOCIRecipeSet,
     NoSourceForOCIRecipe,
     NoSuchOCIRecipe,
+    OCI_RECIPE_ALLOW_CREATE,
     OCI_RECIPE_WEBHOOKS_FEATURE_FLAG,
     OCIRecipeBuildAlreadyPending,
     OCIRecipeNotOwner,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
+from lp.oci.model.ocirecipe import OCIRecipe
 from lp.services.config import config
 from lp.services.database.constants import (
     ONE_DAY_AGO,
@@ -366,10 +369,12 @@ class TestOCIRecipeWebservice(TestCaseWithFactory):
 
     def setUp(self):
         super(TestOCIRecipeWebservice, self).setUp()
-        self.person = self.factory.makePerson(displayname="Test Person")
+        self.person = removeSecurityProxy(self.factory.makePerson(
+            displayname="Test Person"))
         self.webservice = webservice_for_person(
             self.person, permission=OAuthPermission.WRITE_PUBLIC,
             default_api_version="devel")
+        self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
 
     def load_from_api(self, url):
         response = self.webservice.get(url)
@@ -448,4 +453,99 @@ class TestOCIRecipeWebservice(TestCaseWithFactory):
 
         ws_project = self.load_from_api(url)
         self.assertEqual("old description", ws_project['description'])
+
+    def test_api_create_oci_recipe(self):
+        with person_logged_in(self.person):
+            distro = removeSecurityProxy(self.factory.makeDistribution(
+                owner=self.person))
+            project = removeSecurityProxy(self.factory.makeOCIProject(
+                pillar=distro, registrant=self.person))
+            git_ref = self.factory.makeGitRefs()[0]
+
+            project_url = api_url(project)
+            git_ref_url = api_url(git_ref)
+
+        url = '/{distribution}/+oci/{oci_project}/'
+        url = url.format(distribution=distro.name, oci_project=project.name)
+
+        obj = {
+            "name": "My recipe",
+            "oci_project": project_url,
+            "git_ref": git_ref_url,
+            "build_file": "./Dockerfile",
+            "description": "My recipe"}
+
+        resp = self.webservice.named_post(url, "newRecipe", **obj)
+        self.assertEqual(201, resp.status, resp.body)
+
+        result_set = Store.of(project).find(OCIRecipe)
+        self.assertEqual(1, result_set.count())
+
+        recipe = result_set[0]
+        self.assertThat(recipe, MatchesStructure(
+            name=Equals(obj["name"]),
+            oci_project=Equals(project),
+            git_ref=Equals(git_ref),
+            build_file=Equals(obj["build_file"]),
+            description=Equals(obj["description"]),
+            owner=Equals(self.person),
+            registrant=Equals(self.person),
+        ))
+
+    def test_api_create_oci_recipe_non_legitimate_user(self):
+        """Ensure that a non-legitimate user cannot create recipe using API"""
+        self.pushConfig(
+            'launchpad', min_legitimate_karma=9999,
+            min_legitimate_account_age=9999)
+
+        with person_logged_in(self.person):
+            distro = removeSecurityProxy(self.factory.makeDistribution(
+                owner=self.person))
+            project = removeSecurityProxy(self.factory.makeOCIProject(
+                pillar=distro, registrant=self.person))
+            git_ref = self.factory.makeGitRefs()[0]
+
+            project_url = api_url(project)
+            git_ref_url = api_url(git_ref)
+
+        url = '/{distribution}/+oci/{oci_project}/'
+        url = url.format(distribution=distro.name, oci_project=project.name)
+
+        obj = {
+            "name": "My recipe",
+            "oci_project": project_url,
+            "git_ref": git_ref_url,
+            "build_file": "./Dockerfile",
+            "description": "My recipe"}
+
+        resp = self.webservice.named_post(url, "newRecipe", **obj)
+        self.assertEqual(401, resp.status, resp.body)
+
+    def test_api_create_oci_recipe_is_disabled_by_feature_flag(self):
+        """Ensure that OCI newRecipe API method returns HTTP 401 when the
+        feature flag is not set."""
+        self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: ''}))
+
+        with person_logged_in(self.person):
+            distro = removeSecurityProxy(self.factory.makeDistribution(
+                owner=self.person))
+            project = removeSecurityProxy(self.factory.makeOCIProject(
+                pillar=distro, registrant=self.person))
+            git_ref = self.factory.makeGitRefs()[0]
+
+            project_url = api_url(project)
+            git_ref_url = api_url(git_ref)
+
+        url = '/{distribution}/+oci/{oci_project}/'
+        url = url.format(distribution=distro.name, oci_project=project.name)
+
+        obj = {
+            "name": "My recipe",
+            "oci_project": project_url,
+            "git_ref": git_ref_url,
+            "build_file": "./Dockerfile",
+            "description": "My recipe"}
+
+        resp = self.webservice.named_post(url, "newRecipe", **obj)
+        self.assertEqual(401, resp.status, resp.body)
 >>>>>>> lib/lp/oci/tests/test_ocirecipe.py
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index d5875c5..a67c822 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -745,6 +745,9 @@
             permission="launchpad.Edit"
             interface="lp.registry.interfaces.ociproject.IOCIProjectEdit"
             set_schema="lp.registry.interfaces.ociproject.IOCIProjectEditableAttributes" />
+        <require
+            permission="launchpad.AnyLegitimatePerson"
+            interface="lp.registry.interfaces.ociproject.IOCIProjectPublicActions"/>
     </class>
     <subscriber
         for="lp.registry.interfaces.ociproject.IOCIProject zope.lifecycleevent.interfaces.IObjectModifiedEvent"
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index 84561e3..203d6f2 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -12,28 +12,32 @@ __all__ = [
     ]
 
 from lazr.restful.declarations import (
+    call_with,
     export_as_webservice_entry,
+    export_factory_operation,
     exported,
+    operation_for_version,
+    operation_parameters,
+    REQUEST_USER,
     )
 from lazr.restful.fields import (
     CollectionField,
     Reference,
     ReferenceChoice,
     )
-from zope.interface import (
-    Attribute,
-    Interface,
-    )
+from zope.interface import Interface
 from zope.schema import (
     Datetime,
     Int,
     Text,
     TextLine,
     )
+from zope.schema._bootstrapfields import Bool
 
 from lp import _
 from lp.app.validators.name import name_validator
 from lp.bugs.interfaces.bugtarget import IBugTarget
+from lp.code.interfaces.gitref import IGitRef
 from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociprojectname import IOCIProjectName
@@ -99,8 +103,37 @@ class IOCIProjectEdit(Interface):
         """Creates a new `IOCIProjectSeries`."""
 
 
+class IOCIProjectPublicActions(Interface):
+    """IOCIProject methods that require launchpad.AnyLegitimatePerson
+    permission.
+    """
+    @call_with(registrant=REQUEST_USER, owner=REQUEST_USER)
+    @operation_parameters(
+        name=Text(
+            title=_("OCI Recipe name."),
+            description=_("The name of the new OCI Recipe.")),
+        git_ref=Reference(IGitRef, title=_("Git branch.")),
+        build_file=TextLine(
+            title=_("Build file path."),
+            description=_(
+                "The relative path to the file within this recipe's "
+                "branch that defines how to build the recipe.")),
+        description=Text(
+            title=_("Description for this recipe."),
+            description=_("A short description of this recipe.")),
+        official=Bool(
+            title=_("Is this the official recipe?"),
+            description=_("True if this recipe is official for its "
+                          "OCI project.")))
+    @export_factory_operation(Interface, [])
+    @operation_for_version("devel")
+    def newRecipe(name, registrant, owner, git_ref, build_file,
+                  description=None, official=False, require_virtualized=True):
+        """Create an IOCIRecipe for this project."""
+
+
 class IOCIProject(IOCIProjectView, IOCIProjectEdit,
-                       IOCIProjectEditableAttributes):
+                  IOCIProjectEditableAttributes, IOCIProjectPublicActions):
     """A project containing Open Container Initiative recipes."""
 
     export_as_webservice_entry(publish_web_link=True, as_of="devel")
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index c13ee66..916d206 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -1,4 +1,4 @@
-# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2019-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """OCI Project implementation."""
@@ -21,9 +21,14 @@ from storm.locals import (
     )
 from zope.component import getUtility
 from zope.interface import implementer
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from lp.bugs.model.bugtarget import BugTargetBase
+from lp.oci.interfaces.ocirecipe import (
+    IOCIRecipeSet,
+    OCI_RECIPE_ALLOW_CREATE,
+    )
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociproject import (
     IOCIProject,
@@ -42,6 +47,7 @@ from lp.services.database.interfaces import (
     IStore,
     )
 from lp.services.database.stormbase import StormBase
+from lp.services.features import getFeatureFlag
 
 
 def oci_project_modified(oci_project, event):
@@ -88,7 +94,7 @@ class OCIProject(BugTargetBase, StormBase):
         return self.ociprojectname.name
 
     @name.setter
-    def name(self, value):
+    def name_setter(self, value):
         self.ociprojectname = getUtility(IOCIProjectNameSet).getOrCreateByName(
             value)
 
@@ -106,6 +112,23 @@ class OCIProject(BugTargetBase, StormBase):
     bugtargetname = display_name
     bugtargetdisplayname = display_name
 
+    def newRecipe(self, name, registrant, owner, git_ref,
+                  build_file, description=None, official=False,
+                  require_virtualized=True):
+        if not getFeatureFlag(OCI_RECIPE_ALLOW_CREATE):
+            raise Unauthorized("Creating new recipes is not allowed.")
+        return getUtility(IOCIRecipeSet).new(
+            name=name,
+            registrant=registrant,
+            owner=owner,
+            oci_project=self,
+            git_ref=git_ref,
+            build_file=build_file,
+            description=description,
+            official=official,
+            require_virtualized=require_virtualized
+        )
+
     def newSeries(self, name, summary, registrant,
                   status=SeriesStatus.DEVELOPMENT, date_created=DEFAULT):
         """See `IOCIProject`."""