From fe21d29fa8b02f3e6437f035b0af6c58f8f454bb Mon Sep 17 00:00:00 2001 From: "Alexey I. Froloff" Date: Mon, 11 Jul 2016 16:31:09 +0300 Subject: [PATCH] Properly quote IPv6 address in RsyncDriver When IPv6 address literal is used as host in rsync call, it should be enclosed in square brackets. This is already done for copy_file method outside of driver in changeset Ia5f28673e79158d948980f2b3ce496c6a56882af Create helper function format_remote_path(host, path) and use where appropriate. Closes-Bug: 1601822 Change-Id: Ifc386539f33684fb764f5f638a7ee0a10b1ef534 (cherry picked from commit 270be6906c13bc621a7ad507b8ae729a940609d2) --- nova/tests/unit/test_utils.py | 13 ++++++ .../unit/virt/libvirt/volume/test_remotefs.py | 42 +++++++++++++++++++ nova/utils.py | 14 +++++++ nova/virt/libvirt/volume/remotefs.py | 8 ++-- 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index 83b41a15b3ec..1a3323d3c34e 100644 --- a/nova/tests/unit/test_utils.py +++ b/nova/tests/unit/test_utils.py @@ -218,6 +218,19 @@ class GenericUtilsTestCase(test.NoDBTestCase): "::ffff:127.0.0.1")) self.assertEqual("localhost", utils.safe_ip_format("localhost")) + def test_format_remote_path(self): + self.assertEqual("[::1]:/foo/bar", + utils.format_remote_path("::1", "/foo/bar")) + self.assertEqual("127.0.0.1:/foo/bar", + utils.format_remote_path("127.0.0.1", "/foo/bar")) + self.assertEqual("[::ffff:127.0.0.1]:/foo/bar", + utils.format_remote_path("::ffff:127.0.0.1", + "/foo/bar")) + self.assertEqual("localhost:/foo/bar", + utils.format_remote_path("localhost", "/foo/bar")) + self.assertEqual("/foo/bar", utils.format_remote_path(None, + "/foo/bar")) + def test_get_hash_str(self): base_str = b"foo" base_unicode = u"foo" diff --git a/nova/tests/unit/virt/libvirt/volume/test_remotefs.py b/nova/tests/unit/virt/libvirt/volume/test_remotefs.py index f36c79fce7c1..546a166b4f2d 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_remotefs.py +++ b/nova/tests/unit/virt/libvirt/volume/test_remotefs.py @@ -193,3 +193,45 @@ class RemoteFSTestCase(test.NoDBTestCase): '/home/favourite', on_completion=None, on_execute=None) + + @mock.patch('tempfile.mkdtemp', return_value='/tmp/Saturn') + def test_rsync_driver_ipv6(self, mock_mkdtemp): + with mock.patch('nova.utils.execute') as mock_execute: + remotefs.RsyncDriver().create_file('2600::', 'dest_dir', None, + None) + rsync_call_args = mock.call('rsync', '--archive', '--relative', + '--no-implied-dirs', + '/tmp/Saturn/./dest_dir', '[2600::]:/', + on_completion=None, on_execute=None) + self.assertEqual(mock_execute.mock_calls[2], rsync_call_args) + + with mock.patch('nova.utils.execute') as mock_execute: + remotefs.RsyncDriver().create_dir('2600::', 'dest_dir', None, None) + rsync_call_args = mock.call('rsync', '--archive', '--relative', + '--no-implied-dirs', + '/tmp/Saturn/./dest_dir', '[2600::]:/', + on_completion=None, on_execute=None) + self.assertEqual(mock_execute.mock_calls[1], rsync_call_args) + + with mock.patch('nova.utils.execute') as mock_execute: + remotefs.RsyncDriver().remove_file('2600::', 'dest', None, None) + rsync_call_args = mock.call('rsync', '--archive', + '--delete', '--include', + 'dest', '--exclude', '*', + '/tmp/Saturn/', '[2600::]:', + on_completion=None, on_execute=None) + self.assertEqual(mock_execute.mock_calls[0], rsync_call_args) + + with mock.patch('nova.utils.execute') as mock_execute: + remotefs.RsyncDriver().remove_dir('2600::', 'dest', None, None) + rsync_call_args = mock.call('rsync', '--archive', + '--delete-excluded', '/tmp/Saturn/', + '[2600::]:dest', + on_completion=None, on_execute=None) + self.assertEqual(mock_execute.mock_calls[0], rsync_call_args) + rsync_call_args = mock.call('rsync', '--archive', + '--delete', '--include', + 'dest', '--exclude', '*', + '/tmp/Saturn/', '[2600::]:', + on_completion=None, on_execute=None) + self.assertEqual(mock_execute.mock_calls[1], rsync_call_args) diff --git a/nova/utils.py b/nova/utils.py index 72c92e1836b2..1658bc89dc12 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -697,6 +697,20 @@ def safe_ip_format(ip): return ip +def format_remote_path(host, path): + """Returns remote path in format acceptable for scp/rsync. + + If host is IPv6 address literal, return '[host]:path', otherwise + 'host:path' is returned. + + If host is None, only path is returned. + """ + if host is None: + return path + + return "%s:%s" % (safe_ip_format(host), path) + + def monkey_patch(): """If the CONF.monkey_patch set as True, this function patches a decorator diff --git a/nova/virt/libvirt/volume/remotefs.py b/nova/virt/libvirt/volume/remotefs.py index fe9b3d903e49..dc203c8fe6e1 100644 --- a/nova/virt/libvirt/volume/remotefs.py +++ b/nova/virt/libvirt/volume/remotefs.py @@ -277,7 +277,7 @@ class RsyncDriver(RemoteFilesystemDriver): # Remove remote directory's content utils.execute('rsync', '--archive', '--delete-excluded', kwargs['tmp_dir_path'] + os.path.sep, - '%s:%s' % (host, dst), + utils.format_remote_path(host, dst), on_execute=on_execute, on_completion=on_completion) # Delete empty directory @@ -300,7 +300,8 @@ class RsyncDriver(RemoteFilesystemDriver): '--include', os.path.basename(os.path.normpath(dst)), '--exclude', '*', os.path.normpath(src) + os.path.sep, - '%s:%s' % (host, os.path.dirname(os.path.normpath(dst))), + utils.format_remote_path(host, + os.path.dirname(os.path.normpath(dst))), on_execute=on_execute, on_completion=on_completion) @staticmethod @@ -329,7 +330,8 @@ class RsyncDriver(RemoteFilesystemDriver): # Do relative rsync local directory with remote root directory utils.execute('rsync', '--archive', '--relative', '--no-implied-dirs', - relative_tmp_file_path, '%s:%s' % (host, os.path.sep), + relative_tmp_file_path, + utils.format_remote_path(host, os.path.sep), on_execute=on_execute, on_completion=on_completion) def copy_file(self, src, dst, on_execute, on_completion, compression):