Make DB migration creating indexes in RBACs conditional

This patch makes conditional the existing DB migration that adds
the new indexes "target_tenant" and "action" in the "*rbacs" tables.
The rationale of this patch is to be able to manually improve older
systems by just manually creating the indexes in the database.
Once these indexes are added, those operations including RBACs
checks (all these called from non-admin user to RBAC administrated
resourced) will be improved.

This patch is avoiding the migration issue a system could find if
these indexes have been manually added and then the system is
upgraded. The new check added will first retrieve the table indexes;
if the index is already present, the index addition is skipped.

Closes-Bug: #2020802
Change-Id: I1962fbc844bb67180e9071bcee01f8e95853bdda

doc/source/admin/config-rbac.rst

@@ -804,3 +804,37 @@ IDs.
 If an operator wants to prevent normal users from doing this, the
 ``"create_rbac_policy":`` entry in ``policy.yaml`` can be adjusted
 from ``""`` to ``"rule:admin_only"``.
+
+
+Improve database RBAC query operations
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Since [1]_, present in Yoga version, Neutron has indexes for
+"target_tenant" (now "target_project") and "action" columns in all
+RBAC related tables. That improves the SQL queries involving the
+RBAC tables [2]_. Any system before Yoga won't have these indexes
+but the system administrator can manually add them to the Neutron
+database following the next steps:
+
+* Find the RBAC tables:
+
+.. code-block:: console
+
+    $ tables=`mysql -e "use ovs_neutron; show tables;" | grep rbac`
+
+
+* Insert the indexes for the "target_tenant" and "action" columns:
+
+    $ for table in $tables do; mysql -e \
+        "alter table $table add key (action); alter table $table add key (target_tenant);"; done
+
+
+In order to prevent errors during a system upgrade, [3]_ was
+implemented and backported up to Yoga. This patch checks if any index
+is already present in the Neutron tables and avoids executing the
+index creation command again.
+
+
+.. [1] https://review.opendev.org/c/openstack/neutron/+/810072
+.. [2] https://github.com/openstack/neutron-lib/blob/890d62a3df3f35bb18bf1a11e79a9e97e7dd2d2c/neutron_lib/db/model_query.py#L123-L131
+.. [3] https://review.opendev.org/c/openstack/neutron/+/884617

neutron/db/migration/alembic_migrations/versions/yoga/expand/ba859d649675_add_indexes_to_rbacs.py

@@ -14,6 +14,7 @@
 #
 
 from alembic import op
+import sqlalchemy as sa
 
 
 """Add indexes to RBACs
@@ -31,6 +32,27 @@ down_revision = 'c181bb1d89e4'
 OBJECTS = ('network', 'qospolicy', 'securitygroup', 'addressscope',
            'subnetpool', 'addressgroup')
 COLUMNS = ('target_tenant', 'action')
+_INSPECTOR = None
+
+
+def get_inspector():
+    global _INSPECTOR
+    if _INSPECTOR:
+        return _INSPECTOR
+    else:
+        _INSPECTOR = sa.inspect(op.get_bind())
+
+    return _INSPECTOR
+
+
+def has_index(table, column):
+    """Check if the table has an index *using only* the column name provided"""
+    inspector = get_inspector()
+    table_indexes = inspector.get_indexes(table)
+    for index in table_indexes:
+        if [column] == index['column_names']:
+            return True
+    return False
 
 
 def upgrade():
@@ -38,4 +60,6 @@ def upgrade():
         table = object + 'rbacs'
         ix = 'ix_' + table + '_'
         for column in COLUMNS:
-            op.create_index(op.f(ix + column), table, [column], unique=False)
+            if not has_index(table, column):
+                op.create_index(op.f(ix + column), table, [column],
+                                unique=False)

neutron/tests/functional/db/migrations/test_ba859d649675_add_indexes_to_rbacs.py

@@ -0,0 +1,55 @@
+# Copyright 2017 OpenStack Foundation
+#
+#    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_db.sqlalchemy import utils as db_utils
+
+from neutron.db.migration.alembic_migrations.versions.yoga.expand import \
+    ba859d649675_add_indexes_to_rbacs as _migration
+from neutron.tests.functional.db import test_migrations
+
+
+class TestAddIndexesToRbacsMixin(object):
+    """Validates binding_index for NetworkDhcpAgentBinding migration."""
+
+    @staticmethod
+    def get_index(table_indexes, column):
+        for index in table_indexes:
+            if [column] == index['column_names']:
+                return True
+        return False
+
+    def _pre_upgrade_ba859d649675(self, engine):
+        for table in _migration.OBJECTS:
+            table_indexes = db_utils.get_indexes(engine, table + 'rbacs')
+            for column in _migration.COLUMNS:
+                self.assertFalse(self.get_index(table_indexes, column))
+
+    def _check_ba859d649675(self, engine, data):
+        for table in _migration.OBJECTS:
+            table_indexes = db_utils.get_indexes(engine, table + 'rbacs')
+            for column in _migration.COLUMNS:
+                self.assertTrue(self.get_index(table_indexes, column))
+
+
+class TestAddIndexesToRbacsMySQL(
+        TestAddIndexesToRbacsMixin,
+        test_migrations.TestWalkMigrationsMySQL):
+    pass
+
+
+class TestAddIndexesToRbacsPostgreSQL(
+        TestAddIndexesToRbacsMixin,
+        test_migrations.TestWalkMigrationsPostgreSQL):
+    pass