launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26176
[Merge] ~pappacena/launchpad:ocirecipe-private-flag-ui into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:ocirecipe-private-flag-ui into launchpad:master with ~pappacena/launchpad:ocirecipe-private-flag as a prerequisite.
Commit message:
UI to allow editing "private" attribute of OCI recipe
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/397310
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:ocirecipe-private-flag-ui into launchpad:master.
diff --git a/lib/lp/oci/browser/configure.zcml b/lib/lp/oci/browser/configure.zcml
index 6c254fb..c5697ba 100644
--- a/lib/lp/oci/browser/configure.zcml
+++ b/lib/lp/oci/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2020 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2020-2021 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -27,6 +27,12 @@
for="lp.oci.interfaces.ocirecipe.IOCIRecipe"
class="lp.oci.browser.ocirecipe.OCIRecipeView"
permission="launchpad.View"
+ name="+portlet-privacy"
+ template="../templates/ocirecipe-portlet-privacy.pt"/>
+ <browser:page
+ for="lp.oci.interfaces.ocirecipe.IOCIRecipe"
+ class="lp.oci.browser.ocirecipe.OCIRecipeView"
+ permission="launchpad.View"
name="+index"
template="../templates/ocirecipe-index.pt" />
<browser:page
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 4f4253a..4bac850 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -67,6 +67,7 @@ from lp.oci.interfaces.ocirecipe import (
IOCIRecipeSet,
NoSuchOCIRecipe,
OCI_RECIPE_ALLOW_CREATE,
+ OCI_RECIPE_PRIVATE_FEATURE_FLAG,
OCI_RECIPE_WEBHOOKS_FEATURE_FLAG,
OCIRecipeFeatureDisabled,
)
@@ -695,6 +696,7 @@ class IOCIRecipeEditSchema(Interface):
use_template(IOCIRecipe, include=[
"name",
+ "private",
"owner",
"description",
"git_ref",
@@ -880,13 +882,15 @@ class OCIRecipeAddView(LaunchpadFormView, EnableProcessorsMixin,
@action("Create OCI recipe", name="create")
def create_action(self, action, data):
+ private = not getUtility(IOCIRecipeSet).isValidPrivacy(
+ False, data['owner'], data["git_ref"])
recipe = getUtility(IOCIRecipeSet).new(
name=data["name"], registrant=self.user, owner=data["owner"],
oci_project=self.context, git_ref=data["git_ref"],
build_file=data["build_file"], description=data["description"],
build_daily=data["build_daily"], build_args=data["build_args"],
build_path=data["build_path"], processors=data["processors"],
- official=data.get('official_recipe', False))
+ official=data.get('official_recipe', False), private=private)
self.next_url = canonical_url(recipe)
@@ -920,6 +924,22 @@ class BaseOCIRecipeEditView(LaunchpadEditFormView):
"""See `LaunchpadFormView`."""
return {IOCIRecipeEditSchema: self.context}
+ def validate(self, data):
+ super(BaseOCIRecipeEditView, self).validate(data)
+ if data.get('private', self.context.private) is False:
+ if 'private' in data or 'owner' in data:
+ owner = data.get('owner', self.context.owner)
+ if owner is not None and owner.private:
+ self.setFieldError(
+ 'private' if 'private' in data else 'owner',
+ 'A public OCI recipe cannot have a private owner.')
+ if 'private' in data or 'git_ref' in data:
+ ref = data.get('git_ref', self.context.git_ref)
+ if ref is not None and ref.private:
+ self.setFieldError(
+ 'private' if 'private' in data else 'git_ref',
+ 'A public OCI recipe cannot use a private repository.')
+
class OCIRecipeAdminView(BaseOCIRecipeEditView):
"""View for administering OCI recipes."""
@@ -930,7 +950,17 @@ class OCIRecipeAdminView(BaseOCIRecipeEditView):
page_title = "Administer"
- field_names = ("require_virtualized", "allow_internet")
+ field_names = ("private", "require_virtualized", "allow_internet")
+
+ def validate(self, data):
+ super(OCIRecipeAdminView, self).validate(data)
+ # BaseSnapEditView.validate checks the rules for 'private' in
+ # combination with other attributes.
+ if (data.get('private', None) is True and
+ not getFeatureFlag(OCI_RECIPE_PRIVATE_FEATURE_FLAG)):
+ self.setFieldError(
+ 'private',
+ 'You do not have permission to create private OCI recipes.')
class OCIRecipeEditView(BaseOCIRecipeEditView, EnableProcessorsMixin,
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 970c49c..929cf9c 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -34,6 +34,7 @@ from zope.security.proxy import removeSecurityProxy
from zope.testbrowser.browser import LinkNotFoundError
from lp.app.browser.tales import GitRepositoryFormatterAPI
+from lp.app.enums import InformationType
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.processor import IProcessorSet
@@ -56,6 +57,7 @@ from lp.oci.interfaces.ociregistrycredentials import (
IOCIRegistryCredentialsSet,
)
from lp.oci.tests.helpers import OCIConfigHelperMixin
+from lp.registry.enums import PersonVisibility
from lp.registry.interfaces.person import IPersonSet
from lp.services.config import config
from lp.services.database.constants import UTC_NOW
@@ -88,6 +90,7 @@ from lp.testing.matchers import (
from lp.testing.pages import (
extract_text,
find_main_content,
+ find_tag_by_id,
find_tags_by_class,
)
from lp.testing.publication import test_traverse
@@ -151,17 +154,16 @@ class BaseTestOCIRecipeView(BrowserTestCase):
name="test-person", displayname="Test Person")
-class TestOCIRecipeAddView(BaseTestOCIRecipeView):
+class TestOCIRecipeAddView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
def setUp(self):
super(TestOCIRecipeAddView, self).setUp()
self.distroseries = self.factory.makeDistroSeries()
self.distribution = self.distroseries.distribution
- self.useFixture(FeatureFixture({
- OCI_RECIPE_ALLOW_CREATE: "on",
+ self.setConfig({
"oci.build_series.%s" % self.distribution.name:
self.distroseries.name,
- }))
+ })
def setUpDistroSeries(self):
"""Set up self.distroseries with some available processors."""
@@ -263,6 +265,32 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView):
"Build-time\nARG variables:\nVAR1=10\nVAR2=20",
MatchesTagText(content, "build-args"))
+ def test_create_new_ocirecipe_private(self):
+ # Private teams will automatically create private oci recipes.
+ login_person(self.person)
+ self.factory.makeTeam(
+ name='super-private', owner=self.person,
+ visibility=PersonVisibility.PRIVATE)
+
+ oci_project = self.factory.makeOCIProject()
+ [git_ref] = self.factory.makeGitRefs()
+ browser = self.getViewBrowser(
+ oci_project, view_name="+new-recipe", user=self.person)
+ browser.getControl(name="field.owner").value = ['super-private']
+ browser.getControl(name="field.name").value = "recipe-name"
+ browser.getControl("Description").value = "Recipe description"
+ browser.getControl(name="field.git_ref.repository").value = (
+ git_ref.repository.identity)
+ browser.getControl(name="field.git_ref.path").value = git_ref.path
+ browser.getControl("Create OCI recipe").click()
+
+ content = find_main_content(browser.contents)
+ self.assertEqual("recipe-name", extract_text(content.h1))
+ self.assertEqual(
+ 'This OCI recipe contains Private information',
+ extract_text(find_tag_by_id(browser.contents, "privacy"))
+ )
+
def test_create_new_recipe_users_teams_as_owner_options(self):
# Teams that the user is in are options for the OCI recipe owner.
self.factory.makeTeam(
@@ -484,11 +512,11 @@ class TestOCIRecipeAddView(BaseTestOCIRecipeView):
self.assertIn(error_message, browser.contents)
-class TestOCIRecipeAdminView(BaseTestOCIRecipeView):
+class TestOCIRecipeAdminView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
def setUp(self):
super(TestOCIRecipeAdminView, self).setUp()
- self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
+ self.setConfig()
def test_unauthorized(self):
# A non-admin user cannot administer an OCI recipe.
@@ -503,7 +531,7 @@ class TestOCIRecipeAdminView(BaseTestOCIRecipeView):
user=self.person)
def test_admin_recipe(self):
- # Admins can change require_virtualized.
+ # Admins can change require_virtualized and privacy.
login("admin@xxxxxxxxxxxxx")
commercial_admin = self.factory.makePerson(
member_of=[getUtility(ILaunchpadCelebrities).commercial_admin])
@@ -514,6 +542,7 @@ class TestOCIRecipeAdminView(BaseTestOCIRecipeView):
browser = self.getViewBrowser(recipe, user=commercial_admin)
browser.getLink("Administer OCI recipe").click()
+ browser.getControl("Private").selected = True
browser.getControl("Require virtualized builders").selected = False
browser.getControl("Allow external network access").selected = False
browser.getControl("Update OCI recipe").click()
@@ -521,6 +550,7 @@ class TestOCIRecipeAdminView(BaseTestOCIRecipeView):
login_person(self.person)
self.assertFalse(recipe.require_virtualized)
self.assertFalse(recipe.allow_internet)
+ self.assertTrue(recipe.private)
def test_admin_recipe_sets_date_last_modified(self):
# Administering an OCI recipe sets the date_last_modified property.
@@ -538,6 +568,40 @@ class TestOCIRecipeAdminView(BaseTestOCIRecipeView):
self.assertSqlAttributeEqualsDate(
recipe, "date_last_modified", UTC_NOW)
+ def test_admin_privacy_owner_mismatch(self):
+ # Cannot make recipe public if it is still owned by private person.
+ login_person(self.person)
+ team = self.factory.makeTeam(
+ owner=self.person, visibility=PersonVisibility.PRIVATE)
+ recipe = self.factory.makeOCIRecipe(
+ registrant=self.person, owner=team, private=True)
+ commercial_admin = self.factory.makePerson(
+ member_of=[getUtility(ILaunchpadCelebrities).commercial_admin])
+ browser = self.getViewBrowser(recipe, user=commercial_admin)
+ browser.getLink("Administer OCI recipe").click()
+ browser.getControl("Private").selected = False
+ browser.getControl("Update OCI recipe").click()
+ self.assertEqual(
+ 'A public OCI recipe cannot have a private owner.',
+ extract_text(find_tags_by_class(browser.contents, "message")[1]))
+
+ def test_admin_privacy_git_ref_mismatch(self):
+ # Cannot make recipe public if it still uses private git ref.
+ login_person(self.person)
+ [git_ref] = self.factory.makeGitRefs()
+ git_ref.repository.transitionToInformationType(
+ InformationType.PRIVATESECURITY, self.person)
+ recipe = self.factory.makeOCIRecipe(
+ registrant=self.person, owner=self.person,
+ git_ref=git_ref, private=True)
+ admin = self.factory.makeAdministrator()
+ browser = self.getViewBrowser(recipe, '+admin', user=admin)
+ browser.getControl("Private").selected = False
+ browser.getControl("Update OCI recipe").click()
+ self.assertEqual(
+ 'A public OCI recipe cannot use a private repository.',
+ extract_text(find_tags_by_class(browser.contents, "message")[1]))
+
class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
@@ -619,6 +683,42 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView):
"Build schedule:\nBuilt daily\nEdit OCI recipe\n",
MatchesTagText(content, "build-schedule"))
+ def test_edit_public_recipe_private_owner(self):
+ login_person(self.person)
+ recipe = self.factory.makeOCIRecipe(
+ registrant=self.person, owner=self.person)
+ private_team = self.factory.makeTeam(
+ owner=self.person, visibility=PersonVisibility.PRIVATE)
+ private_team_name = private_team.name
+ browser = self.getViewBrowser(recipe, user=self.person)
+ browser.getLink("Edit OCI recipe").click()
+ browser.getControl("Owner").value = [private_team_name]
+ browser.getControl("Update OCI recipe").click()
+ self.assertEqual(
+ "A public OCI recipe cannot have a private owner.",
+ extract_text(find_tags_by_class(browser.contents, "message")[1]))
+
+ def test_edit_public_recipe_private_git_ref(self):
+ login_person(self.person)
+ recipe = self.factory.makeOCIRecipe(
+ registrant=self.person, owner=self.person,
+ git_ref=self.factory.makeGitRefs()[0])
+ login_person(self.person)
+ [private_ref] = self.factory.makeGitRefs(
+ owner=self.person,
+ information_type=InformationType.PRIVATESECURITY)
+ private_ref_identity = private_ref.repository.identity
+ private_ref_path = private_ref.path
+ browser = self.getViewBrowser(recipe, user=self.person)
+ browser.getLink("Edit OCI recipe").click()
+ browser.getControl(name="field.git_ref.repository").value = (
+ private_ref_identity)
+ browser.getControl(name="field.git_ref.path").value = private_ref_path
+ browser.getControl("Update OCI recipe").click()
+ self.assertEqual(
+ "A public OCI recipe cannot use a private repository.",
+ extract_text(find_tags_by_class(browser.contents, "message")[1]))
+
def test_edit_recipe_invalid_branch(self):
oci_project = self.factory.makeOCIProject()
repository = self.factory.makeGitRepository()
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index cc67366..e9b30a7 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -211,8 +211,8 @@ class OCIRecipe(Storm, WebhookTargetMixin):
# callbacks.
self.private = private
self.registrant = registrant
- self.owner = owner
self.oci_project = oci_project
+ self.owner = owner
self.description = description
self.build_file = build_file
self._official = official
diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt
index a3e67bb..829181a 100644
--- a/lib/lp/oci/templates/ocirecipe-index.pt
+++ b/lib/lp/oci/templates/ocirecipe-index.pt
@@ -18,6 +18,7 @@
</metal:registering>
<metal:side fill-slot="side">
+ <div tal:replace="structure context/@@+portlet-privacy" />
<div tal:replace="structure context/@@+global-actions"/>
</metal:side>
diff --git a/lib/lp/oci/templates/ocirecipe-portlet-privacy.pt b/lib/lp/oci/templates/ocirecipe-portlet-privacy.pt
new file mode 100644
index 0000000..024b1fa
--- /dev/null
+++ b/lib/lp/oci/templates/ocirecipe-portlet-privacy.pt
@@ -0,0 +1,16 @@
+<div
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ id="privacy"
+ tal:attributes="
+ class python: 'portlet private' if path('context/private') else 'portlet public'
+ "
+>
+ <span tal:attributes="class python: path('context/private') and 'sprite private' or 'sprite public'"
+ >This OCI recipe contains <strong
+ tal:content="python: 'Private' if path('context/private') else 'Public'"
+ ></strong> information</span>
+</div>
+
+
Follow ups
