Skip to content

Commit

Permalink
Bind mount the configured certs location bsc#1219004
Browse files Browse the repository at this point in the history
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. Default to the
new cert location if a cert location cannot be determined.

Log a message when no certs are present in the cert location, checking
in the system being migrated, and after the bind mount of that path
into the ISO runtime environment.

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.
  • Loading branch information
rtamalin committed Feb 2, 2024
1 parent d58429a commit 8eeedfb
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 8 deletions.
41 changes: 38 additions & 3 deletions suse_migration_services/units/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# You should have received a copy of the GNU General Public License
# along with suse-migration-services. If not, see <http://www.gnu.org/licenses/>
#
import glob
import logging
import os
import shutil
Expand Down Expand Up @@ -67,9 +68,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 = '/usr/lib/regionService/certs'

cloud_register_metadata = ""

Expand All @@ -85,6 +84,13 @@ 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
)
report_if_regionsrv_certs_not_found(
root_path + cloud_register_certs_path, log
)
if os.path.exists(hosts_setup):
shutil.copy(
hosts_setup, '/etc/hosts'
Expand Down Expand Up @@ -196,6 +202,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(
Expand All @@ -210,6 +218,9 @@ def main():
cloud_register_certs_path
]
)
report_if_regionsrv_certs_not_found(
cloud_register_certs_path, log
)
update_smt_cache = '/usr/sbin/updatesmtcache'
if os.path.isfile(update_smt_cache):
log.info('Updating SMT cache')
Expand Down Expand Up @@ -243,6 +254,30 @@ 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 report_if_regionsrv_certs_not_found(certs_path, log):
"""
Log a message if no PEM files are found at the specified certs path
in the system being migrated.
"""
if not glob.glob(os.path.join(certs_path, '*.pem')):
log.info("No certs found under specified certs path: %s",
repr(certs_path))


def update_regionsrv_setup(root_path, suse_cloud_regionsrv_setup):
"""
Note: This method is specific to the SUSE Public Cloud Infrastructure
Expand Down
2 changes: 2 additions & 0 deletions test/data/regionserverclnt-certlocation.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[server]
certLocation = /usr/lib/regionService/certs
2 changes: 2 additions & 0 deletions test/data/regionserverclnt-nocertlocation.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[server]
dummy = value
2 changes: 2 additions & 0 deletions test/data/regionserverclnt-oldcertlocation.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[server]
certLocation = /var/lib/regionService/certs
116 changes: 111 additions & 5 deletions test/unit/units/prepare_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import os
import shutil
from tempfile import NamedTemporaryFile
from tempfile import (
NamedTemporaryFile,
mkdtemp
)
from configparser import ConfigParser
from unittest.mock import (
patch, call, Mock, MagicMock, mock_open
Expand All @@ -8,7 +12,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, report_if_regionsrv_certs_not_found
)

from suse_migration_services.suse_connect import SUSEConnect
Expand Down Expand Up @@ -46,17 +51,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
Expand Down Expand Up @@ -124,6 +136,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')
Expand All @@ -138,7 +151,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
):
Expand Down Expand Up @@ -188,6 +203,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 == [
Expand Down Expand Up @@ -289,6 +308,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')
Expand All @@ -299,7 +319,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()
Expand All @@ -308,6 +329,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
Expand Down Expand Up @@ -382,3 +407,84 @@ 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'

def test_report_if_regionsrv_certs_not_found(
self,
mock_Command_run,
mock_Fstab,
mock_logger_setup,
mock_update_regionsrv_setup
):
tmp_certs_dir = mkdtemp()

log = Mock(spec=['info'])

# verify that log.info() is called when dir has no pem files
report_if_regionsrv_certs_not_found(tmp_certs_dir, log)
assert len(log.info.call_args_list) == 1

# reset the log mock object
log.reset_mock()

# verify that log.info() is not called when dir has pem files
tmp_pem_path = os.path.join(tmp_certs_dir, 'dummy_cert.pem')
open(tmp_pem_path, 'w').close()
report_if_regionsrv_certs_not_found(tmp_certs_dir, log)
assert len(log.info.call_args_list) == 0

# cleanup tmp_certs_dir
shutil.rmtree(tmp_certs_dir, ignore_errors=True)

0 comments on commit 8eeedfb

Please sign in to comment.