Strip the extra properties out when using legacy v2 compatible middle…

…ware

Due to all the input validation with python code already removed for the V2.1
API. So there have some API code can't process the extra properties correctly.
The most safe way is strip the extra properties out the input body. There have
few APIs have validation for the extra properties in V2 API, after this patch
applied the V2.1 legacy v2 compat mode will return 2xx for those APIs. But we
think it is acceptable.

Co-Authored-By: Ghanshyam Mann<[email protected]>
Change-Id: I48b0f1854bce734f63e40a9f566ea2b7dc9cd82e

nova/api/validation/validators.py

@@ -20,6 +20,7 @@
 import re
 
 import jsonschema
+from jsonschema import exceptions as jsonschema_exc
 import netaddr
 from oslo_utils import timeutils
 from oslo_utils import uuidutils
@@ -74,8 +75,41 @@ def _validate_uri(instance):
                                 require_authority=True)
 
 
-def noop_func(*args, **kwargs):
-    pass
+def _soft_validate_additional_properties(validator, aP, instance, schema):
+    """This validator function is used for legacy v2 compatible mode in v2.1.
+    This will skip all the addtional properties checking but keep check the
+    'patternProperties'. 'patternProperties' is used for metadata API.
+
+    """
+    if not validator.is_type(instance, "object"):
+        return
+
+    properties = schema.get("properties", {})
+    patterns = "|".join(schema.get("patternProperties", {}))
+    extra_properties = set()
+    for prop in instance:
+        if prop not in properties:
+            if patterns:
+                if not re.search(patterns, prop):
+                    extra_properties.add(prop)
+            else:
+                extra_properties.add(prop)
+
+    if not extra_properties:
+        return
+
+    if patterns:
+        error = "Additional properties are not allowed (%s %s unexpected)"
+        if len(extra_properties) == 1:
+            verb = "was"
+        else:
+            verb = "were"
+        yield jsonschema_exc.ValidationError(
+            error % (", ".join(repr(extra) for extra in extra_properties),
+                     verb))
+    else:
+        for prop in extra_properties:
+            del instance[prop]
 
 
 class _SchemaValidator(object):
@@ -97,7 +131,8 @@ def __init__(self, schema, relax_additional_properties=False):
             'maximum': self._validate_maximum,
         }
         if relax_additional_properties:
-            validators['additionalProperties'] = noop_func
+            validators[
+                'additionalProperties'] = _soft_validate_additional_properties
 
         validator_cls = jsonschema.validators.extend(self.validator_org,
                                                      validators)

nova/tests/functional/api/client.py

@@ -331,6 +331,12 @@ def delete_server_volume(self, server_id, attachment_id):
         return self.api_delete('/servers/%s/os-volume_attachments/%s' %
                             (server_id, attachment_id))
 
+    def post_server_metadata(self, server_id, metadata):
+        post_body = {'metadata': {}}
+        post_body['metadata'].update(metadata)
+        return self.api_post('/servers/%s/metadata' % server_id,
+                             post_body).body['metadata']
+
 
 class TestOpenStackClientV3(TestOpenStackClient):
     """Simple OpenStack v3 API Client.

nova/tests/functional/test_legacy_v2_compatible_wrapper.py

@@ -16,11 +16,13 @@
 from nova.api import openstack
 from nova.api.openstack import compute
 from nova.api.openstack import wsgi
+from nova.tests.functional.api import client
 from nova.tests.functional import api_paste_fixture
-from nova.tests.functional import integrated_helpers
+from nova.tests.functional import test_servers
+from nova.tests.unit import fake_network
 
 
-class LegacyV2CompatibleTestBase(integrated_helpers._IntegratedTestBase):
+class LegacyV2CompatibleTestBase(test_servers.ServersTestBase):
     _api_version = 'v2'
 
     def setUp(self):
@@ -44,3 +46,26 @@ def test_request_without_addtional_properties_check(self):
         self.assertNotIn(wsgi.API_VERSION_REQUEST_HEADER, response.headers)
         self.assertNotIn('Vary', response.headers)
         self.assertNotIn('type', response.body["keypair"])
+
+    def test_request_with_pattern_properties_check(self):
+        fake_network.set_stub_network_methods(self.stubs)
+        server = self._build_minimal_create_server_request()
+        post = {'server': server}
+        created_server = self.api.post_server(post)
+        self._wait_for_state_change(created_server, 'BUILD')
+        response = self.api.post_server_metadata(created_server['id'],
+                                                 {'a': 'b'})
+        self.assertEqual(response, {'a': 'b'})
+
+    def test_request_with_pattern_properties_with_avoid_metadata(self):
+        fake_network.set_stub_network_methods(self.stubs)
+        server = self._build_minimal_create_server_request()
+        post = {'server': server}
+        created_server = self.api.post_server(post)
+        exc = self.assertRaises(client.OpenStackApiException,
+                                self.api.post_server_metadata,
+                                created_server['id'],
+                                {'a': 'b',
+                                 'x' * 300: 'y',
+                                 'h' * 300: 'i'})
+        self.assertEqual(exc.response.status_code, 400)

nova/tests/unit/api/openstack/test_legacy_v2_compatible_wrapper.py

@@ -13,11 +13,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from jsonschema import exceptions as jsonschema_exc
 import webob
 import webob.dec
 
 import nova.api.openstack
 from nova.api.openstack import wsgi
+from nova.api.validation import validators
 from nova import test
 
 
@@ -106,3 +108,68 @@ def fake_app(req, *args, **kwargs):
 
         wrapper = nova.api.openstack.LegacyV2CompatibleWrapper(fake_app)
         req.get_response(wrapper)
+
+
+class TestSoftAddtionalPropertiesValidation(test.NoDBTestCase):
+
+    def setUp(self):
+        super(TestSoftAddtionalPropertiesValidation, self).setUp()
+        self.schema = {
+            'type': 'object',
+            'properties': {
+                'foo': {'type': 'string'},
+                'bar': {'type': 'string'}
+             },
+            'additionalProperties': False}
+        self.schema_with_pattern = {
+            'type': 'object',
+            'patternProperties': {
+                '^[a-zA-Z0-9-_:. ]{1,255}$': {'type': 'string'}
+            },
+            'additionalProperties': False}
+
+    def test_strip_extra_properties_out_without_extra_props(self):
+        validator = validators._SchemaValidator(self.schema).validator
+        instance = {'foo': '1'}
+        gen = validators._soft_validate_additional_properties(
+            validator, False, instance, self.schema)
+        self.assertRaises(StopIteration, gen.next)
+        self.assertEqual({'foo': '1'}, instance)
+
+    def test_strip_extra_properties_out_with_extra_props(self):
+        validator = validators._SchemaValidator(self.schema).validator
+        instance = {'foo': '1', 'extra_foo': 'extra'}
+        gen = validators._soft_validate_additional_properties(
+            validator, False, instance, self.schema)
+        self.assertRaises(StopIteration, gen.next)
+        self.assertEqual({'foo': '1'}, instance)
+
+    def test_pattern_properties(self):
+        validator = validators._SchemaValidator(
+            self.schema_with_pattern).validator
+        instance = {'foo': '1'}
+        gen = validators._soft_validate_additional_properties(
+            validator, False, instance, self.schema_with_pattern)
+        self.assertRaises(StopIteration, gen.next)
+
+    def test_pattern_properties_with_invalid_property(self):
+        validator = validators._SchemaValidator(
+            self.schema_with_pattern).validator
+        instance = {'foo': '1', 'b' * 300: 'extra'}
+        gen = validators._soft_validate_additional_properties(
+            validator, False, instance, self.schema_with_pattern)
+        exc = gen.next()
+        self.assertIsInstance(exc,
+                              jsonschema_exc.ValidationError)
+        self.assertIn('was', exc.message)
+
+    def test_pattern_properties_with_multiple_invalid_properties(self):
+        validator = validators._SchemaValidator(
+            self.schema_with_pattern).validator
+        instance = {'foo': '1', 'b' * 300: 'extra', 'c' * 300: 'extra'}
+        gen = validators._soft_validate_additional_properties(
+            validator, False, instance, self.schema_with_pattern)
+        exc = gen.next()
+        self.assertIsInstance(exc,
+                              jsonschema_exc.ValidationError)
+        self.assertIn('were', exc.message)