Fix Octavia API HTTP Accept header handling

This patch improves the Octavia API handling of HTTP Accept headers and
ensures the response content type will always be application/json.
Prior to this fix, requests with wildcard Accept headers may have
received a response from the API in a format other than JSON.

Story: 2010447
Task: 46932
Change-Id: Ia557ccb4b9d7576acce308e851ca742624f91d88

api-ref/source/v2/general.inc

@@ -119,13 +119,15 @@ Request format
 --------------
 
 The Octavia API v2 only accepts requests with the JSON data serialization
-format. The ``Content-Type`` header is ignored.
+format. The request must have no ``Accept`` header or an ``Accept`` header that
+is compatible with ``application/json``. The one exception is the Oslo
+middleware healthcheck endpoint.
 
 Response format
 ---------------
 
 The Octavia API v2 always responds with the JSON data serialization
-format. The ``Accept`` header is ignored.
+format. The one exception is the Oslo middleware healthcheck endpoint.
 
 Query extension
     A ``.json`` extension can be added to the request URI. For example, the

octavia/api/common/hooks.py

@@ -11,12 +11,42 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
 from pecan import hooks
+from webob import acceptparse
+from webob import exc
 
 from octavia.api.common import pagination
+from octavia.api.common import utils
 from octavia.common import constants
 from octavia.common import context
+from octavia.i18n import _
+
+_HEALTHCHECK_PATHS = ['/healthcheck', '/load-balancer/healthcheck']
+
+
+class ContentTypeHook(hooks.PecanHook):
+    """Force the request content type to JSON if that is acceptable."""
+
+    def on_route(self, state):
+        # Oslo healthcheck middleware has its own content type handling
+        # so we need to bypass the Octavia content type restrictions.
+        if state.request.path in _HEALTHCHECK_PATHS:
+            return
+        if state.request.accept:
+            best_matches = state.request.accept.acceptable_offers(
+                [constants.APPLICATION_JSON])
+            if not best_matches:
+                # The API reference says we always respond with JSON
+                state.request.accept = acceptparse.create_accept_header(
+                    constants.APPLICATION_JSON)
+                msg = _('Only content type %s is accepted.')
+                raise exc.HTTPNotAcceptable(
+                    msg % constants.APPLICATION_JSON,
+                    json_formatter=utils.json_error_formatter)
+
+        # Force application/json with no other options for the request
+        state.request.accept = acceptparse.create_accept_header(
+            constants.APPLICATION_JSON)
 
 
 class ContextHook(hooks.PecanHook):

octavia/api/common/utils.py

@@ -0,0 +1,44 @@
+#    Copyright 2022 Red Hat
+#
+#    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.
+
+from oslo_middleware import request_id
+import webob
+
+from octavia.common import constants
+
+
+# Inspired by the OpenStack Placement service utils.py
+def json_error_formatter(body, status, title, environ):
+    """A json_formatter for webob exceptions.
+
+    Follows API-WG guidelines at
+    http://specs.openstack.org/openstack/api-wg/guidelines/errors.html
+    """
+    # Clear out the html that webob sneaks in.
+    body = webob.exc.strip_tags(body)
+    # Get status code out of status message. webob's error formatter
+    # only passes entire status string.
+    status_code = int(status.split(None, 1)[0])
+    error_dict = {
+        constants.STATUS: status_code,
+        constants.TITLE: title,
+        constants.DETAIL: body
+    }
+
+    # If the request id middleware has had a chance to add an id,
+    # put it in the error response.
+    if request_id.ENV_REQUEST_ID in environ:
+        error_dict[constants.REQUEST_ID] = environ[request_id.ENV_REQUEST_ID]
+
+    return {constants.ERRORS: [error_dict]}

octavia/api/config.py

@@ -20,6 +20,7 @@ app = {
     'root': 'octavia.api.root_controller.RootController',
     'modules': ['octavia.api'],
     'hooks': [
+        hooks.ContentTypeHook(),
         hooks.ContextHook(),
         hooks.QueryParametersHook()],
     'debug': False

octavia/common/constants.py

@@ -297,6 +297,7 @@ SUPPORTED_TASKFLOW_ENGINE_TYPES = [
                  'non-dependent tasks simultaneously')]
 
 # Task/Flow constants
+ACCEPT = 'accept'
 ACTIVE_CONNECTIONS = 'active_connections'
 ADD_NICS = 'add_nics'
 ADD_SUBNETS = 'add_subnets'
@@ -323,6 +324,7 @@ CA_TLS_CERTIFICATE_ID = 'ca_tls_certificate_id'
 CIDR = 'cidr'
 CLIENT_CA_TLS_CERTIFICATE_ID = 'client_ca_tls_certificate_id'
 CLIENT_CRL_CONTAINER_ID = 'client_crl_container_id'
+CODE = 'code'
 COMPUTE_ID = 'compute_id'
 COMPUTE_OBJ = 'compute_obj'
 COMPUTE_ZONE = 'compute_zone'
@@ -337,8 +339,10 @@ DELTA = 'delta'
 DELTAS = 'deltas'
 DESCRIPTION = 'description'
 DESTINATION = 'destination'
+DETAIL = 'detail'
 DEVICE_OWNER = 'device_owner'
 ENABLED = 'enabled'
+ERRORS = 'errors'
 FAILED_AMP_VRRP_PORT_ID = 'failed_amp_vrrp_port_id'
 FAILED_AMPHORA = 'failed_amphora'
 FAILOVER_AMPHORA = 'failover_amphora'
@@ -410,6 +414,7 @@ REDIRECT_POOL = 'redirect_pool'
 REQ_CONN_TIMEOUT = 'req_conn_timeout'
 REQ_READ_TIMEOUT = 'req_read_timeout'
 REQUEST_ERRORS = 'request_errors'
+REQUEST_ID = 'request_id'
 ROLE = 'role'
 SECURITY_GROUPS = 'security_groups'
 SECURITY_GROUP_RULES = 'security_group_rules'
@@ -425,6 +430,7 @@ SUBNET_ID = 'subnet_id'
 TAGS = 'tags'
 TENANT_ID = 'tenant_id'
 TIMEOUT_DICT = 'timeout_dict'
+TITLE = 'title'
 TLS_CERTIFICATE_ID = 'tls_certificate_id'
 TLS_CONTAINER_ID = 'tls_container_id'
 TOPOLOGY = 'topology'

octavia/tests/functional/api/test_content_types.py

@@ -0,0 +1,135 @@
+#    Copyright 2022 Red Hat
+#
+#    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.
+from unittest import mock
+
+from oslo_config import cfg
+from oslo_config import fixture as oslo_fixture
+import pecan
+
+from octavia.api import config as pconfig
+from octavia.common import constants
+from octavia.tests.functional.db import base as base_db_test
+
+
+class TestContentTypes(base_db_test.OctaviaDBTestBase):
+
+    def setUp(self):
+        super().setUp()
+
+        self.conf = self.useFixture(oslo_fixture.Config(cfg.CONF))
+        # Mock log_opt_values, it prevents the dump of the configuration
+        # with LOG.info for each test. It saves a lot of time when running
+        # the functional tests.
+        self.conf.conf.log_opt_values = mock.MagicMock()
+
+        # Note: we need to set argv=() to stop the wsgi setup_app from
+        # pulling in the testing tool sys.argv
+        self.app = pecan.testing.load_test_app({'app': pconfig.app,
+                                                'wsme': pconfig.wsme},
+                                               argv=())
+
+        def reset_pecan():
+            pecan.set_config({}, overwrite=True)
+
+        self.addCleanup(reset_pecan)
+
+        self.test_url = '/'
+
+    def test_no_accept_header(self):
+        response = self.app.get(self.test_url, status=200, expect_errors=False)
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    # Note: webob will treat invalid content types as no accept header provided
+    def test_bogus_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT: 'bogus'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_valid_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT: constants.APPLICATION_JSON})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_valid_mixed_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT:
+                     'text/html,' + constants.APPLICATION_JSON})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_wildcard_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT: '*/*'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_json_wildcard_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT: constants.APPLICATION_JSON + ', */*'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_json_plain_wildcard_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT: constants.APPLICATION_JSON +
+                     ', text/plain, */*'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_wildcard_mixed_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT:
+                     'text/html,*/*'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_valid_mixed_weighted_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=200, expect_errors=False,
+            headers={constants.ACCEPT:
+                     'text/html,' + constants.APPLICATION_JSON + ';q=0.8'})
+        self.assertEqual(200, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+
+    def test_invalid_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=406, expect_errors=False,
+            headers={constants.ACCEPT: 'application/xml'})
+        self.assertEqual(406, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+        self.assertEqual(406, response.json[constants.CODE])
+        self.assertEqual('Not Acceptable', response.json[constants.TITLE])
+        self.assertEqual('Only content type application/json is accepted.',
+                         response.json[constants.DESCRIPTION])
+
+    def test_invalid_mixed_accept_header(self):
+        response = self.app.get(
+            self.test_url, status=406, expect_errors=False,
+            headers={constants.ACCEPT: 'application/xml,text/html'})
+        self.assertEqual(406, response.status_code)
+        self.assertEqual(constants.APPLICATION_JSON, response.content_type)
+        self.assertEqual(406, response.json[constants.CODE])
+        self.assertEqual('Not Acceptable', response.json[constants.TITLE])
+        self.assertEqual('Only content type application/json is accepted.',
+                         response.json[constants.DESCRIPTION])

octavia/tests/unit/api/test_config.py

@@ -0,0 +1,35 @@
+#    Copyright 2022 Red Hat
+#
+#    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.
+from octavia.api.common import hooks
+from octavia.api.config import app
+from octavia.api.config import wsme
+from octavia.tests.unit import base
+
+
+class TestConfig(base.TestCase):
+
+    def test_app_config(self):
+        self.assertEqual(
+            'octavia.api.root_controller.RootController', app['root'])
+        self.assertEqual(['octavia.api'], app['modules'])
+        expected_hook_types = [
+            hooks.ContentTypeHook,
+            hooks.ContextHook,
+            hooks.QueryParametersHook
+        ]
+        self.assertEqual(expected_hook_types, list(map(type, app['hooks'])))
+        self.assertFalse(app['debug'])
+
+    def test_wsme_config(self):
+        self.assertFalse(wsme['debug'])

releasenotes/notes/EnforceApplicationJSONContentType-65ad696565eac75c.yaml

@@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    The Octavia API will now check that the HTTP Accept header, if present, is
+    compatible with the application/json content type. If not the user will
+    get a 406 status code response, Not Acceptable.
+fixes:
+  - |
+    The Octavia API will now check that the HTTP Accept header, if present, is
+    compatible with the application/json content type. If not the user will
+    get a 406 status code response, Not Acceptable. This change also ensures
+    that the API responses have a content type of application/json.