From 6f6a31d595eec9d5c2735d91317c4d4742b920d7 Mon Sep 17 00:00:00 2001 From: Ben Swartzlander Date: Mon, 13 Feb 2017 22:31:18 -0500 Subject: [PATCH] Only return share host for admins using shares API Also, allow filtering by host based on policy 'list_by_host' that defaults to 'admin only'. Do not bump API, because it is not considered as expected behavior, hence should not be kept for old microversions. Co-Authored-By: Valeriy Ponomaryov APIImpact Change-Id: I799bb7378927b6c3ee0f9fe88fd9876a03dd85b5 Closes bug: 1664370 (cherry picked from commit 15b27e8fbe252328ed6c105942e915c76af23fb4) --- etc/manila/policy.json | 1 + manila/api/views/share_replicas.py | 2 +- manila/api/views/shares.py | 3 +- manila/share/api.py | 2 + manila/tests/api/v1/test_shares.py | 82 +++++++++--------- manila/tests/api/v2/test_share_replicas.py | 2 +- manila/tests/api/v2/test_shares.py | 83 ++++++++++--------- manila/tests/share/test_api.py | 23 +++-- manila_tempest_tests/tests/api/base.py | 3 +- .../tests/api/test_replication.py | 4 +- .../tests/api/test_replication_negative.py | 2 +- manila_tempest_tests/tests/api/test_shares.py | 2 +- .../tests/api/test_shares_actions.py | 19 +---- .../tests/api/test_shares_negative.py | 6 ++ .../tests/scenario/test_share_basic_ops.py | 2 +- ...-shares-and-replicas-a087f85bc4a4ba45.yaml | 10 +++ 16 files changed, 130 insertions(+), 116 deletions(-) create mode 100644 releasenotes/notes/remove-host-field-from-shares-and-replicas-a087f85bc4a4ba45.yaml diff --git a/etc/manila/policy.json b/etc/manila/policy.json index bc5e1bb921..0b98432f25 100644 --- a/etc/manila/policy.json +++ b/etc/manila/policy.json @@ -22,6 +22,7 @@ "share:get": "rule:default", "share:get_all": "rule:default", "share:list_by_share_server_id": "rule:admin_api", + "share:list_by_host": "rule:admin_api", "share:update": "rule:default", "share:access_get": "rule:default", "share:access_get_all": "rule:default", diff --git a/manila/api/views/share_replicas.py b/manila/api/views/share_replicas.py index 1af2c48222..5bcbeb2c5f 100644 --- a/manila/api/views/share_replicas.py +++ b/manila/api/views/share_replicas.py @@ -54,7 +54,6 @@ class ReplicationViewBuilder(common.ViewBuilder): 'share_id': replica.get('share_id'), 'availability_zone': replica.get('availability_zone'), 'created_at': replica.get('created_at'), - 'host': replica.get('host'), 'status': replica.get('status'), 'share_network_id': replica.get('share_network_id'), 'replica_state': replica.get('replica_state'), @@ -63,6 +62,7 @@ class ReplicationViewBuilder(common.ViewBuilder): if context.is_admin: replica_dict['share_server_id'] = replica.get('share_server_id') + replica_dict['host'] = replica.get('host') self.update_versioned_resource_dict(request, replica_dict, replica) return {'share_replica': replica_dict} diff --git a/manila/api/views/shares.py b/manila/api/views/shares.py index 7af27a3b4d..dab6183728 100644 --- a/manila/api/views/shares.py +++ b/manila/api/views/shares.py @@ -80,7 +80,6 @@ class ViewBuilder(common.ViewBuilder): 'name': share.get('display_name'), 'description': share.get('display_description'), 'project_id': share.get('project_id'), - 'host': share_instance.get('host'), 'snapshot_id': share.get('snapshot_id'), 'share_network_id': share_instance.get('share_network_id'), 'share_proto': share.get('share_proto'), @@ -92,12 +91,12 @@ class ViewBuilder(common.ViewBuilder): 'is_public': share.get('is_public'), 'export_locations': export_locations, } - self.update_versioned_resource_dict(request, share_dict, share) if context.is_admin: share_dict['share_server_id'] = share_instance.get( 'share_server_id') + share_dict['host'] = share_instance.get('host') return {'share': share_dict} @common.ViewBuilder.versioned_method("2.2") diff --git a/manila/share/api.py b/manila/share/api.py index 08848cb3b4..03f5c6a248 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -1518,6 +1518,8 @@ class API(base.Base): is_public = strutils.bool_from_string(is_public, strict=True) # Get filtered list of shares + if 'host' in search_opts: + policy.check_policy(context, 'share', 'list_by_host') if 'share_server_id' in search_opts: # NOTE(vponomaryov): this is project_id independent policy.check_policy(context, 'share', 'list_by_share_server_id') diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index c91fed40aa..572baf2ba1 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -90,7 +90,6 @@ class ShareAPITest(test.TestCase): 'export_location': 'fake_location', 'export_locations': ['fake_location', 'fake_location2'], 'project_id': 'fakeproject', - 'host': 'fakehost', 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), 'share_proto': 'FAKEPROTO', 'metadata': {}, @@ -123,6 +122,7 @@ class ShareAPITest(test.TestCase): share['share_proto'] = share['share_proto'].upper() if admin: share['share_server_id'] = 'fake_share_server_id' + share['host'] = 'fakehost' return {'share': share} @ddt.data("1.0", "2.0", "2.1") @@ -501,7 +501,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': 'fake_share_server_id', 'share_type_id': 'fake_share_type_id', 'snapshot_id': 'fake_snapshot_id', - 'host': 'fake_host', 'share_network_id': 'fake_share_network_id', 'metadata': '%7B%27k1%27%3A+%27v1%27%7D', # serialized k1=v1 'extra_specs': '%7B%27k2%27%3A+%27v2%27%7D', # serialized k2=v2 @@ -511,6 +510,8 @@ class ShareAPITest(test.TestCase): 'offset': '1', 'is_public': 'False', } + if use_admin_context: + search_opts['host'] = 'fake_host' # fake_key should be filtered for non-admin url = '/shares?fake_key=fake_value' for k, v in search_opts.items(): @@ -533,7 +534,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': search_opts['share_server_id'], 'share_type_id': search_opts['share_type_id'], 'snapshot_id': search_opts['snapshot_id'], - 'host': search_opts['host'], 'share_network_id': search_opts['share_network_id'], 'metadata': {'k1': 'v1'}, 'extra_specs': {'k2': 'v2'}, @@ -541,6 +541,7 @@ class ShareAPITest(test.TestCase): } if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) + search_opts_expected['host'] = search_opts['host'] share_api.API.get_all.assert_called_once_with( req.environ['manila.context'], sort_key=search_opts['sort_key'], @@ -590,7 +591,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': 'fake_share_server_id', 'share_type_id': 'fake_share_type_id', 'snapshot_id': 'fake_snapshot_id', - 'host': 'fake_host', 'share_network_id': 'fake_share_network_id', 'metadata': '%7B%27k1%27%3A+%27v1%27%7D', # serialized k1=v1 'extra_specs': '%7B%27k2%27%3A+%27v2%27%7D', # serialized k2=v2 @@ -600,6 +600,8 @@ class ShareAPITest(test.TestCase): 'offset': '1', 'is_public': 'False', } + if use_admin_context: + search_opts['host'] = 'fake_host' # fake_key should be filtered for non-admin url = '/shares/detail?fake_key=fake_value' for k, v in search_opts.items(): @@ -630,7 +632,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': search_opts['share_server_id'], 'share_type_id': search_opts['share_type_id'], 'snapshot_id': search_opts['snapshot_id'], - 'host': search_opts['host'], 'share_network_id': search_opts['share_network_id'], 'metadata': {'k1': 'v1'}, 'extra_specs': {'k2': 'v2'}, @@ -638,6 +639,7 @@ class ShareAPITest(test.TestCase): } if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) + search_opts_expected['host'] = search_opts['host'] share_api.API.get_all.assert_called_once_with( req.environ['manila.context'], sort_key=search_opts['sort_key'], @@ -656,8 +658,9 @@ class ShareAPITest(test.TestCase): shares[1]['share_type_id'], result['shares'][0]['share_type']) self.assertEqual( shares[1]['snapshot_id'], result['shares'][0]['snapshot_id']) - self.assertEqual( - shares[1]['instance']['host'], result['shares'][0]['host']) + if use_admin_context: + self.assertEqual( + shares[1]['instance']['host'], result['shares'][0]['host']) self.assertEqual( shares[1]['instance']['share_network_id'], result['shares'][0]['share_network_id']) @@ -668,42 +671,41 @@ class ShareAPITest(test.TestCase): def test_share_list_detail_with_search_opts_by_admin(self): self._share_list_detail_with_search_opts(use_admin_context=True) - def _list_detail_common_expected(self): - return { - 'shares': [ + def _list_detail_common_expected(self, admin=False): + share_dict = { + 'status': 'fakestatus', + 'description': 'displaydesc', + 'export_location': 'fake_location', + 'export_locations': ['fake_location', 'fake_location2'], + 'availability_zone': 'fakeaz', + 'name': 'displayname', + 'share_proto': 'FAKEPROTO', + 'metadata': {}, + 'project_id': 'fakeproject', + + 'id': '1', + 'snapshot_id': '2', + 'snapshot_support': True, + 'share_network_id': None, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'size': 1, + 'share_type': '1', + 'volume_type': '1', + 'is_public': False, + 'links': [ { - 'status': 'fakestatus', - 'description': 'displaydesc', - 'export_location': 'fake_location', - 'export_locations': ['fake_location', 'fake_location2'], - 'availability_zone': 'fakeaz', - 'name': 'displayname', - 'share_proto': 'FAKEPROTO', - 'metadata': {}, - 'project_id': 'fakeproject', - 'host': 'fakehost', - 'id': '1', - 'snapshot_id': '2', - 'snapshot_support': True, - 'share_network_id': None, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'size': 1, - 'share_type': '1', - 'volume_type': '1', - 'is_public': False, - 'links': [ - { - 'href': 'http://localhost/v1/fake/shares/1', - 'rel': 'self' - }, - { - 'href': 'http://localhost/fake/shares/1', - 'rel': 'bookmark' - } - ], + 'href': 'http://localhost/v1/fake/shares/1', + 'rel': 'self' + }, + { + 'href': 'http://localhost/fake/shares/1', + 'rel': 'bookmark' } - ] + ], } + if admin: + share_dict['host'] = 'fakehost' + return {'shares': [share_dict]} def _list_detail_test_common(self, req, expected): self.mock_object(share_api.API, 'get_all', diff --git a/manila/tests/api/v2/test_share_replicas.py b/manila/tests/api/v2/test_share_replicas.py index 196a757cd7..f847579571 100644 --- a/manila/tests/api/v2/test_share_replicas.py +++ b/manila/tests/api/v2/test_share_replicas.py @@ -81,7 +81,6 @@ class ShareReplicasApiTest(test.TestCase): if not summary: expected_replica.update({ - 'host': replica['host'], 'availability_zone': None, 'created_at': None, 'share_network_id': replica['share_network_id'], @@ -90,6 +89,7 @@ class ShareReplicasApiTest(test.TestCase): if admin: expected_replica['share_server_id'] = replica['share_server_id'] + expected_replica['host'] = replica['host'] return replica, expected_replica diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index 8a99e38986..72be15f45d 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -105,7 +105,6 @@ class ShareAPITest(test.TestCase): 'export_location': 'fake_location', 'export_locations': ['fake_location', 'fake_location2'], 'project_id': 'fakeproject', - 'host': 'fakehost', 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), 'share_proto': 'FAKEPROTO', 'metadata': {}, @@ -140,6 +139,7 @@ class ShareAPITest(test.TestCase): share['share_proto'] = share['share_proto'].upper() if admin: share['share_server_id'] = 'fake_share_server_id' + share['host'] = 'fakehost' return {'share': share} def test__revert(self): @@ -1414,7 +1414,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': 'fake_share_server_id', 'share_type_id': 'fake_share_type_id', 'snapshot_id': 'fake_snapshot_id', - 'host': 'fake_host', 'share_network_id': 'fake_share_network_id', 'metadata': '%7B%27k1%27%3A+%27v1%27%7D', # serialized k1=v1 'extra_specs': '%7B%27k2%27%3A+%27v2%27%7D', # serialized k2=v2 @@ -1424,6 +1423,8 @@ class ShareAPITest(test.TestCase): 'offset': '1', 'is_public': 'False', } + if use_admin_context: + search_opts['host'] = 'fake_host' # fake_key should be filtered for non-admin url = '/shares?fake_key=fake_value' for k, v in search_opts.items(): @@ -1446,7 +1447,7 @@ class ShareAPITest(test.TestCase): 'share_server_id': search_opts['share_server_id'], 'share_type_id': search_opts['share_type_id'], 'snapshot_id': search_opts['snapshot_id'], - 'host': search_opts['host'], + 'share_network_id': search_opts['share_network_id'], 'metadata': {'k1': 'v1'}, 'extra_specs': {'k2': 'v2'}, @@ -1454,6 +1455,7 @@ class ShareAPITest(test.TestCase): } if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) + search_opts_expected['host'] = search_opts['host'] share_api.API.get_all.assert_called_once_with( req.environ['manila.context'], sort_key=search_opts['sort_key'], @@ -1503,7 +1505,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': 'fake_share_server_id', 'share_type_id': 'fake_share_type_id', 'snapshot_id': 'fake_snapshot_id', - 'host': 'fake_host', 'share_network_id': 'fake_share_network_id', 'metadata': '%7B%27k1%27%3A+%27v1%27%7D', # serialized k1=v1 'extra_specs': '%7B%27k2%27%3A+%27v2%27%7D', # serialized k2=v2 @@ -1513,6 +1514,8 @@ class ShareAPITest(test.TestCase): 'offset': '1', 'is_public': 'False', } + if use_admin_context: + search_opts['host'] = 'fake_host' # fake_key should be filtered for non-admin url = '/shares/detail?fake_key=fake_value' for k, v in search_opts.items(): @@ -1545,7 +1548,6 @@ class ShareAPITest(test.TestCase): 'share_server_id': search_opts['share_server_id'], 'share_type_id': search_opts['share_type_id'], 'snapshot_id': search_opts['snapshot_id'], - 'host': search_opts['host'], 'share_network_id': search_opts['share_network_id'], 'metadata': {'k1': 'v1'}, 'extra_specs': {'k2': 'v2'}, @@ -1553,6 +1555,7 @@ class ShareAPITest(test.TestCase): } if use_admin_context: search_opts_expected.update({'fake_key': 'fake_value'}) + search_opts_expected['host'] = search_opts['host'] share_api.API.get_all.assert_called_once_with( req.environ['manila.context'], sort_key=search_opts['sort_key'], @@ -1571,8 +1574,9 @@ class ShareAPITest(test.TestCase): shares[1]['share_type_id'], result['shares'][0]['share_type']) self.assertEqual( shares[1]['snapshot_id'], result['shares'][0]['snapshot_id']) - self.assertEqual( - shares[1]['instance']['host'], result['shares'][0]['host']) + if use_admin_context: + self.assertEqual( + shares[1]['instance']['host'], result['shares'][0]['host']) self.assertEqual( shares[1]['instance']['share_network_id'], result['shares'][0]['share_network_id']) @@ -1583,42 +1587,40 @@ class ShareAPITest(test.TestCase): def test_share_list_detail_with_search_opts_by_admin(self): self._share_list_detail_with_search_opts(use_admin_context=True) - def _list_detail_common_expected(self): - return { - 'shares': [ + def _list_detail_common_expected(self, admin=False): + share_dict = { + 'status': 'fakestatus', + 'description': 'displaydesc', + 'export_location': 'fake_location', + 'export_locations': ['fake_location', 'fake_location2'], + 'availability_zone': 'fakeaz', + 'name': 'displayname', + 'share_proto': 'FAKEPROTO', + 'metadata': {}, + 'project_id': 'fakeproject', + 'id': '1', + 'snapshot_id': '2', + 'snapshot_support': True, + 'share_network_id': None, + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'size': 1, + 'share_type': '1', + 'volume_type': '1', + 'is_public': False, + 'links': [ { - 'status': 'fakestatus', - 'description': 'displaydesc', - 'export_location': 'fake_location', - 'export_locations': ['fake_location', 'fake_location2'], - 'availability_zone': 'fakeaz', - 'name': 'displayname', - 'share_proto': 'FAKEPROTO', - 'metadata': {}, - 'project_id': 'fakeproject', - 'host': 'fakehost', - 'id': '1', - 'snapshot_id': '2', - 'snapshot_support': True, - 'share_network_id': None, - 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), - 'size': 1, - 'share_type': '1', - 'volume_type': '1', - 'is_public': False, - 'links': [ - { - 'href': 'http://localhost/v1/fake/shares/1', - 'rel': 'self' - }, - { - 'href': 'http://localhost/fake/shares/1', - 'rel': 'bookmark' - } - ], + 'href': 'http://localhost/v1/fake/shares/1', + 'rel': 'self' + }, + { + 'href': 'http://localhost/fake/shares/1', + 'rel': 'bookmark' } - ] + ], } + if admin: + share_dict['host'] = 'fakehost' + return {'shares': [share_dict]} def _list_detail_test_common(self, req, expected): self.mock_object(share_api.API, 'get_all', @@ -1692,7 +1694,6 @@ class ShareAPITest(test.TestCase): 'metadata': {}, 'project_id': 'fakeproject', 'access_rules_status': 'active', - 'host': 'fakehost', 'id': '1', 'snapshot_id': '2', 'share_network_id': None, diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 5086f18047..125a06a57d 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -249,24 +249,29 @@ class ShareAPITestCase(test.TestCase): ctx, sort_dir='desc', sort_key='created_at', filters={}) self.assertEqual(_FAKE_LIST_OF_ALL_SHARES, shares) - def test_get_all_non_admin_filter_by_share_server(self): + @ddt.data( + ({'share_server_id': 'fake_share_server'}, 'list_by_share_server_id'), + ({'host': 'fake_host'}, 'list_by_host'), + ) + @ddt.unpack + def test_get_all_by_non_admin_using_admin_filter(self, filters, policy): def fake_policy_checker(*args, **kwargs): - if 'list_by_share_server_id' == args[2] and not args[0].is_admin: + if policy == args[2] and not args[0].is_admin: raise exception.NotAuthorized ctx = context.RequestContext('fake_uid', 'fake_pid_1', is_admin=False) - self.mock_object(share_api.policy, 'check_policy', - mock.Mock(side_effect=fake_policy_checker)) + self.mock_object( + share_api.policy, 'check_policy', + mock.Mock(side_effect=fake_policy_checker)) + self.assertRaises( exception.NotAuthorized, - self.api.get_all, - ctx, - {'share_server_id': 'fake'}, - ) + self.api.get_all, ctx, filters) + share_api.policy.check_policy.assert_has_calls([ mock.call(ctx, 'share', 'get_all'), - mock.call(ctx, 'share', 'list_by_share_server_id'), + mock.call(ctx, 'share', policy), ]) def test_get_all_admin_filter_by_share_server_and_all_tenants(self): diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py index 9cf3e69817..c2b8112637 100644 --- a/manila_tempest_tests/tests/api/base.py +++ b/manila_tempest_tests/tests/api/base.py @@ -661,7 +661,8 @@ class BaseSharesTest(test.BaseTestCase): def get_pools_for_replication_domain(self): # Get the list of pools for the replication domain pools = self.admin_client.list_pools(detail=True)['pools'] - instance_host = self.shares[0]['host'] + instance_host = self.admin_client.get_share( + self.shares[0]['id'])['host'] host_pool = [p for p in pools if p['name'] == instance_host][0] rep_domain = host_pool['capabilities']['replication_domain'] pools_in_rep_domain = [p for p in pools if p['capabilities'][ diff --git a/manila_tempest_tests/tests/api/test_replication.py b/manila_tempest_tests/tests/api/test_replication.py index 1a1a7c9167..589f47cfb3 100644 --- a/manila_tempest_tests/tests/api/test_replication.py +++ b/manila_tempest_tests/tests/api/test_replication.py @@ -25,7 +25,7 @@ from manila_tempest_tests.tests.api import base CONF = config.CONF _MIN_SUPPORTED_MICROVERSION = '2.11' SUMMARY_KEYS = ['share_id', 'id', 'replica_state', 'status'] -DETAIL_KEYS = SUMMARY_KEYS + ['availability_zone', 'host', 'updated_at', +DETAIL_KEYS = SUMMARY_KEYS + ['availability_zone', 'updated_at', 'share_network_id', 'created_at'] @@ -201,7 +201,7 @@ class ReplicationTest(base.BaseSharesMixedTest): cleanup_in_class=False) self.shares_v2_client.get_share_replica(share_replica2['id']) - share_replicas = self.shares_v2_client.list_share_replicas( + share_replicas = self.admin_client.list_share_replicas( share_id=self.shares[0]["id"]) replica_host_set = {r['host'] for r in share_replicas} diff --git a/manila_tempest_tests/tests/api/test_replication_negative.py b/manila_tempest_tests/tests/api/test_replication_negative.py index c5ee02102b..21a5ef24cd 100644 --- a/manila_tempest_tests/tests/api/test_replication_negative.py +++ b/manila_tempest_tests/tests/api/test_replication_negative.py @@ -185,7 +185,7 @@ class ReplicationNegativeTest(base.BaseSharesMixedTest): hosts = [p['name'] for p in pools] self.create_share_replica(self.share1["id"], self.replica_zone, cleanup_in_class=False) - share_host = self.share1['host'] + share_host = self.admin_client.get_share(self.share1['id'])['host'] for host in hosts: if host != share_host: diff --git a/manila_tempest_tests/tests/api/test_shares.py b/manila_tempest_tests/tests/api/test_shares.py index 277f76e5f3..a02b5f8bfc 100644 --- a/manila_tempest_tests/tests/api/test_shares.py +++ b/manila_tempest_tests/tests/api/test_shares.py @@ -42,7 +42,7 @@ class SharesNFSTest(base.BaseSharesTest): share = self.create_share(self.protocol) detailed_elements = {'name', 'id', 'availability_zone', 'description', 'project_id', - 'host', 'created_at', 'share_proto', 'metadata', + 'created_at', 'share_proto', 'metadata', 'size', 'snapshot_id', 'share_network_id', 'status', 'share_type', 'volume_type', 'links', 'is_public'} diff --git a/manila_tempest_tests/tests/api/test_shares_actions.py b/manila_tempest_tests/tests/api/test_shares_actions.py index cfebdd31e2..0f59d7a0c6 100644 --- a/manila_tempest_tests/tests/api/test_shares_actions.py +++ b/manila_tempest_tests/tests/api/test_shares_actions.py @@ -88,7 +88,7 @@ class SharesActionsTest(base.BaseSharesTest): "status", "description", "links", "availability_zone", "created_at", "project_id", "volume_type", "share_proto", "name", "snapshot_id", "id", "size", "share_network_id", "metadata", - "host", "snapshot_id", "is_public", + "snapshot_id", "is_public", ] if utils.is_microversion_lt(version, '2.9'): expected_keys.extend(["export_location", "export_locations"]) @@ -196,7 +196,7 @@ class SharesActionsTest(base.BaseSharesTest): "status", "description", "links", "availability_zone", "created_at", "project_id", "volume_type", "share_proto", "name", "snapshot_id", "id", "size", "share_network_id", "metadata", - "host", "snapshot_id", "is_public", "share_type", + "snapshot_id", "is_public", "share_type", ] if utils.is_microversion_lt(version, '2.9'): keys.extend(["export_location", "export_locations"]) @@ -283,19 +283,6 @@ class SharesActionsTest(base.BaseSharesTest): if CONF.share.capability_create_share_from_snapshot_support: self.assertFalse(self.shares[1]['id'] in [s['id'] for s in shares]) - @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) - def test_list_shares_with_detail_filter_by_host(self): - base_share = self.shares_client.get_share(self.shares[0]['id']) - filters = {'host': base_share['host']} - - # list shares - shares = self.shares_client.list_shares_with_detail(params=filters) - - # verify response - self.assertGreater(len(shares), 0) - for share in shares: - self.assertEqual(filters['host'], share['host']) - @tc.attr(base.TAG_POSITIVE, base.TAG_API_WITH_BACKEND) @testtools.skipIf( not CONF.share.multitenancy_enabled, "Only for multitenancy.") @@ -407,7 +394,7 @@ class SharesActionsTest(base.BaseSharesTest): keys = [ "status", "description", "links", "availability_zone", - "created_at", "export_location", "share_proto", "host", + "created_at", "export_location", "share_proto", "name", "snapshot_id", "id", "size", "project_id", "is_public", ] [self.assertIn(key, sh.keys()) for sh in shares for key in keys] diff --git a/manila_tempest_tests/tests/api/test_shares_negative.py b/manila_tempest_tests/tests/api/test_shares_negative.py index 42204e1edd..3229f2f1df 100644 --- a/manila_tempest_tests/tests/api/test_shares_negative.py +++ b/manila_tempest_tests/tests/api/test_shares_negative.py @@ -193,6 +193,12 @@ class SharesAPIOnlyNegativeTest(base.BaseSharesTest): 'fake-host', 'nfs', '/export/path', 'fake-type') + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) + def test_list_by_user_with_host_filter(self): + self.assertRaises(lib_exc.Forbidden, + self.shares_v2_client.list_shares, + params={'host': 'fake_host'}) + @tc.attr(base.TAG_NEGATIVE, base.TAG_API) def test_list_by_share_server_by_user(self): self.assertRaises(lib_exc.Forbidden, diff --git a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py index cd772944c8..835e3259db 100644 --- a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py +++ b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py @@ -312,7 +312,7 @@ class ShareBasicOpsBase(manager.ShareScenarioTest): instance = self.boot_instance(wait_until="BUILD") self.create_share() instance = self.wait_for_active_instance(instance["id"]) - self.share = self.shares_client.get_share(self.share['id']) + self.share = self.shares_admin_v2_client.get_share(self.share['id']) default_type = self.shares_v2_client.list_share_types( default=True)['share_type'] diff --git a/releasenotes/notes/remove-host-field-from-shares-and-replicas-a087f85bc4a4ba45.yaml b/releasenotes/notes/remove-host-field-from-shares-and-replicas-a087f85bc4a4ba45.yaml new file mode 100644 index 0000000000..f62002608e --- /dev/null +++ b/releasenotes/notes/remove-host-field-from-shares-and-replicas-a087f85bc4a4ba45.yaml @@ -0,0 +1,10 @@ +--- +critical: + - The "host" field is no longer returned in the JSON response of the /shares + and /share-replicas APIs when these APIs are invoked with non-admin + privileges. Applications that depend on this field must be updated as + necessary. The value of this field is privileged information and the + request context must specify administrator privileges when using these + APIs for the "host" field to be present. The use of "host" as a filter + key in the GET /shares API is controlled with the policy "list_by_host". + This policy defaults to "rule:admin_api".