Merge ~cjwatson/launchpad:oci-recipe-push-rules-add into launchpad:master
- Git
- lp:~cjwatson/launchpad
- oci-recipe-push-rules-add
- Merge into master
Proposed by
Colin Watson
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | f2ce0ac900cac32a41f01a37ec77242c3d2bb45e |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~cjwatson/launchpad:oci-recipe-push-rules-add |
Merge into: | launchpad:master |
Diff against target: |
960 lines (+696/-56) 6 files modified
lib/lp/oci/browser/ocirecipe.py (+203/-32) lib/lp/oci/browser/tests/test_ocirecipe.py (+267/-5) lib/lp/oci/javascript/ocirecipe.edit.js (+38/-0) lib/lp/oci/templates/ocirecipe-edit-push-rules.pt (+126/-19) lib/lp/oci/vocabularies.py (+52/-0) lib/lp/oci/vocabularies.zcml (+10/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Thiago F. Pappacena (community) | Approve | ||
Review via email: mp+389875@code.launchpad.net |
Commit message
Allow adding push rules on OCIRecipe:
Description of the change
This is https:/
Screenshot: https:/
To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py |
2 | index 3dd764c..c2dceb2 100644 |
3 | --- a/lib/lp/oci/browser/ocirecipe.py |
4 | +++ b/lib/lp/oci/browser/ocirecipe.py |
5 | @@ -25,12 +25,18 @@ from lazr.restful.interface import ( |
6 | ) |
7 | from zope.component import getUtility |
8 | from zope.formlib.form import FormFields |
9 | +from zope.formlib.widget import ( |
10 | + DisplayWidget, |
11 | + renderElement, |
12 | + ) |
13 | from zope.interface import Interface |
14 | from zope.schema import ( |
15 | Bool, |
16 | Choice, |
17 | List, |
18 | + Password, |
19 | TextLine, |
20 | + ValidationError, |
21 | ) |
22 | |
23 | from lp.app.browser.launchpadform import ( |
24 | @@ -44,7 +50,10 @@ from lp.app.errors import UnexpectedFormData |
25 | from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget |
26 | from lp.buildmaster.interfaces.processor import IProcessorSet |
27 | from lp.code.browser.widgets.gitref import GitRefWidget |
28 | -from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet |
29 | +from lp.oci.interfaces.ocipushrule import ( |
30 | + IOCIPushRuleSet, |
31 | + OCIPushRuleAlreadyExists, |
32 | + ) |
33 | from lp.oci.interfaces.ocirecipe import ( |
34 | IOCIRecipe, |
35 | IOCIRecipeSet, |
36 | @@ -56,6 +65,8 @@ from lp.oci.interfaces.ocirecipe import ( |
37 | ) |
38 | from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet |
39 | from lp.oci.interfaces.ociregistrycredentials import ( |
40 | + IOCIRegistryCredentialsSet, |
41 | + OCIRegistryCredentialsAlreadyExist, |
42 | user_can_edit_credentials_for_owner, |
43 | ) |
44 | from lp.services.features import getFeatureFlag |
45 | @@ -72,6 +83,7 @@ from lp.services.webapp import ( |
46 | stepthrough, |
47 | structured, |
48 | ) |
49 | +from lp.services.webapp.authorization import check_permission |
50 | from lp.services.webapp.batching import BatchNavigator |
51 | from lp.services.webapp.breadcrumb import NameBreadcrumb |
52 | from lp.services.webhooks.browser import WebhookTargetNavigationMixin |
53 | @@ -264,7 +276,17 @@ def new_builds_notification_text(builds, already_pending=None): |
54 | return builds_text |
55 | |
56 | |
57 | -class OCIRecipeEditPushRulesView(LaunchpadEditFormView): |
58 | +class InvisibleCredentialsWidget(DisplayWidget): |
59 | + """A widget that just displays a private icon. |
60 | + |
61 | + This indicates invisible credentials. |
62 | + """ |
63 | + |
64 | + def __call__(self): |
65 | + return renderElement("span", id=self.name, cssClass="sprite private") |
66 | + |
67 | + |
68 | +class OCIRecipeEditPushRulesView(LaunchpadFormView): |
69 | """View for +ocirecipe-edit-push-rules.pt.""" |
70 | |
71 | class schema(Interface): |
72 | @@ -311,41 +333,103 @@ class OCIRecipeEditPushRulesView(LaunchpadEditFormView): |
73 | return field_type, rule_id |
74 | |
75 | def setUpFields(self): |
76 | - """See `LaunchpadEditFormView`.""" |
77 | - LaunchpadEditFormView.setUpFields(self) |
78 | - |
79 | - image_fields = [] |
80 | + super(OCIRecipeEditPushRulesView, self).setUpFields() |
81 | + image_name_fields = [] |
82 | + url_fields = [] |
83 | + private_url_fields = [] |
84 | + username_fields = [] |
85 | + private_username_fields = [] |
86 | + password_fields = [] |
87 | delete_fields = [] |
88 | - creds = [] |
89 | for elem in list(self.context.push_rules): |
90 | - image_fields.append( |
91 | + image_name_fields.append( |
92 | TextLine( |
93 | - title=u'Image name', |
94 | __name__=self._getFieldName('image_name', elem.id), |
95 | default=elem.image_name, |
96 | required=True, readonly=False)) |
97 | + if check_permission('launchpad.View', elem.registry_credentials): |
98 | + url_fields.append( |
99 | + TextLine( |
100 | + __name__=self._getFieldName('url', elem.id), |
101 | + default=elem.registry_credentials.url, |
102 | + required=True, readonly=True)) |
103 | + username_fields.append( |
104 | + TextLine( |
105 | + __name__=self._getFieldName('username', elem.id), |
106 | + default=elem.registry_credentials.username, |
107 | + required=True, readonly=True)) |
108 | + else: |
109 | + # XXX cjwatson 2020-08-27: Ideally we'd be able to just show |
110 | + # the URL, and maybe the username too, but the |
111 | + # launchpad.View security adapter for OCIRegistryCredentials |
112 | + # doesn't currently allow that in some cases (e.g. a |
113 | + # team-owned recipe with a push rule using credentials owned |
114 | + # by another team member). In future it might make sense to |
115 | + # add a launchpad.LimitedView adapter that grants access if |
116 | + # the credentials are used by a push rule on one of the |
117 | + # viewer's recipes. |
118 | + private_url_fields.append( |
119 | + TextLine( |
120 | + __name__=self._getFieldName('url', elem.id), |
121 | + default='', required=True, readonly=True)) |
122 | + private_username_fields.append( |
123 | + TextLine( |
124 | + __name__=self._getFieldName('username', elem.id), |
125 | + default='', required=True, readonly=True)) |
126 | delete_fields.append( |
127 | Bool( |
128 | - title=u'Delete', |
129 | __name__=self._getFieldName('delete', elem.id), |
130 | default=False, |
131 | required=True, readonly=False)) |
132 | - creds.append( |
133 | - TextLine( |
134 | - title=u'Username', |
135 | - __name__=self._getFieldName('username', elem.id), |
136 | - default=elem.registry_credentials.username, |
137 | - required=True, readonly=True)) |
138 | - creds.append( |
139 | - TextLine( |
140 | - title=u'Registry URL', |
141 | - __name__=self._getFieldName('url', elem.id), |
142 | - default=elem.registry_credentials.url, |
143 | - required=True, readonly=True)) |
144 | - |
145 | - self.form_fields += FormFields(*image_fields) |
146 | - self.form_fields += FormFields(*creds) |
147 | - self.form_fields += FormFields(*delete_fields) |
148 | + image_name_fields.append( |
149 | + TextLine( |
150 | + __name__=u'add_image_name', |
151 | + required=False, readonly=False)) |
152 | + add_credentials = Choice( |
153 | + __name__='add_credentials', |
154 | + default='existing', values=('existing', 'new'), |
155 | + required=False, readonly=False) |
156 | + existing_credentials = Choice( |
157 | + vocabulary='OCIRegistryCredentials', |
158 | + required=False, |
159 | + __name__=u'existing_credentials') |
160 | + url_fields.append( |
161 | + TextLine( |
162 | + __name__=u'add_url', |
163 | + required=False, readonly=False)) |
164 | + username_fields.append( |
165 | + TextLine( |
166 | + __name__=u'add_username', |
167 | + required=False, readonly=False)) |
168 | + password_fields.append( |
169 | + Password( |
170 | + __name__=u'add_password', |
171 | + required=False, readonly=False)) |
172 | + password_fields.append( |
173 | + Password( |
174 | + __name__=u'add_confirm_password', |
175 | + required=False, readonly=False)) |
176 | + |
177 | + self.form_fields = ( |
178 | + FormFields(*image_name_fields) + |
179 | + FormFields(*url_fields) + |
180 | + FormFields( |
181 | + *private_url_fields, |
182 | + custom_widget=InvisibleCredentialsWidget) + |
183 | + FormFields(*username_fields) + |
184 | + FormFields( |
185 | + *private_username_fields, |
186 | + custom_widget=InvisibleCredentialsWidget) + |
187 | + FormFields(*password_fields) + |
188 | + FormFields(*delete_fields) + |
189 | + FormFields(add_credentials, existing_credentials)) |
190 | + |
191 | + def setUpWidgets(self, context=None): |
192 | + """See `LaunchpadFormView`.""" |
193 | + super(OCIRecipeEditPushRulesView, self).setUpWidgets(context=context) |
194 | + for widget in self.widgets: |
195 | + widget.display_label = False |
196 | + widget.hint = None |
197 | |
198 | @property |
199 | def label(self): |
200 | @@ -357,28 +441,60 @@ class OCIRecipeEditPushRulesView(LaunchpadEditFormView): |
201 | def cancel_url(self): |
202 | return canonical_url(self.context) |
203 | |
204 | - def getRulesWidgets(self, rule): |
205 | - |
206 | + def getRuleWidgets(self, rule): |
207 | widgets_by_name = {widget.name: widget for widget in self.widgets} |
208 | + url_field_name = ( |
209 | + "field." + self._getFieldName("url", rule.id)) |
210 | image_field_name = ( |
211 | "field." + self._getFieldName("image_name", rule.id)) |
212 | username_field_name = ( |
213 | "field." + self._getFieldName("username", rule.id)) |
214 | - url_field_name = ( |
215 | - "field." + self._getFieldName("url", rule.id)) |
216 | delete_field_name = ( |
217 | "field." + self._getFieldName("delete", rule.id)) |
218 | return { |
219 | + "url": widgets_by_name[url_field_name], |
220 | "image_name": widgets_by_name[image_field_name], |
221 | "username": widgets_by_name[username_field_name], |
222 | - "url": widgets_by_name[url_field_name], |
223 | "delete": widgets_by_name[delete_field_name], |
224 | } |
225 | |
226 | + def getNewRuleWidgets(self): |
227 | + widgets_by_name = {widget.name: widget for widget in self.widgets} |
228 | + return { |
229 | + "image_name": widgets_by_name["field.add_image_name"], |
230 | + "existing_credentials": |
231 | + widgets_by_name["field.existing_credentials"], |
232 | + "url": widgets_by_name["field.add_url"], |
233 | + "username": widgets_by_name["field.add_username"], |
234 | + "password": widgets_by_name["field.add_password"], |
235 | + "confirm_password": widgets_by_name["field.add_confirm_password"], |
236 | + } |
237 | + |
238 | def parseData(self, data): |
239 | """Rearrange form data to make it easier to process.""" |
240 | parsed_data = {} |
241 | - |
242 | + add_image_name = data.get("add_image_name") |
243 | + add_url = data.get("add_url") |
244 | + add_username = data.get("add_username") |
245 | + add_password = data.get("add_password") |
246 | + add_confirm_password = data.get("add_confirm_password") |
247 | + add_existing_credentials = data.get("existing_credentials") |
248 | + |
249 | + # parse data from the Add new rule section of the form |
250 | + if (add_url or add_username or add_password or |
251 | + add_confirm_password or add_image_name or |
252 | + add_existing_credentials): |
253 | + parsed_data.setdefault(None, { |
254 | + "image_name": add_image_name, |
255 | + "url": add_url, |
256 | + "username": add_username, |
257 | + "password": add_password, |
258 | + "confirm_password": add_confirm_password, |
259 | + "existing_credentials": data["existing_credentials"], |
260 | + "add_credentials": data["add_credentials"], |
261 | + "action": "add", |
262 | + }) |
263 | + # parse data from the Edit existing rule section of the form |
264 | for field_name in sorted( |
265 | name for name in data if name.split(".")[0] == "image_name"): |
266 | _, rule_id = self._parseFieldName(field_name) |
267 | @@ -395,6 +511,59 @@ class OCIRecipeEditPushRulesView(LaunchpadEditFormView): |
268 | |
269 | return parsed_data |
270 | |
271 | + def addNewRule(self, parsed_data): |
272 | + add_data = parsed_data[None] |
273 | + image_name = add_data.get("image_name") |
274 | + url = add_data.get("url") |
275 | + password = add_data.get("password") |
276 | + confirm_password = add_data.get("confirm_password") |
277 | + username = add_data.get("username") |
278 | + existing_credentials = add_data.get("existing_credentials") |
279 | + add_credentials = add_data.get("add_credentials") |
280 | + |
281 | + if not image_name: |
282 | + self.setFieldError("add_image_name", "Image name must be set.") |
283 | + return |
284 | + |
285 | + if add_credentials == "existing": |
286 | + if not existing_credentials: |
287 | + return |
288 | + |
289 | + credentials = existing_credentials |
290 | + |
291 | + elif add_credentials == "new": |
292 | + if not url: |
293 | + self.setFieldError("add_url", "Registry URL must be set.") |
294 | + return |
295 | + if password != confirm_password: |
296 | + self.setFieldError( |
297 | + "add_confirm_password", "Passwords do not match.") |
298 | + return |
299 | + |
300 | + credentials_set = getUtility(IOCIRegistryCredentialsSet) |
301 | + try: |
302 | + credentials = credentials_set.getOrCreate( |
303 | + owner=self.context.owner, url=url, |
304 | + credentials={'username': username, 'password': password}) |
305 | + except OCIRegistryCredentialsAlreadyExist: |
306 | + self.setFieldError( |
307 | + "add_url", |
308 | + "Credentials already exist with the same URL and " |
309 | + "username.") |
310 | + return |
311 | + except ValidationError: |
312 | + self.setFieldError("add_url", "Not a valid URL.") |
313 | + return |
314 | + |
315 | + try: |
316 | + getUtility(IOCIPushRuleSet).new( |
317 | + self.context, credentials, image_name) |
318 | + except OCIPushRuleAlreadyExists: |
319 | + self.setFieldError( |
320 | + "add_image_name", |
321 | + "A push rule already exists with the same URL, image name, " |
322 | + "and credentials.") |
323 | + |
324 | def updatePushRulesFromData(self, parsed_data): |
325 | rules_map = { |
326 | rule.id: rule |
327 | @@ -415,6 +584,8 @@ class OCIRecipeEditPushRulesView(LaunchpadEditFormView): |
328 | rule.setNewImageName(image_name) |
329 | elif action == "delete": |
330 | rule.destroySelf() |
331 | + elif action == "add": |
332 | + self.addNewRule(parsed_data) |
333 | else: |
334 | raise AssertionError("unknown action: %s" % action) |
335 | |
336 | diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py |
337 | index e2cdd77..8b17168 100644 |
338 | --- a/lib/lp/oci/browser/tests/test_ocirecipe.py |
339 | +++ b/lib/lp/oci/browser/tests/test_ocirecipe.py |
340 | @@ -17,9 +17,12 @@ import re |
341 | |
342 | from fixtures import FakeLogger |
343 | import pytz |
344 | +from six.moves.urllib.parse import quote |
345 | import soupmatchers |
346 | +from storm.locals import Store |
347 | from testtools.matchers import ( |
348 | Equals, |
349 | + Is, |
350 | MatchesDict, |
351 | MatchesSetwise, |
352 | MatchesStructure, |
353 | @@ -887,12 +890,20 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin, |
354 | "oci.build_series.%s" % self.distroseries.distribution.name: |
355 | self.distroseries.name, |
356 | })) |
357 | - oci_project = self.factory.makeOCIProject( |
358 | + self.oci_project = self.factory.makeOCIProject( |
359 | pillar=self.distroseries.distribution, |
360 | ociprojectname="oci-project-name") |
361 | + |
362 | + self.member = self.factory.makePerson() |
363 | + self.team = self.factory.makeTeam(members=[self.person, self.member]) |
364 | + |
365 | self.recipe = self.factory.makeOCIRecipe( |
366 | name="recipe-name", registrant=self.person, owner=self.person, |
367 | - oci_project=oci_project) |
368 | + oci_project=self.oci_project) |
369 | + |
370 | + self.team_owned_recipe = self.factory.makeOCIRecipe( |
371 | + name="recipe-name", registrant=self.person, owner=self.team, |
372 | + oci_project=self.oci_project) |
373 | |
374 | self.setConfig() |
375 | |
376 | @@ -1013,7 +1024,7 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin, |
377 | "Image name", "td", text=image_name)))) |
378 | |
379 | def test_edit_oci_push_rules(self): |
380 | - url = unicode(self.factory.getUniqueURL()) |
381 | + url = self.factory.getUniqueURL() |
382 | credentials = {'username': 'foo', 'password': 'bar'} |
383 | registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
384 | owner=self.person, |
385 | @@ -1077,8 +1088,52 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin, |
386 | self.assertRaises(OCIPushRuleAlreadyExists, |
387 | browser.getControl("Save").click) |
388 | |
389 | + def test_edit_oci_push_rules_non_owner_of_credentials(self): |
390 | + url = self.factory.getUniqueURL() |
391 | + credentials = {'username': 'foo', 'password': 'bar'} |
392 | + registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
393 | + owner=self.person, |
394 | + url=url, |
395 | + credentials=credentials) |
396 | + image_names = [self.factory.getUniqueUnicode() for _ in range(2)] |
397 | + push_rules = [ |
398 | + getUtility(IOCIPushRuleSet).new( |
399 | + recipe=self.team_owned_recipe, |
400 | + registry_credentials=registry_credentials, |
401 | + image_name=image_name) |
402 | + for image_name in image_names] |
403 | + Store.of(push_rules[-1]).flush() |
404 | + push_rule_ids = [push_rule.id for push_rule in push_rules] |
405 | + browser = self.getViewBrowser(self.team_owned_recipe, user=self.member) |
406 | + browser.getLink("Edit push rules").click() |
407 | + row = soupmatchers.Tag( |
408 | + "push rule row", "tr", attrs={"class": "push-rule"}) |
409 | + self.assertThat(browser.contents, soupmatchers.HTMLContains( |
410 | + soupmatchers.Within( |
411 | + row, |
412 | + soupmatchers.Tag( |
413 | + "username widget", "span", |
414 | + attrs={ |
415 | + "id": "field.username.%d" % push_rule_ids[0], |
416 | + "class": "sprite private", |
417 | + })), |
418 | + soupmatchers.Within( |
419 | + row, |
420 | + soupmatchers.Tag( |
421 | + "url widget", "span", |
422 | + attrs={ |
423 | + "id": "field.url.%d" % push_rule_ids[0], |
424 | + "class": "sprite private", |
425 | + })))) |
426 | + browser.getControl( |
427 | + name="field.image_name.%d" % push_rule_ids[0]).value = "image1" |
428 | + browser.getControl("Save").click() |
429 | + with person_logged_in(self.member): |
430 | + self.assertEqual("image1", push_rules[0].image_name) |
431 | + self.assertEqual(image_names[1], push_rules[1].image_name) |
432 | + |
433 | def test_delete_oci_push_rules(self): |
434 | - url = unicode(self.factory.getUniqueURL()) |
435 | + url = self.factory.getUniqueURL() |
436 | credentials = {'username': 'foo', 'password': 'bar'} |
437 | registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
438 | owner=self.person, |
439 | @@ -1100,8 +1155,215 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin, |
440 | self.assertIsNone( |
441 | getUtility(IOCIPushRuleSet).getByID(push_rule.id)) |
442 | |
443 | + def test_add_oci_push_rules_validations(self): |
444 | + # Add new rule works when there are no rules in the DB. |
445 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
446 | + browser.getLink("Edit push rules").click() |
447 | + |
448 | + # Save does not error if there are no changes on the form. |
449 | + browser.getControl("Save").click() |
450 | + self.assertIn("Saved push rules", browser.contents) |
451 | + |
452 | + # If only an image name is given but no registry URL, we fail with |
453 | + # "Registry URL must be set". |
454 | + browser.getLink("Edit push rules").click() |
455 | + browser.getControl(name="field.add_image_name").value = "imagename1" |
456 | + browser.getControl(name="field.add_credentials").value = "new" |
457 | + browser.getControl("Save").click() |
458 | + self.assertIn("Registry URL must be set", browser.contents) |
459 | + |
460 | + # No image name entered on the form. We assume user is only editing |
461 | + # and we allow saving the form. |
462 | + browser.getControl(name="field.add_image_name").value = "" |
463 | + browser.getControl("Save").click() |
464 | + self.assertIn("Saved push rules", browser.contents) |
465 | + |
466 | + def test_add_oci_push_rules_new_empty_credentials(self): |
467 | + # Supplying an image name and registry URL creates a credentials |
468 | + # object without username or password, and a valid push rule based |
469 | + # on that credentials object. |
470 | + url = self.factory.getUniqueURL() |
471 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
472 | + browser.getLink("Edit push rules").click() |
473 | + browser.getControl(name="field.add_credentials").value = "new" |
474 | + browser.getControl(name="field.add_image_name").value = "imagename1" |
475 | + browser.getControl(name="field.add_url").value = url |
476 | + browser.getControl("Save").click() |
477 | + with person_logged_in(self.person): |
478 | + rules = list(removeSecurityProxy( |
479 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
480 | + self.assertEqual(len(rules), 1) |
481 | + rule = rules[0] |
482 | + self.assertThat(rule, MatchesStructure( |
483 | + image_name=Equals("imagename1"), |
484 | + registry_url=Equals(url), |
485 | + registry_credentials=MatchesStructure( |
486 | + url=Equals(url), |
487 | + username=Is(None)))) |
488 | + |
489 | + with person_logged_in(self.person): |
490 | + self.assertEqual( |
491 | + {"password": None}, rule.registry_credentials.getCredentials()) |
492 | + |
493 | + def test_add_oci_push_rules_new_username_password(self): |
494 | + # Supplying an image name, registry URL, username, and password |
495 | + # creates a credentials object with the given username or password, |
496 | + # and a valid push rule based on that credentials object. |
497 | + url = self.factory.getUniqueURL() |
498 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
499 | + browser.getLink("Edit push rules").click() |
500 | + browser.getControl(name="field.add_credentials").value = "new" |
501 | + browser.getControl(name="field.add_image_name").value = "imagename3" |
502 | + browser.getControl(name="field.add_url").value = url |
503 | + browser.getControl(name="field.add_username").value = "username" |
504 | + browser.getControl(name="field.add_password").value = "password" |
505 | + browser.getControl( |
506 | + name="field.add_confirm_password").value = "password" |
507 | + browser.getControl("Save").click() |
508 | + with person_logged_in(self.person): |
509 | + rules = list(removeSecurityProxy( |
510 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
511 | + self.assertEqual(len(rules), 1) |
512 | + rule = rules[0] |
513 | + self.assertThat(rule, MatchesStructure( |
514 | + image_name=Equals("imagename3"), |
515 | + registry_url=Equals(url), |
516 | + registry_credentials=MatchesStructure.byEquality( |
517 | + url=url, |
518 | + username="username"))) |
519 | + with person_logged_in(self.person): |
520 | + self.assertEqual( |
521 | + {"username": "username", "password": "password"}, |
522 | + rule.registry_credentials.getCredentials()) |
523 | + |
524 | + def test_add_oci_push_rules_existing_credentials_duplicate(self): |
525 | + # Adding a new push rule using existing credentials fails if a rule |
526 | + # with the same image name already exists. |
527 | + existing_rule = self.factory.makeOCIPushRule( |
528 | + recipe=self.recipe, |
529 | + registry_credentials=self.factory.makeOCIRegistryCredentials( |
530 | + owner=self.recipe.owner)) |
531 | + existing_image_name = existing_rule.image_name |
532 | + existing_registry_url = existing_rule.registry_url |
533 | + existing_username = existing_rule.username |
534 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
535 | + browser.getLink("Edit push rules").click() |
536 | + browser.getControl(name="field.add_credentials").value = "existing" |
537 | + browser.getControl(name="field.add_image_name").value = ( |
538 | + existing_image_name) |
539 | + browser.getControl(name="field.existing_credentials").value = ( |
540 | + "%s %s" % (quote(existing_registry_url), quote(existing_username))) |
541 | + browser.getControl("Save").click() |
542 | + self.assertIn( |
543 | + "A push rule already exists with the same URL, " |
544 | + "image name, and credentials.", browser.contents) |
545 | + |
546 | + def test_add_oci_push_rules_existing_credentials(self): |
547 | + # Previously added registry credentials can be chosen from the radio |
548 | + # widget when adding a new rule. |
549 | + # We correctly display the radio buttons widget when the |
550 | + # username is empty in registry credentials and |
551 | + # allow correctly adding new rule based on it |
552 | + existing_rule = self.factory.makeOCIPushRule( |
553 | + recipe=self.recipe, |
554 | + registry_credentials=self.factory.makeOCIRegistryCredentials( |
555 | + owner=self.recipe.owner, credentials={})) |
556 | + existing_registry_url = existing_rule.registry_url |
557 | + browser = self.getViewBrowser(self.recipe, user=self.person) |
558 | + browser.getLink("Edit push rules").click() |
559 | + browser.getControl(name="field.add_credentials").value = "existing" |
560 | + browser.getControl(name="field.add_image_name").value = "imagename2" |
561 | + browser.getControl(name="field.existing_credentials").value = ( |
562 | + quote(existing_registry_url)) |
563 | + browser.getControl("Save").click() |
564 | + with person_logged_in(self.person): |
565 | + rules = list(removeSecurityProxy( |
566 | + getUtility(IOCIPushRuleSet).findByRecipe(self.recipe))) |
567 | + self.assertEqual(len(rules), 2) |
568 | + rule = rules[1] |
569 | + self.assertThat(rule, MatchesStructure( |
570 | + image_name=Equals("imagename2"), |
571 | + registry_url=Equals(existing_registry_url), |
572 | + registry_credentials=MatchesStructure( |
573 | + url=Equals(existing_registry_url), |
574 | + username=Is(None)))) |
575 | + with person_logged_in(self.person): |
576 | + self.assertEqual({}, rule.registry_credentials.getCredentials()) |
577 | + |
578 | + def test_add_oci_push_rules_team_owned(self): |
579 | + url = self.factory.getUniqueURL() |
580 | + browser = self.getViewBrowser(self.team_owned_recipe, user=self.member) |
581 | + browser.getLink("Edit push rules").click() |
582 | + browser.getControl( |
583 | + name="field.add_image_name").value = "imagename1" |
584 | + browser.getControl( |
585 | + name="field.add_url").value = url |
586 | + browser.getControl(name="field.add_credentials").value = "new" |
587 | + browser.getControl("Save").click() |
588 | + |
589 | + with person_logged_in(self.member): |
590 | + rules = list(removeSecurityProxy( |
591 | + getUtility(IOCIPushRuleSet).findByRecipe( |
592 | + self.team_owned_recipe))) |
593 | + self.assertEqual(len(rules), 1) |
594 | + rule = rules[0] |
595 | + self.assertThat(rule, MatchesStructure( |
596 | + image_name=Equals(u'imagename1'), |
597 | + registry_url=Equals(url), |
598 | + registry_credentials=MatchesStructure( |
599 | + url=Equals(url), |
600 | + username=Is(None)))) |
601 | + |
602 | + with person_logged_in(self.member): |
603 | + self.assertThat( |
604 | + rule.registry_credentials.getCredentials(), |
605 | + MatchesDict( |
606 | + {"password": Equals(None)})) |
607 | + |
608 | + def test_edit_oci_push_rules_team_owned(self): |
609 | + url = self.factory.getUniqueURL() |
610 | + browser = self.getViewBrowser(self.team_owned_recipe, user=self.member) |
611 | + browser.getLink("Edit push rules").click() |
612 | + browser.getControl( |
613 | + name="field.add_image_name").value = "imagename1" |
614 | + browser.getControl( |
615 | + name="field.add_url").value = url |
616 | + browser.getControl(name="field.add_credentials").value = "new" |
617 | + browser.getControl("Save").click() |
618 | + |
619 | + # push rules created by another team member (self.member) |
620 | + # can be edited by self.person |
621 | + browser = self.getViewBrowser(self.team_owned_recipe, user=self.person) |
622 | + browser.getLink("Edit push rules").click() |
623 | + with person_logged_in(self.person): |
624 | + rules = list(removeSecurityProxy( |
625 | + getUtility(IOCIPushRuleSet).findByRecipe( |
626 | + self.team_owned_recipe))) |
627 | + self.assertEqual(len(rules), 1) |
628 | + rule = rules[0] |
629 | + self.assertEqual("imagename1", browser.getControl( |
630 | + name="field.image_name.%d" % rule.id).value) |
631 | + |
632 | + # set image name to valid string |
633 | + with person_logged_in(self.person): |
634 | + browser.getControl( |
635 | + name="field.image_name.%d" % rule.id).value = "image1" |
636 | + browser.getControl("Save").click() |
637 | + |
638 | + # and assert model changed |
639 | + with person_logged_in(self.member): |
640 | + self.assertEqual( |
641 | + rule.image_name, "image1") |
642 | + |
643 | + # self.member will see the new image name |
644 | + browser = self.getViewBrowser(self.team_owned_recipe, user=self.member) |
645 | + browser.getLink("Edit push rules").click() |
646 | + with person_logged_in(self.member): |
647 | + self.assertEqual("image1", browser.getControl( |
648 | + name="field.image_name.%d" % rule.id).value) |
649 | + |
650 | def test_edit_oci_registry_creds(self): |
651 | - url = unicode(self.factory.getUniqueURL()) |
652 | + url = self.factory.getUniqueURL() |
653 | credentials = {'username': 'foo', 'password': 'bar'} |
654 | image_name = self.factory.getUniqueUnicode() |
655 | registry_credentials = getUtility(IOCIRegistryCredentialsSet).new( |
656 | diff --git a/lib/lp/oci/javascript/ocirecipe.edit.js b/lib/lp/oci/javascript/ocirecipe.edit.js |
657 | new file mode 100644 |
658 | index 0000000..8b3d82d |
659 | --- /dev/null |
660 | +++ b/lib/lp/oci/javascript/ocirecipe.edit.js |
661 | @@ -0,0 +1,38 @@ |
662 | +/* Copyright 2015-2020 Canonical Ltd. This software is licensed under the |
663 | + * GNU Affero General Public License version 3 (see the file LICENSE). |
664 | + * |
665 | + * @module Y.lp.oci.ocirecipe.edit |
666 | + * @requires node, DOM |
667 | + */ |
668 | +YUI.add('lp.oci.ocirecipe.edit', function(Y) { |
669 | + var module = Y.namespace('lp.oci.ocirecipe.edit'); |
670 | + |
671 | + module.set_enabled = function(field_id, is_enabled) { |
672 | + var field = Y.DOM.byId(field_id); |
673 | + if (field !== null) { |
674 | + field.disabled = !is_enabled; |
675 | + } |
676 | + }; |
677 | + |
678 | + module.onclick_add_credentials = function(e) { |
679 | + var value = ''; |
680 | + Y.all('input[name="field.add_credentials"]').each(function(node) { |
681 | + if (node.get('checked')) { |
682 | + value = node.get('value'); |
683 | + } |
684 | + }); |
685 | + module.set_enabled('field.existing_credentials', value === 'existing'); |
686 | + module.set_enabled('field.add_url', value === 'new'); |
687 | + module.set_enabled('field.add_username', value === 'new'); |
688 | + module.set_enabled('field.add_password', value === 'new'); |
689 | + module.set_enabled('field.add_confirm_password', value === 'new'); |
690 | + }; |
691 | + |
692 | + module.setup = function() { |
693 | + Y.all('input[name="field.add_credentials"]').on( |
694 | + 'click', module.onclick_add_credentials); |
695 | + |
696 | + // Set the initial state. |
697 | + module.onclick_add_credentials(); |
698 | + }; |
699 | +}, '0.1', {'requires': ['node', 'DOM']}); |
700 | diff --git a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt |
701 | index ccfe716..eee7db7 100644 |
702 | --- a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt |
703 | +++ b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt |
704 | @@ -7,35 +7,142 @@ |
705 | i18n:domain="launchpad"> |
706 | <body> |
707 | |
708 | +<metal:block fill-slot="head_epilogue"> |
709 | + <style type="text/css"> |
710 | + table.push-rules-table { |
711 | + max-width: 60%; |
712 | + margin-bottom: 1em; |
713 | + } |
714 | + table.push-rules-table tr.even { |
715 | + background-color: #eee; |
716 | + } |
717 | + /* These add up to 100%. */ |
718 | + tr .push-rule-url { |
719 | + width: 35%; |
720 | + } |
721 | + tr .push-rule-image-name { |
722 | + width: 20%; |
723 | + } |
724 | + tr .push-rule-username { |
725 | + width: 10%; |
726 | + } |
727 | + tr .push-rule-password, tr .push-rule-confirm-password { |
728 | + width: 15%; |
729 | + } |
730 | + tr .push-rule-delete { |
731 | + width: 5%; |
732 | + } |
733 | + </style> |
734 | +</metal:block> |
735 | + |
736 | <div metal:fill-slot="main"> |
737 | <div metal:use-macro="context/@@launchpad_form/form"> |
738 | <metal:formbody fill-slot="widgets"> |
739 | <p condition="view/can_edit_credentials"> |
740 | <a class="sprite edit" tal:attributes="href context/owner/fmt:url/+edit-oci-registry-credentials">Edit OCI registry credentials</a> |
741 | </p> |
742 | - <table class="form"> |
743 | - <tr tal:repeat="rule context/push_rules"> |
744 | - <tal:rules_widgets |
745 | - define="rules_widgets python:view.getRulesWidgets(rule); |
746 | - parity python:'even' if repeat['rule'].even() else 'odd'"> |
747 | - <td tal:define="widget nocall:rules_widgets/image_name"> |
748 | - <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
749 | - </td> |
750 | - <td tal:define="widget nocall:rules_widgets/username"> |
751 | - <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
752 | - </td> |
753 | - <td tal:define="widget nocall:rules_widgets/url"> |
754 | - <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
755 | - </td> |
756 | - <td tal:define="widget nocall:rules_widgets/delete"> |
757 | - <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
758 | - </td> |
759 | - </tal:rules_widgets> |
760 | - </tr> |
761 | + <table class="listing push-rules-table"> |
762 | + <thead> |
763 | + <tr> |
764 | + <th class="push-rule-image-name">Image name</th> |
765 | + <th class="push-rule-url">Registry URL</th> |
766 | + <th class="push-rule-username">Username</th> |
767 | + <th class="push-rule-password">Password</th> |
768 | + <th class="push-rule-confirm-password">Confirm password</th> |
769 | + <th class="push-rule-delete">Delete?</th> |
770 | + </tr> |
771 | + </thead> |
772 | + <tbody> |
773 | + <tal:rule repeat="rule view/push_rules"> |
774 | + <tal:rule_widgets |
775 | + define="rule_widgets python:view.getRuleWidgets(rule); |
776 | + parity python:'even' if repeat['rule'].even() else 'odd'"> |
777 | + <tr tal:attributes="class string:push-rule ${parity}"> |
778 | + <td class="push-rule-image-name" |
779 | + tal:define="widget nocall:rule_widgets/image_name"> |
780 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
781 | + </td> |
782 | + <td class="push-rule-url" |
783 | + tal:define="widget nocall:rule_widgets/url"> |
784 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
785 | + </td> |
786 | + <td class="push-rule-username" |
787 | + tal:define="widget nocall:rule_widgets/username"> |
788 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
789 | + </td> |
790 | + <td colspan="2" /> |
791 | + <td class="push-rule-delete" |
792 | + tal:define="widget nocall:rule_widgets/delete"> |
793 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
794 | + </td> |
795 | + </tr> |
796 | + </tal:rule_widgets> |
797 | + </tal:rule> |
798 | + <tal:new-rule |
799 | + define="new_rule_widgets python:view.getNewRuleWidgets(); |
800 | + parity python:'odd' if len(view.push_rules) % 2 |
801 | + else 'even'"> |
802 | + <tr tal:attributes="class parity"> |
803 | + <td class="push-rule-image-name" |
804 | + tal:define="widget nocall:new_rule_widgets/image_name"> |
805 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
806 | + </td> |
807 | + <td colspan="5" /> |
808 | + </tr> |
809 | + <tr tal:attributes="class parity"> |
810 | + <td> |
811 | + <label> |
812 | + <input type="radio" name="field.add_credentials" |
813 | + value="existing" checked="checked" /> |
814 | + Use existing credentials: |
815 | + </label> |
816 | + </td> |
817 | + <td colspan="4" |
818 | + tal:define="widget nocall:new_rule_widgets/existing_credentials"> |
819 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
820 | + </td> |
821 | + <td /> |
822 | + </tr> |
823 | + <tr tal:attributes="class parity"> |
824 | + <td> |
825 | + <label> |
826 | + <input type="radio" name="field.add_credentials" |
827 | + value="new" /> |
828 | + Add new credentials: |
829 | + </label> |
830 | + </td> |
831 | + <td class="push-rule-url" |
832 | + tal:define="widget nocall:new_rule_widgets/url"> |
833 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
834 | + </td> |
835 | + <td class="push-rule-username" |
836 | + tal:define="widget nocall:new_rule_widgets/username"> |
837 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
838 | + </td> |
839 | + <td class="push-rule-password" |
840 | + tal:define="widget nocall:new_rule_widgets/password"> |
841 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
842 | + </td> |
843 | + <td class="push-rule-confirm-password" |
844 | + tal:define="widget nocall:new_rule_widgets/confirm_password"> |
845 | + <metal:widget use-macro="context/@@launchpad_form/widget_div" /> |
846 | + </td> |
847 | + <td /> |
848 | + </tr> |
849 | + </tal:new-rule> |
850 | + </tbody> |
851 | </table> |
852 | </metal:formbody> |
853 | </div> |
854 | |
855 | + <script type="text/javascript"> |
856 | + LPJS.use('lp.oci.ocirecipe.edit', function(Y) { |
857 | + Y.on('domready', function(e) { |
858 | + Y.lp.oci.ocirecipe.edit.setup(); |
859 | + }, window); |
860 | + }); |
861 | + </script> |
862 | + |
863 | </div> |
864 | </body> |
865 | </html> |
866 | diff --git a/lib/lp/oci/vocabularies.py b/lib/lp/oci/vocabularies.py |
867 | index aa1fae3..e29fcd9 100644 |
868 | --- a/lib/lp/oci/vocabularies.py |
869 | +++ b/lib/lp/oci/vocabularies.py |
870 | @@ -8,10 +8,19 @@ from __future__ import absolute_import, print_function, unicode_literals |
871 | __metaclass__ = type |
872 | __all__ = [] |
873 | |
874 | +from six.moves.urllib.parse import ( |
875 | + quote, |
876 | + unquote, |
877 | + ) |
878 | +from zope.component import getUtility |
879 | from zope.interface import implementer |
880 | from zope.schema.vocabulary import SimpleTerm |
881 | |
882 | +from lp.oci.interfaces.ociregistrycredentials import ( |
883 | + IOCIRegistryCredentialsSet, |
884 | + ) |
885 | from lp.oci.model.ocirecipe import OCIRecipe |
886 | +from lp.oci.model.ociregistrycredentials import OCIRegistryCredentials |
887 | from lp.services.webapp.vocabulary import ( |
888 | IHugeVocabulary, |
889 | StormVocabularyBase, |
890 | @@ -35,6 +44,49 @@ class OCIRecipeDistroArchSeriesVocabulary(StormVocabularyBase): |
891 | return len(self.context.getAllowedArchitectures()) |
892 | |
893 | |
894 | +class OCIRegistryCredentialsVocabulary(StormVocabularyBase): |
895 | + |
896 | + _table = OCIRegistryCredentials |
897 | + |
898 | + def toTerm(self, obj): |
899 | + if obj.username: |
900 | + token = "%s %s" % (quote(obj.url), quote(obj.username)) |
901 | + title = "%s (%s)" % (obj.url, obj.username) |
902 | + else: |
903 | + token = quote(obj.url) |
904 | + title = obj.url |
905 | + |
906 | + return SimpleTerm(obj, token, title) |
907 | + |
908 | + @property |
909 | + def _entries(self): |
910 | + return list(getUtility( |
911 | + IOCIRegistryCredentialsSet).findByOwner(self.context.owner)) |
912 | + |
913 | + def __contains__(self, value): |
914 | + """See `IVocabulary`.""" |
915 | + return value in self._entries |
916 | + |
917 | + def __len__(self): |
918 | + return len(self._entries) |
919 | + |
920 | + def getTermByToken(self, token): |
921 | + """See `IVocabularyTokenized`.""" |
922 | + try: |
923 | + if ' ' in token: |
924 | + url, username = token.split(' ', 1) |
925 | + url = unquote(url) |
926 | + username = unquote(username) |
927 | + else: |
928 | + username = None |
929 | + url = unquote(token) |
930 | + for obj in self._entries: |
931 | + if obj.url == url and obj.username == username: |
932 | + return self.toTerm(obj) |
933 | + except ValueError: |
934 | + raise LookupError(token) |
935 | + |
936 | + |
937 | @implementer(IHugeVocabulary) |
938 | class OCIRecipeVocabulary(StormVocabularyBase): |
939 | """All OCI Recipes of a given OCI project.""" |
940 | diff --git a/lib/lp/oci/vocabularies.zcml b/lib/lp/oci/vocabularies.zcml |
941 | index 1a6b75c..24a57d8 100644 |
942 | --- a/lib/lp/oci/vocabularies.zcml |
943 | +++ b/lib/lp/oci/vocabularies.zcml |
944 | @@ -16,6 +16,16 @@ |
945 | </class> |
946 | |
947 | <securedutility |
948 | + name="OCIRegistryCredentials" |
949 | + component="lp.oci.vocabularies.OCIRegistryCredentialsVocabulary" |
950 | + provides="zope.schema.interfaces.IVocabularyFactory"> |
951 | + <allow interface="zope.schema.interfaces.IVocabularyFactory" /> |
952 | + </securedutility> |
953 | + |
954 | + <class class="lp.oci.vocabularies.OCIRegistryCredentialsVocabulary"> |
955 | + <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary" /> |
956 | + </class> |
957 | + <securedutility |
958 | name="OCIRecipe" |
959 | component="lp.oci.vocabularies.OCIRecipeVocabulary" |
960 | provides="zope.schema.interfaces.IVocabularyFactory"> |
LGTM (and nice visual!). I've added just a couple of comments.