Merge "db: Handle parameters in DB strings"

nova/db/migration.py

@@ -128,10 +128,15 @@ def db_sync(version=None, database='main', context=None):
 
     repository = _find_migrate_repo(database)
     config = _find_alembic_conf(database)
-    # discard the URL encoded in alembic.ini in favour of the URL configured
-    # for the engine by the database fixtures, casting from
-    # 'sqlalchemy.engine.url.URL' to str in the process
-    config.set_main_option('sqlalchemy.url', str(engine.url))
+    # discard the URL stored in alembic.ini in favour of the URL configured
+    # for the engine, casting from 'sqlalchemy.engine.url.URL' to str in the
+    # process
+    # NOTE(sean-k-mooney): the engine has already url encoded the connection
+    # string using a mix of url encode styles for different parts of the url.
+    # since we are updating the alembic config parser instance we need to
+    # escape '%' to '%%' to account for ConfigParser's string interpolation.
+    url = str(engine.url).replace('%', '%%')
+    config.set_main_option('sqlalchemy.url', url)
 
     # if we're in a deployment where sqlalchemy-migrate is already present,
     # then apply all the updates for that and fake apply the initial alembic

nova/tests/unit/db/test_migration.py

@@ -12,11 +12,17 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import urllib
+
+import fixtures
+import mock
+
 from alembic import command as alembic_api
 from alembic.runtime import migration as alembic_migration
 from migrate import exceptions as migrate_exceptions
 from migrate.versioning import api as migrate_api
-import mock
+
+from oslo_db.sqlalchemy import enginefacade
 
 from nova.db.api import api as api_db_api
 from nova.db.main import api as main_db_api
@@ -25,6 +31,36 @@ from nova import exception
 from nova import test
 
 
+class TestDBURL(test.NoDBTestCase):
+
+    def test_db_sync_with_special_symbols_in_connection_string(self):
+        qargs = 'read_default_group=data with/a+percent_%-and%20symbols!'
+        url = f"sqlite:///:memory:?{qargs}"
+        self.flags(connection=url, group='database')
+        # since the engine.url is immutable it will never get updated
+        # once its created so reusing the engine instance would break
+        # this test.
+        engine = enginefacade.writer.get_engine()
+        self.useFixture(
+            fixtures.MonkeyPatch(
+                'nova.db.migration._get_engine',
+                mock.Mock(return_value=engine)))
+        alembic_config = migration._find_alembic_conf()
+        with mock.patch.object(
+                migration, '_find_alembic_conf', return_value=alembic_config):
+            migration.db_sync()
+        actual = alembic_config.get_main_option('sqlalchemy.url')
+        expected = (
+            "sqlite:///:memory:?read_default_group=data+with%2Fa"
+            "+percent_%25-and+symbols%21"
+        )
+        self.assertEqual(expected, actual)
+        self.assertEqual(
+            urllib.parse.unquote_plus(url),
+            urllib.parse.unquote_plus(actual)
+        )
+
+
 class TestDBSync(test.NoDBTestCase):
 
     def test_db_sync_invalid_databse(self):
@@ -50,6 +86,13 @@ class TestDBSync(test.NoDBTestCase):
         mock_find_conf, mock_is_migrate, mock_is_alembic, mock_init,
         mock_upgrade,
     ):
+
+        # return an encoded URL to mimic sqlalchemy
+        mock_get_engine.return_value.url = (
+           'mysql+pymysql://nova:pass@192.168.24.3/nova?'
+           'read_default_file=%2Fetc%2Fmy.cnf.d%2Fnova.cnf'
+           '&read_default_group=nova'
+       )
         mock_is_migrate.return_value = has_migrate
         mock_is_alembic.return_value = has_alembic
 
@@ -59,7 +102,10 @@ class TestDBSync(test.NoDBTestCase):
         mock_find_repo.assert_called_once_with('main')
         mock_find_conf.assert_called_once_with('main')
         mock_find_conf.return_value.set_main_option.assert_called_once_with(
-            'sqlalchemy.url', str(mock_get_engine.return_value.url),
+            'sqlalchemy.url',
+            'mysql+pymysql://nova:pass@192.168.24.3/nova?'  # ...
+            'read_default_file=%%2Fetc%%2Fmy.cnf.d%%2Fnova.cnf'  # ...
+            '&read_default_group=nova'
         )
         mock_is_migrate.assert_called_once_with(
             mock_get_engine.return_value, mock_find_repo.return_value)