Correctly handle integrity errors on MySQL 8.x

We attempt to parse the 'DBDuplicateEntry' exceptions raised by oslo.db
in the event of an integrity error to provide more useful information to
the user about what they did wrong. To do that, we're looking at the
'columns' attribute of this exception, which should list the conflicting
column(s), and extracting entries from the user-provided data based on
these columns names. However, on MySQL 8.x this does not return a column
name but rather a constraint name. This results in a KeyError when we
attempt to show the offending data by pulling it out by "column" name.
For example:

  KeyError: 'resource_providers.uniq_resource_providers0name'

The solution is simple. Since there are only two unique constraints for
the 'ResourceProvider' model, we can simply check for these and map them
to their corresponding columns if found. The existing code can be
retained as a fallback for other database backends and legacy MySQL
versions.

Change-Id: I1dda02d46cb019eedd6e3ef64e327ac85bb7c484
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Story: 2008750
Task: 42110

placement/handlers/resource_provider.py

@@ -105,11 +105,26 @@ def create_resource_provider(req):
         # Whether exc.columns has one or two entries (in the event
         # of both fields being duplicates) appears to be database
         # dependent, so going with the complete solution here.
-        duplicate = ', '.join(
-            ['%s: %s' % (column, data[column]) for column in exc.columns])
+        duplicates = []
+        for column in exc.columns:
+            # For MySQL, this is error 1062:
+            #
+            #   Duplicate entry '%s' for key %d
+            #
+            # The 'key' value is captured in 'DBDuplicateEntry.columns'.
+            # Despite the name, this isn't always a column name. While MySQL
+            # 5.x does indeed use the name of the column, 8.x uses the name of
+            # the constraint. oslo.db should probably fix this, but until that
+            # happens we need to handle both cases
+            if column == 'uniq_resource_providers0uuid':
+                duplicates.append(f'uuid: {data["uuid"]}')
+            elif column == 'uniq_resource_providers0name':
+                duplicates.append(f'name: {data["name"]}')
+            else:
+                duplicates.append(f'{column}: {data[column]}')
         raise webob.exc.HTTPConflict(
             'Conflicting resource provider %(duplicate)s already exists.' %
-            {'duplicate': duplicate},
+            {'duplicate': ', '.join(duplicates)},
             comment=errors.DUPLICATE_NAME)
     except exception.ObjectActionError as exc:
         raise webob.exc.HTTPBadRequest(

placement/tests/unit/handlers/test_resource_provider.py

@@ -0,0 +1,75 @@
+# 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.
+
+"""
+Unit tests for code in the resource provider handler that gabbi isn't covering.
+"""
+
+from unittest import mock
+
+import microversion_parse
+from oslo_db import exception as db_exc
+import webob
+
+from placement import context
+from placement.handlers import resource_provider
+from placement.tests.unit import base
+
+
+class TestAggregateHandlerErrors(base.ContextTestCase):
+
+    @mock.patch('placement.context.RequestContext.can', new=mock.Mock())
+    def _test_duplicate_error_parsing_mysql(self, key):
+        fake_context = context.RequestContext(
+            user_id='fake', project_id='fake')
+
+        req = webob.Request.blank(
+            '/resource_providers',
+            method='POST',
+            content_type='application/json')
+        req.body = b'{"name": "foobar"}'
+        req.environ['placement.context'] = fake_context
+
+        parse_version = microversion_parse.parse_version_string
+        microversion = parse_version('1.15')
+        microversion.max_version = parse_version('9.99')
+        microversion.min_version = parse_version('1.0')
+        req.environ['placement.microversion'] = microversion
+
+        with mock.patch(
+            'placement.objects.resource_provider.ResourceProvider.create',
+            side_effect=db_exc.DBDuplicateEntry(columns=[key]),
+        ):
+            response = req.get_response(
+                resource_provider.create_resource_provider)
+
+        self.assertEqual('409 Conflict', response.status)
+        self.assertIn(
+            'Conflicting resource provider name: foobar already exists.',
+            response.text)
+
+    def test_duplicate_error_parsing_mysql_5x(self):
+        """Ensure we parse the correct column on MySQL 5.x.
+
+        On MySQL 5.x, DBDuplicateEntry.columns will contain the name of the
+        column causing the integrity error.
+        """
+        self._test_duplicate_error_parsing_mysql('name')
+
+    def test_duplicate_error_parsing_mysql_8x(self):
+        """Ensure we parse the correct column on MySQL 5.x.
+
+        On MySQL 5.x, DBDuplicateEntry.columns will contain the name of the
+        constraint causing the integrity error.
+        """
+        self._test_duplicate_error_parsing_mysql(
+            'uniq_resource_providers0name')