From 8277db2f1e3026c64b9554ac0a47a65442b6c0de Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Tue, 7 May 2024 15:26:17 -0400 Subject: [PATCH 1/7] Provide error message to user on registration failure If base product registration fails we swallow the error message and it is only available in the log file. We correctly exit with error code 1. Provide the error message we get from the underlying registration tool used on stderr. --- usr/sbin/registercloudguest | 1 + 1 file changed, 1 insertion(+) diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index a7e86a5..d41a5b6 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -503,6 +503,7 @@ while not base_registered: logging.error('Baseproduct registration failed') logging.error('\t%s' % error_message) cleanup() + print(error_message, file=sys.stderr) sys.exit(1) for smt_srv in region_smt_servers: target_smt_ipv4 = registration_target.get_ipv4() From 79f93f97657026fbb120aa84747fc4c2e8818a0d Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Tue, 7 May 2024 18:36:34 -0400 Subject: [PATCH 2/7] Support transactional-update for registration On SUSE Micro systems with a RO root filesystem we need to use the transactional-update register command to complete the registration. SUSEConnect disables itself in this situation and produces and error. Signed-off-by: Robert Schweikert --- usr/sbin/registercloudguest | 39 ++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index d41a5b6..c66b5e0 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -440,8 +440,41 @@ if not utils.is_registration_supported(cfg): # Check SUSE/SLES registration command to be present register_cmd = '/usr/sbin/SUSEConnect' +# For transactional systems we need to do a few extra things +p = subprocess.Popen( + ['findmnt', '--noheadings', '--json', '/'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE +) +res = p.communicate() +# If we get an error from findmnt move forward on a best effort basis +if p.returncode: + logging.warning('Unable to find filesystem information for "/"') +else: + fsinfo = json.loads(res[0]) + fsdata = fsinfo.get('filesystems') + if fsdata: + fsoptions = fsdata[0].get('options') + # If we are on a RO system we need to use the + # transactional-update command + if 'ro' in fsoptions.split(','): + cmd_name = 'transactional-update' + for path in ['/sbin/','/usr/sbin/']: + exec_path = path + cmd_name + if os.path.exists(exec_path): + register_cmd = exec_path + break + else: + err_msg = 'transactional-update command not found.' + err_msg += 'But is required on a RO filesystem for ' + err_msg += 'registration' + logging.error(err_msg) + print(err_msg, file=sys.stderr) + sys.exit(1) if not (os.path.exists(register_cmd) and os.access(register_cmd, os.X_OK)): - logging.error('No registration executable found') + err_msg = 'No registration executable found' + logging.error(err_msg) + print(err_msg, file=sys.stderr) sys.exit(1) # get product list @@ -460,8 +493,12 @@ failed_smts = [] while not base_registered: utils.add_hosts_entry(registration_target) + sub_cmd = '' + if 'transactional' in register_cmd: + sub_cmd = 'register' cmd = [ register_cmd, + sub_cmd, '--url', 'https://%s' % registration_target.get_FQDN() ] From 83150288a836926f026569961b2f6b9af6fac925 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Wed, 8 May 2024 13:22:25 -0400 Subject: [PATCH 3/7] Handle zypper operation on a different root If zypper is instructed to operate on a different root with the "--root" argument the credentials are also expected to be found in this seperate root tree. WE need to accomodate this redirection. --- lib/cloudregister/registerutils.py | 25 ++++- usr/lib/zypp/plugins/urlresolver/susecloud | 2 + usr/sbin/registercloudguest | 117 ++++++++++++--------- 3 files changed, 94 insertions(+), 50 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index a41a971..3c0fa46 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -159,10 +159,13 @@ def clear_rmt_as_scc_proxy_flag(): def credentials_files_are_equal(repo_credentials): """Compare the base credentials files the the repo header and make sure they have the same values.""" - credentials_location = '/etc/zypp/credentials.d/' + if not repo_credentials or not isinstance(repo_credentials, str): return False - + + base_credentials_location = '/etc/zypp/credentials.d/' + target_root = get_zypper_target_root() + credentials_location = target_root + base_credentials_location credentials_base = os.path.join(credentials_location, 'SCCcredentials') credentials_header = os.path.join(credentials_location, repo_credentials) ref_user, ref_pass = get_credentials(credentials_base) @@ -522,7 +525,8 @@ def get_credentials_file(update_server, service_name=None): after the system is properly registered. """ credentials_file = '' - credentials_loc = '/etc/zypp/credentials.d/' + target_root = get_zypper_target_root() + credentials_loc = target_root + '/etc/zypp/credentials.d/' credential_names = [ '*' + update_server.get_FQDN().replace('.', '_'), 'SCC*' @@ -874,6 +878,21 @@ def get_zypper_pid_cache(): return zypper_state_file.read() +# ---------------------------------------------------------------------------- +def get_zypper_target_root(): + """Return the target root if zypper has the --root argument to + specify a target directory in which to operate. + """ + zypper_cmd = get_zypper_command() + target_root = '' + for root_arg in ('-R', '--root'): + if zypper_cmd and root_arg in zypper_cmd: + target_root = zypper_cmd.split(root_arg)[-1].split()[0].strip() + break + + return target_root + + # ---------------------------------------------------------------------------- def has_ipv6_access(smt): """IPv6 access is possible if we have an SMT server that has an IPv6 diff --git a/usr/lib/zypp/plugins/urlresolver/susecloud b/usr/lib/zypp/plugins/urlresolver/susecloud index 0416790..cb7f1dd 100755 --- a/usr/lib/zypp/plugins/urlresolver/susecloud +++ b/usr/lib/zypp/plugins/urlresolver/susecloud @@ -82,7 +82,9 @@ class SUSECloudPlugin(Plugin): if repo_credentials: repo_url += '?credentials=' + repo_credentials if verify_credentials: + target_root = utils.get_zypper_target_root() credentials_file_path = ( + target_root + '/etc/zypp/credentials.d/%s' % repo_credentials ) user, password = utils.get_credentials( diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index c66b5e0..6ca1c1d 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -52,10 +52,50 @@ from requests.auth import HTTPBasicAuth urllib3.disable_warnings() registration_returncode = 0 +# ---------------------------------------------------------------------------- +def get_register_cmd(): + """Determine which command we need to use to register the system""" + + register_cmd = '/usr/sbin/SUSEConnect' + # Figure out if we are on RO transactional-update system + p = subprocess.Popen( + ['findmnt', '--noheadings', '--json', '/'], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE + ) + res = p.communicate() + # If we get an error from findmnt move forward on a best effort basis + if p.returncode: + logging.warning('Unable to find filesystem information for "/"') + else: + fsinfo = json.loads(res[0]) + fsdata = fsinfo.get('filesystems') + if fsdata: + fsoptions = fsdata[0].get('options') + # If we are on a RO system we need to use the + # transactional-update command + if 'ro' in fsoptions.split(','): + cmd_name = 'transactional-update' + for path in ['/sbin/','/usr/sbin/']: + exec_path = path + cmd_name + if os.path.exists(exec_path): + register_cmd = exec_path + break + else: + err_msg = 'transactional-update command not found.' + err_msg += 'But is required on a RO filesystem for ' + err_msg += 'registration' + logging.error(err_msg) + print(err_msg, file=sys.stderr) + sys.exit(1) + + return register_cmd + # ---------------------------------------------------------------------------- def register_modules(extensions, products, registered=[], failed=[]): """Register modules obeying dependencies""" global registration_returncode + register_cmd = get_register_cmd() for extension in extensions: # If the extension is recommended it gets installed with the # baseproduct registration. No need to run another registration @@ -70,13 +110,23 @@ def register_modules(extensions, products, registered=[], failed=[]): triplet = '/'.join((identifier, version, arch)) if triplet in products and triplet not in registered: registered.append(triplet) - cmd = [ - register_cmd, - '--url', - 'https://%s' % registration_target.get_FQDN(), - '--product', - triplet - ] + if 'transactional' in register_cmd: + cmd = [ + register_cmd, + sub_cmd, + '--url', + 'https://%s' % registration_target.get_FQDN(), + '--product', + triplet + ] + else: + cmd = [ + register_cmd, + '--url', + 'https://%s' % registration_target.get_FQDN(), + '--product', + triplet + ] if os.path.exists(instance_data_filepath): cmd.append('--instance-data') cmd.append(instance_data_filepath) @@ -438,39 +488,7 @@ if instance_data: if not utils.is_registration_supported(cfg): sys.exit(0) -# Check SUSE/SLES registration command to be present -register_cmd = '/usr/sbin/SUSEConnect' -# For transactional systems we need to do a few extra things -p = subprocess.Popen( - ['findmnt', '--noheadings', '--json', '/'], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE -) -res = p.communicate() -# If we get an error from findmnt move forward on a best effort basis -if p.returncode: - logging.warning('Unable to find filesystem information for "/"') -else: - fsinfo = json.loads(res[0]) - fsdata = fsinfo.get('filesystems') - if fsdata: - fsoptions = fsdata[0].get('options') - # If we are on a RO system we need to use the - # transactional-update command - if 'ro' in fsoptions.split(','): - cmd_name = 'transactional-update' - for path in ['/sbin/','/usr/sbin/']: - exec_path = path + cmd_name - if os.path.exists(exec_path): - register_cmd = exec_path - break - else: - err_msg = 'transactional-update command not found.' - err_msg += 'But is required on a RO filesystem for ' - err_msg += 'registration' - logging.error(err_msg) - print(err_msg, file=sys.stderr) - sys.exit(1) +register_cmd = get_register_cmd() if not (os.path.exists(register_cmd) and os.access(register_cmd, os.X_OK)): err_msg = 'No registration executable found' logging.error(err_msg) @@ -495,13 +513,18 @@ while not base_registered: utils.add_hosts_entry(registration_target) sub_cmd = '' if 'transactional' in register_cmd: - sub_cmd = 'register' - cmd = [ - register_cmd, - sub_cmd, - '--url', - 'https://%s' % registration_target.get_FQDN() - ] + cmd = [ + register_cmd, + 'register', + '--url', + 'https://%s' % registration_target.get_FQDN() + ] + else: + cmd = [ + register_cmd, + '--url', + 'https://%s' % registration_target.get_FQDN() + ] if os.path.exists(instance_data_filepath): cmd.append('--instance-data') cmd.append(instance_data_filepath) From 6ccaa96dedb2cb227084ca7f004ea5a586e4c85a Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 9 May 2024 09:51:15 -0400 Subject: [PATCH 4/7] Make test work with pystest When running the tests with pytest only we need to properly set the path to the current location to find the modules we want to import. While the path calculation was alreday part of the test, the created path was not properly inserted into the environment. --- tests/test_cloudguestregistryauth.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_cloudguestregistryauth.py b/tests/test_cloudguestregistryauth.py index ddacdc8..2536040 100644 --- a/tests/test_cloudguestregistryauth.py +++ b/tests/test_cloudguestregistryauth.py @@ -4,7 +4,6 @@ from lxml import etree -from cloudregister.smt import SMT from importlib.machinery import SourceFileLoader from pytest import raises @@ -18,6 +17,10 @@ sys.path.insert(0, code_path) +from cloudregister.smt import SMT # noqa + +sys.path.insert(0, code_path) + # Hack to get the script without the .py imported for testing cloudguestregistryauth = SourceFileLoader( 'cloudguestregistryauth', From ed4ad6a21037be7136e66146259e137554b337a7 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 9 May 2024 09:53:04 -0400 Subject: [PATCH 5/7] Add unit tests for new method To support a transactional system we need to look at the process table to see if zypper is operating on a directory other than the default root path. A new function was added to retrieve the target path form the zypper command line. Test the various command line options. --- integration_test-process.txt | 3 +++ lib/cloudregister/registerutils.py | 4 +-- tests/test_registerutils.py | 39 ++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/integration_test-process.txt b/integration_test-process.txt index e5f17c3..65fed26 100644 --- a/integration_test-process.txt +++ b/integration_test-process.txt @@ -19,6 +19,8 @@ After installing the test package with "zypper in" + no error no message + zypper lr has no repos + nothing in /etc/zypp/credentials.d + + nothing in /etc/zypp/services.d/ + + no pkl files in /var/cache/cloudregister/ - registercloudguest + no error + sucees message on stdout @@ -75,6 +77,7 @@ After installing the test package with "zypper in" + no error no message + zypper lr has no repos + nothing in /etc/zypp/credentials.d + + nothing in /etc/zypp/services.d/ - registercloudguest -r XXX + no error + sucees message on stdout diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 3c0fa46..173b499 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -159,10 +159,10 @@ def clear_rmt_as_scc_proxy_flag(): def credentials_files_are_equal(repo_credentials): """Compare the base credentials files the the repo header and make sure they have the same values.""" - + if not repo_credentials or not isinstance(repo_credentials, str): return False - + base_credentials_location = '/etc/zypp/credentials.d/' target_root = get_zypper_target_root() credentials_location = target_root + base_credentials_location diff --git a/tests/test_registerutils.py b/tests/test_registerutils.py index 01f050a..f4829ff 100644 --- a/tests/test_registerutils.py +++ b/tests/test_registerutils.py @@ -84,6 +84,45 @@ def test_get_zypper_pid_cache_no_cache(path_exists): assert utils.get_zypper_pid_cache() == 0 +@patch('cloudregister.registerutils.get_zypper_command') +def test_get_zypper_target_root_no_zypper(zypp_cmd): + """Test behavior when zypper is not running""" + zypp_cmd.return_value = None + assert utils.get_zypper_target_root() == '' + + +@patch('cloudregister.registerutils.get_zypper_command') +def test_get_zypper_target_root_set_R_short(zypp_cmd): + """Test behavior when zypper is "running" and has root set using -R and no + other args""" + zypp_cmd.return_value = '-R /foobar' + assert utils.get_zypper_target_root() == '/foobar' + + +@patch('cloudregister.registerutils.get_zypper_command') +def test_get_zypper_target_root_set_R_long(zypp_cmd): + """Test behavior when zypper is "running" and has root set using -R and + other args""" + zypp_cmd.return_value = '-R /foobar --no-interactive' + assert utils.get_zypper_target_root() == '/foobar' + + +@patch('cloudregister.registerutils.get_zypper_command') +def test_get_zypper_target_root_set_root_short(zypp_cmd): + """Test behavior when zypper is "running" and has root set using --root + and no other args""" + zypp_cmd.return_value = '--root /foobar' + assert utils.get_zypper_target_root() == '/foobar' + + +@patch('cloudregister.registerutils.get_zypper_command') +def test_get_zypper_target_root_set_root_long(zypp_cmd): + """Test behavior when zypper is "running" and has root set using --root + and other args""" + zypp_cmd.return_value = '--root /foobar --no-interactive' + assert utils.get_zypper_target_root() == '/foobar' + + @patch('cloudregister.registerutils.__get_region_server_args') @patch('cloudregister.registerutils.__get_framework_plugin') @patch('cloudregister.registerutils.get_framework_identifier_path') From 8334902e3e99e42ad7e8b38fa0a8af7fdfd67ca8 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 9 May 2024 11:13:43 -0400 Subject: [PATCH 6/7] Fix uninitialized variable THe command generation for registration had to be split, no need to have a variable for a fixed value when it is used only once. --- usr/sbin/registercloudguest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 6ca1c1d..ee2e486 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -113,7 +113,7 @@ def register_modules(extensions, products, registered=[], failed=[]): if 'transactional' in register_cmd: cmd = [ register_cmd, - sub_cmd, + 'register', '--url', 'https://%s' % registration_target.get_FQDN(), '--product', From e177d43924b0bd76e9d87076232744ccaec631f4 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 9 May 2024 13:30:38 -0400 Subject: [PATCH 7/7] Version update to 10.2.0 --- cloud-regionsrv-client.spec | 2 +- lib/cloudregister/VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cloud-regionsrv-client.spec b/cloud-regionsrv-client.spec index ee4cd05..897de66 100644 --- a/cloud-regionsrv-client.spec +++ b/cloud-regionsrv-client.spec @@ -16,7 +16,7 @@ # -%define base_version 10.1.8 +%define base_version 10.2.0 Name: cloud-regionsrv-client Version: %{base_version} Release: 0 diff --git a/lib/cloudregister/VERSION b/lib/cloudregister/VERSION index 22a2908..2bd6f7e 100644 --- a/lib/cloudregister/VERSION +++ b/lib/cloudregister/VERSION @@ -1 +1 @@ -10.1.8 +10.2.0