launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-no-push-rules-for-distribution-projects into launchpad:master.

Commit message:
Prevent push rules being created in a Distribution Credentials using context

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Existing push rules from before the distribution credentials were added will still exist, but be ignored.
We should prevent new ones from being added however, so error if that's attempted.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-no-push-rules-for-distribution-projects into launchpad:master.

diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 1cd81dd..3a9b4d7 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -24,6 +24,7 @@ __all__ = [
     'OCI_RECIPE_ALLOW_CREATE',
     'OCI_RECIPE_BUILD_DISTRIBUTION',
     'OCI_RECIPE_WEBHOOKS_FEATURE_FLAG',
+    'UsingDistributionCredentials',
     ]
 
 from lazr.lifecycle.snapshot import doNotSnapshot
@@ -123,6 +124,15 @@ class NoSuchOCIRecipe(NameLookupFailed):
 
 
 @error_status(http_client.BAD_REQUEST)
+class UsingDistributionCredentials(Exception):
+    """The OCI Recipe is in a Distribution that has credentials set."""
+
+    def __init__(self):
+        super(UsingDistributionCredentials, self).__init__(
+            "The OCI Recipe is in a Distribution that has credentials set.")
+
+
+@error_status(http_client.BAD_REQUEST)
 class NoSourceForOCIRecipe(Exception):
     """OCI Recipes must have a source and build file."""
 
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 3dcb917..adcbd6c 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -89,6 +89,7 @@ from lp.oci.interfaces.ocirecipe import (
     OCIRecipeFeatureDisabled,
     OCIRecipeNotOwner,
     OCIRecipePrivacyMismatch,
+    UsingDistributionCredentials,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
@@ -788,6 +789,8 @@ class OCIRecipe(Storm, WebhookTargetMixin):
             # credentials owner via the webservice API, so for compatibility
             # we give it a default.
             credentials_owner = self.owner
+        if self.use_distribution_credentials:
+            raise UsingDistributionCredentials()
         oci_credentials = getUtility(IOCIRegistryCredentialsSet).getOrCreate(
             registrant, credentials_owner, registry_url, credentials)
         push_rule = getUtility(IOCIPushRuleSet).new(
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index c3fa762..055d1f7 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -53,6 +53,7 @@ from lp.oci.interfaces.ocirecipe import (
     OCIRecipeBuildRequestStatus,
     OCIRecipeNotOwner,
     OCIRecipePrivacyMismatch,
+    UsingDistributionCredentials,
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
@@ -587,6 +588,29 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
                     recipe.registrant, url, image_name, credentials,
                     credentials_owner=other_team)
 
+    def test_newPushRule_distribution_credentials(self):
+        # If the OCIRecipe is in a Distribution with credentials set
+        # we cannot create new push rules
+        self.setConfig()
+        distribution = self.factory.makeDistribution()
+        credentials = self.factory.makeOCIRegistryCredentials()
+        project = self.factory.makeOCIProject(pillar=distribution)
+        recipe = self.factory.makeOCIRecipe(oci_project=project)
+        with person_logged_in(distribution.owner):
+            distribution.oci_registry_credentials = credentials
+            project.setOfficialRecipeStatus(recipe, True)
+
+        url = 'asdf://foo.com'
+        image_name = self.factory.getUniqueUnicode()
+        credentials = {
+            "username": "test-username", "password": "test-password"}
+
+        with person_logged_in(recipe.owner):
+            with ExpectedException(UsingDistributionCredentials):
+                recipe.newPushRule(
+                    recipe.registrant, url, image_name, credentials,
+                    credentials_owner=recipe.registrant)
+
     def test_set_official_directly_is_forbidden(self):
         recipe = self.factory.makeOCIRecipe()
         self.assertRaises(
@@ -1547,6 +1571,31 @@ class TestOCIRecipeWebservice(OCIConfigHelperMixin, TestCaseWithFactory):
             "username": Equals("foo"),
             }))
 
+    def test_api_create_new_push_rule_distribution_credentials(self):
+        """Should not be able to create a push rule in a Distribution."""
+
+        self.setConfig()
+
+        with person_logged_in(self.person):
+            distribution = self.factory.makeDistribution()
+            credentials = self.factory.makeOCIRegistryCredentials()
+            project = self.factory.makeOCIProject(
+                pillar=distribution, registrant=self.person)
+            recipe = self.factory.makeOCIRecipe(
+                oci_project=project, owner=self.person, registrant=self.person)
+            with person_logged_in(distribution.owner):
+                distribution.oci_registry_credentials = credentials
+                project.setOfficialRecipeStatus(recipe, True)
+            url = api_url(recipe)
+
+        obj = {
+            "registry_url": self.factory.getUniqueURL(),
+            "image_name": self.factory.getUniqueUnicode(),
+            "credentials": {"username": "foo", "password": "bar"}}
+
+        resp = self.webservice.named_post(url, "newPushRule", **obj)
+        self.assertEqual(400, resp.status, resp.body)
+
     def test_api_push_rules_exported(self):
         """Are push rules exported for a recipe?"""
         self.setConfig()