From 5b0134ea95c0b94f1e9c9f2a483b16523266c8c7 Mon Sep 17 00:00:00 2001
From: Stephen Finucane <stephenfin@redhat.com>
Date: Fri, 21 Mar 2025 14:27:44 +0000
Subject: [PATCH] conf: Add '[api] response_validation' option

It doesn't make sense to return a HTTP 500 in production and after
everything has actually been processed just because schema validation
failed. Instead, we should only do this in test environments. Make it
so via a new config option which is now set by default in unit and
integration tests.

Change-Id: I3cac41e024d569dfe05f21767d90d585f54e3eac
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
---
 devstack/lib/ironic                           |  1 +
 ironic/api/validation/__init__.py             | 32 +++++++++++++----
 ironic/conf/api.py                            | 35 +++++++++++++++++++
 ironic/tests/base.py                          |  1 +
 .../api-validation-eface4a013c58a70.yaml      |  6 ++++
 5 files changed, 68 insertions(+), 7 deletions(-)
 create mode 100644 releasenotes/notes/api-validation-eface4a013c58a70.yaml

diff --git a/devstack/lib/ironic b/devstack/lib/ironic
index dbbc945028..c6f9aaff54 100644
--- a/devstack/lib/ironic
+++ b/devstack/lib/ironic
@@ -1685,6 +1685,7 @@ function configure_ironic {
     rm -f $IRONIC_CONF_FILE
 
     iniset $IRONIC_CONF_FILE DEFAULT debug True
+    iniset $IRONIC_CONF_FILE api response_validation error
     inicomment $IRONIC_CONF_FILE DEFAULT log_file
     iniset $IRONIC_CONF_FILE database connection `database_connection_url ironic`
     iniset $IRONIC_CONF_FILE DEFAULT state_path $IRONIC_STATE_PATH
diff --git a/ironic/api/validation/__init__.py b/ironic/api/validation/__init__.py
index b909356c83..91647ecc92 100644
--- a/ironic/api/validation/__init__.py
+++ b/ironic/api/validation/__init__.py
@@ -16,6 +16,9 @@ import functools
 import inspect
 import typing as ty
 
+import jsonschema.exceptions
+from oslo_config import cfg
+from oslo_log import log
 from oslo_serialization import jsonutils
 from webob import exc as webob_exc
 
@@ -23,6 +26,9 @@ from ironic import api
 from ironic.api.validation import validators
 from ironic.common.i18n import _
 
+CONF = cfg.CONF
+LOG = log.getLogger(__name__)
+
 
 def api_version(
     min_version: ty.Optional[int],
@@ -350,6 +356,11 @@ def response_body_schema(
         def wrapper(*args, **kwargs):
             response = func(*args, **kwargs)
 
+            if CONF.api.response_validation == 'ignore':
+                # don't waste our time checking anything if we're ignoring
+                # schema errors
+                return response
+
             # FIXME(stephenfin): How is ironic/pecan doing jsonification? The
             # below will fail on e.g. date-time fields
 
@@ -362,13 +373,20 @@ def response_body_schema(
             else:
                 body = jsonutils.loads(_body)
 
-            _schema_validator(
-                schema,
-                body,
-                min_version,
-                max_version,
-                is_body=True,
-            )
+            try:
+                _schema_validator(
+                    schema,
+                    body,
+                    min_version,
+                    max_version,
+                    is_body=True,
+                )
+            except jsonschema.exceptions.ValidationError:
+                if CONF.api.response_validation == 'warn':
+                    LOG.exception('Schema failed to validate')
+                else:
+                    raise
+
             return response
 
         if hasattr(func, 'arguments_transformed'):
diff --git a/ironic/conf/api.py b/ironic/conf/api.py
index b04048d1d8..925eaa5632 100644
--- a/ironic/conf/api.py
+++ b/ironic/conf/api.py
@@ -105,6 +105,40 @@ opts = [
                        "during enrollment. Eg: ['bios']")),
 ]
 
+
+validation_opts = [
+    cfg.StrOpt(
+        'response_validation',
+        choices=(
+            (
+                'error',
+                'Raise a HTTP 500 (Server Error) for responses that fail '
+                'response body schema validation',
+            ),
+            (
+                'warn',
+                'Log a warning for responses that fail response body schema '
+                'validation',
+            ),
+            (
+                'ignore',
+                'Ignore response body schema validation failures',
+            ),
+        ),
+        default='warn',
+        help=_("""\
+Configure validation of API responses.
+
+``warn`` is the current recommendation for production environments and
+``error`` should only be used in testing environments.
+
+If you find it necessary to enable the ``ignore`` option, please report the
+issues you are seeing to the Nova team so we can improve our schemas.
+"""),
+    ),
+]
+
+
 opt_group = cfg.OptGroup(name='api',
                          title='Options for the ironic-api service')
 
@@ -112,3 +146,4 @@ opt_group = cfg.OptGroup(name='api',
 def register_opts(conf):
     conf.register_group(opt_group)
     conf.register_opts(opts, group=opt_group)
+    conf.register_opts(validation_opts, group=opt_group)
diff --git a/ironic/tests/base.py b/ironic/tests/base.py
index c7c5570b75..71f1c5daae 100644
--- a/ironic/tests/base.py
+++ b/ironic/tests/base.py
@@ -202,6 +202,7 @@ class TestCase(oslo_test_base.BaseTestCase):
                     group='neutron')
         self.config(enabled_hardware_types=['fake-hardware',
                                             'manual-management'])
+        self.config(response_validation='error', group='api')
         self.config(initial_grub_template=None, group='pxe')
         for iface in drivers_base.ALL_INTERFACES:
             default = None
diff --git a/releasenotes/notes/api-validation-eface4a013c58a70.yaml b/releasenotes/notes/api-validation-eface4a013c58a70.yaml
new file mode 100644
index 0000000000..32fc3fff81
--- /dev/null
+++ b/releasenotes/notes/api-validation-eface4a013c58a70.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    A new configuration option, ``[api] response_validation``, has been added.
+    This allows operators to configure the behavior of ``ironic-api`` when a
+    response fails schema validation