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 <sfinucan@redhat.com>

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"
     }

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"
         }

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

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

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``.

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']

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)

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])

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.