launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24607
[Merge] ~twom/launchpad:oci-registry-credentials-api into launchpad:master
Tom Wardill has proposed merging ~twom/launchpad:oci-registry-credentials-api into launchpad:master.
Commit message:
Add API for OCI Registry Credentials
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/382651
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-registry-credentials-api into launchpad:master.
diff --git a/lib/lp/oci/browser/configure.zcml b/lib/lp/oci/browser/configure.zcml
index 3fbb0f1..1a3843b 100644
--- a/lib/lp/oci/browser/configure.zcml
+++ b/lib/lp/oci/browser/configure.zcml
@@ -101,5 +101,10 @@
for="lp.oci.interfaces.ocirecipebuild.IOCIRecipeBuild"
factory="lp.services.webapp.breadcrumb.TitleBreadcrumb"
permission="zope.Public" />
+
+ <browser:url
+ for="lp.oci.interfaces.ocipushrule.IOCIPushRule"
+ path_expression="string:+pushrule/${id}"
+ attribute_to_parent="recipe" />
</facet>
</configure>
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 42a32a7..a0ee61d 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -39,6 +39,7 @@ from lp.app.browser.tales import format_link
from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget
from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet
from lp.oci.interfaces.ocirecipe import (
IOCIRecipe,
IOCIRecipeSet,
@@ -80,6 +81,11 @@ class OCIRecipeNavigation(WebhookTargetNavigationMixin, Navigation):
return None
return build
+ @stepthrough('+pushrule')
+ def traverse_pushrule(self, id):
+ id = int(id)
+ return getUtility(IOCIPushRuleSet).getByID(id)
+
class OCIRecipeBreadcrumb(NameBreadcrumb):
diff --git a/lib/lp/oci/interfaces/ocipushrule.py b/lib/lp/oci/interfaces/ocipushrule.py
index 2351edf..0eaa24b 100644
--- a/lib/lp/oci/interfaces/ocipushrule.py
+++ b/lib/lp/oci/interfaces/ocipushrule.py
@@ -8,10 +8,17 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
__all__ = [
'IOCIPushRule',
- 'IOCIPushRuleSet'
+ 'IOCIPushRuleSet',
+ 'OCIPushRuleAlreadyExists',
]
+from lazr.restful.declarations import (
+ error_status,
+ export_as_webservice_entry,
+ exported,
+ )
from lazr.restful.fields import Reference
+from six.moves import http_client
from zope.interface import Interface
from zope.schema import (
Int,
@@ -23,6 +30,18 @@ from lp.oci.interfaces.ocirecipe import IOCIRecipe
from lp.oci.interfaces.ociregistrycredentials import IOCIRegistryCredentials
+@error_status(http_client.BAD_REQUEST)
+class OCIPushRuleAlreadyExists(Exception):
+ """A new OCIPushRuleAlreadyExists was added with the
+ same details as an existing one.
+ """
+
+ def __init__(self):
+ super(OCIPushRuleAlreadyExists, self).__init__(
+ "A push rule already exists with the same image_name "
+ "and credentials")
+
+
class IOCIPushRuleView(Interface):
"""`IOCIPushRule` methods that require launchpad.View
permission.
@@ -51,11 +70,11 @@ class IOCIPushRuleEditableAttributes(Interface):
required=True,
readonly=False)
- image_name = TextLine(
+ image_name = exported(TextLine(
title=_("Image name"),
description=_("The intended name of the image on the registry."),
required=True,
- readonly=False)
+ readonly=False))
class IOCIPushRuleEdit(Interface):
@@ -71,9 +90,15 @@ class IOCIPushRule(IOCIPushRuleEdit, IOCIPushRuleEditableAttributes,
IOCIPushRuleView):
"""A rule for pushing builds of an OCI recipe to a registry."""
+ export_as_webservice_entry(
+ publish_web_link=True, as_of="devel", singular_name="oci_push_rule")
+
class IOCIPushRuleSet(Interface):
"""A utility to create and access OCI Push Rules."""
def new(recipe, registry_credentials, image_name):
"""Create an `IOCIPushRule`."""
+
+ def getByID(id):
+ """Get a single `IOCIPushRule` by it's ID."""
diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index eda2782..2593b60 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -27,6 +27,7 @@ from lazr.restful.declarations import (
call_with,
error_status,
export_as_webservice_entry,
+ export_factory_operation,
export_write_operation,
exported,
operation_for_version,
@@ -46,6 +47,7 @@ from zope.interface import (
from zope.schema import (
Bool,
Datetime,
+ Dict,
Int,
List,
Text,
@@ -227,6 +229,27 @@ class IOCIRecipeEdit(IWebhookTarget):
def destroySelf():
"""Delete this OCI recipe, provided that it has no builds."""
+ @call_with(owner=REQUEST_USER)
+ @operation_parameters(
+ url=TextLine(
+ title=_("OCI Registry URL"),
+ description=_("URL for the target OCI Registry"),
+ required=True),
+ image_name=TextLine(
+ title=_("Image name"),
+ description=_("Name of the image to push to on the OCI Registry"),
+ required=True),
+ credentials=Dict(
+ title=_("Registry credentials"),
+ description=_(
+ "The credentials to use in pushing the image to the registry"),
+ required=True))
+ # Really IOCIPushRule, patched in lp.oci.interfaces.webservice.
+ @export_factory_operation(Interface, [])
+ @operation_for_version("devel")
+ def createPushRule(owner, url, image_name, credentials):
+ """Create a new `OCIPushRule` and `OCICredentials` for this recipe."""
+
class IOCIRecipeEditableAttributes(IHasOwner):
"""`IOCIRecipe` attributes that can be edited.
diff --git a/lib/lp/oci/interfaces/ociregistrycredentials.py b/lib/lp/oci/interfaces/ociregistrycredentials.py
index 0357d9e..7142b70 100644
--- a/lib/lp/oci/interfaces/ociregistrycredentials.py
+++ b/lib/lp/oci/interfaces/ociregistrycredentials.py
@@ -9,8 +9,11 @@ __metaclass__ = type
__all__ = [
'IOCIRegistryCredentials',
'IOCIRegistryCredentialsSet',
+ 'OCIRegistryCredentialsAlreadyExist',
]
+from lazr.restful.declarations import error_status
+from six.moves import http_client
from zope.interface import Interface
from zope.schema import (
Int,
@@ -22,6 +25,17 @@ from lp.registry.interfaces.role import IHasOwner
from lp.services.fields import PersonChoice
+@error_status(http_client.BAD_REQUEST)
+class OCIRegistryCredentialsAlreadyExist(Exception):
+ """A new `OCIRegistryCredentials` was added with the
+ same details as an existing one.
+ """
+
+ def __init__(self):
+ super(OCIRegistryCredentialsAlreadyExist, self).__init__(
+ "Credentials already exist with the same URL and username.")
+
+
class IOCIRegistryCredentialsView(Interface):
id = Int(title=_("ID"), required=True, readonly=True)
@@ -72,5 +86,9 @@ class IOCIRegistryCredentialsSet(Interface):
def new(owner, url, credentials):
"""Create an `IOCIRegistryCredentials`."""
+ def getOrCreate(owner, url, credentials):
+ """Get an `IOCIRegistryCredentials` that match the url and username
+ or create a new object."""
+
def findByOwner(owner):
"""Find matching `IOCIRegistryCredentials` by owner."""
diff --git a/lib/lp/oci/interfaces/webservice.py b/lib/lp/oci/interfaces/webservice.py
index 0a6933b..0edbb30 100644
--- a/lib/lp/oci/interfaces/webservice.py
+++ b/lib/lp/oci/interfaces/webservice.py
@@ -7,8 +7,17 @@ __all__ = [
'IOCIProject',
'IOCIProjectSeries',
'IOCIRecipe',
+ 'IOCIPushRule',
]
-from lp.oci.interfaces.ocirecipe import IOCIRecipe
+from lp.oci.interfaces.ocirecipe import (
+ IOCIRecipe,
+ IOCIRecipeEdit,
+ )
from lp.registry.interfaces.ociproject import IOCIProject
from lp.registry.interfaces.ociprojectseries import IOCIProjectSeries
+from lp.oci.interfaces.ocipushrule import IOCIPushRule
+from lp.services.webservice.apihelpers import patch_entry_return_type
+
+
+patch_entry_return_type(IOCIRecipeEdit, 'createPushRule', IOCIPushRule)
diff --git a/lib/lp/oci/model/ocipushrule.py b/lib/lp/oci/model/ocipushrule.py
index 84e4a1c..7033a1d 100644
--- a/lib/lp/oci/model/ocipushrule.py
+++ b/lib/lp/oci/model/ocipushrule.py
@@ -22,6 +22,7 @@ from zope.interface import implementer
from lp.oci.interfaces.ocipushrule import (
IOCIPushRule,
IOCIPushRuleSet,
+ OCIPushRuleAlreadyExists,
)
from lp.services.database.interfaces import IStore
@@ -58,4 +59,16 @@ class OCIPushRuleSet:
def new(self, recipe, registry_credentials, image_name):
"""See `IOCIPushRuleSet`."""
+ for existing in recipe.push_rules:
+ credentials_match = (
+ existing.registry_credentials == registry_credentials)
+ image_match = (existing.image_name == image_name)
+ if credentials_match and image_match:
+ raise OCIPushRuleAlreadyExists()
return OCIPushRule(recipe, registry_credentials, image_name)
+
+ def getByID(self, id):
+ """See `IOCIPushRuleSet`."""
+ return IStore(OCIPushRule).find(
+ OCIPushRule,
+ OCIPushRule.id == id).one()
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index aa69900..1bca554 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -42,6 +42,7 @@ from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.buildmaster.model.buildqueue import BuildQueue
from lp.buildmaster.model.processor import Processor
+from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet
from lp.oci.interfaces.ocirecipe import (
CannotModifyOCIRecipeProcessor,
DuplicateOCIRecipeName,
@@ -55,6 +56,7 @@ from lp.oci.interfaces.ocirecipe import (
OCIRecipeNotOwner,
)
from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
+from lp.oci.interfaces.ociregistrycredentials import IOCIRegistryCredentialsSet
from lp.oci.model.ocipushrule import OCIPushRule
from lp.oci.model.ocirecipebuild import OCIRecipeBuild
from lp.registry.interfaces.person import IPersonSet
@@ -381,6 +383,16 @@ class OCIRecipe(Storm, WebhookTargetMixin):
def can_upload_to_registry(self):
return not self.push_rules.is_empty()
+ def createPushRule(self, owner, url, image_name, credentials):
+ """See `IOCIRecipe`."""
+
+ oci_credentials = getUtility(IOCIRegistryCredentialsSet).getOrCreate(
+ owner, url, credentials)
+ push_rule = getUtility(IOCIPushRuleSet).new(
+ self, oci_credentials, image_name)
+ Store.of(push_rule).flush()
+ return push_rule
+
class OCIRecipeArch(Storm):
"""Link table to back `OCIRecipe.processors`."""
diff --git a/lib/lp/oci/model/ociregistrycredentials.py b/lib/lp/oci/model/ociregistrycredentials.py
index 03b0f72..a477b31 100644
--- a/lib/lp/oci/model/ociregistrycredentials.py
+++ b/lib/lp/oci/model/ociregistrycredentials.py
@@ -28,6 +28,7 @@ from zope.security.proxy import removeSecurityProxy
from lp.oci.interfaces.ociregistrycredentials import (
IOCIRegistryCredentials,
IOCIRegistryCredentialsSet,
+ OCIRegistryCredentialsAlreadyExist,
)
from lp.services.config import config
from lp.services.crypto.interfaces import (
@@ -106,8 +107,26 @@ class OCIRegistryCredentialsSet:
def new(self, owner, url, credentials):
"""See `IOCIRegistryCredentialsSet`."""
+ for existing in self.findByOwner(owner):
+ # If the urls are different, we can ignore these
+ if existing.url != url:
+ continue
+ username = existing.getCredentials().get('username')
+ if username == credentials.get('username'):
+ raise OCIRegistryCredentialsAlreadyExist
return OCIRegistryCredentials(owner, url, credentials)
+ def getOrCreate(self, owner, url, credentials):
+ """See `IOCIRegistryCredentialsSet`."""
+ for existing in self.findByOwner(owner):
+ # If the urls are different, we can ignore these
+ if existing.url != url:
+ continue
+ username = existing.getCredentials().get('username')
+ if username == credentials.get('username'):
+ return existing
+ return self.new(owner, url, credentials)
+
def findByOwner(self, owner):
"""See `IOCIRegistryCredentialsSet`."""
store = IStore(OCIRegistryCredentials)
diff --git a/lib/lp/oci/tests/test_ocipushrule.py b/lib/lp/oci/tests/test_ocipushrule.py
index 90ddd96..ccf3a2e 100644
--- a/lib/lp/oci/tests/test_ocipushrule.py
+++ b/lib/lp/oci/tests/test_ocipushrule.py
@@ -11,6 +11,7 @@ from zope.component import getUtility
from lp.oci.interfaces.ocipushrule import (
IOCIPushRule,
IOCIPushRuleSet,
+ OCIPushRuleAlreadyExists,
)
from lp.oci.tests.helpers import OCIConfigHelperMixin
from lp.testing import (
@@ -68,3 +69,17 @@ class TestOCIPushRuleSet(OCIConfigHelperMixin, TestCaseWithFactory):
recipe=recipe,
registry_credentials=registry_credentials,
image_name=image_name))
+
+ def test_new_with_existing(self):
+ recipe = self.factory.makeOCIRecipe()
+ registry_credentials = self.factory.makeOCIRegistryCredentials()
+ image_name = self.factory.getUniqueUnicode()
+ getUtility(IOCIPushRuleSet).new(
+ recipe=recipe,
+ registry_credentials=registry_credentials,
+ image_name=image_name)
+
+ self.assertRaises(
+ OCIPushRuleAlreadyExists,
+ getUtility(IOCIPushRuleSet).new,
+ recipe, registry_credentials, image_name)
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 06ed382..1a272b0 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -5,12 +5,13 @@
from __future__ import absolute_import, print_function, unicode_literals
-import base64
import json
from fixtures import FakeLogger
-from nacl.public import PrivateKey
-from six import string_types
+from six import (
+ ensure_text,
+ string_types,
+ )
from storm.exceptions import LostObjectError
from testtools.matchers import (
ContainsDict,
@@ -24,6 +25,7 @@ from zope.security.proxy import removeSecurityProxy
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.processor import IProcessorSet
+from lp.oci.interfaces.ocipushrule import OCIPushRuleAlreadyExists
from lp.oci.interfaces.ocirecipe import (
CannotModifyOCIRecipeProcessor,
DuplicateOCIRecipeName,
@@ -37,6 +39,10 @@ from lp.oci.interfaces.ocirecipe import (
OCIRecipeNotOwner,
)
from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
+from lp.oci.interfaces.ociregistrycredentials import (
+ OCIRegistryCredentialsAlreadyExist,
+ )
+from lp.oci.tests.helpers import OCIConfigHelperMixin
from lp.services.config import config
from lp.services.database.constants import (
ONE_DAY_AGO,
@@ -59,7 +65,7 @@ from lp.testing.layers import DatabaseFunctionalLayer
from lp.testing.pages import webservice_for_person
-class TestOCIRecipe(TestCaseWithFactory):
+class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -205,10 +211,7 @@ class TestOCIRecipe(TestCaseWithFactory):
self.assertEqual([], list(oci_recipe.pending_builds))
def test_push_rules(self):
- self.pushConfig(
- "oci",
- registry_secrets_public_key=base64.b64encode(
- bytes(PrivateKey.generate().public_key)).decode("UTF-8"))
+ self.setConfig()
oci_recipe = self.factory.makeOCIRecipe()
for _ in range(3):
self.factory.makeOCIPushRule(recipe=oci_recipe)
@@ -219,6 +222,47 @@ class TestOCIRecipe(TestCaseWithFactory):
for rule in oci_recipe.push_rules:
self.assertEqual(rule.recipe, oci_recipe)
+ def test_createPushRule(self):
+ self.setConfig()
+ recipe = self.factory.makeOCIRecipe()
+ url = ensure_text(self.factory.getUniqueURL())
+ image_name = ensure_text(self.factory.getUniqueString())
+ credentials = {
+ "username": "test-username", "password": "test-password"}
+
+ with person_logged_in(recipe.owner):
+ push_rule = recipe.createPushRule(
+ recipe.owner, url, image_name, credentials)
+ self.assertEqual(
+ image_name,
+ push_rule.image_name)
+ self.assertEqual(
+ url,
+ push_rule.registry_credentials.url)
+ self.assertEqual(
+ credentials,
+ push_rule.registry_credentials.getCredentials())
+
+ self.assertEqual(
+ push_rule,
+ recipe.push_rules[0])
+
+ def test_createPushRule_same_details(self):
+ self.setConfig()
+ recipe = self.factory.makeOCIRecipe()
+ url = ensure_text(self.factory.getUniqueURL())
+ image_name = ensure_text(self.factory.getUniqueString())
+ credentials = {
+ "username": "test-username", "password": "test-password"}
+
+ with person_logged_in(recipe.owner):
+ recipe.createPushRule(
+ recipe.owner, url, image_name, credentials)
+ self.assertRaises(
+ OCIPushRuleAlreadyExists,
+ recipe.createPushRule,
+ recipe.owner, url, image_name, credentials)
+
class TestOCIRecipeProcessors(TestCaseWithFactory):
@@ -512,7 +556,7 @@ class TestOCIRecipeSet(TestCaseWithFactory):
oci_recipe, "date_last_modified", UTC_NOW)
-class TestOCIRecipeWebservice(TestCaseWithFactory):
+class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
layer = DatabaseFunctionalLayer
def setUp(self):
@@ -694,3 +738,28 @@ class TestOCIRecipeWebservice(TestCaseWithFactory):
resp = self.webservice.named_post(oci_project_url, "newRecipe", **obj)
self.assertEqual(401, resp.status, resp.body)
+
+ def test_api_create_new_push_rule(self):
+ """Can you create a new push rule for a recipe via the API."""
+
+ self.setConfig()
+
+ with person_logged_in(self.person):
+ oci_project = self.factory.makeOCIProject(
+ registrant=self.person)
+ recipe = self.factory.makeOCIRecipe(
+ oci_project=oci_project, owner=self.person,
+ registrant=self.person)
+ url = api_url(recipe)
+
+ obj = {
+ "url": self.factory.getUniqueURL(),
+ "image_name": self.factory.getUniqueUnicode(),
+ "credentials": {"username": "foo", "password": "bar"}}
+
+ resp = self.webservice.named_post(url, "createPushRule", **obj)
+ self.assertEqual(201, resp.status, resp.body)
+
+ new_obj_url = resp.getHeader("Location")
+ ws_push_rule = self.load_from_api(new_obj_url)
+ self.assertEqual(obj["image_name"], ws_push_rule["image_name"])
diff --git a/lib/lp/oci/tests/test_ociregistrycredentials.py b/lib/lp/oci/tests/test_ociregistrycredentials.py
index 50953da..a4852ad 100644
--- a/lib/lp/oci/tests/test_ociregistrycredentials.py
+++ b/lib/lp/oci/tests/test_ociregistrycredentials.py
@@ -19,6 +19,7 @@ from zope.security.proxy import removeSecurityProxy
from lp.oci.interfaces.ociregistrycredentials import (
IOCIRegistryCredentials,
IOCIRegistryCredentialsSet,
+ OCIRegistryCredentialsAlreadyExist,
)
from lp.oci.tests.helpers import OCIConfigHelperMixin
from lp.services.crypto.interfaces import IEncryptedContainer
@@ -93,6 +94,48 @@ class TestOCIRegistryCredentialsSet(OCIConfigHelperMixin, TestCaseWithFactory):
self.assertEqual(oci_credentials.url, url)
self.assertEqual(oci_credentials.getCredentials(), credentials)
+ def test_new_with_existing(self):
+ owner = self.factory.makePerson()
+ url = unicode(self.factory.getUniqueURL())
+ credentials = {'username': 'foo', 'password': 'bar'}
+ getUtility(IOCIRegistryCredentialsSet).new(
+ owner=owner,
+ url=url,
+ credentials=credentials)
+ self.assertRaises(
+ OCIRegistryCredentialsAlreadyExist,
+ getUtility(IOCIRegistryCredentialsSet).new,
+ owner, url, credentials)
+
+ def test_getOrCreate_existing(self):
+ owner = self.factory.makePerson()
+ url = unicode(self.factory.getUniqueURL())
+ credentials = {'username': 'foo', 'password': 'bar'}
+ new = getUtility(IOCIRegistryCredentialsSet).new(
+ owner=owner,
+ url=url,
+ credentials=credentials)
+
+ existing = getUtility(IOCIRegistryCredentialsSet).getOrCreate(
+ owner=owner,
+ url=url,
+ credentials=credentials)
+
+ self.assertEqual(new.id, existing.id)
+
+ def test_getOrCreate_new(self):
+ owner = self.factory.makePerson()
+ url = unicode(self.factory.getUniqueURL())
+ credentials = {'username': 'foo', 'password': 'bar'}
+ new = getUtility(IOCIRegistryCredentialsSet).getOrCreate(
+ owner=owner,
+ url=url,
+ credentials=credentials)
+
+ self.assertEqual(new.owner, owner)
+ self.assertEqual(new.url, url)
+ self.assertEqual(new.getCredentials(), credentials)
+
def test_findByOwner(self):
owner = self.factory.makePerson()
for _ in range(3):
