From da544d12ac0c39a54936a6e126c2e1447d344323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Thu, 5 Dec 2024 15:31:13 +0100 Subject: [PATCH 1/8] feat: Print config on connection test For non-legacy, --test-connection dumps a user-friendly connection configuration. First, the authentication information is printed starting with type. For BASIC, username is printed. For CERT, certificate and key paths are verified and printed. In case of missing files or credentials, the connection test fails immediately. Second, tested URLs (base, Ingress, Inventory, API cast) are listed and server type (production, staging, Satellite) is determined. HTTPS proxy information is included. --- insights/client/connection.py | 98 +++++++++++++++++++++++++++++++---- insights/client/phase/v1.py | 1 - 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 656ba1b448..09e3134de1 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -124,7 +124,10 @@ def __init__(self, config): else: self.upload_url = self.base_url + '/ingress/v1/upload' - self.api_url = self.base_url + if self.config.legacy_upload: + self.api_url = self.base_url + "/platform" + else: + self.api_url = self.base_url self.branch_info_url = self.config.branch_info_url if self.branch_info_url is None: # workaround for a workaround for a workaround @@ -132,6 +135,11 @@ def __init__(self, config): self.branch_info_url = base_url_base + '/v1/branch_info' self.inventory_url = self.api_url + "/inventory/v1" + if self.config.legacy_upload: + self.ping_url = self.base_url + else: + self.ping_url = self.base_url + '/apicast-tests/ping' + self.authmethod = self.config.authmethod self.systemid = self.config.systemid or None self.get_proxies() @@ -402,21 +410,94 @@ def _test_urls(self, url, method): print(exc) raise + def _test_auth_config(self): + errors = [] + if self.authmethod == "BASIC": + logger.info("Authentication: login credentials ({})".format(self.authmethod)) + + for desc, var, placeholder in [ + ("Username", self.username, None), + ("Password", self.password, "********"), + ]: + if not var: + error = "{} not set.".format(desc) + errors.append(error) + + val = placeholder or var if var else "not set" + logger.info(" %s: %s", desc, val) + elif self.authmethod == "CERT": + logger.info("Authentication: identity certificate ({})".format(self.authmethod)) + + for desc, path_func in [ + ("Certificate", rhsmCertificate.certpath), + ("Key", rhsmCertificate.keypath), + ]: + path = path_func() + exists = os.path.exists(path) + if exists: + exists_description = "exists" + else: + exists_description = "NOT FOUND" + error = "{} file '{}' missing.".format(desc, path) + errors.append(error) + logger.info(" %s: %s (%s)", desc, path, exists_description) + else: + logger.info("Authentication: unknown") + errors.append = "Unknown authentication method '{}'.".format(self.authmethod) + logger.info("") + + if errors: + logger.error("") + logger.error("ERROR. Cannot authenticate:") + for error in errors: + logger.error(" %s", error) + + return not errors + + def _dump_urls(self): + base_parsed = urlparse(self.base_url) + if base_parsed.hostname.endswith("stage.redhat.com"): + hostname_desc = "Red Hat Insights (staging)" + elif base_parsed.hostname.endswith("redhat.com"): + if self.config.verbose: + hostname_desc = "Red Hat Insights (production)" + else: + hostname_desc = "Red Hat Insights" + else: + hostname_desc = "Satellite" + + logger.info("Running Connection Tests against %s...", hostname_desc) + + logger.info(" Base URL: %s", self.base_url) + logger.info(" Upload URL: %s", self.upload_url) + logger.info(" Inventory URL: %s", self.inventory_url) + logger.info(" Ping URL: %s", self.ping_url) + + if self.proxies: + for proxy_type, proxy_url in self.proxies.items(): + logger.info(" %s proxy: %s", proxy_type.upper(), proxy_url) + else: + logger.info(" Proxy: not set") + + logger.info("") + def test_connection(self, rc=0): """ Test connection to Red Hat """ - logger.debug("Proxy config: %s", self.proxies) + auth_config_ok = self._test_auth_config() + if not auth_config_ok: + return 1 + + self._dump_urls() + try: logger.info("=== Begin Upload URL Connection Test ===") upload_success = self._test_urls(self.upload_url, "POST") logger.info("=== End Upload URL Connection Test: %s ===\n", "SUCCESS" if upload_success else "FAILURE") logger.info("=== Begin API URL Connection Test ===") - if self.config.legacy_upload: - api_success = self._test_urls(self.base_url, "GET") - else: - api_success = self._test_urls(self.base_url + '/apicast-tests/ping', 'GET') + api_success = self._test_urls(self.ping_url, 'GET') logger.info("=== End API URL Connection Test: %s ===\n", "SUCCESS" if api_success else "FAILURE") if upload_success and api_success: @@ -741,10 +822,7 @@ def _fetch_system_by_machine_id(self): machine_id = generate_machine_id() try: # [circus music] - if self.config.legacy_upload: - url = self.base_url + '/platform/inventory/v1/hosts?insights_id=' + machine_id - else: - url = self.inventory_url + '/hosts?insights_id=' + machine_id + url = self.inventory_url + '/hosts?insights_id=' + machine_id res = self.get(url) except REQUEST_FAILED_EXCEPTIONS as e: _api_request_failed(e) diff --git a/insights/client/phase/v1.py b/insights/client/phase/v1.py index 11332172c8..11817c8283 100644 --- a/insights/client/phase/v1.py +++ b/insights/client/phase/v1.py @@ -93,7 +93,6 @@ def pre_update(client, config): # test the insights connection if config.test_connection: - logger.info("Running Connection Tests...") rc = client.test_connection() if rc == 0: sys.exit(constants.sig_kill_ok) From 77d505ed7829e2f74051239151910387dd698c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Thu, 12 Dec 2024 16:54:18 +0100 Subject: [PATCH 2/8] feat: Improve test URLs output _test_urls and _legacy_test_urls output is nicer, with clear SUCCESS/FAILURE statement. URLs are consistently listed, so is legacy fallback. With --verbose turned on, more information about requests, responses and errors are printed. The readability of the output improved drastically, with only little changes to the logging and tiny touches to the logic. The generic HTTP method logs information about the request. To make the log messages blend nicely into the connection test, introduced logging-related arguments: * To keep the output concise by default, but more helpful with --verbose, log_level suppresses HTTP details. * To match indentation with messages outside the request method, log_prefix allows to add spaces to the beginning. --- insights/client/connection.py | 105 ++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 09e3134de1..1ad7faba64 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -168,7 +168,7 @@ def _init_session(self): session.trust_env = False return session - def _http_request(self, url, method, log_response_text=True, **kwargs): + def _http_request(self, url, method, log_response_text=True, log_prefix="", log_level=NETWORK, **kwargs): ''' Perform an HTTP request, net logging, and error handling Parameters @@ -178,7 +178,7 @@ def _http_request(self, url, method, log_response_text=True, **kwargs): Returns HTTP response object ''' - log_message = "{method} {url}".format(method=method, url=url) + log_message = "{log_prefix}{method} {url}".format(log_prefix=log_prefix, method=method, url=url) if "data" in kwargs.keys(): log_message += " data={data}".format(data=kwargs["data"]) if "json" in kwargs.keys(): @@ -193,14 +193,14 @@ def _http_request(self, url, method, log_response_text=True, **kwargs): else: attachments.append(name) log_message += " attachments={files}".format(files=",".join(attachments)) - logger.log(NETWORK, log_message) + logger.log(log_level, log_message) try: res = self.session.request(url=url, method=method, timeout=self.config.http_timeout, **kwargs) except Exception: raise - logger.log(NETWORK, "HTTP Status: %d %s", res.status_code, res.reason) + logger.log(log_level, "%sHTTP Status: %d %s", log_prefix, res.status_code, res.reason) if log_response_text or res.status_code // 100 != 2: - logger.log(NETWORK, "HTTP Response Text: %s", res.text) + logger.log(log_level, "%sHTTP Response Text: %s", log_prefix, res.text) return res def get(self, url, **kwargs): @@ -357,26 +357,24 @@ def _legacy_test_urls(self, url, method): test_url = url.scheme + "://" + url.netloc last_ex = None paths = (url.path + '/', '', '/r', '/r/insights') + log_level = NETWORK if self.config.verbose else logging.DEBUG for ext in paths: try: - logger.log(NETWORK, "Testing: %s", test_url + ext) + logger.info(" Testing %s", test_url + ext) if method == "POST": - test_req = self.post(test_url + ext, data=test_flag) + test_req = self.post(test_url + ext, data=test_flag, log_prefix=" ", log_level=log_level) elif method == "GET": - test_req = self.get(test_url + ext) + test_req = self.get(test_url + ext, log_prefix=" ", log_level=log_level) # Strata returns 405 on a GET sometimes, this isn't a big deal if test_req.status_code in (200, 201): - logger.info( - "Successfully connected to: %s", test_url + ext) return True else: - logger.info("Connection failed") + logger.error(" Failed.") return False except REQUEST_FAILED_EXCEPTIONS as exc: last_ex = exc - logger.error( - "Could not successfully connect to: %s", test_url + ext) - print(exc) + logger.debug(" Caught %s: %s", type(exc).__name__, exc) + logger.error(" Failed.") if last_ex: raise last_ex @@ -387,27 +385,26 @@ def _test_urls(self, url, method): if self.config.legacy_upload: return self._legacy_test_urls(url, method) try: - logger.log(NETWORK, 'Testing %s', url) + logger.info(' Testing %s', url) + + log_prefix = " " + log_level = NETWORK if self.config.verbose else logging.DEBUG + if method == 'POST': test_tar = TemporaryFile(mode='rb', suffix='.tar.gz') test_files = { 'file': ('test.tar.gz', test_tar, 'application/vnd.redhat.advisor.collection+tgz'), 'metadata': '{\"test\": \"test\"}' } - test_req = self.post(url, files=test_files) + test_req = self.post(url, files=test_files, log_prefix=log_prefix, log_level=log_level) elif method == "GET": - test_req = self.get(url) + test_req = self.get(url, log_prefix=log_prefix, log_level=log_level) if test_req.status_code in (200, 201, 202): - logger.info( - "Successfully connected to: %s", url) return True else: - logger.info("Connection failed") return False except REQUEST_FAILED_EXCEPTIONS as exc: - logger.error( - "Could not successfully connect to: %s", url) - print(exc) + logger.debug(" Caught %s: %s", type(exc).__name__, exc) raise def _test_auth_config(self): @@ -468,10 +465,13 @@ def _dump_urls(self): logger.info("Running Connection Tests against %s...", hostname_desc) - logger.info(" Base URL: %s", self.base_url) - logger.info(" Upload URL: %s", self.upload_url) - logger.info(" Inventory URL: %s", self.inventory_url) - logger.info(" Ping URL: %s", self.ping_url) + urls = [ + (self.base_url, "Base"), + (self.upload_url, "Upload"), + (self.ping_url, "Ping"), + ] + for url, title in urls: + logger.info(" %s URL: %s", title, url) if self.proxies: for proxy_type, proxy_url in self.proxies.items(): @@ -491,28 +491,35 @@ def test_connection(self, rc=0): self._dump_urls() - try: - logger.info("=== Begin Upload URL Connection Test ===") - upload_success = self._test_urls(self.upload_url, "POST") - logger.info("=== End Upload URL Connection Test: %s ===\n", - "SUCCESS" if upload_success else "FAILURE") - logger.info("=== Begin API URL Connection Test ===") - api_success = self._test_urls(self.ping_url, 'GET') - logger.info("=== End API URL Connection Test: %s ===\n", - "SUCCESS" if api_success else "FAILURE") - if upload_success and api_success: - logger.info("Connectivity tests completed successfully") - print("See %s for more details." % self.config.logging_file) - else: - logger.info("Connectivity tests completed with some errors") - print("See %s for more details." % self.config.logging_file) - rc = 1 - except REQUEST_FAILED_EXCEPTIONS: - logger.error('Connectivity test failed! ' - 'Please check your network configuration') - print('Additional information may be in %s' % self.config.logging_file) - return 1 - return rc + urls = [ + ("POST", self.upload_url, "Uploading a file to Ingress"), + ("GET", self.ping_url, "Pinging the API"), + ] + for method, url, description in urls: + logger.info(" %s...", description) + + try: + success = self._test_urls(url, method) + except REQUEST_FAILED_EXCEPTIONS as exc: + last_ex = exc + success = False + + if not success: + break + + logger.info(" SUCCESS.") + logger.info("") + else: + logger.info(" See %s for more details." % self.config.logging_file) + return rc + + logger.error(" FAILED.") + logger.error("") + logger.error(" Please check your network configuration") + logger.error(" Additional information may be in %s" % self.config.logging_file) + logger.error("") + + return 1 def handle_fail_rcs(self, req): """ From fe3b3532c79d1f0b5471ae6699d4547aee2f3d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Mon, 30 Dec 2024 13:57:25 +0100 Subject: [PATCH 3/8] chore: Use return for flow control Exceptions in _(legacy_)test_urls are merely used for control-flow. Known ones are re-thrown and re-caught in test_connection, unknown ones are not caught at all. Return is more appropriate: _test_urls passes the result, test_connection decides how to handle it. --- insights/client/connection.py | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 1ad7faba64..56e4f80d4e 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -362,21 +362,14 @@ def _legacy_test_urls(self, url, method): try: logger.info(" Testing %s", test_url + ext) if method == "POST": - test_req = self.post(test_url + ext, data=test_flag, log_prefix=" ", log_level=log_level) + return self.post(test_url + ext, data=test_flag, log_prefix=" ", log_level=log_level) elif method == "GET": - test_req = self.get(test_url + ext, log_prefix=" ", log_level=log_level) - # Strata returns 405 on a GET sometimes, this isn't a big deal - if test_req.status_code in (200, 201): - return True - else: - logger.error(" Failed.") - return False + return self.get(test_url + ext, log_prefix=" ", log_level=log_level) except REQUEST_FAILED_EXCEPTIONS as exc: last_ex = exc logger.debug(" Caught %s: %s", type(exc).__name__, exc) logger.error(" Failed.") - if last_ex: - raise last_ex + return last_ex def _test_urls(self, url, method): ''' @@ -396,16 +389,12 @@ def _test_urls(self, url, method): 'file': ('test.tar.gz', test_tar, 'application/vnd.redhat.advisor.collection+tgz'), 'metadata': '{\"test\": \"test\"}' } - test_req = self.post(url, files=test_files, log_prefix=log_prefix, log_level=log_level) + return self.post(url, files=test_files, log_prefix=log_prefix, log_level=log_level) elif method == "GET": - test_req = self.get(url, log_prefix=log_prefix, log_level=log_level) - if test_req.status_code in (200, 201, 202): - return True - else: - return False + return self.get(url, log_prefix=log_prefix, log_level=log_level) except REQUEST_FAILED_EXCEPTIONS as exc: logger.debug(" Caught %s: %s", type(exc).__name__, exc) - raise + return exc def _test_auth_config(self): errors = [] @@ -498,13 +487,12 @@ def test_connection(self, rc=0): for method, url, description in urls: logger.info(" %s...", description) - try: - success = self._test_urls(url, method) - except REQUEST_FAILED_EXCEPTIONS as exc: - last_ex = exc - success = False + result = self._test_urls(url, method) + if isinstance(result, REQUEST_FAILED_EXCEPTIONS): + break - if not success: + # Strata returns 405 on a GET sometimes, this isn't a big deal + if result.status_code not in (requests.codes.ok, requests.codes.created, requests.codes.accepted): break logger.info(" SUCCESS.") From 879ab60ee7c5a31ea60f965d5d00a0890df0a072 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Mon, 16 Dec 2024 10:52:09 +0100 Subject: [PATCH 4/8] feat: Test GET from Inventory Inventory is tested along with Ingress and an API ping. Hosts are listed as the most basic Inventory GET request. --- insights/client/connection.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 56e4f80d4e..6147ce6a18 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -136,7 +136,7 @@ def __init__(self, config): self.inventory_url = self.api_url + "/inventory/v1" if self.config.legacy_upload: - self.ping_url = self.base_url + self.ping_url = self.base_url + "/" else: self.ping_url = self.base_url + '/apicast-tests/ping' @@ -356,7 +356,7 @@ def _legacy_test_urls(self, url, method): url = urlparse(url) test_url = url.scheme + "://" + url.netloc last_ex = None - paths = (url.path + '/', '', '/r', '/r/insights') + paths = (url.path, '', '/r', '/r/insights') log_level = NETWORK if self.config.verbose else logging.DEBUG for ext in paths: try: @@ -457,6 +457,7 @@ def _dump_urls(self): urls = [ (self.base_url, "Base"), (self.upload_url, "Upload"), + (self.inventory_url, "Inventory"), (self.ping_url, "Ping"), ] for url, title in urls: @@ -482,6 +483,7 @@ def test_connection(self, rc=0): urls = [ ("POST", self.upload_url, "Uploading a file to Ingress"), + ("GET", self.inventory_url + "/hosts", "Getting hosts from Inventory"), ("GET", self.ping_url, "Pinging the API"), ] for method, url, description in urls: From c552b7aa976ce7d7d83d9a79b9f29e2d8e18ca33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Mon, 16 Dec 2024 14:43:58 +0100 Subject: [PATCH 5/8] feat: Check connection In case of DNS failure. The DNS is queried, then a connection is established to the resolved IP. If resolving fails, a hard-coded IP is tried for production or staging. In case of either failure, DNS query for a public CloudFlare URL one.one.one.one and its IP 1.1.1.1 is tried. --- insights/client/connection.py | 49 +++++++++++++++++++++++++++++++++++ insights/client/constants.py | 4 +++ 2 files changed, 53 insertions(+) diff --git a/insights/client/connection.py b/insights/client/connection.py index 6147ce6a18..9e0d2f03f4 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -4,6 +4,7 @@ from __future__ import print_function from __future__ import absolute_import import requests +import socket import os import six import json @@ -18,10 +19,12 @@ try: # python 2 from urlparse import urlparse + from urlparse import urlunparse from urllib import quote except ImportError: # python 3 from urllib.parse import urlparse + from urllib.parse import urlunparse from urllib.parse import quote from .utilities import (determine_hostname, generate_machine_id, @@ -74,6 +77,26 @@ def _api_request_failed(exception, message='The Insights API could not be reache logger.error(message) +def _is_dns_error(exception): + while isinstance(exception, Exception): + if isinstance(exception, socket.gaierror): + return True + + exception = exception.__context__ + + return False + + +def _fallback_ip(hostname): + if hostname.endswith("redhat.com"): + if hostname.endswith("stage.redhat.com"): + return constants.insights_ip_stage + else: + return constants.insights_ip_prod + else: + return None + + class InsightsConnection(object): """ @@ -471,6 +494,29 @@ def _dump_urls(self): logger.info("") + def _test_connection(self, url): + parsed_url = urlparse(url) + logger.error(" Could not resolve %s.", parsed_url.hostname) + fallback = [constants.stable_public_url, constants.stable_public_ip] + ip = _fallback_ip(parsed_url.hostname) + if ip: + fallback = [ip] + fallback + for fallback_url in fallback: + parsed_ip_url = urlunparse((parsed_url.scheme, fallback_url, "/", "", "", "")) + try: + logger.info(' Testing %s', parsed_ip_url) + log_prefix = " " + log_level = NETWORK if self.config.verbose else logging.DEBUG + self.get(parsed_ip_url, log_prefix=log_prefix, log_level=log_level, verify=False) + except REQUEST_FAILED_EXCEPTIONS as exc: + logger.debug(" Caught %s: %s", type(exc).__name__, exc) + logger.error(" Failed.") + else: + logger.info(" SUCCESS.") + break + else: + logger.info(" FAILED.") + def test_connection(self, rc=0): """ Test connection to Red Hat @@ -509,6 +555,9 @@ def test_connection(self, rc=0): logger.error(" Additional information may be in %s" % self.config.logging_file) logger.error("") + if _is_dns_error(result): + self._test_connection(url) + return 1 def handle_fail_rcs(self, req): diff --git a/insights/client/constants.py b/insights/client/constants.py index 688878ad4a..6087096a90 100644 --- a/insights/client/constants.py +++ b/insights/client/constants.py @@ -90,3 +90,7 @@ class InsightsConstants(object): rhsm_facts_file = os.path.join(os.sep, 'etc', 'rhsm', 'facts', 'insights-client.facts') # In MB archive_filesize_max = 100 + insights_ip_prod = "23.37.45.238" + insights_ip_stage = "23.53.5.13" + stable_public_url = "one.one.one.one" # Public CloudFlare DNS + stable_public_ip = "1.1.1.1" # Public CloudFlare DNS From aecf4833cf9d87a291190f02383614cf72856923 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Tue, 31 Dec 2024 14:39:47 +0100 Subject: [PATCH 6/8] feat: Recognize more errors * 429 Too Many requests means the rate limit was hit. * 401 Unauthorized from gateway means the username/password is invalid. * SSLError means the key/certificate pair is invalid. * SSL: WRONG_VERSION_NUMBER in the SSLError means that HTTPS has been used to contact an HTTP server. * ConnectionTimeout and ReadTimeout may mean the connection is slow. --- insights/client/connection.py | 75 +++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 9e0d2f03f4..95fbd14ed6 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -77,16 +77,32 @@ def _api_request_failed(exception, message='The Insights API could not be reache logger.error(message) -def _is_dns_error(exception): - while isinstance(exception, Exception): - if isinstance(exception, socket.gaierror): - return True +def _is_basic_auth_error(result): + if result.status_code != 401 or result.headers["Content-Type"] != "application/json": + return False - exception = exception.__context__ + try: + body = result.json() + except requests.exceptions.JSONDecodeError: + return False + + if not isinstance(body, dict) or "errors" not in body or not isinstance(body["errors"], list): + return False + + for error in body["errors"]: + if "status" in error and error["status"] == 401 and "meta" in error and isinstance(error["meta"], dict) and "response_by" in error["meta"] and error["meta"]["response_by"] == "gateway": + return True return False +def _exception_root_cause(exception): + while True: + if not exception.__context__: + return exception + exception = exception.__context__ + + def _fallback_ip(hostname): if hostname.endswith("redhat.com"): if hostname.endswith("stage.redhat.com"): @@ -217,6 +233,8 @@ def _http_request(self, url, method, log_response_text=True, log_prefix="", log_ attachments.append(name) log_message += " attachments={files}".format(files=",".join(attachments)) logger.log(log_level, log_message) + if "verify" not in kwargs: + kwargs["verify"] = "/home/insights/simple-http-server/cert.pem" try: res = self.session.request(url=url, method=method, timeout=self.config.http_timeout, **kwargs) except Exception: @@ -494,15 +512,13 @@ def _dump_urls(self): logger.info("") - def _test_connection(self, url): - parsed_url = urlparse(url) - logger.error(" Could not resolve %s.", parsed_url.hostname) + def _test_connection(self, scheme, hostname): fallback = [constants.stable_public_url, constants.stable_public_ip] - ip = _fallback_ip(parsed_url.hostname) + ip = _fallback_ip(hostname) if ip: fallback = [ip] + fallback for fallback_url in fallback: - parsed_ip_url = urlunparse((parsed_url.scheme, fallback_url, "/", "", "", "")) + parsed_ip_url = urlunparse((scheme, fallback_url, "/", "", "", "")) try: logger.info(' Testing %s', parsed_ip_url) log_prefix = " " @@ -555,8 +571,43 @@ def test_connection(self, rc=0): logger.error(" Additional information may be in %s" % self.config.logging_file) logger.error("") - if _is_dns_error(result): - self._test_connection(url) + if isinstance(result, REQUEST_FAILED_EXCEPTIONS): + root_cause = _exception_root_cause(result) + if isinstance(result, requests.exceptions.SSLError): + if "[SSL: WRONG_VERSION_NUMBER]" in str(root_cause): + logger.error(" Invalid protocol.") + else: + logger.error(" Invalid key or certificate.") + elif isinstance(result, requests.exceptions.Timeout): + if isinstance(result, requests.exceptions.ConnectTimeout): + logger.error(" Connection timed out.") + elif isinstance(result, requests.exceptions.ReadTimeout): + logger.error(" Read timed out.") + else: + logger.error(" Timeout.") # Cannot happen. + + parsed_url = urlparse(url) + self._test_connection(parsed_url.scheme, parsed_url.hostname) + elif isinstance(result, requests.exceptions.ConnectionError): + if isinstance(root_cause, socket.gaierror): + parsed_url = urlparse(url) + logger.error(" Could not resolve base URL host %s.", parsed_url.hostname) + self._test_connection(parsed_url.scheme, parsed_url.hostname) + if isinstance(root_cause, (ConnectionRefusedError, ConnectionResetError)): + logger.error(" Connection refused.") + else: + logger.error(" Connection error.") # Should not happen. + else: + logger.error(" Unknown error %s.", result) # Should not happen. + elif isinstance(result, requests.Response): + if _is_basic_auth_error(result): + logger.error(" Invalid username or password.") + elif result.status_code == requests.codes.too_many_requests: + logger.error(" Too many requests.") + else: + logger.error(" Unknown response %s %s.", result.status_code, result.reason) + else: + logger.error(" Unknown result %s.", result) # Cannot happen. return 1 From 7fa1127ebd4ecf3c8d8b3e04b7f607536cc777ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Thu, 2 Jan 2025 17:26:38 +0100 Subject: [PATCH 7/8] feat: Detect proxy errors HTTPS proxy introduces several possible error cases, similar to the actual remote server connection: * proxy name resolution (DNS) error, * proxy connection error, * proxy authentication error. The proxy authentication error can only be recognized by a string in the underlying OSError: the outer exception is a plain remote server connection error. Although the proxy is used for HTTPS connection, the actual communication for the proxy itself is HTTP. Thus, specifying a HTTPS protocol for the proxy causes a specific WRONG_VERSION_NUMBER SSL error. --- insights/client/connection.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 95fbd14ed6..f36bb4c732 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -573,7 +573,19 @@ def test_connection(self, rc=0): if isinstance(result, REQUEST_FAILED_EXCEPTIONS): root_cause = _exception_root_cause(result) - if isinstance(result, requests.exceptions.SSLError): + if isinstance(result, requests.exceptions.ProxyError): + scheme = urlparse(url).scheme + proxy_url = self.proxies[scheme] + if isinstance(root_cause, socket.gaierror): + logger.error(" Could not resolve proxy host %s.", proxy_url) + self._test_connection(scheme, proxy_url) + elif "407 Proxy Authentication Required" in str(root_cause): + logger.error(" Invalid proxy credentials %s.", proxy_url) + elif isinstance(root_cause, (ConnectionRefusedError, ConnectionResetError)): + logger.error(" Connection to proxy %s refused.", proxy_url) + else: + logger.error(" Proxy %s error %s.", proxy_url, result) + elif isinstance(result, requests.exceptions.SSLError): if "[SSL: WRONG_VERSION_NUMBER]" in str(root_cause): logger.error(" Invalid protocol.") else: From b7d593c87b171d1e0ec58a8623d2ae30ee2aca11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Fri, 3 Jan 2025 13:24:40 +0100 Subject: [PATCH 8/8] feat: Validate URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit urlparse from Python stdlib doesn’t fail on an invalid URL. parse_url from urllib3 used by requests does though. Invalid base URL or proxy URL raises thus an uncaught exception. --- insights/client/connection.py | 40 ++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index f36bb4c732..3ac96e365b 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -4,6 +4,7 @@ from __future__ import print_function from __future__ import absolute_import import requests +import urllib3 import socket import os import six @@ -481,6 +482,31 @@ def _test_auth_config(self): return not errors + def _test_url_config(self): + logger.info("URL configuration:") + + urls = [("Base URL", self.base_url)] + if self.proxies: + for proxy_protocol, proxy_url in self.proxies.items(): + proxy_description = "{} proxy URL".format(proxy_protocol.upper()) + urls.append((proxy_description, proxy_url)) + + valid = True + for description, url in urls: + try: + urllib3.util.url.parse_url(url) + except urllib3.exceptions.LocationParseError: + valid = False + logger.error(" %s: %s (INVALID!)", description, url) + else: + logger.info(" %s: %s", description, url) + if not self.proxies: + logger.info(" No proxy.") + + logger.info("") + + return valid + def _dump_urls(self): base_parsed = urlparse(self.base_url) if base_parsed.hostname.endswith("stage.redhat.com"): @@ -496,7 +522,6 @@ def _dump_urls(self): logger.info("Running Connection Tests against %s...", hostname_desc) urls = [ - (self.base_url, "Base"), (self.upload_url, "Upload"), (self.inventory_url, "Inventory"), (self.ping_url, "Ping"), @@ -504,12 +529,6 @@ def _dump_urls(self): for url, title in urls: logger.info(" %s URL: %s", title, url) - if self.proxies: - for proxy_type, proxy_url in self.proxies.items(): - logger.info(" %s proxy: %s", proxy_type.upper(), proxy_url) - else: - logger.info(" Proxy: not set") - logger.info("") def _test_connection(self, scheme, hostname): @@ -537,9 +556,10 @@ def test_connection(self, rc=0): """ Test connection to Red Hat """ - auth_config_ok = self._test_auth_config() - if not auth_config_ok: - return 1 + for config_test in [self._test_auth_config, self._test_url_config]: + success = config_test() + if not success: + return 1 self._dump_urls()