Fix share replica details page for non-admins

Non-administrators cannot access share instance
export locations by the virtue of default policy.

Fix this retrieval to not error-out showing the
whole page.

Change-Id: I5f78e7fbfbf4ce93f9c1658609f20f179c6995a6
Closes-Bug: #1787016
This commit is contained in:
Goutham Pacha Ravi 2019-04-03 12:18:19 -07:00
parent ca9cb26eee
commit 8aa0993154
4 changed files with 50 additions and 31 deletions

View File

@ -87,20 +87,25 @@ class DetailReplicaView(tabs.TabView):
try:
replica_id = self.kwargs['replica_id']
replica = manila.share_replica_get(self.request, replica_id)
replica.export_locations = (
manila.share_instance_export_location_list(
self.request, replica_id))
export_locations = [
exp['path'] for exp in replica.export_locations
]
replica.el_size = ui_utils.calculate_longest_str_size(
export_locations)
try:
# The default policy for this API does not allow
# non-admins to retrieve export locations.
replica.export_locations = (
manila.share_instance_export_location_list(
self.request, replica_id))
export_locations = [
exp['path'] for exp in replica.export_locations
]
replica.el_size = ui_utils.calculate_longest_str_size(
export_locations)
except Exception:
replica.export_locations = []
except Exception:
redirect = reverse(self._redirect_url)
exceptions.handle(
self.request,
_('Unable to retrieve replica %sdetails.') % replica_id,
redirect=redirect)
_('Unable to retrieve details of replica %s') %
self.kwargs['replica_id'], redirect=redirect)
return replica
def get_tabs(self, request, *args, **kwargs):

View File

@ -13,20 +13,22 @@
<dd>{{ replica.status|capfirst }}</dd>
<dt>{% trans "Replica state" %}</dt>
<dd>{{ replica.replica_state|capfirst }}</dd>
<dt>{% trans "Export locations" %}</dt>
{% for el in replica.export_locations %}
<dd><p>
<div><b>Path:</b>
<input type="text" readonly="true"
value="{{ el.path }}" size="{{ replica.el_size }}"
onClick="this.setSelectionRange(0, this.value.length)">
</div>
<div><b>Preferred:</b> {{ el.preferred }}</div>
{% if el.is_admin_only == True or el.is_admin_only == False %}
<div><b>Is admin only:</b> {{ el.is_admin_only }}</div>
{% endif %}
</p></dd>
{% endfor %}
{% if replica.export_locations %}
<dt>{% trans "Export locations" %}</dt>
{% for el in replica.export_locations %}
<dd><p>
<div><b>Path:</b>
<input type="text" readonly="true"
value="{{ el.path }}" size="{{ replica.el_size }}"
onClick="this.setSelectionRange(0, this.value.length)">
</div>
<div><b>Preferred:</b> {{ el.preferred }}</div>
{% if el.is_admin_only == True or el.is_admin_only == False %}
<div><b>Is admin only:</b> {{ el.is_admin_only }}</div>
{% endif %}
</p></dd>
{% endfor %}
{% endif %}
<dt>{% trans "Availability zone" %}</dt>
<dd>{{ replica.availability_zone }}</dd>

View File

@ -103,17 +103,22 @@ class ReplicasTests(test.TestCase):
mock.Mock(side_effect=Exception("Fake exception")))
self._create_post()
def test_detail(self):
@ddt.data(True, False)
def test_detail_with_export_locations_available(self, exports_available):
url = reverse(
"horizon:project:shares:replica_detail",
args=[self.share_replica.id])
export_locations = test_data.export_locations
export_locations_call_behavior = (
{'return_value': test_data.export_locations} if exports_available
else {'side_effect': Exception("Access denied to this resource")}
)
self.mock_object(
api_manila, "share_replica_get",
mock.Mock(return_value=self.share_replica))
self.mock_object(
api_manila, "share_instance_export_location_list",
mock.Mock(return_value=export_locations))
mock.Mock(**export_locations_call_behavior))
res = self.client.get(url)
@ -125,10 +130,12 @@ class ReplicasTests(test.TestCase):
self.assertContains(res, "<dd>%s</dd>" % self.share_replica.id, 1, 200)
self.assertContains(
res, "<dd>%s</dd>" % self.share.availability_zone, 1, 200)
for el in export_locations:
self.assertContains(res, "value=\"%s\"" % el.path, 1, 200)
self.assertContains(
res, "<div><b>Preferred:</b> %s</div>" % el.preferred, 1, 200)
if exports_available:
for el in test_data.export_locations:
self.assertContains(res, "value=\"%s\"" % el.path, 1, 200)
self.assertContains(
res, "<div><b>Preferred:</b> %s</div>" % el.preferred,
1, 200)
self.assertContains(
res, "<div><b>Is admin only:</b> %s</div>" % el.is_admin_only,
1, 200)

View File

@ -0,0 +1,5 @@
---
fixes:
- |
The Share replica details page has been fixed to render correctly for
users with non-admin roles.