Validate transport_url in nova-manage cell_v2 commands

In the three commands that take a --transport-url option,
or reads it from config, this will validate the tranport URL is
valid by calling the parsing code in oslo.messaging and fail
if the URL does not parse correctly.

Change-Id: If60cdf697cab2f035cd22830303f5ecaba0f3969
Closes-Bug: #1770341

doc/source/cli/nova-manage.rst

@@ -142,8 +142,8 @@ Nova Cells v2
     specified, it will use the one defined by ``[DEFAULT]/transport_url``
     in the configuration file. Returns 0 if setup is completed
     (or has already been done), 1 if no hosts are reporting (and cannot be
-    mapped), 1 if the transport url is missing, and 2 if run in a cells v1
-    environment.
+    mapped), 1 if the transport url is missing or invalid, and 2 if run in a
+    cells v1 environment.
 
 ``nova-manage cell_v2 map_cell0 [--database_connection <database_connection>]``
     Create a cell mapping to the database connection for the cell0 database.
@@ -177,7 +177,7 @@ Nova Cells v2
     use the one defined by ``[DEFAULT]/transport_url`` in the configuration
     file. This command is idempotent (can be run multiple times), and the
     verbose option will print out the resulting cell mapping uuid. Returns 0
-    on successful completion, and 1 if the transport url is missing.
+    on successful completion, and 1 if the transport url is missing or invalid.
 
 ``nova-manage cell_v2 verify_instance --uuid <instance_uuid> [--quiet]``
     Verify instance mapping to a cell. This command is useful to determine if
@@ -204,7 +204,8 @@ Nova Cells v2
     explained below:
 
     * Returns 0 if the cell mapping was successfully created.
-    * Returns 1 if the transport url or database connection was missing.
+    * Returns 1 if the transport url or database connection was missing
+      or invalid.
     * Returns 2 if another cell is already using that transport url and/or
       database connection combination.
 

nova/cmd/manage.py

@@ -1022,6 +1022,14 @@ def _validate_transport_url(self, transport_url):
         if not transport_url:
             print('Must specify --transport-url if [DEFAULT]/transport_url '
                   'is not set in the configuration file.')
+            return None
+
+        try:
+            messaging.TransportURL.parse(conf=CONF, url=transport_url)
+        except (messaging.InvalidTransportURL, ValueError) as e:
+            print(_('Invalid transport URL: %s') % six.text_type(e))
+            return None
+
         return transport_url
 
     def _non_unique_transport_url_database_connection_checker(self, ctxt,
@@ -1436,11 +1444,10 @@ def status_fn(msg):
     def create_cell(self, name=None, database_connection=None,
                     transport_url=None, verbose=False, disabled=False):
         ctxt = context.get_context()
-        transport_url = transport_url or CONF.transport_url
+        transport_url = self._validate_transport_url(transport_url)
         if not transport_url:
-            print(_('Must specify --transport-url if [DEFAULT]/transport_url '
-                    'is not set in the configuration file.'))
             return 1
+
         database_connection = database_connection or CONF.database.connection
         if not database_connection:
             print(_('Must specify --database_connection '

nova/tests/unit/test_nova_manage.py

@@ -1728,25 +1728,29 @@ def test_discover_hosts_by_service(self, mock_discover_hosts):
                                                     True)
 
     def test_validate_transport_url_in_conf(self):
-        from_conf = 'fake://user:pass@host:port/'
+        from_conf = 'fake://user:pass@host:5672/'
         self.flags(transport_url=from_conf)
         self.assertEqual(from_conf,
                          self.commands._validate_transport_url(None))
 
     def test_validate_transport_url_on_command_line(self):
-        from_cli = 'fake://user:pass@host:port/'
+        from_cli = 'fake://user:pass@host:5672/'
         self.assertEqual(from_cli,
                          self.commands._validate_transport_url(from_cli))
 
     def test_validate_transport_url_missing(self):
         self.assertIsNone(self.commands._validate_transport_url(None))
 
     def test_validate_transport_url_favors_command_line(self):
-        self.flags(transport_url='fake://user:pass@host:port/')
-        from_cli = 'fake://otheruser:otherpass@otherhost:otherport'
+        self.flags(transport_url='fake://user:pass@host:5672/')
+        from_cli = 'fake://otheruser:otherpass@otherhost:5673'
         self.assertEqual(from_cli,
                          self.commands._validate_transport_url(from_cli))
 
+    def test_validate_transport_url_invalid_url(self):
+        self.assertIsNone(self.commands._validate_transport_url('not-a-url'))
+        self.assertIn('Invalid transport URL', self.output.getvalue())
+
     def test_non_unique_transport_url_database_connection_checker(self):
         ctxt = context.RequestContext()
         cell1 = objects.CellMapping(context=ctxt, uuid=uuidsentinel.cell1,
@@ -1780,7 +1784,7 @@ def test_create_cell_use_params(self):
         ctxt = context.get_context()
         kwargs = dict(
             name='fake-name',
-            transport_url='fake-transport-url',
+            transport_url='http://fake-transport-url',
             database_connection='fake-db-connection')
         status = self.commands.create_cell(verbose=True, **kwargs)
         self.assertEqual(0, status)
@@ -1795,7 +1799,7 @@ def test_create_cell_use_params(self):
 
     def test_create_cell_use_config_values(self):
         settings = dict(
-            transport_url='fake-conf-transport-url',
+            transport_url='http://fake-conf-transport-url',
             database_connection='fake-conf-db-connection')
         self.flags(connection=settings['database_connection'],
                    group='database')
@@ -1814,7 +1818,7 @@ def test_create_cell_use_config_values(self):
     def test_create_cell_failed_if_non_unique(self):
         kwargs = dict(
             name='fake-name',
-            transport_url='fake-transport-url',
+            transport_url='http://fake-transport-url',
             database_connection='fake-db-connection')
         status1 = self.commands.create_cell(verbose=True, **kwargs)
         status2 = self.commands.create_cell(verbose=True, **kwargs)
@@ -1829,15 +1833,15 @@ def test_create_cell_failed_if_no_transport_url(self):
 
     def test_create_cell_failed_if_no_database_connection(self):
         self.flags(connection=None, group='database')
-        status = self.commands.create_cell(transport_url='fake-transport-url')
+        status = self.commands.create_cell(transport_url='http://fake-url')
         self.assertEqual(1, status)
         self.assertIn('--database_connection', self.output.getvalue())
 
     def test_create_cell_pre_disabled(self):
         ctxt = context.get_context()
         kwargs = dict(
             name='fake-name1',
-            transport_url='fake-transport-url1',
+            transport_url='http://fake-transport-url1',
             database_connection='fake-db-connection1')
         status1 = self.commands.create_cell(verbose=True, disabled=True,
                                             **kwargs)