Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Print config on connection test #4301

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Glutexo
Copy link
Collaborator

@Glutexo Glutexo commented Dec 7, 2024

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • No Sensitive Data in this change?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

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.

Card IDs:

@Glutexo Glutexo self-assigned this Dec 7, 2024
@Glutexo Glutexo added the client These issues represent work to be done by the "client" team. label Dec 7, 2024
@Glutexo Glutexo force-pushed the test-connection branch 3 times, most recently from ca48f81 to e3c1f79 Compare December 20, 2024 19:28
@Glutexo Glutexo force-pushed the test-connection branch 5 times, most recently from 5ba1101 to 27550e3 Compare January 3, 2025 13:01
@Glutexo Glutexo force-pushed the test-connection branch 3 times, most recently from 338f48b to 7a1a1b4 Compare January 8, 2025 11:10
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.
_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.
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.
Inventory is tested along with Ingress and an API ping. Hosts are listed as the most basic Inventory GET request.
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.
* 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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client These issues represent work to be done by the "client" team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant