From ffe3a13e22ffd8537d4914111bc29daea2507e83 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 29 Aug 2023 14:20:30 +0100 Subject: [PATCH 01/14] Check if instance has Ipv4 or IPv6 enabled When using '--force-new --smt-ip ...' options, validate that the instance can use the --smt-ip value in order to save debugging or unnecessary errors --- lib/cloudregister/registerutils.py | 34 ++++++++++++++++++++++++++++++ usr/sbin/registercloudguest | 8 +++++++ 2 files changed, 42 insertions(+) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 877f5516..2b3b84b7 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1407,6 +1407,40 @@ def write_framework_identifier(cfg): framework_file.write(json.dumps(identifier)) +# ---------------------------------------------------------------------------- +def is_user_smt_ip_enabled(arg_ip_addr): + cfg = get_config() + try: + data_provider = cfg['instance']['dataProvider'] + except KeyError as err: + return False, err + + cmd_list = data_provider.split()[:3] + if isinstance(ipaddress.ip_address(arg_ip_addr), ipaddress.IPv6Address): + cmd_list.append('--ipv6') + else: + cmd_list.append('--public-ipv4') + + output, errors = exec_subprocess(cmd_list, return_output=True) + if output == -1: + msg = 'Could not find configured dataProvider: %s' % ' '.join(cmd_list) + return False, msg + + if 'Error' in errors.decode(): + # get 'ipv6' or 'ipv4' string from the command + ip_format = cmd_list[-1][-4:] + msg = 'This instance does not have {} enabled.'.format(ip_format) + return False, msg + + metadata_ip_addr = output.decode().strip() + try: + return ipaddress.ip_address(metadata_ip_addr), None + except ValueError as err: + msg = 'This instance does not have {} enabled.'.format(ip_format) + return False, msg + + + # Private # ---------------------------------------------------------------------------- def __get_framework_plugin(cfg): diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index c81d5119..c00700c1 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -29,6 +29,7 @@ import argparse import ipaddress +from ipaddress import IPv4Address, IPv6Address import json import logging import os @@ -200,6 +201,12 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: print(msg, file=sys.stderr) sys.exit(1) + # check the instance has IPv6 or Ipv4 enabled + # so smt-ip can be either + is_enabled, error_msg = utils.is_user_smt_ip_enabled(args.user_smt_ip) + if not is_enabled: + print(error_msg, file=sys.stderr) + sys.exit(1) if args.clean_up and args.force_new_registration: msg = '--clean and --force-new are incompatible, use one or the other' @@ -214,6 +221,7 @@ if (args.email and not args.reg_code): time.sleep(int(args.delay_time)) + config_file = args.config_file if config_file: config_file = os.path.expanduser(args.config_file) From 71b4a352ddde0f85c9bceaa158435256fc0e9097 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 5 Sep 2023 10:19:25 +0100 Subject: [PATCH 02/14] Leverage has_ipv6_access - Leverage has_ipv6_access - Refactor has_ipv6_access to accomodate IPv4 check --- lib/cloudregister/registerutils.py | 80 ++++++++++++------------------ usr/sbin/registercloudguest | 9 +++- 2 files changed, 40 insertions(+), 49 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 2b3b84b7..0bcd1306 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -878,22 +878,8 @@ def has_ipv6_access(smt): address and it can be accessed over IPv6""" if not smt.get_ipv6(): return False - logging.info('Attempt to access update server over IPv6') - protocol = 'http' # Default for backward compatibility - if https_only(get_config()): - protocol = 'https' - try: - # Per rfc3986 IPv6 addresses in a URI are enclosed in [] - cert_res = requests.get( - '%s://[%s]/smt.crt' % (protocol, smt.get_ipv6()), - timeout=3, - verify=False - ) - except Exception: - logging.info('Update server not reachable over IPv6') - return False - if cert_res and cert_res.status_code == 200: - return True + # Per rfc3986 IPv6 addresses in a URI are enclosed in [] + return __connection_check('[{}]'.format(smt.get_ipv6()), 'IPv6') # ---------------------------------------------------------------------------- @@ -1408,37 +1394,15 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def is_user_smt_ip_enabled(arg_ip_addr): - cfg = get_config() - try: - data_provider = cfg['instance']['dataProvider'] - except KeyError as err: - return False, err - - cmd_list = data_provider.split()[:3] - if isinstance(ipaddress.ip_address(arg_ip_addr), ipaddress.IPv6Address): - cmd_list.append('--ipv6') - else: - cmd_list.append('--public-ipv4') - - output, errors = exec_subprocess(cmd_list, return_output=True) - if output == -1: - msg = 'Could not find configured dataProvider: %s' % ' '.join(cmd_list) - return False, msg - - if 'Error' in errors.decode(): - # get 'ipv6' or 'ipv4' string from the command - ip_format = cmd_list[-1][-4:] - msg = 'This instance does not have {} enabled.'.format(ip_format) - return False, msg - - metadata_ip_addr = output.decode().strip() - try: - return ipaddress.ip_address(metadata_ip_addr), None - except ValueError as err: - msg = 'This instance does not have {} enabled.'.format(ip_format) - return False, msg - +def is_user_smt_ip_enabled(ip_format): + for rmt_server in get_available_smt_servers(): + if 'ipv6' in ip_format: + if has_ipv6_access(rmt_server): + return True + else: + # check IPv4 + if __connection_check(rmt_server.get_ipv4(), 'IPv4'): + return True # Private @@ -1669,3 +1633,25 @@ def __replace_url_target(config_files, new_smt): current_service_server, new_smt.get_FQDN()) ) + + +# ----------------------------------------------------------------------------- +def __connection_check(smt_ip_addr, ip_format): + if not smt_ip_addr: + # in case RMT ever dropped IPv4 support + return False + logging.info('Attempt to access update server over {}'.format(ip_format)) + protocol = 'http' # Default for backward compatibility + if https_only(get_config()): + protocol = 'https' + try: + cert_res = requests.get( + '%s://%s/smt.crt' % (protocol, smt_ip_addr), + timeout=3, + verify=False + ) + except Exception: + logging.info('Update server not reachable over %s' % ip_format) + return False + if cert_res and cert_res.status_code == 200: + return True diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index c00700c1..98393d0e 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -203,9 +203,14 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: # check the instance has IPv6 or Ipv4 enabled # so smt-ip can be either - is_enabled, error_msg = utils.is_user_smt_ip_enabled(args.user_smt_ip) + ip_format = ['ipv4'] + if isinstance(ipaddress.ip_address(arg_ip_addr), ipaddress.IPv6Address): + ip_format = ['ipv6'] + + is_enabled = utils.is_user_smt_ip_enabled(ip_format) if not is_enabled: - print(error_msg, file=sys.stderr) + msg = 'Instance does not have {} enabled'.format(ip_format[0]) + print(msg, file=sys.stderr) sys.exit(1) if args.clean_up and args.force_new_registration: From 162dc2997277c35330f973a018f328ead202fd8a Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 25 Oct 2023 15:44:27 +0100 Subject: [PATCH 03/14] Check if RMT serves the selected IP address type --- lib/cloudregister/registerutils.py | 13 +++++++++++++ usr/sbin/registercloudguest | 18 +++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 0bcd1306..bb592e9d 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1405,6 +1405,19 @@ def is_user_smt_ip_enabled(ip_format): return True +# ----------------------------------------------------------------------------- +def does_rmt_serve_ip(ip_format): + try: + rmt_server = get_available_smt_servers()[0] + except IndexError: + logging.error('No RMT servers available') + return False + + if 'IPv6' in ip_format: + return rmt_server.has_ipv6() + + return rmt_server.has_ipv4() + # Private # ---------------------------------------------------------------------------- def __get_framework_plugin(cfg): diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 98393d0e..390503fb 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -203,13 +203,17 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: # check the instance has IPv6 or Ipv4 enabled # so smt-ip can be either - ip_format = ['ipv4'] - if isinstance(ipaddress.ip_address(arg_ip_addr), ipaddress.IPv6Address): - ip_format = ['ipv6'] - - is_enabled = utils.is_user_smt_ip_enabled(ip_format) - if not is_enabled: - msg = 'Instance does not have {} enabled'.format(ip_format[0]) + ip_format = ['IPv4'] + if isinstance(ipaddress.ip_address(args.user_smt_ip), IPv6Address): + ip_format = ['IPv6'] + + if utils.does_rmt_serve_ip(ip_format): + if not utils.is_user_smt_ip_enabled(ip_format): + msg = 'Instance does not have {} enabled'.format(ip_format[0]) + print(msg, file=sys.stderr) + sys.exit(1) + else: + msg = 'RMT does not serve {} addresses'.format(ip_format[0]) print(msg, file=sys.stderr) sys.exit(1) From 54ed7f64a34f4a78a4968cf631f51e2b0f8cae81 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 25 Oct 2023 15:46:05 +0100 Subject: [PATCH 04/14] Remove unnecessary conditions --- lib/cloudregister/registerutils.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index bb592e9d..0a3624c0 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1397,12 +1397,10 @@ def write_framework_identifier(cfg): def is_user_smt_ip_enabled(ip_format): for rmt_server in get_available_smt_servers(): if 'ipv6' in ip_format: - if has_ipv6_access(rmt_server): - return True + return has_ipv6_access(rmt_server) else: # check IPv4 - if __connection_check(rmt_server.get_ipv4(), 'IPv4'): - return True + return __connection_check(rmt_server.get_ipv4(), 'IPv4') # ----------------------------------------------------------------------------- @@ -1666,5 +1664,5 @@ def __connection_check(smt_ip_addr, ip_format): except Exception: logging.info('Update server not reachable over %s' % ip_format) return False - if cert_res and cert_res.status_code == 200: - return True + + return cert_res and cert_res.status_code == 200 From 241a93c8dcf91ee077e673f91105c55600b2a2dd Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Mon, 13 Nov 2023 10:46:17 +0000 Subject: [PATCH 05/14] Refactor infra reaching check - Less knowledge littering - Check request exception for - established connection - timeout - Handle general request exception --- lib/cloudregister/registerutils.py | 61 +++++++++++++++++------------- usr/sbin/registercloudguest | 22 ++++------- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 0a3624c0..a62deb8c 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -878,8 +878,7 @@ def has_ipv6_access(smt): address and it can be accessed over IPv6""" if not smt.get_ipv6(): return False - # Per rfc3986 IPv6 addresses in a URI are enclosed in [] - return __connection_check('[{}]'.format(smt.get_ipv6()), 'IPv6') + return __connection_check(smt.get_ipv6()) # ---------------------------------------------------------------------------- @@ -1394,27 +1393,33 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def is_user_smt_ip_enabled(ip_format): - for rmt_server in get_available_smt_servers(): - if 'ipv6' in ip_format: - return has_ipv6_access(rmt_server) - else: - # check IPv4 - return __connection_check(rmt_server.get_ipv4(), 'IPv4') - - -# ----------------------------------------------------------------------------- -def does_rmt_serve_ip(ip_format): +def can_reach_infra(rmt_ip): + """Check if the RMT server with the provided IP address is reachable.""" try: - rmt_server = get_available_smt_servers()[0] - except IndexError: - logging.error('No RMT servers available') - return False + if not __connection_check(rmt_ip, True): + # instance could make Ipv4 or IPv6 request + # but connection failed + return False, 'Instance could not connect to {}'.format(rmt_ip) + except requests.exceptions.ConnectionError as err: + if ('Failed to establish a new connection' in str(err) and + 'Network is unreachable' in str(err) + ): + # could not establish a connection to IPv4 or IPv6 + ip_format = 'IPv4' + if isinstance(ipaddress.ip_address(rmt_ip), ipaddress.IPv6Address): + ip_format = 'IPv6' + message = ( + 'Connection error: Could not establish a connection to {ip}.' + 'Check {ip_format} is enabled and working properly'.format( + ip=rmt_ip, ip_format=ip_format + ) + ) + return False, message + elif 'Connection to {} timed out.'.format(rmt_ip) in str(err): + return False, 'Connection to {} timed out'.format(rmt_ip) - if 'IPv6' in ip_format: - return rmt_server.has_ipv6() + return True, None - return rmt_server.has_ipv4() # Private # ---------------------------------------------------------------------------- @@ -1647,11 +1652,11 @@ def __replace_url_target(config_files, new_smt): # ----------------------------------------------------------------------------- -def __connection_check(smt_ip_addr, ip_format): - if not smt_ip_addr: - # in case RMT ever dropped IPv4 support - return False - logging.info('Attempt to access update server over {}'.format(ip_format)) +def __connection_check(smt_ip_addr, propagate=False): + if isinstance(smt_ip_addr, ipaddress.IPv6Address): + # Per rfc3986 IPv6 addresses in a URI are enclosed in [] + smt_ip_addr= "[{}]".format(smt_ip_addr) + protocol = 'http' # Default for backward compatibility if https_only(get_config()): protocol = 'https' @@ -1661,8 +1666,12 @@ def __connection_check(smt_ip_addr, ip_format): timeout=3, verify=False ) + except requests.exceptions.ConnectionError as err: + if propagate: + raise + logging.error(err) except Exception: - logging.info('Update server not reachable over %s' % ip_format) + logging.error('Update server not reachable for %s' % smt_ip_addr) return False return cert_res and cert_res.status_code == 200 diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 390503fb..1d107ff7 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -29,7 +29,6 @@ import argparse import ipaddress -from ipaddress import IPv4Address, IPv6Address import json import logging import os @@ -201,22 +200,15 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: print(msg, file=sys.stderr) sys.exit(1) - # check the instance has IPv6 or Ipv4 enabled - # so smt-ip can be either - ip_format = ['IPv4'] - if isinstance(ipaddress.ip_address(args.user_smt_ip), IPv6Address): - ip_format = ['IPv6'] - - if utils.does_rmt_serve_ip(ip_format): - if not utils.is_user_smt_ip_enabled(ip_format): - msg = 'Instance does not have {} enabled'.format(ip_format[0]) - print(msg, file=sys.stderr) - sys.exit(1) - else: - msg = 'RMT does not serve {} addresses'.format(ip_format[0]) - print(msg, file=sys.stderr) + logging.info( + 'Attempt to access update server with IP {}'.format(args.user_smt_ip) + ) + can_reach_infra, error_message = utils.can_reach_infra(args.user_smt_ip) + if not can_reach_infra: + print(error_message, file=sys.stderr) sys.exit(1) + if args.clean_up and args.force_new_registration: msg = '--clean and --force-new are incompatible, use one or the other' print(msg, file=sys.stderr) From 527d407fbe7746bf8daa1f824b98499378757d33 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Mon, 13 Nov 2023 18:09:17 +0000 Subject: [PATCH 06/14] Remove if for connection check - The message is checked if connection failed, use that and remove the if to set the message --- lib/cloudregister/registerutils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index a62deb8c..fa1723dd 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1396,10 +1396,7 @@ def write_framework_identifier(cfg): def can_reach_infra(rmt_ip): """Check if the RMT server with the provided IP address is reachable.""" try: - if not __connection_check(rmt_ip, True): - # instance could make Ipv4 or IPv6 request - # but connection failed - return False, 'Instance could not connect to {}'.format(rmt_ip) + check = __connection_check(rmt_ip, True) except requests.exceptions.ConnectionError as err: if ('Failed to establish a new connection' in str(err) and 'Network is unreachable' in str(err) @@ -1418,7 +1415,10 @@ def can_reach_infra(rmt_ip): elif 'Connection to {} timed out.'.format(rmt_ip) in str(err): return False, 'Connection to {} timed out'.format(rmt_ip) - return True, None + # if check failed, it means + # instance could make Ipv4 or IPv6 request + # but connection failed + return check, 'Instance could not connect to {}'.format(rmt_ip) # Private From 4cba1cb703b0cb704a68dab1e33ad0cc83b67a54 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 21 Nov 2023 10:52:18 +0000 Subject: [PATCH 07/14] Rename and refactor method - Rename method to reflect the correct intend - Check if a request could be made, handle behaviour accordingly --- lib/cloudregister/registerutils.py | 27 +++++++++------------------ usr/sbin/registercloudguest | 8 ++++++-- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index fa1723dd..f4717890 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1393,32 +1393,23 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def can_reach_infra(rmt_ip): - """Check if the RMT server with the provided IP address is reachable.""" +def can_make_ip_request(rmt_ip): + """Check if the instance can make IPv4 or IPv6 requests.""" try: - check = __connection_check(rmt_ip, True) + __connection_check(rmt_ip, True) except requests.exceptions.ConnectionError as err: if ('Failed to establish a new connection' in str(err) and 'Network is unreachable' in str(err) ): # could not establish a connection to IPv4 or IPv6 - ip_format = 'IPv4' - if isinstance(ipaddress.ip_address(rmt_ip), ipaddress.IPv6Address): - ip_format = 'IPv6' - message = ( - 'Connection error: Could not establish a connection to {ip}.' - 'Check {ip_format} is enabled and working properly'.format( - ip=rmt_ip, ip_format=ip_format - ) - ) - return False, message - elif 'Connection to {} timed out.'.format(rmt_ip) in str(err): - return False, 'Connection to {} timed out'.format(rmt_ip) - + return False + # something went wrong with the request (i.e. connection timed out) + # but a request was made + pass # if check failed, it means # instance could make Ipv4 or IPv6 request - # but connection failed - return check, 'Instance could not connect to {}'.format(rmt_ip) + # but request went wrong or its status code was not 200 + return True # Private diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 1d107ff7..4ce81605 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -203,8 +203,12 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: logging.info( 'Attempt to access update server with IP {}'.format(args.user_smt_ip) ) - can_reach_infra, error_message = utils.can_reach_infra(args.user_smt_ip) - if not can_reach_infra: + if not utils.can_make_ip_request(args.user_smt_ip): + error_message = ( + 'Connection error: Could not establish a connection to {ip}.' + 'Please, make sure the instance has that IP format enabled ' + 'and working properly'.format(ip=args.user_smt_ip) + ) print(error_message, file=sys.stderr) sys.exit(1) From 00b33028fa234e3733f55c8dc3397c205dc16778 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 21 Nov 2023 19:09:38 +0000 Subject: [PATCH 08/14] Refactor method to check IP enablement - Check socket connection - Restore has_ipv6_access as it was - Remove __connection_check --- lib/cloudregister/registerutils.py | 64 +++++++++++------------------- usr/sbin/registercloudguest | 5 +-- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index f4717890..f9101f73 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -878,7 +878,22 @@ def has_ipv6_access(smt): address and it can be accessed over IPv6""" if not smt.get_ipv6(): return False - return __connection_check(smt.get_ipv6()) + logging.info('Attempt to access update server over IPv6') + protocol = 'http' # Default for backward compatibility + if https_only(get_config()): + protocol = 'https' + try: + # Per rfc3986 IPv6 addresses in a URI are enclosed in [] + cert_res = requests.get( + '%s://[%s]/smt.crt' % (protocol, smt.get_ipv6()), + timeout=3, + verify=False + ) + except Exception: + logging.info('Update server not reachable over IPv6') + return False + if cert_res and cert_res.status_code == 200: + return True # ---------------------------------------------------------------------------- @@ -1393,22 +1408,15 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def can_make_ip_request(rmt_ip): +def instance_has_ip_enabled(rmt_ip): """Check if the instance can make IPv4 or IPv6 requests.""" try: - __connection_check(rmt_ip, True) - except requests.exceptions.ConnectionError as err: - if ('Failed to establish a new connection' in str(err) and - 'Network is unreachable' in str(err) - ): - # could not establish a connection to IPv4 or IPv6 - return False - # something went wrong with the request (i.e. connection timed out) - # but a request was made - pass - # if check failed, it means - # instance could make Ipv4 or IPv6 request - # but request went wrong or its status code was not 200 + socket.create_connection((rmt_ip, 443), timeout=2) + except OSError as e: + # check socket connection produced + # Network is unreachable error + return e.errno != errno.ENETUNREACH + return True @@ -1640,29 +1648,3 @@ def __replace_url_target(config_files, new_smt): current_service_server, new_smt.get_FQDN()) ) - - -# ----------------------------------------------------------------------------- -def __connection_check(smt_ip_addr, propagate=False): - if isinstance(smt_ip_addr, ipaddress.IPv6Address): - # Per rfc3986 IPv6 addresses in a URI are enclosed in [] - smt_ip_addr= "[{}]".format(smt_ip_addr) - - protocol = 'http' # Default for backward compatibility - if https_only(get_config()): - protocol = 'https' - try: - cert_res = requests.get( - '%s://%s/smt.crt' % (protocol, smt_ip_addr), - timeout=3, - verify=False - ) - except requests.exceptions.ConnectionError as err: - if propagate: - raise - logging.error(err) - except Exception: - logging.error('Update server not reachable for %s' % smt_ip_addr) - return False - - return cert_res and cert_res.status_code == 200 diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 4ce81605..63236929 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -200,10 +200,7 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: print(msg, file=sys.stderr) sys.exit(1) - logging.info( - 'Attempt to access update server with IP {}'.format(args.user_smt_ip) - ) - if not utils.can_make_ip_request(args.user_smt_ip): + if not utils.instance_has_ip_enabled(args.user_smt_ip): error_message = ( 'Connection error: Could not establish a connection to {ip}.' 'Please, make sure the instance has that IP format enabled ' From 3ec3e12ceaf19a9c188283ed16a6f4172d7233f3 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 30 Apr 2024 14:48:57 +0100 Subject: [PATCH 09/14] Updates - Close socket connection - Return False for any socket error --- lib/cloudregister/registerutils.py | 9 ++++----- usr/sbin/registercloudguest | 10 ++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index f9101f73..30e30456 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1408,15 +1408,14 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def instance_has_ip_enabled(rmt_ip): +def has_network_access_by_ipversion(rmt_ip): """Check if the instance can make IPv4 or IPv6 requests.""" try: - socket.create_connection((rmt_ip, 443), timeout=2) + rmt_connection = socket.create_connection((rmt_ip, 443), timeout=2) except OSError as e: - # check socket connection produced - # Network is unreachable error - return e.errno != errno.ENETUNREACH + return False + rmt_connection.close() return True diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index 63236929..ea8ea45b 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -200,14 +200,13 @@ if args.user_smt_ip or args.user_smt_fqdn or args.user_smt_fp: print(msg, file=sys.stderr) sys.exit(1) - if not utils.instance_has_ip_enabled(args.user_smt_ip): + if not utils.has_network_access_by_ipversion(args.user_smt_ip): error_message = ( 'Connection error: Could not establish a connection to {ip}.' - 'Please, make sure the instance has that IP format enabled ' - 'and working properly'.format(ip=args.user_smt_ip) + 'Please, make sure the network configuration supports the ' + 'provided {ip} address version.'.format(ip=args.user_smt_ip) ) - print(error_message, file=sys.stderr) - sys.exit(1) + sys.exit(error_message) if args.clean_up and args.force_new_registration: @@ -223,7 +222,6 @@ if (args.email and not args.reg_code): time.sleep(int(args.delay_time)) - config_file = args.config_file if config_file: config_file = os.path.expanduser(args.config_file) From ab444b418d0d3b56d7bcd351aee89b40bbccb4bc Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Tue, 30 Apr 2024 18:14:41 +0100 Subject: [PATCH 10/14] Log error if socket connection failed --- lib/cloudregister/registerutils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 30e30456..e366f21f 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1408,11 +1408,12 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- -def has_network_access_by_ipversion(rmt_ip): - """Check if the instance can make IPv4 or IPv6 requests.""" +def has_network_access_by_ipversion(server_ip): + """Check if we can connect to the given server""" try: - rmt_connection = socket.create_connection((rmt_ip, 443), timeout=2) + rmt_connection = socket.create_connection((server_ip, 443), timeout=2) except OSError as e: + logging.info('Network access error: "%s"', e) return False rmt_connection.close() From 3e05174575edde5a37701f3d72b08b52803982c9 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 8 May 2024 13:00:16 +0100 Subject: [PATCH 11/14] Refactor IPv6 access checks - When checking if instance can access IPv6 infra server, make sure the instance can handle IPv6 addresses --- lib/cloudregister/registerutils.py | 20 ++++++++++++++++---- usr/sbin/registercloudguest | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index e366f21f..ab5652d2 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -53,7 +53,7 @@ def add_hosts_entry(smt_server): smt_hosts_entry_comment = '\n# Added by SMT registration do not remove, ' smt_hosts_entry_comment += 'retain comment as well\n' smt_ip = smt_server.get_ipv4() - if has_ipv6_access(smt_server): + if has_rmt_ipv6_access(smt_server): smt_ip = smt_server.get_ipv6() entry = '%s\t%s\t%s\n' % ( smt_ip, @@ -264,7 +264,7 @@ def fetch_smt_data(cfg, proxies, quiet=False): region_servers_ipv4.append(ip_addr) random.shuffle(region_servers_ipv4) random.shuffle(region_servers_ipv6) - if socket.has_ipv6: + if has_ipv6_access(): region_servers = region_servers_ipv6 + region_servers_ipv4 else: region_servers = region_servers_ipv4 @@ -873,10 +873,10 @@ def get_zypper_pid_cache(): # ---------------------------------------------------------------------------- -def has_ipv6_access(smt): +def has_rmt_ipv6_access(smt): """IPv6 access is possible if we have an SMT server that has an IPv6 address and it can be accessed over IPv6""" - if not smt.get_ipv6(): + if not has_ipv6_access() or not smt.get_ipv6(): return False logging.info('Attempt to access update server over IPv6') protocol = 'http' # Default for backward compatibility @@ -1407,6 +1407,18 @@ def write_framework_identifier(cfg): framework_file.write(json.dumps(identifier)) +# ---------------------------------------------------------------------------- +def has_ipv4_access(): + """Check if we have IPv4 network configuration""" + return has_network_access_by_ipversion('8.8.8.8') + + +# ---------------------------------------------------------------------------- +def has_ipv6_access(): + """Check if we have IPv6 network configuration""" + return has_network_access_by_ipversion('2001:4860:4860::8888') + + # ---------------------------------------------------------------------------- def has_network_access_by_ipversion(server_ip): """Check if we can connect to the given server""" diff --git a/usr/sbin/registercloudguest b/usr/sbin/registercloudguest index ea8ea45b..e5f3ebb4 100755 --- a/usr/sbin/registercloudguest +++ b/usr/sbin/registercloudguest @@ -367,7 +367,7 @@ if registration_smt: ): continue smt_ip = new_target.get_ipv4() - if utils.has_ipv6_access(new_target): + if utils.has_rmt_ipv6_access(new_target): smt_ip = new_target.get_ipv6() msg = 'Configured update server is unresponsive, switching ' msg += 'to equivalent update server with ip %s' % smt_ip From 3c0509ee83d35e4f6b95c0076bb24b6bada4ef88 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Wed, 8 May 2024 15:05:35 +0100 Subject: [PATCH 12/14] Update tests --- tests/test_registerutils.py | 113 +++++++++++++++++++++++++----------- 1 file changed, 80 insertions(+), 33 deletions(-) diff --git a/tests/test_registerutils.py b/tests/test_registerutils.py index 01f050a2..1bb34154 100644 --- a/tests/test_registerutils.py +++ b/tests/test_registerutils.py @@ -503,8 +503,8 @@ def test_clean_host_file_raised_exception(): assert m().write.mock_calls == [] -@patch('cloudregister.registerutils.has_ipv6_access') -def test_add_hosts_entry(mock_has_ipv6_access): +@patch('cloudregister.registerutils.has_rmt_ipv6_access') +def test_add_hosts_entry(mock_has_rmt_ipv6_access): """Test hosts entry has a new entry added by us.""" smt_data_ipv46 = dedent('''\ ''') smt_server = SMT(etree.fromstring(smt_data_ipv46)) - mock_has_ipv6_access.return_value = True + mock_has_rmt_ipv6_access.return_value = True with patch('builtins.open', create=True) as mock_open: mock_open.return_value = MagicMock(spec=io.IOBase) file_handle = mock_open.return_value.__enter__.return_value utils.add_hosts_entry(smt_server) - mock_open.assert_called_once_with('/etc/hosts', 'a') - file_content_comment = ( - '\n# Added by SMT registration do not remove, ' - 'retain comment as well\n' - ) - file_content_entry = '{ip}\t{fqdn}\t{name}\n'.format( - ip=smt_server.get_ipv6(), - fqdn=smt_server.get_FQDN(), - name=smt_server.get_name() - ) - assert file_handle.write.mock_calls == [ - call(file_content_comment), - call(file_content_entry) - ] + mock_open.assert_called_once_with('/etc/hosts', 'a') + file_content_comment = ( + '\n# Added by SMT registration do not remove, ' + 'retain comment as well\n' + ) + file_content_entry = '{ip}\t{fqdn}\t{name}\n'.format( + ip=smt_server.get_ipv6(), + fqdn=smt_server.get_FQDN(), + name=smt_server.get_name() + ) + assert file_handle.write.mock_calls == [ + call(file_content_comment), + call(file_content_entry) + ] @patch('cloudregister.amazonec2.generateRegionSrvArgs') @@ -708,17 +708,20 @@ def test_fetch_smt_data_metadata_server( etree.tostring(smt_server, encoding='utf-8') +@patch('cloudregister.registerutils.has_network_access_by_ipversion') @patch('cloudregister.registerutils.time.sleep') @patch('cloudregister.registerutils.logging') def test_fetch_smt_data_api_no_answer( mock_logging, - mock_time_sleep + mock_time_sleep, + mock_has_network_access ): cfg = get_test_config() del cfg['server']['metadata_server'] cfg.set('server', 'regionsrv', '1.1.1.1') with raises(SystemExit): utils.fetch_smt_data(cfg, None) + mock_has_network_access.return_value = False assert mock_logging.info.call_args_list == [ call('Using API: regionInfo'), call('Getting update server information, attempt 1'), @@ -749,7 +752,7 @@ def test_fetch_smt_data_api_no_answer( ] -@patch('cloudregister.registerutils.socket.has_ipv6', False) +@patch('cloudregister.registerutils.has_network_access_by_ipversion') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -759,6 +762,7 @@ def test_fetch_smt_data_api_answered( mock_time_sleep, mock_os_path_isfile, mock_request_get, + mock_has_network_access ): cfg = get_test_config() del cfg['server']['metadata_server'] @@ -776,6 +780,7 @@ def test_fetch_smt_data_api_answered( ''') response.text = smt_xml mock_request_get.return_value = response + mock_has_network_access.return_value = False utils.fetch_smt_data(cfg, None) assert mock_logging.info.call_args_list == [ call('Using API: regionInfo'), @@ -819,6 +824,7 @@ def test_fetch_smt_data_api_no_valid_ip( assert etree.tostring(smt_data, encoding='utf-8') == smt_xml.encode() +@patch('cloudregister.registerutils.has_network_access_by_ipversion') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -828,6 +834,7 @@ def test_fetch_smt_data_api_error_response( mock_time_sleep, mock_os_path_isfile, mock_request_get, + mock_has_network_access ): cfg = get_test_config() del cfg['server']['metadata_server'] @@ -837,8 +844,10 @@ def test_fetch_smt_data_api_error_response( response.status_code = 422 response.reason = 'well, you shall not pass' mock_request_get.return_value = response + mock_has_network_access.return_value = False with raises(SystemExit): utils.fetch_smt_data(cfg, None) + print(mock_logging.info.call_args_list) assert mock_logging.info.call_args_list == [ call('Using API: regionInfo'), call('Getting update server information, attempt 1'), @@ -871,6 +880,7 @@ def test_fetch_smt_data_api_error_response( ] +@patch('cloudregister.registerutils.has_network_access_by_ipversion') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -879,7 +889,8 @@ def test_fetch_smt_data_api_exception( mock_logging, mock_time_sleep, mock_os_path_isfile, - mock_request_get + mock_request_get, + mock_has_network_access ): cfg = get_test_config() del cfg['server']['metadata_server'] @@ -889,6 +900,7 @@ def test_fetch_smt_data_api_exception( response.status_code = 422 response.reason = 'well, you shall not pass' mock_request_get.side_effect = requests.exceptions.RequestException('foo') + mock_has_network_access.return_value = True with raises(SystemExit): utils.fetch_smt_data(cfg, None) assert mock_logging.info.call_args_list == [ @@ -917,6 +929,7 @@ def test_fetch_smt_data_api_exception( ] +@patch('cloudregister.registerutils.has_network_access_by_ipversion') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -925,7 +938,8 @@ def test_fetch_smt_data_api_exception_quiet( mock_logging, mock_time_sleep, mock_os_path_isfile, - mock_request_get + mock_request_get, + mock_has_network_access ): cfg = get_test_config() del cfg['server']['metadata_server'] @@ -935,6 +949,7 @@ def test_fetch_smt_data_api_exception_quiet( response.status_code = 422 response.reason = 'well, you shall not pass' mock_request_get.side_effect = requests.exceptions.RequestException('foo') + mock_has_network_access.return_value = True with raises(SystemExit): utils.fetch_smt_data(cfg, 'foo', quiet=True) assert mock_logging.info.call_args_list == [ @@ -1178,16 +1193,21 @@ def test_get_current_smt_no_match(mock_get_smt_from_store, mock_os_unlink): utils.get_current_smt() +@patch('cloudregister.registerutils.glob.glob') @patch('cloudregister.registerutils.get_smt_from_store') -def test_get_current_smt_no_registered(mock_get_smt_from_store): +def test_get_current_smt_no_registered( + mock_get_smt_from_store, mock_glob_glob +): smt_data_ipv46 = dedent('''\ ''') - smt_server = SMT(etree.fromstring(smt_data_ipv46)) - mock_get_smt_from_store.return_value = smt_server + mock_get_smt_from_store.return_value = SMT( + etree.fromstring(smt_data_ipv46) + ) + mock_glob_glob.return_value = [] hosts_content = """ # simulates hosts file containing the ipv4 we are looking for in the test @@ -1783,20 +1803,26 @@ def test_get_zypper_pid(mock_popen): assert utils.get_zypper_pid() == 'pid' -def test_has_ipv6_access_no_ipv6_defined(): +@patch('cloudregister.registerutils.has_ipv6_access') +def test_has_rmt_ipv6_access_no_ipv6_defined(mock_ipv6_access): smt_data_ipv4 = dedent('''\ ''') smt_server = SMT(etree.fromstring(smt_data_ipv4)) - assert utils.has_ipv6_access(smt_server) is False + mock_ipv6_access.return_value = True + assert utils.has_rmt_ipv6_access(smt_server) is False +@patch('cloudregister.registerutils.has_ipv6_access') @patch('cloudregister.registerutils.get_config') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.https_only') -def test_has_ipv6_access_https(mock_https_only, mock_request, mock_get_config): +def test_has_rmt_ipv6_access_https( + mock_https_only, mock_request, + mock_get_config, mock_ipv6_access +): smt_data_ipv46 = dedent('''\ Date: Mon, 13 May 2024 09:32:59 +0100 Subject: [PATCH 13/14] Better name for variable --- lib/cloudregister/registerutils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index 9036756d..b8e60ada 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1458,12 +1458,12 @@ def has_ipv6_access(): def has_network_access_by_ipversion(server_ip): """Check if we can connect to the given server""" try: - rmt_connection = socket.create_connection((server_ip, 443), timeout=2) + connection = socket.create_connection((server_ip, 443), timeout=2) except OSError as e: logging.info('Network access error: "%s"', e) return False - rmt_connection.close() + connection.close() return True From 6d8b2acc0859b7f97a24af5c78fc624d498cae26 Mon Sep 17 00:00:00 2001 From: Jesus Bermudez Velazquez Date: Fri, 17 May 2024 10:55:42 +0100 Subject: [PATCH 14/14] Rename method for accuracy --- lib/cloudregister/registerutils.py | 6 +++--- tests/test_registerutils.py | 18 +++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/cloudregister/registerutils.py b/lib/cloudregister/registerutils.py index b8e60ada..1e7157d7 100644 --- a/lib/cloudregister/registerutils.py +++ b/lib/cloudregister/registerutils.py @@ -1445,17 +1445,17 @@ def write_framework_identifier(cfg): # ---------------------------------------------------------------------------- def has_ipv4_access(): """Check if we have IPv4 network configuration""" - return has_network_access_by_ipversion('8.8.8.8') + return has_network_access_by_ip_address('8.8.8.8') # ---------------------------------------------------------------------------- def has_ipv6_access(): """Check if we have IPv6 network configuration""" - return has_network_access_by_ipversion('2001:4860:4860::8888') + return has_network_access_by_ip_address('2001:4860:4860::8888') # ---------------------------------------------------------------------------- -def has_network_access_by_ipversion(server_ip): +def has_network_access_by_ip_address(server_ip): """Check if we can connect to the given server""" try: connection = socket.create_connection((server_ip, 443), timeout=2) diff --git a/tests/test_registerutils.py b/tests/test_registerutils.py index 0c1297ec..aab3474f 100644 --- a/tests/test_registerutils.py +++ b/tests/test_registerutils.py @@ -747,7 +747,7 @@ def test_fetch_smt_data_metadata_server( etree.tostring(smt_server, encoding='utf-8') -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') @patch('cloudregister.registerutils.time.sleep') @patch('cloudregister.registerutils.logging') def test_fetch_smt_data_api_no_answer( @@ -791,7 +791,7 @@ def test_fetch_smt_data_api_no_answer( ] -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -863,7 +863,7 @@ def test_fetch_smt_data_api_no_valid_ip( assert etree.tostring(smt_data, encoding='utf-8') == smt_xml.encode() -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -919,7 +919,7 @@ def test_fetch_smt_data_api_error_response( ] -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -968,7 +968,7 @@ def test_fetch_smt_data_api_exception( ] -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') @patch('cloudregister.registerutils.requests.get') @patch('cloudregister.registerutils.os.path.isfile') @patch('cloudregister.registerutils.time.sleep') @@ -3046,21 +3046,21 @@ def test_remove_service( mock_logging.info.not_called() -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') def test_has_ipv4_access(mock_has_network_access): mock_has_network_access.return_value = True assert utils.has_ipv4_access() -@patch('cloudregister.registerutils.has_network_access_by_ipversion') +@patch('cloudregister.registerutils.has_network_access_by_ip_address') def test_has_ipv6_access(mock_has_network_access): mock_has_network_access.return_value = True assert utils.has_ipv6_access() @patch('cloudregister.registerutils.socket.create_connection') -def test_has_network_access_by_ipversion(mock_socket_create_connection): - assert utils.has_network_access_by_ipversion('1.1.1.1') +def test_has_network_access_by_ip_address(mock_socket_create_connection): + assert utils.has_network_access_by_ip_address('1.1.1.1') # ---------------------------------------------------------------------------