Bind mount the configured certs location bsc#1219004

When the migration is running ensure that the correct certs directory
is bind mounted from the /system-root into the ISO boot's runtime
environment, by retrieving the configured server.certlocation setting
from the /system-root/etc/regionserverclnt.cfg file.

Add unit tests for new get_regionsrv_certs_path() function and tweak
some existing tests, that depend upon mocking calls to builtin.open(),
so that they mock any calls to get_regionsrv_certs_path() so as to
avoid triggering additional open() calls that would break those tests.

suse_migration_services/units/prepare.py

@@ -67,9 +67,7 @@ def main():
         '/etc/pki/trust/anchors/'
     ]
     cache_cloudregister_path = '/var/cache/cloudregister'
-    cloud_register_certs_path = '/var/lib/regionService/certs'
-    cloud_register_certs_bind_mount_path = \
-        root_path + cloud_register_certs_path
+    cloud_register_certs_path_fallback = '/var/lib/regionService/certs'
 
     cloud_register_metadata = ""
 
@@ -85,6 +83,10 @@ def main():
         update_regionsrv_setup(
             root_path, migration_suse_cloud_regionsrv_setup
         )
+        cloud_register_certs_path = get_regionsrv_certs_path(
+            suse_cloud_regionsrv_setup,
+            cloud_register_certs_path_fallback
+        )
     if os.path.exists(hosts_setup):
         shutil.copy(
             hosts_setup, '/etc/hosts'
@@ -196,6 +198,8 @@ def main():
                     cache_cloudregister_path
                 ]
             )
+        cloud_register_certs_bind_mount_path = \
+            root_path + cloud_register_certs_path
         if os.path.exists(cloud_register_certs_bind_mount_path):
             log.info(
                 'Bind mounting {0} from {1}'.format(
@@ -243,6 +247,20 @@ def main():
         )
 
 
+def get_regionsrv_certs_path(suse_cloud_regionsrv_setup, fallback):
+    """
+    Note: This method is specific to the SUSE Public Cloud Infrastructure
+
+    Retrieve the certlocation from the specified suse_cloud_regionsrv_setup
+    config's server.certlocation setting, defaulting to fallback value if not
+    found.
+    """
+    regionsrv_setup = ConfigParser()
+    regionsrv_setup.read(suse_cloud_regionsrv_setup)
+
+    return regionsrv_setup.get('server', 'certlocation', fallback=fallback)
+
+
 def update_regionsrv_setup(root_path, suse_cloud_regionsrv_setup):
     """
     Note: This method is specific to the SUSE Public Cloud Infrastructure

test/data/regionserverclnt-certlocation.cfg

@@ -0,0 +1,2 @@
+[server]
+certLocation = /usr/lib/regionService/certs

test/data/regionserverclnt-nocertlocation.cfg

@@ -0,0 +1,2 @@
+[server]
+dummy = value

test/data/regionserverclnt-oldcertlocation.cfg

@@ -0,0 +1,2 @@
+[server]
+certLocation = /var/lib/regionService/certs

test/unit/units/prepare_test.py

@@ -8,7 +8,8 @@
 from pytest import raises
 
 from suse_migration_services.units.prepare import (
-    main, update_regionsrv_setup, get_regionsrv_client_file_location
+    main, update_regionsrv_setup, get_regionsrv_client_file_location,
+    get_regionsrv_certs_path
 )
 
 from suse_migration_services.suse_connect import SUSEConnect
@@ -46,17 +47,24 @@ def test_main_raises_on_zypp_bind(
         with raises(DistMigrationZypperMetaDataException):
             main()
 
+    @patch('suse_migration_services.units.prepare.get_regionsrv_certs_path')
     @patch('suse_migration_services.units.prepare.get_regionsrv_client_file_location')
     @patch('shutil.copy')
     @patch('os.listdir')
     @patch('os.path.exists')
     def test_main_raises_on_get_regionsrv_client_file_location(
         self, mock_os_path_exists, mock_os_listdir, mock_shutil_copy,
-        mock_get_regionsrv_client_file_location, mock_Command_run,
+        mock_get_regionsrv_client_file_location,
+        mock_get_regionsrv_certs_path,
+        mock_Command_run,
         mock_Fstab, mock_logger_setup,
         mock_update_regionsrv_setup
     ):
         mock_os_listdir.return_value = None
+        # need to override the get_regionsrv_certs_path() call to avoid
+        # introducing a additional open()
+        mock_get_regionsrv_certs_path.return_value = \
+            '/var/lib/regionService/certs'
         mock_get_regionsrv_client_file_location.side_effect = \
             [DistMigrationZypperMetaDataException('foo')]
         mock_os_path_exists.return_value = True
@@ -124,6 +132,7 @@ def test_main_raises_and_umount_file_system(
                 call(['umount', '/system-root/dev'], raise_on_error=False)
             ]
 
+    @patch('suse_migration_services.units.prepare.get_regionsrv_certs_path')
     @patch('os.path.isfile')
     @patch.object(SUSEConnect, 'is_registered')
     @patch('suse_migration_services.units.prepare.MigrationConfig')
@@ -138,7 +147,9 @@ def test_main(
         self, mock_os_path_exists, mock_readlink, mock_os_path_islink,
         mock_path_isdir, mock_os_listdir,
         mock_shutil_copy, mock_Path, mock_MigrationConfig,
-        mock_is_registered, mock_is_file, mock_Command_run,
+        mock_is_registered, mock_is_file,
+        mock_get_regionsrv_certs_path,
+        mock_Command_run,
         mock_Fstab, mock_logger_setup,
         mock_update_regionsrv_setup
     ):
@@ -188,6 +199,10 @@ def test_main(
             FileNotFoundError('cert copy failed')
         ]
         mock_is_file.return_value = True
+        # need to override the get_regionsrv_certs_path() call to avoid
+        # introducing a additional open() call
+        mock_get_regionsrv_certs_path.return_value = \
+            '/var/lib/regionService/certs'
         with patch('builtins.open', mock_open(read_data='foo.susecloud.net')):
             main()
         assert mock_shutil_copy.call_args_list == [
@@ -289,6 +304,7 @@ def test_main(
             ['cat', '/proc/net/bonding/bond*'], raise_on_error=False
         )
 
+    @patch('suse_migration_services.units.prepare.get_regionsrv_certs_path')
     @patch('suse_migration_services.units.prepare.get_regionsrv_client_file_location')
     @patch.object(SUSEConnect, 'is_registered')
     @patch('suse_migration_services.units.prepare.MigrationConfig')
@@ -299,7 +315,8 @@ def test_main(
     def test_main_no_registered_instance(
         self, mock_os_path_exists, mock_os_listdir, mock_shutil_copy,
         mock_Path, mock_MigrationConfig, mock_is_registered,
-        mock_get_regionsrv_client_file_location, mock_Command_run,
+        mock_get_regionsrv_client_file_location,
+        mock_get_regionsrv_certs_path, mock_Command_run,
         mock_Fstab, mock_logger_setup, mock_update_regionsrv_setup
     ):
         migration_config = Mock()
@@ -308,6 +325,10 @@ def test_main_no_registered_instance(
         mock_MigrationConfig.return_value = migration_config
         fstab = Mock()
         mock_Fstab.return_value = fstab
+        # need to override the get_regionsrv_certs_path() call to avoid
+        # introducing a additional open() call
+        mock_get_regionsrv_certs_path.return_value = \
+            '/var/lib/regionService/certs'
         mock_os_listdir.return_value = ['foo', 'bar']
         mock_os_path_exists.return_value = True
         mock_is_registered.return_value = False
@@ -382,3 +403,57 @@ def test_get_regionsrv_client_file_location_raises(
         mock_path_isdir.side_effect = [False, False]
         with raises(DistMigrationZypperMetaDataException):
             get_regionsrv_client_file_location('/foo')
+
+    def test_get_regionsrv_certs_path_certlocation(
+        self,
+        mock_Command_run,
+        mock_Fstab,
+        mock_logger_setup,
+        mock_update_regionsrv_setup,
+    ):
+        tmp_regionserverclnt = NamedTemporaryFile()
+
+        # assert fallback path returned if client config is missing
+        assert get_regionsrv_certs_path(
+            '/some/file/that/does/not/exist',
+            'foo'
+        ) == 'foo'
+
+        # assert fallback path returned if client config is empty
+        assert get_regionsrv_certs_path(
+            tmp_regionserverclnt.name,
+            'foo'
+        ) == 'foo'
+
+        # test we get the fallback certlocation when none specified in config
+        shutil.copy(
+            '../data/regionserverclnt-nocertlocation.cfg',
+            tmp_regionserverclnt.name
+        )
+
+        assert get_regionsrv_certs_path(
+            tmp_regionserverclnt.name,
+            'foo'
+        ) == 'foo'
+
+        # test we get the standard certlocation from the config
+        shutil.copy(
+            '../data/regionserverclnt-certlocation.cfg',
+            tmp_regionserverclnt.name
+        )
+
+        assert get_regionsrv_certs_path(
+            tmp_regionserverclnt.name,
+            'foo'
+        ) == '/usr/lib/regionService/certs'
+
+        # test we get the old certlocation from an old config
+        shutil.copy(
+            '../data/regionserverclnt-oldcertlocation.cfg',
+            tmp_regionserverclnt.name
+        )
+
+        assert get_regionsrv_certs_path(
+            tmp_regionserverclnt.name,
+            'foo'
+        ) == '/var/lib/regionService/certs'