launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25752
[Merge] ~pappacena/launchpad:public-ecr-aws-ui into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:public-ecr-aws-ui into launchpad:master with ~pappacena/launchpad:public-ecr-aws as a prerequisite.
Commit message:
UI to allow adding region to OCI registry credentials
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394599
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:public-ecr-aws-ui into launchpad:master.
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index 460b108..2103e1e 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -344,7 +344,9 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
super(OCIRecipeEditPushRulesView, self).setUpFields()
image_name_fields = []
url_fields = []
+ region_fields = []
private_url_fields = []
+ private_region_fields = []
username_fields = []
private_username_fields = []
password_fields = []
@@ -361,6 +363,13 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
__name__=self._getFieldName('url', elem.id),
default=elem.registry_credentials.url,
required=True, readonly=True))
+ region = elem.registry_credentials.getCredentialsValue(
+ 'region')
+ region_fields.append(
+ TextLine(
+ __name__=self._getFieldName('region', elem.id),
+ default=region,
+ required=False, readonly=True))
username_fields.append(
TextLine(
__name__=self._getFieldName('username', elem.id),
@@ -380,6 +389,10 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
TextLine(
__name__=self._getFieldName('url', elem.id),
default='', required=True, readonly=True))
+ private_region_fields.append(
+ TextLine(
+ __name__=self._getFieldName('region', elem.id),
+ default='', required=True, readonly=True))
private_username_fields.append(
TextLine(
__name__=self._getFieldName('username', elem.id),
@@ -405,6 +418,10 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
TextLine(
__name__=u'add_url',
required=False, readonly=False))
+ region_fields.append(
+ TextLine(
+ __name__=u'add_region',
+ required=False, readonly=False))
username_fields.append(
TextLine(
__name__=u'add_username',
@@ -424,6 +441,10 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
FormFields(
*private_url_fields,
custom_widget=InvisibleCredentialsWidget) +
+ FormFields(*region_fields) +
+ FormFields(
+ *private_region_fields,
+ custom_widget=InvisibleCredentialsWidget) +
FormFields(*username_fields) +
FormFields(
*private_username_fields,
@@ -453,6 +474,8 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
widgets_by_name = {widget.name: widget for widget in self.widgets}
url_field_name = (
"field." + self._getFieldName("url", rule.id))
+ region_field_name = (
+ "field." + self._getFieldName("region", rule.id))
image_field_name = (
"field." + self._getFieldName("image_name", rule.id))
username_field_name = (
@@ -461,6 +484,7 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
"field." + self._getFieldName("delete", rule.id))
return {
"url": widgets_by_name[url_field_name],
+ "region": widgets_by_name[region_field_name],
"image_name": widgets_by_name[image_field_name],
"username": widgets_by_name[username_field_name],
"delete": widgets_by_name[delete_field_name],
@@ -473,6 +497,7 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
"existing_credentials":
widgets_by_name["field.existing_credentials"],
"url": widgets_by_name["field.add_url"],
+ "region": widgets_by_name["field.add_region"],
"username": widgets_by_name["field.add_username"],
"password": widgets_by_name["field.add_password"],
"confirm_password": widgets_by_name["field.add_confirm_password"],
@@ -483,18 +508,20 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
parsed_data = {}
add_image_name = data.get("add_image_name")
add_url = data.get("add_url")
+ add_region = data.get("add_region")
add_username = data.get("add_username")
add_password = data.get("add_password")
add_confirm_password = data.get("add_confirm_password")
add_existing_credentials = data.get("existing_credentials")
# parse data from the Add new rule section of the form
- if (add_url or add_username or add_password or
+ if (add_url or add_region or add_username or add_password or
add_confirm_password or add_image_name or
add_existing_credentials):
parsed_data.setdefault(None, {
"image_name": add_image_name,
"url": add_url,
+ "region": add_region,
"username": add_username,
"password": add_password,
"confirm_password": add_confirm_password,
@@ -523,6 +550,7 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
add_data = parsed_data[None]
image_name = add_data.get("image_name")
url = add_data.get("url")
+ region = add_data.get("region")
password = add_data.get("password")
confirm_password = add_data.get("confirm_password")
username = add_data.get("username")
@@ -550,9 +578,12 @@ class OCIRecipeEditPushRulesView(LaunchpadFormView):
credentials_set = getUtility(IOCIRegistryCredentialsSet)
try:
+ credential_data = {'username': username, 'password': password}
+ if region is not None:
+ credential_data['region'] = region
credentials = credentials_set.getOrCreate(
registrant=self.user, owner=self.context.owner, url=url,
- credentials={'username': username, 'password': password})
+ credentials=credential_data)
except OCIRegistryCredentialsAlreadyExist:
self.setFieldError(
"add_url",
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index 038e602..0ab9a59 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -1111,6 +1111,10 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin,
text=soupmatchers._not_passed)),
soupmatchers.Within(
row,
+ soupmatchers.Tag("Region", "td",
+ text=soupmatchers._not_passed)),
+ soupmatchers.Within(
+ row,
soupmatchers.Tag("Username", "td",
text=soupmatchers._not_passed)),
soupmatchers.Within(
@@ -1330,6 +1334,7 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin,
browser.getControl(name="field.add_credentials").value = "new"
browser.getControl(name="field.add_image_name").value = "imagename3"
browser.getControl(name="field.add_url").value = url
+ browser.getControl(name="field.add_region").value = "somewhere-02"
browser.getControl(name="field.add_username").value = "username"
browser.getControl(name="field.add_password").value = "password"
browser.getControl(
@@ -1347,8 +1352,9 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin,
url=url,
username="username")))
with person_logged_in(self.person):
- self.assertEqual(
- {"username": "username", "password": "password"},
+ self.assertEqual({
+ "username": "username", "password": "password",
+ "region": "somewhere-02"},
rule.registry_credentials.getCredentials())
def test_add_oci_push_rules_existing_credentials_duplicate(self):
@@ -1494,6 +1500,7 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin,
browser.getLink("Edit OCI registry credentials").click()
browser.getControl(name="field.add_url").value = url
+ browser.getControl(name="field.add_region").value = "new_region1"
browser.getControl(name="field.add_username").value = "new_username"
browser.getControl(name="field.add_password").value = "password"
browser.getControl(name="field.add_confirm_password"
@@ -1508,8 +1515,10 @@ class TestOCIRecipeEditPushRulesView(OCIConfigHelperMixin,
self.assertEqual(url, creds[1].url)
self.assertThat(
(creds[1]).getCredentials(),
- MatchesDict({"username": Equals("new_username"),
- "password": Equals("password")}))
+ MatchesDict({
+ "username": Equals("new_username"),
+ "password": Equals("password"),
+ "region": Equals("new_region1")}))
class TestOCIProjectRecipesView(BaseTestOCIRecipeView):
diff --git a/lib/lp/oci/javascript/ocirecipe.edit.js b/lib/lp/oci/javascript/ocirecipe.edit.js
index 8b3d82d..c31f532 100644
--- a/lib/lp/oci/javascript/ocirecipe.edit.js
+++ b/lib/lp/oci/javascript/ocirecipe.edit.js
@@ -23,6 +23,7 @@ YUI.add('lp.oci.ocirecipe.edit', function(Y) {
});
module.set_enabled('field.existing_credentials', value === 'existing');
module.set_enabled('field.add_url', value === 'new');
+ module.set_enabled('field.add_region', value === 'new');
module.set_enabled('field.add_username', value === 'new');
module.set_enabled('field.add_password', value === 'new');
module.set_enabled('field.add_confirm_password', value === 'new');
diff --git a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
index eee7db7..d78d51d 100644
--- a/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
+++ b/lib/lp/oci/templates/ocirecipe-edit-push-rules.pt
@@ -46,6 +46,7 @@
<tr>
<th class="push-rule-image-name">Image name</th>
<th class="push-rule-url">Registry URL</th>
+ <th class="push-rule-region">Region</th>
<th class="push-rule-username">Username</th>
<th class="push-rule-password">Password</th>
<th class="push-rule-confirm-password">Confirm password</th>
@@ -66,6 +67,10 @@
tal:define="widget nocall:rule_widgets/url">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
+ <td class="push-rule-region"
+ tal:define="widget nocall:rule_widgets/region">
+ <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+ </td>
<td class="push-rule-username"
tal:define="widget nocall:rule_widgets/username">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
@@ -115,6 +120,10 @@
tal:define="widget nocall:new_rule_widgets/url">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
+ <td class="push-rule-region"
+ tal:define="widget nocall:new_rule_widgets/region">
+ <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+ </td>
<td class="push-rule-username"
tal:define="widget nocall:new_rule_widgets/username">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index aef4eb0..55596e9 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -3738,17 +3738,25 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
default=credentials.url,
required=True, readonly=False)
+ region = TextLine(
+ __name__=self._getFieldName('region', id),
+ default=credentials.getCredentialsValue("region"),
+ required=False, readonly=False)
+
delete = Bool(
__name__=self._getFieldName('delete', id),
default=False,
required=True, readonly=False)
- return owner, username, password, confirm_password, url, delete
+ return owner, username, password, confirm_password, url, region, delete
def getAddFieldsRow(self):
add_url = TextLine(
__name__=u'add_url',
required=False, readonly=False)
+ add_region = TextLine(
+ __name__=u'add_region',
+ required=False, readonly=False)
add_owner = Choice(
__name__=u'add_owner',
vocabulary=(
@@ -3766,7 +3774,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
required=False, readonly=False)
return (
- add_url, add_owner, add_username,
+ add_url, add_region, add_owner, add_username,
add_password, add_confirm_password)
def _parseFieldName(self, field_name):
@@ -3825,6 +3833,8 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"field." + self._getFieldName("confirm_password",
credentials.id))
url_field_name = "field." + self._getFieldName("url", credentials.id)
+ region_field_name = "field." + self._getFieldName(
+ "region", credentials.id)
delete_field_name = (
"field." + self._getFieldName("delete", credentials.id))
return {
@@ -3833,6 +3843,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"password": widgets_by_name[password_field_name],
"confirm_password": widgets_by_name[confirm_password_field_name],
"url": widgets_by_name[url_field_name],
+ "region": widgets_by_name[region_field_name],
"delete": widgets_by_name[delete_field_name]
}
@@ -3840,6 +3851,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"""Rearrange form data to make it easier to process."""
parsed_data = {}
add_url = data["add_url"]
+ add_region = data["add_region"]
add_owner = data["add_owner"]
add_username = data["add_username"]
add_password = data["add_password"]
@@ -3850,6 +3862,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"password": add_password,
"confirm_password": add_confirm_password,
"url": add_url,
+ "region": add_region,
"owner": add_owner,
"action": "add",
})
@@ -3865,6 +3878,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
confirm_password_field_name = self._getFieldName(
"confirm_password", credentials_id)
url_field_name = self._getFieldName("url", credentials_id)
+ region_field_name = self._getFieldName("region", credentials_id)
delete_field_name = self._getFieldName("delete", credentials_id)
if data.get(delete_field_name):
action = "delete"
@@ -3875,6 +3889,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
"password": data.get(password_field_name),
"confirm_password": data.get(confirm_password_field_name),
"url": data.get(url_field_name),
+ "region": data.get(region_field_name),
"owner": data.get(owner_field_name),
"action": action,
})
@@ -3882,6 +3897,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
return parsed_data
def changeCredentials(self, parsed_credentials, credentials):
+ region = parsed_credentials["region"]
username = parsed_credentials["username"]
password = parsed_credentials["password"]
confirm_password = parsed_credentials["confirm_password"]
@@ -3902,6 +3918,11 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
credentials.url = parsed_credentials["url"]
elif parsed_credentials["url"] != credentials.url:
credentials.url = parsed_credentials["url"]
+ if credentials.getCredentialsValue('region') != region:
+ credentials_data = removeSecurityProxy(
+ credentials.getCredentials())
+ credentials_data["region"] = region
+ credentials.setCredentials(credentials_data)
if owner != credentials.owner:
credentials.owner = owner
@@ -3919,6 +3940,7 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
def addCredentials(self, parsed_add_credentials):
url = parsed_add_credentials["url"]
+ region = parsed_add_credentials["region"]
owner = parsed_add_credentials["owner"]
password = parsed_add_credentials["password"]
confirm_password = parsed_add_credentials["confirm_password"]
@@ -3936,6 +3958,8 @@ class PersonEditOCIRegistryCredentialsView(LaunchpadFormView):
credentials = {
'username': username,
'password': password}
+ if region:
+ credentials["region"] = region
try:
getUtility(IOCIRegistryCredentialsSet).new(
registrant=self.user, owner=owner, url=url,
diff --git a/lib/lp/registry/browser/tests/test_person.py b/lib/lp/registry/browser/tests/test_person.py
index 07ef8d4..350afcb 100644
--- a/lib/lp/registry/browser/tests/test_person.py
+++ b/lib/lp/registry/browser/tests/test_person.py
@@ -1455,13 +1455,16 @@ class TestPersonOCIRegistryCredentialsView(
"password": Equals("bar"),
})))
- # change only the registry url
+ # change only the registry url and region
browser = self.getViewBrowser(
self.owner, view_name='+oci-registry-credentials', user=self.user)
browser.getLink("Edit OCI registry credentials").click()
url_control = browser.getControl(
name="field.url.%d" % registry_credentials_id)
url_control.value = newurl
+ url_control = browser.getControl(
+ name="field.region.%d" % registry_credentials_id)
+ url_control.value = 'us-west-2'
browser.getControl("Save").click()
with person_logged_in(self.user):
self.assertThat(
@@ -1470,6 +1473,7 @@ class TestPersonOCIRegistryCredentialsView(
MatchesDict({
"username": Equals("different_username"),
"password": Equals("bar"),
+ "region": Equals("us-west-2"),
})))
# change only the password
@@ -1484,7 +1488,7 @@ class TestPersonOCIRegistryCredentialsView(
self.assertIn(
"Passwords do not match.", six.ensure_text(browser.contents))
- # change all fields with one edit action
+ # change all fields (except region) with one edit action
username_control = browser.getControl(
name="field.username.%d" % registry_credentials_id)
username_control.value = 'third_different_username'
@@ -1506,6 +1510,7 @@ class TestPersonOCIRegistryCredentialsView(
MatchesDict({
"username": Equals("third_different_username"),
"password": Equals("third_newpassword"),
+ "region": Equals("us-west-2"),
})))
def test_add_oci_registry_credentials(self):
@@ -1530,6 +1535,7 @@ class TestPersonOCIRegistryCredentialsView(
[owner_name], browser.getControl(name="field.add_owner").value)
browser.getControl(name="field.add_url").value = url
+ browser.getControl(name="field.add_region").value = "sa-east-1"
browser.getControl(name="field.add_owner").value = [new_owner_name]
browser.getControl(name="field.add_username").value = "new_username"
browser.getControl(name="field.add_password").value = "password"
@@ -1555,6 +1561,7 @@ class TestPersonOCIRegistryCredentialsView(
MatchesDict({
"username": Equals("new_username"),
"password": Equals("password"),
+ "region": Equals("sa-east-1"),
}))))
def test_delete_oci_registry_credentials(self):
diff --git a/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
index 1b0fd0f..610f9de 100644
--- a/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
+++ b/lib/lp/registry/templates/person-edit-ociregistrycredentials.pt
@@ -43,6 +43,7 @@
<thead>
<tr>
<th class="credentials-url">Registry URL</th>
+ <th class="credentials-region">Region</th>
<th class="credentials-owner">Owner</th>
<th class="credentials-username">Username</th>
<th class="credentials-password">Password</th>
@@ -60,6 +61,10 @@
tal:define="widget nocall:credentials_widgets/url">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
+ <td class="credentials-region"
+ tal:define="widget nocall:credentials_widgets/region">
+ <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+ </td>
<td class="credentials-owner"
tal:define="widget nocall:credentials_widgets/owner">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
@@ -91,6 +96,10 @@
tal:define="widget nocall:view/widgets/add_url">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
</td>
+ <td class="credentials-region"
+ tal:define="widget nocall:view/widgets/add_region">
+ <metal:widget use-macro="context/@@launchpad_form/widget_div" />
+ </td>
<td class="credentials-owner"
tal:define="widget nocall:view/widgets/add_owner">
<metal:widget use-macro="context/@@launchpad_form/widget_div" />
Follow ups
