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.

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.

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.

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.

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.

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.

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.

feat: Validate URLs

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.

Card IDs:
* CCT-963

Signed-off-by: Štěpán Tomsa <[email protected]>

Author: Glutexo

insights/client/connection.py

@@ -447,11 +447,16 @@ def _test_auth_config(self):
                 ("Password", self.password, "********"),
             ]:
                 if not var:
-                    error = ["  %s NOT SET.", desc]
-                    errors.append(error)
+                    errors.append(["  %s NOT SET.", desc])
 
                 val = placeholder or var if var else "NOT SET"
                 logger.info("  %s: %s", desc, val)
+
+            if errors:
+                errors.append([
+                    "  Check \"username\" and \"password\" in %s.",
+                    self.config.conf
+                ])
         elif self.authmethod == "CERT":
             logger.info("Authentication: identity certificate (%s)", self.authmethod)
 
@@ -465,12 +470,22 @@ def _test_auth_config(self):
                     exists_description = "exists"
                 else:
                     exists_description = "NOT FOUND"
-                    error = ["  %s file %s MISSING.", desc, path]
-                    errors.append(error)
+                    errors.append(["  %s file %s MISSING.", desc, path])
                 logger.info("  %s: %s (%s)", desc, path, exists_description)
+
+            if errors:
+                errors.append([
+                    "  Re-register the system by running \"subscription-manager unregister\" and then "
+                    "\"subscription-manager register\"."
+                ])
         else:
             logger.info("Authentication: unknown")  # Should not happen..
-            errors.append(["  Unknown authentication method %s.", self.authmethod])
+            errors.append(["  Unknown authentication method \"%s\".", self.authmethod]),
+            errors.append([
+                "  Set \"authmethod\" in %s to \"BASIC\" for username/password login, or to \"CERT\" for authentication"
+                " with a certificate.",
+                self.config.conf
+            ])
         logger.info("")
 
         if errors:
@@ -485,28 +500,45 @@ def _test_auth_config(self):
     def _test_url_config(self):
         logger.info("URL configuration:")
 
-        urls = [("Base", self.base_url)]
+        urls = [(
+            "Base",
+            self.base_url,
+            ["  Check \"base_url\" in %s.", self.config.conf]
+        )]
         if self.proxies:
             for proxy_protocol, proxy_url in self.proxies.items():
                 proxy_description = "{} proxy".format(proxy_protocol.upper())
-                urls.append((proxy_description, proxy_url))
+                urls.append((
+                    proxy_description,
+                    proxy_url,
+                    [
+                        "  Check \"proxy\" in %s and \"%s_proxy\" environment value.",
+                        self.config.conf,
+                        proxy_protocol
+                    ]
+                ))
 
         errors = []
-        for description, url in urls:
+        for description, url, error_message in urls:
+            url_errors = []
             try:
                 parsed = urllib3.util.url.parse_url(url)
             except urllib3.exceptions.LocationParseError:
                 logger.error("  %s URL: %s (INVALID!)", description, url)
-                errors.append(("  INVALID %s URL.", description))
+                url_errors.append(["  INVALID %s URL.", description])
             else:
                 if not parsed.scheme:
                     logger.error("  %s URL: %s (INCOMPLETE!)", description, url)
-                    errors.append(("  Protocol MISSING in %s URL.", description))
+                    url_errors.append(["  Protocol MISSING in %s URL.", description])
                 elif not parsed.netloc:
                     logger.error("  %s URL: %s (INCOMPLETE!)", description, url)
-                    errors.append(("  Hostname MISSING in %s URL.", description))
+                    url_errors.append(["  Hostname MISSING in %s URL.", description])
                 else:
                     logger.info("  %s URL: %s", description, url)
+
+            if url_errors:
+                errors.extend(url_errors)
+                errors.append(error_message)
         if not self.proxies:
             logger.info("  No proxy.")
         logger.info("")
@@ -597,7 +629,7 @@ def test_connection(self, rc=0):
             else:
                 break  # Cannot happen.
         else:
-            logger.info("    See %s for more details." % self.config.logging_file)
+            logger.info("    See %s or use --verbose for more details." % self.config.logging_file)
             logger.info("")
             return rc
 

insights/tests/client/connection/test_test_connection.py

@@ -850,7 +850,7 @@ def test_test_connection_print_success(test_urls, insights_connection, capsys):
     insights_connection.test_connection()
 
     out, err = capsys.readouterr()
-    assert out == "See {0} for more details.\n".format(LOGGING_FILE)
+    assert out == "See {0} or use --verbose for more details.\n".format(LOGGING_FILE)
     assert not err
 
 
@@ -973,7 +973,7 @@ def test_test_connection_non_legacy_print_success(
     insights_connection.test_connection()
 
     out, err = capsys.readouterr()
-    assert out == "See {0} for more details.\n".format(LOGGING_FILE)
+    assert out == "See {0} or use --verbose for more details.\n".format(LOGGING_FILE)
     assert not err
 
 
@@ -1110,7 +1110,7 @@ def test_test_connection_legacy_print_success(
     insights_connection.test_connection()
 
     out, err = capsys.readouterr()
-    assert out == "See {0} for more details.\n".format(LOGGING_FILE)
+    assert out == "See {0} or use --verbose for more details.\n".format(LOGGING_FILE)
     assert not err
 
 
@@ -1127,7 +1127,7 @@ def test_test_connection_legacy_print_one_fail(
     insights_connection.test_connection()
 
     out, err = capsys.readouterr()
-    assert out == "{0}\nSee {1} for more details.\n".format(
+    assert out == "{0}\nSee {1} or use --verbose for more details.\n".format(
         EXCEPTION_MESSAGE, LOGGING_FILE
     )
     assert not err
@@ -1196,6 +1196,12 @@ def test_test_connection_basic_auth_incomplete_credentials_log(
         ]
         + [(logging.ERROR, "  {}.".format(error)) for error in errors]
         + [
+            (
+                logging.ERROR,
+                '  Check "username" and "password" in {}.'.format(
+                    insights_connection.config.conf
+                ),
+            ),
             (logging.ERROR, ""),
         ]
     )
@@ -1294,6 +1300,11 @@ def test_test_connection_basic_auth_incomplete_key_pair_log(
         ]
         + [(logging.ERROR, "  {}".format(error)) for error in errors]
         + [
+            (
+                logging.ERROR,
+                '  Re-register the system by running "subscription-manager unregister" and then '
+                '"subscription-manager register".',
+            ),
             (logging.ERROR, ""),
         ]
     )
@@ -1310,15 +1321,18 @@ def test_test_connection_unknown_auth_log(insights_connection, caplog):
     with caplog.at_level(logging.INFO):
         insights_connection.test_connection()
 
-    messages = (
-        [
-            (logging.INFO, "Authentication: unknown"),
-            (logging.INFO, ""),
-            (logging.ERROR, "ERROR. Cannot authenticate:"),
-            (logging.ERROR, "  Unknown authentication method INVALID."),
-            (logging.ERROR, ""),
-        ]
-    )
+    messages = [
+        (logging.INFO, "Authentication: unknown"),
+        (logging.INFO, ""),
+        (logging.ERROR, "ERROR. Cannot authenticate:"),
+        (logging.ERROR, '  Unknown authentication method "INVALID".'),
+        (
+            logging.ERROR,
+            '  Set "authmethod" in {} to "BASIC" for username/password login, or to "CERT" for authentication'
+            " with a certificate.".format(insights_connection.config.conf),
+        ),
+        (logging.ERROR, ""),
+    ]
 
     record_tuples = [
         ("insights.client.connection", loglevel, message)
@@ -1446,17 +1460,27 @@ def test_test_connection_url_invalid_log(
         insights_connection.test_connection()
 
     errors = base_url_errors + proxy_url_errors
-    messages = (
-        [
-            (logging.INFO, "URL configuration:"),
-            (base_url_loglevel, "  {}".format(base_url_line)),
-            (proxy_url_loglevel, "  {}".format(proxy_url_line)),
-            (logging.INFO, ""),
-            (logging.ERROR, "ERROR. Invalid URL configuration:"),
+    messages = [
+        (logging.INFO, "URL configuration:"),
+        (base_url_loglevel, "  {}".format(base_url_line)),
+        (proxy_url_loglevel, "  {}".format(proxy_url_line)),
+        (logging.INFO, ""),
+        (logging.ERROR, "ERROR. Invalid URL configuration:"),
+    ]
+
+    for url_errors, url_message in [
+        (base_url_errors, 'Check "base_url" in {}'),
+        (proxy_url_errors, 'Check "proxy" in {} and "https_proxy" environment value'),
+    ]:
+        messages += [(logging.ERROR, "  {}.".format(error)) for error in url_errors]
+        messages += [
+            (
+                logging.ERROR,
+                "  {}.".format(url_message.format(insights_connection.config.conf)),
+            )
         ]
-        + [(logging.ERROR, "  {}.".format(error)) for error in errors]
-        + [(logging.ERROR, "")]
-    )
+
+    messages += [(logging.ERROR, "")]
 
     record_tuples = [
         ("insights.client.connection", loglevel, message)
@@ -1580,7 +1604,7 @@ def test_test_connection_urls_legacy_log(
         "    Testing {}".format(full_ping_url),
         "    SUCCESS.",
         "",
-        "    See {} for more details.".format(LOGGING_FILE),
+        "    See {} or use --verbose for more details.".format(LOGGING_FILE),
         "",
     ]
     record_tuples = [
@@ -1641,7 +1665,7 @@ def test_test_connection_hostname_description_log(
         "    Testing {}".format(insights_connection.ping_url),
         "    SUCCESS.",
         "",
-        "    See {} for more details.".format(LOGGING_FILE),
+        "    See {} or use --verbose for more details.".format(LOGGING_FILE),
         "",
     ]
     record_tuples = [
@@ -1690,7 +1714,7 @@ def test_test_connection_success_status_code_log(legacy_upload, status_code, cap
         "    Testing {}".format(connection.ping_url),
         "    SUCCESS.",
         "",
-        "    See {} for more details.".format(LOGGING_FILE),
+        "    See {} or use --verbose for more details.".format(LOGGING_FILE),
         "",
     ]
     record_tuples = [
@@ -2722,7 +2746,7 @@ def test_test_connection_legacy_path_fallback_log(
             (logging.INFO, "    Testing https://www.example.com/"),
             (logging.INFO, "    SUCCESS."),
             (logging.INFO, ""),
-            (logging.INFO, "    See {} for more details.".format(LOGGING_FILE)),
+            (logging.INFO, "    See {} or use --verbose for more details.".format(LOGGING_FILE)),
             (logging.INFO, ""),
         ]
     elif isinstance(side_effect[-1], requests.exceptions.ConnectionError):
@@ -2847,7 +2871,7 @@ def test_test_connection_legacy_path_fallback_return_value(
                 ),
                 (logging.INFO, "    SUCCESS."),
                 (logging.INFO, ""),
-                (logging.INFO, "    See {} for more details.".format(LOGGING_FILE)),
+                (logging.INFO, "    See {} or use --verbose for more details.".format(LOGGING_FILE)),
                 (logging.INFO, ""),
             ],
         ),