From 3f49bcd2b3138e1bea72f79f38a0cb1f122e2384 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rados=C5=82aw=20Piliszek?= <radoslaw.piliszek@gmail.com>
Date: Wed, 10 Mar 2021 14:42:53 +0000
Subject: [PATCH] Fix versioned schema validation

Makes API microversion bounds respected on the schema validator.
Adds extra tests that check this.

This fixes the new 'enabled' flag for which we used the bounds.
No need to backport currently. No impact on stable.

Change-Id: I57f83cebdc02eee892d6269836566b6d07c609f9
---
 masakari/api/validation/__init__.py        | 12 +--
 masakari/tests/unit/test_api_validation.py | 86 +++++++++++++++++++++-
 2 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/masakari/api/validation/__init__.py b/masakari/api/validation/__init__.py
index eb5a0cc7..1647e7cc 100644
--- a/masakari/api/validation/__init__.py
+++ b/masakari/api/validation/__init__.py
@@ -21,7 +21,7 @@ Request Body validating middleware.
 import functools
 
 from masakari.api import api_version_request as api_version
-from masakari.api.validation import validators
+from masakari.api.validation.validators import _SchemaValidator
 
 
 def schema(request_body_schema, min_version=None, max_version=None):
@@ -45,13 +45,13 @@ def schema(request_body_schema, min_version=None, max_version=None):
             else:
                 ver = args[1].api_version_request
 
-            ver.matches(min_ver, max_ver)
             # Only validate against the schema if it lies within
-            # the version range specified. Note that if both min
-            # and max are not specified the validator will always
+            # the version range specified. Note that, if both min
+            # and max are not specified, the validator will always
             # be run.
-            schema_validator = validators._SchemaValidator(request_body_schema)
-            schema_validator.validate(kwargs['body'])
+            if ver.matches(min_ver, max_ver):
+                schema_validator = _SchemaValidator(request_body_schema)
+                schema_validator.validate(kwargs['body'])
 
             return func(*args, **kwargs)
         return wrapper
diff --git a/masakari/tests/unit/test_api_validation.py b/masakari/tests/unit/test_api_validation.py
index adeeca9f..2e090f4b 100644
--- a/masakari/tests/unit/test_api_validation.py
+++ b/masakari/tests/unit/test_api_validation.py
@@ -28,8 +28,10 @@ from masakari import test
 
 
 class FakeRequest(object):
-    api_version_request = api_version.APIVersionRequest("1.0")
-    environ = {}
+    def __init__(self, version=None):
+        if version is None:
+            version = '1.0'
+        self.api_version_request = api_version.APIVersionRequest(version)
 
 
 class ValidationRegex(test.NoDBTestCase):
@@ -552,3 +554,83 @@ class DatetimeTestCase(APIValidationTestCase):
                          self.post(body={
                              'foo': '2016-01-14T01:00:00Z'}, req=FakeRequest()
                          ))
+
+
+class VersionedApiValidationTestCase(APIValidationTestCase):
+
+    def setUp(self):
+        super(__class__, self).setUp()
+
+        schema_pre13 = {
+            'type': 'object',
+            'properties': {
+                'foo': {
+                    'type': 'string',
+                },
+            },
+            'additionalProperties': False,
+        }
+
+        schema_post13 = {
+            'type': 'object',
+            'properties': {
+                'bar': {
+                    'type': 'boolean',
+                },
+            },
+            'additionalProperties': False,
+        }
+
+        @validation.schema(request_body_schema=schema_pre13,
+                           min_version='1.1',
+                           max_version='1.2')
+        @validation.schema(request_body_schema=schema_post13,
+                           min_version='1.3')
+        def post(req, body):
+            return 'Validation succeeded.'
+
+        self.post = post
+
+    def check_validation_error(self, body, req):
+        try:
+            self.post(body=body, req=req)
+        except exception.ValidationError as ex:
+            self.assertEqual(http.BAD_REQUEST, ex.kwargs['code'])
+        except Exception as ex:
+            self.fail('An unexpected exception happens: %s' % ex)
+        else:
+            self.fail('Any exception does not happen.')
+
+    def test_validate_with_proper_microversions(self):
+        self.assertEqual('Validation succeeded.',
+                         self.post(body={
+                             'foo': 'ahappystring'}, req=FakeRequest('1.1')
+                         ))
+        self.assertEqual('Validation succeeded.',
+                         self.post(body={
+                             'foo': 'ahappystring'}, req=FakeRequest('1.2')
+                         ))
+        self.assertEqual('Validation succeeded.',
+                         self.post(body={
+                             'bar': True}, req=FakeRequest('1.3')
+                         ))
+        self.assertEqual('Validation succeeded.',
+                         self.post(body={
+                             'bar': True}, req=FakeRequest('1.10')
+                         ))
+        self.assertEqual('Validation succeeded.',
+                         self.post(body={
+                             'whatever': None}, req=FakeRequest('1.0')
+                         ))
+
+    def test_validate_with_improper_microversions(self):
+        self.check_validation_error(body={'bar': False},
+                                    req=FakeRequest('1.1'))
+        self.check_validation_error(body={'bar': False},
+                                    req=FakeRequest('1.2'))
+        self.check_validation_error(body={'foo': 'asadstring'},
+                                    req=FakeRequest('1.3'))
+        self.check_validation_error(body={'foo': 'asadstring'},
+                                    req=FakeRequest('1.10'))
+        self.check_validation_error(body={'foo': 'asadstring'},
+                                    req=FakeRequest('2.0'))