From 4018d6fb71a4e5bb0554ac36479cb217f55a6fcf Mon Sep 17 00:00:00 2001 From: Victor Coutellier Date: Wed, 8 Jan 2020 21:09:58 +0100 Subject: [PATCH] Non-Admin user can filter their instances by more filters Microversion bump to allow non-admin user to use more filters key when listing instances. In order to stay coherent, all existing instance filters who are related to a field readable by default to non admin users when showing instance details, should be allowed by default without policy modification. Implements: blueprint non-admin-filter-instance-by-az Change-Id: Ia66d3a1ceb74ed521cf44922929b2a502f3ee935 --- api-guide/source/server_concepts.rst | 23 +++++-- api-ref/source/parameters.yaml | 55 +++++++++------- api-ref/source/servers.inc | 12 +++- .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- doc/source/contributor/microversions.rst | 8 +++ nova/api/openstack/api_version_request.py | 4 +- .../compute/rest_api_version_history.rst | 18 ++++++ nova/api/openstack/compute/servers.py | 5 ++ .../api/openstack/compute/test_serversV21.py | 64 +++++++++++++++++++ ...instance-more-filter-ea5abad7c32ff328.yaml | 17 +++++ 11 files changed, 178 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/allow-non-admin-filter-instance-more-filter-ea5abad7c32ff328.yaml diff --git a/api-guide/source/server_concepts.rst b/api-guide/source/server_concepts.rst index 1a42ffc3cf4f..ef7bf49eabd0 100644 --- a/api-guide/source/server_concepts.rst +++ b/api-guide/source/server_concepts.rst @@ -159,6 +159,17 @@ For different user roles, the user has different query options set: - ``tags-any`` (New in version 2.26) - ``changes-before`` (New in version 2.66) - ``locked`` (New in version 2.73) + - ``availability_zone`` (New in version 2.83) + - ``config_drive`` (New in version 2.83) + - ``key_name`` (New in version 2.83) + - ``created_at`` (New in version 2.83) + - ``launched_at`` (New in version 2.83) + - ``terminated_at`` (New in version 2.83) + - ``power_state`` (New in version 2.83) + - ``task_state`` (New in version 2.83) + - ``vm_state`` (New in version 2.83) + - ``progress`` (New in version 2.83) + - ``user_id`` (New in version 2.83) Other options will be ignored by nova silently. @@ -177,12 +188,12 @@ Precondition: there are 2 servers existing in cloud with following info:: "servers": [ { "name": "t1", - "OS-EXT-STS:vm_state": "active", + "OS-EXT-SRV-ATTR:host": "devstack1", ... }, { "name": "t2", - "OS-EXT-STS:vm_state": "stopped", + "OS-EXT-SRV-ATTR:host": "devstack2", ... } ] @@ -190,13 +201,13 @@ Precondition: there are 2 servers existing in cloud with following info:: **Example: General user query server with administrator only options** -Request with non-administrator context: ``GET /servers/detail?vm_state=active`` +Request with non-administrator context: ``GET /servers/detail?host=devstack1`` .. note:: - The ``vm_state`` query parameter is only for administrator users and + The ``host`` query parameter is only for administrator users and the query parameter is ignored if specified by non-administrator users. - Thus the API returns servers of both ``active`` and ``stopped`` + Thus the API returns servers of both ``devstack1`` and ``devstack2`` in this example. Response:: @@ -216,7 +227,7 @@ Response:: **Example: Administrator query server with administrator only options** -Request with administrator context: ``GET /servers/detail?vm_state=active`` +Request with administrator context: ``GET /servers/detail?host=devstack1`` Response:: diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index bc828c85f0c1..fc579791c777 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -417,8 +417,9 @@ availability_zone_query_server: description: | Filter the server list result by server availability zone. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -579,8 +580,9 @@ config_drive_query_server: description: | Filter the server list result by the config drive setting of the server. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -597,8 +599,9 @@ created_at_query_server: For example, ``2015-08-27T09:49:58-05:00``. If you omit the time zone, the UTC time zone is assumed. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -888,8 +891,9 @@ key_name_query_server: description: | Filter the server list result by keypair name. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -942,8 +946,9 @@ launched_at_query_server: For example, ``2015-08-27T09:49:58-05:00``. If you omit the time zone, the UTC time zone is assumed. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -1121,15 +1126,17 @@ power_state_query_server: 6: CRASHED 7: SUSPENDED - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. progress_query_server: description: | Filter the server list result by the progress of the server. The value could be from 0 to 100 as integer. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: integer @@ -1371,8 +1378,9 @@ task_state_query_server: description: | Filter the server list result by task state. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. tenant_id_query: description: | Specify the project ID (tenant ID) to show the rate and absolute limits. @@ -1392,8 +1400,9 @@ terminated_at_query_server: For example, ``2015-08-27T09:49:58-05:00``. If you omit the time zone, the UTC time zone is assumed. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -1439,8 +1448,9 @@ user_id_query_server: description: | Filter the list of servers by the given user ID. - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string @@ -1469,8 +1479,9 @@ vm_state_query_server: - ``STOPPED`` - ``SUSPENDED`` - This parameter is only valid when specified by administrators. - If non-admin users specify this parameter, it is ignored. + This parameter is restricted to administrators until microversion 2.82. + If non-admin users specify this parameter before microversion 2.83, it + is ignored. in: query required: false type: string diff --git a/api-ref/source/servers.inc b/api-ref/source/servers.inc index 6334a96fefba..c6e6560b329e 100644 --- a/api-ref/source/servers.inc +++ b/api-ref/source/servers.inc @@ -173,7 +173,17 @@ whitelist will be silently ignored. - ``tags-any`` (New in version 2.26) - ``changes-before`` (New in version 2.66) - ``locked`` (New in version 2.73) - + - ``availability_zone`` (New in version 2.83) + - ``config_drive`` (New in version 2.83) + - ``key_name`` (New in version 2.83) + - ``created_at`` (New in version 2.83) + - ``launched_at`` (New in version 2.83) + - ``terminated_at`` (New in version 2.83) + - ``power_state`` (New in version 2.83) + - ``task_state`` (New in version 2.83) + - ``vm_state`` (New in version 2.83) + - ``progress`` (New in version 2.83) + - ``user_id`` (New in version 2.83) - For admin user, whitelist includes all filter keys mentioned in :ref:`list-server-request` Section. diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 395eca072e9b..a17f60f5ea4a 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.82", + "version": "2.83", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index 874f5a4afef4..4d05fabc5734 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.82", + "version": "2.83", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/source/contributor/microversions.rst b/doc/source/contributor/microversions.rst index 65d8739b6302..092272152481 100644 --- a/doc/source/contributor/microversions.rst +++ b/doc/source/contributor/microversions.rst @@ -374,9 +374,17 @@ necessary to add changes to other places which describe your change: * If the microversion changes the response schema, a new schema and test for the microversion must be added to Tempest. +* If applicable, add Functional sample tests under + ``nova/tests/functional/api_sample_tests``. Also, add JSON examples to + ``doc/api_samples`` directory which can be generated automatically via tox + env ``api-samples`` or run test with env var ``GENERATE_SAMPLES`` True. + * Update the `API Reference`_ documentation as appropriate. The source is located under `api-ref/source/`. +* If the microversion changes servers related APIs, update the + `api-guide/source/server_concepts.rst` accordingly. + .. _API Reference: https://docs.openstack.org/api-ref/compute/ Allocating a microversion diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index 0ca661d66cdc..a20ef3c13872 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -224,6 +224,8 @@ REST_API_VERSION_HISTORY = """REST API Version History: ``os-server-external-events`` API. This event is sent by Cyborg to indicate completion of ARQ binding. The ARQs can be obtained from Cyborg with ``GET /v2/accelerator_requests?instance={uuid}`` + * 2.83 - Allow more filter parameters for ``GET /servers/detail`` and + ``GET /servers`` for non-admin. """ # The minimum and maximum versions of the API supported @@ -232,7 +234,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: # Note(cyeoh): This only applies for the v2.1 API once microversions # support is fully merged. It does not affect the V2 API. _MIN_API_VERSION = "2.1" -_MAX_API_VERSION = "2.82" +_MAX_API_VERSION = "2.83" DEFAULT_API_VERSION = _MIN_API_VERSION # Almost all proxy APIs which are related to network, images and baremetal diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index 44cd73e65990..817106efb4c3 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -1087,3 +1087,21 @@ Adds support for image cache management by aggregate by adding Adds ``accelerator-request-bound`` event to ``os-server-external-events`` API. This event is sent by Cyborg to indicate completion of the binding event for one accelerator request (ARQ) associated with an instance. + +2.83 +---- + +Allow the following filter parameters for ``GET /servers/detail`` +and ``GET /servers`` for non-admin : + +* ``availability_zone`` +* ``config_drive`` +* ``key_name`` +* ``created_at`` +* ``launched_at`` +* ``terminated_at`` +* ``power_state`` +* ``task_state`` +* ``vm_state`` +* ``progress`` +* ``user_id`` diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 8aad815e33b8..44a9edaeed9f 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -1268,6 +1268,11 @@ class ServersController(wsgi.Controller): opt_list += ('changes-before',) if api_version_request.is_supported(req, min_version='2.73'): opt_list += ('locked',) + if api_version_request.is_supported(req, min_version='2.83'): + opt_list += ('availability_zone', 'config_drive', 'key_name', + 'created_at', 'launched_at', 'terminated_at', + 'power_state', 'task_state', 'vm_state', 'progress', + 'user_id',) return opt_list def _get_instance(self, context, instance_uuid): diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index f7a97822483d..8af543105ecb 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -2739,6 +2739,70 @@ class ServersControllerTestV275(ControllerTest): self.assertIn('OS-EXT-IPS-MAC:mac_addr', item) +class ServersControllerTestV283(ControllerTest): + filters = ['availability_zone', 'config_drive', 'key_name', + 'created_at', 'launched_at', 'terminated_at', + 'power_state', 'task_state', 'vm_state', 'progress', + 'user_id'] + + def test_get_servers_by_new_filter_for_non_admin(self): + def fake_get_all(context, search_opts=None, **kwargs): + self.assertIsNotNone(search_opts) + for f in self.filters: + self.assertIn(f, search_opts) + return objects.InstanceList( + objects=[fakes.stub_instance_obj(100, uuid=uuids.fake)]) + + self.mock_get_all.side_effect = fake_get_all + + query_str = '&'.join('%s=test_value' % f for f in self.filters) + req = fakes.HTTPRequest.blank(self.path_with_query % query_str, + version='2.83') + servers = self.controller.index(req)['servers'] + + self.assertEqual(1, len(servers)) + self.assertEqual(uuids.fake, servers[0]['id']) + + def test_get_servers_new_filters_for_non_admin_old_version(self): + def fake_get_all(context, search_opts=None, **kwargs): + self.assertIsNotNone(search_opts) + for f in self.filters: + self.assertNotIn(f, search_opts) + return objects.InstanceList( + objects=[]) + + # Without policy edition, test will fail and admin filter will work. + self.policy.set_rules({'os_compute_api:servers:index': ''}) + self.mock_get_all.side_effect = fake_get_all + + query_str = '&'.join('%s=test_value' % f for f in self.filters) + req = fakes.HTTPRequest.blank(self.path_with_query % query_str, + version='2.82') + servers = self.controller.index(req)['servers'] + + self.assertEqual(0, len(servers)) + + def test_get_servers_by_node_fail_non_admin(self): + def fake_get_all(context, search_opts=None, **kwargs): + self.assertIsNotNone(search_opts) + self.assertNotIn('node', search_opts) + return objects.InstanceList( + objects=[fakes.stub_instance_obj(100, uuid=uuids.fake)]) + + server_filter_rule = 'os_compute_api:servers:allow_all_filters' + self.policy.set_rules({'os_compute_api:servers:index': '', + server_filter_rule: 'role:admin'}) + self.mock_get_all.side_effect = fake_get_all + + query_str = "node=node1" + req = fakes.HTTPRequest.blank(self.path_with_query % query_str, + version='2.83') + servers = self.controller.index(req)['servers'] + + self.assertEqual(1, len(servers)) + self.assertEqual(uuids.fake, servers[0]['id']) + + class ServersControllerDeleteTest(ControllerTest): def setUp(self): diff --git a/releasenotes/notes/allow-non-admin-filter-instance-more-filter-ea5abad7c32ff328.yaml b/releasenotes/notes/allow-non-admin-filter-instance-more-filter-ea5abad7c32ff328.yaml new file mode 100644 index 000000000000..158fdcb33f14 --- /dev/null +++ b/releasenotes/notes/allow-non-admin-filter-instance-more-filter-ea5abad7c32ff328.yaml @@ -0,0 +1,17 @@ +--- + features: + - | + Allow the following filter parameters for ``GET /servers/detail`` + and ``GET /servers`` for non-admin in microversion 2.83: + + - availability_zone + - config_drive + - key_name + - created_at + - launched_at + - terminated_at + - power_state + - task_state + - vm_state + - progress + - user_id