Limit ml2_vlan_allocations.vlan_id value in DB backend

Limit "ml2_vlan_allocations.vlan_id" values stored in the DB by
adding a check constraint in the DB engine. This check will
verify that vlan_id is in the interval [1, 4094].

Change-Id: Ie6453cb7a2ef0c43baf540c49a03079f4c8d3818
Closes-Bug: #1870400

neutron/cmd/upgrade_checks/checks.py

@@ -18,10 +18,12 @@ from neutron_lib.db import model_query
 from oslo_config import cfg
 from oslo_serialization import jsonutils
 from oslo_upgradecheck import upgradecheck
+from sqlalchemy import or_
 
 from neutron._i18n import _
 from neutron.cmd.upgrade_checks import base
 from neutron.db.models import agent as agent_model
+from neutron.db.models.plugins.ml2 import vlanallocation
 from neutron.db import models_v2
 
 
@@ -71,6 +73,15 @@ def get_ovn_db_revisions():
         "SELECT version_num from %s;" % OVN_ALEMBIC_TABLE_NAME)]  # nosec
 
 
+def count_vlan_allocations_invalid_segmentation_id():
+    ctx = context.get_admin_context()
+    query = ctx.session.query(vlanallocation.VlanAllocation)
+    query = query.filter(or_(
+        vlanallocation.VlanAllocation.vlan_id < constants.MIN_VLAN_TAG,
+        vlanallocation.VlanAllocation.vlan_id > constants.MAX_VLAN_TAG))
+    return query.count()
+
+
 class CoreChecks(base.BaseChecks):
 
     def get_checks(self):
@@ -83,7 +94,9 @@ class CoreChecks(base.BaseChecks):
             (_("Networking-ovn database revision"),
              self.ovn_db_revision_check),
             (_("NIC Switch agent check kernel"),
-             self.nic_switch_agent_min_kernel_check)
+             self.nic_switch_agent_min_kernel_check),
+            (_("VLAN allocations valid segmentation ID check"),
+             self.vlan_allocations_segid_check),
         ]
 
     @staticmethod
@@ -240,3 +253,31 @@ class CoreChecks(base.BaseChecks):
             return upgradecheck.Result(
                 upgradecheck.Code.SUCCESS,
                 _("No NIC Switch agents detected."))
+
+    @staticmethod
+    def vlan_allocations_segid_check(checker):
+        """Checks that "ml2_vlan_allocations.vlan_id" has a valid value
+
+        Database register column "ml2_vlan_allocations.vlan_id" must be between
+        1 and 4094.
+        """
+        if not cfg.CONF.database.connection:
+            return upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _("Database connection string is not set. Check for VLAN "
+                  "allocations with invalid segmentation IDs can't be done."))
+
+        count = count_vlan_allocations_invalid_segmentation_id()
+        if count:
+            return upgradecheck.Result(
+                upgradecheck.Code.WARNING,
+                _("There are %(count)s registers in 'ml2_vlan_allocations' "
+                  "table with an invalid segmentation ID. 'vlan_id' must be "
+                  "between %(min_vlan)s and %(max_vlan)s") %
+                {'count': count, 'min_vlan': constants.MIN_VLAN_TAG,
+                 'max_vlan': constants.MAX_VLAN_TAG})
+
+        return upgradecheck.Result(
+            upgradecheck.Code.SUCCESS,
+            _("All 'ml2_vlan_allocations' registers have a valid "
+              "segmentation ID."))

neutron/db/migration/alembic_migrations/versions/EXPAND_HEAD

@@ -1 +1 @@
-d8bdf05313f4
+dfe425060830

neutron/db/migration/alembic_migrations/versions/victoria/expand/dfe425060830_limit_vlan_allocation_id_values.py

@@ -0,0 +1,55 @@
+# Copyright 2020 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 alembic import op
+from neutron_lib import constants
+
+
+"""limit vlan allocation id values
+
+Revision ID: dfe425060830
+Revises: d8bdf05313f4
+Create Date: 2020-04-27 16:58:33.110194
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'dfe425060830'
+down_revision = 'd8bdf05313f4'
+
+
+# NOTE(ralonsoh): "CHECK CONSTRAINT" is a feature heterogeneously implemented
+# in different database engines and versions. For example:
+# - MySQL: since version 8.0.16 (April 2019)
+#   https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-16.html
+# - MariaDB: since version 10.2.1 (July 2016)
+#   https://mariadb.com/kb/en/mariadb-1021-release-notes/
+# - PostgreSQL: since version 9.4 (December 2014)
+#   https://www.postgresql.org/docs/9.4/ddl-constraints.html
+#
+# If the DB engine does not support yet this feature, it will be ignored. The
+# VLAN tag constraint is enforced in the Neutron API. This extra enforcement
+# in the DB engine wants to mimic the limitation imposed in the API side and
+# avoid any spurious modification.
+
+def upgrade():
+    # https://docs.sqlalchemy.org/en/13/core/constraints.html#check-constraint
+    constraint = ('vlan_id>=%(min_vlan)s AND vlan_id<=%(max_vlan)s' %
+                  {'min_vlan': constants.MIN_VLAN_TAG,
+                   'max_vlan': constants.MAX_VLAN_TAG})
+    op.create_check_constraint(
+        constraint_name='check_ml2_vlan_allocations0vlan_id',
+        table_name='ml2_vlan_allocations',
+        condition=constraint)

neutron/db/models/plugins/ml2/vlanallocation.py

@@ -10,10 +10,17 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron_lib import constants
 from neutron_lib.db import model_base
 import sqlalchemy as sa
 
 
+# https://docs.sqlalchemy.org/en/13/core/constraints.html#check-constraint
+VLAN_CONSTRAINT = ('vlan_id>=%(min_vlan)s AND vlan_id<=%(max_vlan)s' %
+                   {'min_vlan': constants.MIN_VLAN_TAG,
+                    'max_vlan': constants.MAX_VLAN_TAG})
+
+
 class VlanAllocation(model_base.BASEV2):
     """Represent allocation state of a vlan_id on a physical network.
 
@@ -32,7 +39,10 @@ class VlanAllocation(model_base.BASEV2):
     __table_args__ = (
         sa.Index('ix_ml2_vlan_allocations_physical_network_allocated',
                  'physical_network', 'allocated'),
-        model_base.BASEV2.__table_args__,)
+        sa.CheckConstraint(sqltext=VLAN_CONSTRAINT,
+                           name='check_ml2_vlan_allocations0vlan_id'),
+        model_base.BASEV2.__table_args__,
+    )
 
     physical_network = sa.Column(sa.String(64), nullable=False,
                                  primary_key=True)

neutron/tests/unit/cmd/upgrade_checks/test_checks.py

@@ -179,3 +179,14 @@ class TestChecks(base.BaseTestCase):
             self.assertEqual(Code.WARNING, result.code)
             self.assertIn('Host A', result.details)
             self.assertIn('Host B', result.details)
+
+    def test_vlan_allocations_segid_check(self):
+        cases = ([0, Code.SUCCESS], [1, Code.WARNING])
+        with mock.patch.object(
+                checks, 'count_vlan_allocations_invalid_segmentation_id') \
+                as mock_count:
+            for count, returned_code in cases:
+                mock_count.return_value = count
+                result = checks.CoreChecks.vlan_allocations_segid_check(
+                    mock.ANY)
+                self.assertEqual(returned_code, result.code)

releasenotes/notes/limit_vlan_allocations_in_db-93083c6c4923403a.yaml

@@ -0,0 +1,8 @@
+---
+upgrade:
+  - |
+    Limit the ML2 VLAN allocations to [1, 4094] values in the database engine.
+    This constraint, enforced in the database engine, could not be supported
+    yet. In this case, it will be ignored. For more information, see the note
+    in
+    ``neutron.db.migration.alembic_migrations.versions.victoria.expand.dfe425060830_limit_vlan_allocation_id_values.py``.