From 8b5b9cb83be616bdcfd4fb3b9d62fc41a9bcbca4 Mon Sep 17 00:00:00 2001 From: Dina Saparbaeva Date: Thu, 25 Mar 2021 15:43:31 +0000 Subject: [PATCH] API v2.45, adds metadata support to access rules Added access metadata for share access and also introduced the GET /share-access-rules API. The prior API to retrieve access rules will not work with API version >=2.45 Closes-Bug: #1920687 Change-Id: Iec3a3fad5e2bdf854f04ae974248d899f90bd894 --- doc/source/user/index.rst | 15 +++++ manila_ui/api/manila.py | 23 ++++++-- manila_ui/dashboards/project/shares/forms.py | 49 +++++++++++++++- manila_ui/dashboards/project/shares/tables.py | 31 +++++++--- .../shares/_update_rule_metadata.html | 13 +++++ .../shares/update_rule_metadata.html | 7 +++ manila_ui/dashboards/project/shares/urls.py | 4 ++ manila_ui/dashboards/project/shares/views.py | 53 ++++++++++++++++- manila_ui/tests/api/test_manila.py | 58 +++++++++++++++---- .../tests/dashboards/project/shares/tests.py | 43 +++++++++++++- .../tests/dashboards/project/test_data.py | 51 ++++++++++++---- ...d-share-access-rules-66a86a45a1f4a9c8.yaml | 5 ++ 12 files changed, 316 insertions(+), 36 deletions(-) create mode 100644 manila_ui/dashboards/project/shares/templates/shares/_update_rule_metadata.html create mode 100644 manila_ui/dashboards/project/shares/templates/shares/update_rule_metadata.html create mode 100644 releasenotes/notes/add-share-access-rules-66a86a45a1f4a9c8.yaml diff --git a/doc/source/user/index.rst b/doc/source/user/index.rst index 16c64480..cabb2fe2 100644 --- a/doc/source/user/index.rst +++ b/doc/source/user/index.rst @@ -81,6 +81,21 @@ Deny access A message indicates whether the action was successful. +Edit share access metadata +-------------------------- + +#. Log in to the dashboard, choose a project, and click :guilabel:`Shares`. + +#. Go to the share that you want to deny access and choose + :guilabel:`Manage Rules` from Actions. + +#. Choose the rule you want to edit. + +#. Click :guilabel:`Edit Rule Metadata`: To add share access metadata, use key=value. +To unset metadata, use key. + + A message indicates whether the action was successful. + Edit share metadata ------------------- diff --git a/manila_ui/api/manila.py b/manila_ui/api/manila.py index 9d27813c..3fbd2c90 100644 --- a/manila_ui/api/manila.py +++ b/manila_ui/api/manila.py @@ -28,7 +28,7 @@ from manilaclient import client as manila_client LOG = logging.getLogger(__name__) MANILA_UI_USER_AGENT_REPR = "manila_ui_plugin_for_horizon" -MANILA_VERSION = "2.44" +MANILA_VERSION = "2.45" MANILA_SERVICE_TYPE = "sharev2" # API static values @@ -125,7 +125,21 @@ def share_update(request, share_id, name, description, is_public=False): def share_rules_list(request, share_id): - return manilaclient(request).shares.access_list(share_id) + return manilaclient(request).share_access_rules.access_list(share_id) + + +def share_rule_get(request, rule_id): + return manilaclient(request).share_access_rules.get(rule_id) + + +def share_rule_set_metadata(request, rule, metadata): + return manilaclient(request).share_access_rules.set_metadata( + rule, metadata) + + +def share_rule_unset_metadata(request, rule, keys): + return manilaclient(request).share_access_rules.unset_metadata( + rule, keys) def share_export_location_list(request, share_id): @@ -137,9 +151,10 @@ def share_instance_export_location_list(request, share_instance_id): share_instance_id) -def share_allow(request, share_id, access_type, access_to, access_level): +def share_allow(request, share_id, access_type, access_to, access_level, + metadata=None): return manilaclient(request).shares.allow( - share_id, access_type, access_to, access_level) + share_id, access_type, access_to, access_level, metadata) def share_deny(request, share_id, rule_id): diff --git a/manila_ui/dashboards/project/shares/forms.py b/manila_ui/dashboards/project/shares/forms.py index a8752177..d6af51ab 100644 --- a/manila_ui/dashboards/project/shares/forms.py +++ b/manila_ui/dashboards/project/shares/forms.py @@ -334,15 +334,29 @@ class AddRule(forms.SelfHandlingForm): choices=(('rw', 'read-write'), ('ro', 'read-only'),)) access_to = forms.CharField( label=_("Access To"), max_length="255", required=True) + metadata = forms.CharField( + label=_("Metadata"), required=False, + widget=forms.Textarea(attrs={'rows': 4})) def handle(self, request, data): share_id = self.initial['share_id'] + metadata = {} + try: + set_dict, unset_list = utils.parse_str_meta(data['metadata']) + if unset_list: + msg = _("Expected only pairs of key=value.") + raise ValidationError(message=msg) + metadata = set_dict + except ValidationError as e: + self.api_error(e.messages[0]) + return False try: manila.share_allow( request, share_id, access_to=data['access_to'], access_type=data['access_type'], - access_level=data['access_level']) + access_level=data['access_level'], + metadata=metadata) message = _('Creating rule for "%s"') % data['access_to'] messages.success(request, message) return True @@ -353,6 +367,39 @@ class AddRule(forms.SelfHandlingForm): request, _('Unable to add rule.'), redirect=redirect) +class UpdateRuleMetadataForm(forms.SelfHandlingForm): + metadata = forms.CharField(widget=forms.Textarea, + label=_("Metadata"), required=False) + + def __init__(self, *args, **kwargs): + super(UpdateRuleMetadataForm, self).__init__(*args, **kwargs) + rule_metadata = utils.metadata_to_str( + self.initial["metadata"] + ).replace('
', '\r\n') + self.initial["metadata"] = rule_metadata + + def handle(self, request, data): + rule_id = self.initial['rule_id'] + try: + rule = manila.share_rule_get(self.request, rule_id) + set_dict, unset_list = utils.parse_str_meta(data['metadata']) + if set_dict: + manila.share_rule_set_metadata(request, rule, set_dict) + if unset_list: + manila.share_rule_unset_metadata(request, rule, unset_list) + message = _('Updating share access rule metadata ') + messages.success(request, message) + return True + except ValidationError as e: + self.api_error(e.messages[0]) + return False + except Exception: + redirect = reverse("horizon:project:shares:manage_rules") + exceptions.handle(request, + _('Unable to update rule metadata.'), + redirect=redirect) + + class ResizeForm(forms.SelfHandlingForm): name = forms.CharField( max_length="255", label=_("Share Name"), diff --git a/manila_ui/dashboards/project/shares/tables.py b/manila_ui/dashboards/project/shares/tables.py index ecabf233..38e99117 100644 --- a/manila_ui/dashboards/project/shares/tables.py +++ b/manila_ui/dashboards/project/shares/tables.py @@ -347,16 +347,30 @@ class DeleteRule(tables.DeleteAction): exceptions.handle(request, msg) +class EditRuleMetadata(tables.LinkAction): + name = "update_rule_metadata" + verbose_name = _("Edit Rule Metadata") + url = "horizon:project:shares:update_rule_metadata" + classes = ("ajax-modal", "btn-create") + policy_rules = (("share_access_metadata", "share_access_metadata:update"),) + + def get_policy_target(self, request, datum=None): + project_id = None + if datum: + project_id = getattr(datum, "os-share-tenant-attr:tenant_id", None) + return {"project_id": project_id} + + def allowed(self, request, rule=None): + return rule.state == "active" + + class UpdateRuleRow(tables.Row): ajax = True def get_data(self, request, rule_id): - rules = manila.share_rules_list(request, self.table.kwargs['share_id']) - if rules: - for rule in rules: - if rule.id == rule_id: - return rule - raise exceptions.NotFound + rule = manila.share_rule_get(request, rule_id) + rule.metadata = utils.metadata_to_str(rule.metadata) + return rule class RulesTable(tables.DataTable): @@ -364,6 +378,8 @@ class RulesTable(tables.DataTable): access_to = tables.Column("access_to", verbose_name=_("Access to")) access_level = tables.Column( "access_level", verbose_name=_("Access Level")) + metadata = tables.Column( + "metadata", verbose_name=_("Metadata")) status = tables.Column("state", verbose_name=_("Status")) access_key = tables.Column("access_key", verbose_name=_("Access Key")) created_at = tables.Column("created_at", verbose_name=_("Created At"), @@ -383,7 +399,8 @@ class RulesTable(tables.DataTable): AddRule, DeleteRule) row_actions = ( - DeleteRule,) + DeleteRule, + EditRuleMetadata,) def get_share_network(share): diff --git a/manila_ui/dashboards/project/shares/templates/shares/_update_rule_metadata.html b/manila_ui/dashboards/project/shares/templates/shares/_update_rule_metadata.html new file mode 100644 index 00000000..3201e78d --- /dev/null +++ b/manila_ui/dashboards/project/shares/templates/shares/_update_rule_metadata.html @@ -0,0 +1,13 @@ +{% extends "horizon/common/_modal_form.html" %} +{% load i18n %} +{% block modal-body-right %} +

{% trans "Metadata" %}:

+

+ {% trans "One line - one action. Empty strings will be ignored." %}
+ {% trans "To add metadata use:" %} +

key=value
+ {% trans "To unset metadata use:" %} +
key
+ {% trans "All pairs that are in field for left are set for this metadata." %} +

+{% endblock %} diff --git a/manila_ui/dashboards/project/shares/templates/shares/update_rule_metadata.html b/manila_ui/dashboards/project/shares/templates/shares/update_rule_metadata.html new file mode 100644 index 00000000..e5db8c8c --- /dev/null +++ b/manila_ui/dashboards/project/shares/templates/shares/update_rule_metadata.html @@ -0,0 +1,7 @@ +{% extends 'base.html' %} +{% load i18n %} +{% block title %}{% trans "Edit Rule Metadata" %}{% endblock %} + +{% block main %} + {% include 'project/shares/_update_rule_metadata.html' %} +{% endblock %} diff --git a/manila_ui/dashboards/project/shares/urls.py b/manila_ui/dashboards/project/shares/urls.py index 7d81011b..a1c33826 100644 --- a/manila_ui/dashboards/project/shares/urls.py +++ b/manila_ui/dashboards/project/shares/urls.py @@ -36,6 +36,10 @@ urlpatterns = [ r'^(?P[^/]+)/rule_add/$', shares_views.AddRuleView.as_view(), name='rule_add'), + urls.url( + r'^rules/(?P[^/]+)/update_rule_metadata/$', + shares_views.UpdateRuleMetadataView.as_view(), + name='update_rule_metadata'), urls.url( r'^(?P[^/]+)/$', shares_views.DetailView.as_view(), diff --git a/manila_ui/dashboards/project/shares/views.py b/manila_ui/dashboards/project/shares/views.py index 019adb66..19fc3236 100644 --- a/manila_ui/dashboards/project/shares/views.py +++ b/manila_ui/dashboards/project/shares/views.py @@ -249,6 +249,53 @@ class AddRuleView(forms.ModalFormView): args=[self.kwargs['share_id']]) +class UpdateRuleMetadataView(forms.ModalFormView): + form_class = share_form.UpdateRuleMetadataForm + form_id = "" + template_name = 'project/shares/update_rule_metadata.html' + modal_header = _("Edit Rule Metadata") + modal_id = "update_rule_metadata_modal" + submit_label = _("Save Changes") + submit_url = "horizon:project:shares:update_rule_metadata" + success_url = reverse_lazy("horizon:project:shares:index") + page_title = _('Edit Rule Metadata') + + def get_object(self): + if not hasattr(self, "_object"): + rule_id = self.kwargs['rule_id'] + try: + self._object = manila.share_rule_get(self.request, rule_id) + except Exception: + msg = _('Unable to retrieve share access rule.') + url = reverse('horizon:project:shares:manage_rules') + exceptions.handle(self.request, msg, redirect=url) + return self._object + + def get_context_data(self, **kwargs): + context = super(UpdateRuleMetadataView, self).\ + get_context_data(**kwargs) + args = (self.get_object().id,) + context['submit_url'] = reverse(self.submit_url, args=args) + return context + + def get_initial(self): + rule = self.get_object() + return {'rule_id': self.kwargs["rule_id"], + 'metadata': rule.metadata} + + # To redirect to Rules Table page, after updating the rule + # metadata. Loop is to get the share_id neccessary to display + # Rules Table page. + def get_success_url(self): + shares = manila.share_list(self.request) + for share in shares: + rules = manila.share_rules_list(self.request, share.id) + for rule in rules: + if rule.id == self.kwargs["rule_id"]: + return reverse("horizon:project:shares:manage_rules", + args=[share.id]) + + class ManageRulesView(tables.DataTableView): table_class = shares_tables.RulesTable template_name = 'project/shares/manage_rules.html' @@ -257,7 +304,6 @@ class ManageRulesView(tables.DataTableView): context = super(ManageRulesView, self).get_context_data(**kwargs) share = manila.share_get(self.request, self.kwargs['share_id']) context['share_display_name'] = share.name or share.id - context["share"] = self.get_data() context["page_title"] = _("Share Rules: " "%(share_display_name)s") % { 'share_display_name': context['share_display_name']} @@ -273,6 +319,11 @@ class ManageRulesView(tables.DataTableView): exceptions.handle(self.request, _('Unable to retrieve share rules.'), redirect=redirect) + return [] + + for rule in rules: + rule.metadata = ui_utils.metadata_to_str(rule.metadata) + return rules diff --git a/manila_ui/tests/api/test_manila.py b/manila_ui/tests/api/test_manila.py index ccf11fd8..911c1a34 100644 --- a/manila_ui/tests/api/test_manila.py +++ b/manila_ui/tests/api/test_manila.py @@ -103,9 +103,42 @@ class ManilaApiTests(base.APITestCase): result = api.share_rules_list(self.request, self.id) self.assertEqual( - self.manilaclient.shares.access_list.return_value, result) + self.manilaclient.share_access_rules. + access_list.return_value, result) - self.manilaclient.shares.access_list.assert_called_once_with(self.id) + self.manilaclient.share_access_rules.\ + access_list.assert_called_once_with(self.id) + + def test_share_rule_get(self): + result = api.share_rule_get(self.request, self.id) + + self.assertEqual( + self.manilaclient.share_access_rules.get. + return_value, result) + + self.manilaclient.share_access_rules.get.\ + assert_called_once_with(self.id) + + def test_share_rule_set_metadata(self): + fake_metadata = { + "aim": "testing", + "project": "test", + } + + api.share_rule_set_metadata( + self.request, self.id, fake_metadata) + + self.manilaclient.share_access_rules.\ + set_metadata.assert_called_once_with(self.id, fake_metadata) + + def test_share_rule_unset_metadata(self): + fake_key = "test" + + api.share_rule_unset_metadata( + self.request, self.id, fake_key) + + self.manilaclient.share_access_rules.\ + unset_metadata.assert_called_once_with(self.id, fake_key) def test_list_share_export_locations(self): api.share_export_location_list(self.request, self.id) @@ -120,19 +153,22 @@ class ManilaApiTests(base.APITestCase): client.share_instance_export_locations.list.assert_called_once_with( self.id) - @ddt.data(("ip", "10.0.0.13", "rw"), ("ip", "10.0.0.13", None), - ("ip", "10.0.0.13", "ro"), - ("user", "demo", "rw"), - ("user", "demo", None), ("user", "demo", "ro"), - ("cephx", "alice", "rw"), - ("cephx", "alice", None), ("cephx", "alice", "ro")) + @ddt.data(("ip", "10.0.0.13", "rw", {'testkey': "testval"}), + ("ip", "10.0.0.13", None, None), + ("ip", "10.0.0.13", "ro", {'key1': "val1"}), + ("user", "demo", "rw", {'key2': "val2"}), + ("user", "demo", None, None), + ("user", "demo", "ro", None), + ("cephx", "alice", "rw", {'test': "some"}), + ("cephx", "alice", None, None), + ("cephx", "alice", "ro", None)) @ddt.unpack - def test_share_allow(self, access_type, access_to, access_level): + def test_share_allow(self, access_type, access_to, access_level, metadata): api.share_allow(self.request, self.id, access_type, - access_to, access_level) + access_to, access_level, metadata) self.manilaclient.shares.allow.assert_called_once_with( - self.id, access_type, access_to, access_level) + self.id, access_type, access_to, access_level, metadata) def test_share_deny(self): fake_rule_id = "fake_rule_id" diff --git a/manila_ui/tests/dashboards/project/shares/tests.py b/manila_ui/tests/dashboards/project/shares/tests.py index b69e8ce0..e25f4524 100644 --- a/manila_ui/tests/dashboards/project/shares/tests.py +++ b/manila_ui/tests/dashboards/project/shares/tests.py @@ -300,7 +300,7 @@ class ShareViewTests(test.APITestCase): [mock.call(mock.ANY, self.share.id) for i in (1, 2)]) def test_list_rules(self): - rules = [test_data.ip_rule, test_data.user_rule, test_data.cephx_rule] + rules = test_data.share_access_list self.mock_object( api_manila, "share_rules_list", mock.Mock(return_value=rules)) url = reverse( @@ -331,6 +331,7 @@ class ShareViewTests(test.APITestCase): 'method': 'CreateForm', 'access_to': 'someuser', 'access_level': 'rw', + 'metadata': {}, } res = self.client.post(url, formData) @@ -338,7 +339,8 @@ class ShareViewTests(test.APITestCase): api_manila.share_allow.assert_called_once_with( mock.ANY, self.share.id, access_type=formData['access_type'], access_to=formData['access_to'], - access_level=formData['access_level']) + access_level=formData['access_level'], + metadata=formData['metadata']) self.assertRedirectsNoFollow( res, reverse('horizon:project:shares:manage_rules', @@ -360,6 +362,43 @@ class ShareViewTests(test.APITestCase): mock.ANY, self.share.id, rule.id) api_manila.share_rules_list.assert_called_with(mock.ANY, self.share.id) + def test_update_share_rule_metadata_get(self): + rule = test_data.user_rule + url = reverse( + 'horizon:project:shares:update_rule_metadata', args=[rule.id]) + self.mock_object( + api_manila, "share_rule_get", mock.Mock(return_value=rule)) + res = self.client.get(url) + api_manila.share_rule_get.assert_called_once_with(mock.ANY, rule.id) + self.assertNoMessages() + self.assertTemplateUsed( + res, 'project/shares/update_rule_metadata.html') + + def test_update_share_rule_metadata_post(self): + rule = test_data.user_rule + data = { + 'metadata': 'aaa=ccc', + } + form_data = { + 'metadata': {'aaa': 'ccc'}, + } + url = reverse( + 'horizon:project:shares:update_rule_metadata', args=[rule.id]) + self.mock_object( + api_manila, "share_list", mock.Mock( + return_value=test_data.shares_list)) + self.mock_object( + api_manila, "share_rules_list", mock.Mock( + return_value=test_data.share_access_list)) + self.mock_object( + api_manila, "share_rule_get", mock.Mock(return_value=rule)) + self.mock_object(api_manila, "share_rule_set_metadata") + + self.client.post(url, data) + + api_manila.share_rule_set_metadata.assert_called_once_with( + mock.ANY, rule, form_data['metadata']) + def test_resize_share_get(self): share = test_data.share url = reverse('horizon:project:shares:resize', args=[share.id]) diff --git a/manila_ui/tests/dashboards/project/test_data.py b/manila_ui/tests/dashboards/project/test_data.py index 2ef65351..8cbc4e7b 100644 --- a/manila_ui/tests/dashboards/project/test_data.py +++ b/manila_ui/tests/dashboards/project/test_data.py @@ -11,10 +11,10 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. -import collections from manilaclient.v2 import messages from manilaclient.v2 import security_services +from manilaclient.v2 import share_access_rules from manilaclient.v2 import share_export_locations from manilaclient.v2 import share_group_snapshots from manilaclient.v2 import share_group_types @@ -102,6 +102,8 @@ other_share = shares.Share( 'replication_type': 'readable', 'mount_snapshot_support': False}) +shares_list = [share, nameless_share, other_share] + share_replica = share_replicas.ShareReplica( share_replicas.ShareReplicaManager(FakeAPIClient), {'id': '11023e92-8008-4c8b-8059-replica00001', @@ -207,17 +209,46 @@ user_snapshot_export_locations = [ 'path': '1.1.1.2:/not/too/long/path/to/user/share_snapshot'} ) ] +ip_rule = share_access_rules.ShareAccessRule( + share_access_rules.ShareAccessRuleManager(FakeAPIClient), + {'id': 'ca8b755c-fe13-497f-81d0-fd2f13a30a78', + 'access_level': 'rw', + 'access_to': '1.1.1.1', + 'access_type': 'ip', + 'state': 'active', + 'access_key': '', + 'created_at': '2021-03-03T14:29:41.000000', + 'updated_at': '', + "metadata": {}, + }) -rule = collections.namedtuple('Access', ['access_type', 'access_to', 'state', - 'id', 'access_level', 'access_key']) +user_rule = share_access_rules.ShareAccessRule( + share_access_rules.ShareAccessRuleManager(FakeAPIClient), + {'id': '0837072-c49e-11e3-bd64-60a44c371189', + 'access_level': 'rw', + 'access_to': 'someuser', + 'access_type': 'user', + 'state': 'active', + 'access_key': '', + 'created_at': '2021-03-03T14:29:41.000000', + 'updated_at': '', + 'metadata': {'abc': 'ddd'}, + }) -user_rule = rule('user', 'someuser', 'active', - '10837072-c49e-11e3-bd64-60a44c371189', 'rw', '') -ip_rule = rule('ip', '1.1.1.1', 'active', - '2cc8e2f8-c49e-11e3-bd64-60a44c371189', 'rw', '') -cephx_rule = rule('cephx', 'alice', 'active', - '235481bc-1a84-11e6-9666-68f728a0492e', 'rw', - 'AQAdFCNYDCapMRAANuK/CiEZbog2911a+t5dcQ==') +cephx_rule = share_access_rules.ShareAccessRule( + share_access_rules.ShareAccessRuleManager(FakeAPIClient), + {'id': '235481bc-1a84-11e6-9666-68f728a0492e', + 'access_level': 'rw', + 'access_to': 'alice', + 'access_type': 'cephx', + 'state': 'active', + 'access_key': 'AQAdFCNYDCapMRAANuK/CiEZbog2911a+t5dcQ==', + 'created_at': '2021-03-03T14:29:41.000000', + 'updated_at': '', + 'metadata': {'test': 'true'}, + }) + +share_access_list = [user_rule, cephx_rule, ip_rule] snapshot = share_snapshots.ShareSnapshot( share_snapshots.ShareSnapshotManager(FakeAPIClient), diff --git a/releasenotes/notes/add-share-access-rules-66a86a45a1f4a9c8.yaml b/releasenotes/notes/add-share-access-rules-66a86a45a1f4a9c8.yaml new file mode 100644 index 00000000..0026dbad --- /dev/null +++ b/releasenotes/notes/add-share-access-rules-66a86a45a1f4a9c8.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Added access metadata for share access and + also introduced the GET /share-access-rules API.