From d8e9daafe8018a92fbd87f3b71defb9dfde29b66 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 26 Mar 2020 15:37:40 +0000 Subject: [PATCH] api: Add microversion for extra spec validation Enable support for API-based extra spec validation. Since most of the hard work has been done in previous patches, all that's necessary here is to wire up the microversion handling and turn things on. Part of blueprint flavor-extra-spec-validators Change-Id: If67f0d924ea372746a6dc440ea7bdc655e4f0bea Signed-off-by: Stephen Finucane --- .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 7 +- .../openstack/compute/flavors_extraspecs.py | 6 +- .../compute/rest_api_version_history.rst | 14 ++ nova/tests/functional/api/client.py | 28 +++- .../functional/test_flavor_extraspecs.py | 143 ++++++++++++++++++ .../compute/test_flavors_extra_specs.py | 15 +- ...xtra-spec-validators-76d1f2e52ba753db.yaml | 6 + 9 files changed, 204 insertions(+), 19 deletions(-) create mode 100644 nova/tests/functional/test_flavor_extraspecs.py create mode 100644 releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index 2d1760aa312f..7e05eed56d5e 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.85", + "version": "2.86", "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 d0a29d7ff181..3555b6b72073 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.85", + "version": "2.86", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index b6e354e32caf..ac187e6d5ffa 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -231,6 +231,9 @@ REST_API_VERSION_HISTORY = """REST API Version History: ``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` which supports specifying the ``delete_on_termination`` field in the request body to change the attached volume's flag. + * 2.86 - Add support for validation of known extra specs to the + ``POST /flavors/{flavor_id}/os-extra_specs`` and + ``PUT /flavors/{flavor_id}/os-extra_specs/{id}`` APIs. """ # The minimum and maximum versions of the API supported @@ -238,8 +241,8 @@ REST_API_VERSION_HISTORY = """REST API Version History: # minimum version of the API supported. # 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.85" +_MIN_API_VERSION = '2.1' +_MAX_API_VERSION = '2.86' 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/flavors_extraspecs.py b/nova/api/openstack/compute/flavors_extraspecs.py index cabae5c6e38d..0bfe07abe409 100644 --- a/nova/api/openstack/compute/flavors_extraspecs.py +++ b/nova/api/openstack/compute/flavors_extraspecs.py @@ -16,6 +16,7 @@ import six import webob +from nova.api.openstack import api_version_request from nova.api.openstack import common from nova.api.openstack.compute.schemas import flavors_extraspecs from nova.api.openstack import wsgi @@ -35,8 +36,9 @@ class FlavorExtraSpecsController(wsgi.Controller): return dict(extra_specs=flavor.extra_specs) def _check_extra_specs_value(self, req, specs): - # TODO(stephenfin): Wire this up to check the API microversion - validation_supported = False + validation_supported = api_version_request.is_supported( + req, min_version='2.86', + ) for name, value in specs.items(): # NOTE(gmann): Max length for numeric value is being checked diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index cb51613d6a6a..5168890b2346 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -1120,3 +1120,17 @@ server with status ``ERROR``. Adds the ability to specify ``delete_on_termination`` in the ``PUT /servers/{server_id}/os-volume_attachments/{volume_id}`` API, which allows changing the behavior of volume deletion on instance deletion. + +2.86 +---- + +Add support for validation of known extra specs. This is enabled by default +for the following APIs: + +* ``POST /flavors/{flavor_id}/os-extra_specs`` +* ``PUT /flavors/{flavor_id}/os-extra_specs/{id}`` + +Validation is only used for recognized extra spec namespaces, namely: +``accel``, ``aggregate_instance_extra_specs``, ``capabilities``, ``hw``, +``hw_rng``, ``hw_video``, ``os``, ``pci_passthrough``, ``powervm``, ``quota``, +``resources``, ``trait``, and ``vmware``. diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py index dd38a9275060..df7f9a8ea38e 100644 --- a/nova/tests/functional/api/client.py +++ b/nova/tests/functional/api/client.py @@ -314,9 +314,31 @@ class TestOpenStackClient(object): def delete_flavor(self, flavor_id): return self.api_delete('/flavors/%s' % flavor_id) - def post_extra_spec(self, flavor_id, spec): - return self.api_post('/flavors/%s/os-extra_specs' % - flavor_id, spec) + def get_extra_specs(self, flavor_id): + return self.api_get( + '/flavors/%s/os-extra_specs' % flavor_id + ).body['extra_specs'] + + def get_extra_spec(self, flavor_id, spec_id): + return self.api_get( + '/flavors/%s/os-extra_specs/%s' % (flavor_id, spec_id), + ).body + + def post_extra_spec(self, flavor_id, body, **_params): + url = '/flavors/%s/os-extra_specs' % flavor_id + if _params: + query_string = '?%s' % parse.urlencode(list(_params.items())) + url += query_string + + return self.api_post(url, body) + + def put_extra_spec(self, flavor_id, spec_id, body, **_params): + url = '/flavors/%s/os-extra_specs/%s' % (flavor_id, spec_id) + if _params: + query_string = '?%s' % parse.urlencode(list(_params.items())) + url += query_string + + return self.api_put(url, body) def get_volume(self, volume_id): return self.api_get('/os-volumes/%s' % volume_id).body['volume'] diff --git a/nova/tests/functional/test_flavor_extraspecs.py b/nova/tests/functional/test_flavor_extraspecs.py new file mode 100644 index 000000000000..cb8afe9a9db5 --- /dev/null +++ b/nova/tests/functional/test_flavor_extraspecs.py @@ -0,0 +1,143 @@ +# Copyright 2020, Red Hat, Inc. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Tests for os-extra_specs API.""" + +import testtools + +from nova.tests.functional.api import client as api_client +from nova.tests.functional import integrated_helpers + + +class FlavorExtraSpecsTest(integrated_helpers._IntegratedTestBase): + api_major_version = 'v2' + + def setUp(self): + super(FlavorExtraSpecsTest, self).setUp() + self.flavor_id = self._create_flavor() + + def test_create(self): + """Test creating flavor extra specs with valid specs.""" + body = { + 'extra_specs': {'hw:numa_nodes': '1', 'hw:cpu_policy': 'shared'}, + } + self.admin_api.post_extra_spec(self.flavor_id, body) + self.assertEqual( + body['extra_specs'], self.admin_api.get_extra_specs(self.flavor_id) + ) + + def test_create_invalid_spec(self): + """Test creating flavor extra specs with invalid specs. + + This should pass because validation is not enabled in this API + microversion. + """ + body = {'extra_specs': {'hw:numa_nodes': '1', 'foo': 'bar'}} + self.admin_api.post_extra_spec(self.flavor_id, body) + self.assertEqual( + body['extra_specs'], self.admin_api.get_extra_specs(self.flavor_id) + ) + + def test_update(self): + """Test updating extra specs with valid specs.""" + spec_id = 'hw:numa_nodes' + body = {'hw:numa_nodes': '1'} + self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) + self.assertEqual( + body, self.admin_api.get_extra_spec(self.flavor_id, spec_id) + ) + + def test_update_invalid_spec(self): + """Test updating extra specs with invalid specs. + + This should pass because validation is not enabled in this API + microversion. + """ + spec_id = 'foo:bar' + body = {'foo:bar': 'baz'} + self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) + self.assertEqual( + body, self.admin_api.get_extra_spec(self.flavor_id, spec_id) + ) + + +class FlavorExtraSpecsV286Test(FlavorExtraSpecsTest): + api_major_version = 'v2.1' + microversion = '2.86' + + def test_create_invalid_spec(self): + """Test creating extra specs with invalid specs.""" + body = {'extra_specs': {'hw:numa_nodes': 'foo', 'foo': 'bar'}} + + # this should fail because 'foo' is not a suitable value for + # 'hw:numa_nodes' + with testtools.ExpectedException( + api_client.OpenStackApiException + ) as exc: + self.admin_api.post_extra_spec(self.flavor_id, body) + self.assertEqual(400, exc.response.status_code) + + # ...and the extra specs should not be saved + self.assertEqual({}, self.admin_api.get_extra_specs(self.flavor_id)) + + def test_create_unknown_spec(self): + """Test creating extra specs with unknown specs.""" + body = {'extra_specs': {'hw:numa_nodes': '1', 'foo': 'bar'}} + + # this should pass because we don't recognize the extra spec but it's + # not in a namespace we care about + self.admin_api.post_extra_spec(self.flavor_id, body) + + body = {'extra_specs': {'hw:numa_nodes': '1', 'hw:foo': 'bar'}} + + # ...but this should fail because we do recognize the namespace + with testtools.ExpectedException( + api_client.OpenStackApiException + ) as exc: + self.admin_api.post_extra_spec(self.flavor_id, body) + self.assertEqual(400, exc.response.status_code) + + def test_update_invalid_spec(self): + """Test updating extra specs with invalid specs.""" + spec_id = 'hw:foo' + body = {'hw:foo': 'bar'} + + # this should fail because we don't recognize the extra spec + with testtools.ExpectedException( + api_client.OpenStackApiException + ) as exc: + self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) + self.assertEqual(400, exc.response.status_code) + + spec_id = 'hw:numa_nodes' + body = {'hw:numa_nodes': 'foo'} + + # ...while this should fail because the value is not valid + with testtools.ExpectedException( + api_client.OpenStackApiException + ) as exc: + self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) + self.assertEqual(400, exc.response.status_code) + + # ...and neither extra spec should be saved + self.assertEqual({}, self.admin_api.get_extra_specs(self.flavor_id)) + + def test_update_unknown_spec(self): + """Test updating extra specs with unknown specs.""" + spec_id = 'foo:bar' + body = {'foo:bar': 'baz'} + + # this should pass because we don't recognize the extra spec but it's + # not in a namespace we care about + self.admin_api.put_extra_spec(self.flavor_id, spec_id, body) diff --git a/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py b/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py index e9386211cb87..d973e1d08011 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py +++ b/nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py @@ -15,7 +15,6 @@ import mock import testtools -import unittest import webob from nova.api.openstack.compute import flavors_extraspecs \ @@ -266,8 +265,6 @@ class FlavorsExtraSpecsTestV21(test.TestCase): self.assertRaises(self.bad_request, self.controller.create, req, 1, body=body) - # TODO(stephenfin): Wire the microversion up - @unittest.expectedFailure def test_create_invalid_known_namespace(self): """Test behavior of validator with specs from known namespace.""" invalid_specs = { @@ -279,7 +276,7 @@ class FlavorsExtraSpecsTestV21(test.TestCase): for key, value in invalid_specs.items(): body = {'extra_specs': {key: value}} req = self._get_request( - '1/os-extra_specs', use_admin_context=True, version='2.82', + '1/os-extra_specs', use_admin_context=True, version='2.86', ) with testtools.ExpectedException( self.bad_request, 'Validation failed; .*' @@ -296,7 +293,7 @@ class FlavorsExtraSpecsTestV21(test.TestCase): for key, value in unknown_specs.items(): body = {'extra_specs': {key: value}} req = self._get_request( - '1/os-extra_specs', use_admin_context=True, version='2.82', + '1/os-extra_specs', use_admin_context=True, version='2.86', ) self.controller.create(req, 1, body=body) @@ -403,8 +400,6 @@ class FlavorsExtraSpecsTestV21(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update, req, 1, 'hw:numa_nodes', body=body) - # TODO(stephenfin): Wire the microversion up - @unittest.expectedFailure def test_update_invalid_specs_known_namespace(self): """Test behavior of validator with specs from known namespace.""" invalid_specs = { @@ -417,7 +412,7 @@ class FlavorsExtraSpecsTestV21(test.TestCase): body = {key: value} req = self._get_request( '1/os-extra_specs/{key}', - use_admin_context=True, version='2.82', + use_admin_context=True, version='2.86', ) with testtools.ExpectedException( self.bad_request, 'Validation failed; .*' @@ -435,7 +430,7 @@ class FlavorsExtraSpecsTestV21(test.TestCase): body = {key: value} req = self._get_request( f'1/os-extra_specs/{key}', - use_admin_context=True, version='2.82', + use_admin_context=True, version='2.86', ) self.controller.update(req, 1, key, body=body) @@ -452,7 +447,7 @@ class FlavorsExtraSpecsTestV21(test.TestCase): body = {key: value} req = self._get_request( f'1/os-extra_specs/{key}', use_admin_context=True, - version='2.82', + version='2.86', ) res_dict = self.controller.update(req, 1, key, body=body) self.assertEqual(value, res_dict[key]) diff --git a/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml b/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml new file mode 100644 index 000000000000..76c96e755ac2 --- /dev/null +++ b/releasenotes/notes/flavor-extra-spec-validators-76d1f2e52ba753db.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + The 2.86 microversion adds support for flavor extra spec validation when + creating or updating flavor extra specs. Use of an unrecognized or invalid + flavor extra spec will result in a HTTP 400 response.