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

Allow disabling TLS explicitly #546

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Allow disabling TLS explicitly #546

merged 1 commit into from
Jul 18, 2024

Conversation

mieciu
Copy link
Member

@mieciu mieciu commented Jul 18, 2024

So we have this logic to ignore creating TLS-based connections when we get specific TLS-related errors (assuming that plaintext connection should be used).

However, when we start Quesma while ClickHouse is dead, aforementioned logic doesn't work and we don't fallback to non-TLS connection properly, as the error is just "connection refused":
image (12)
So eventually when plaintext ClickHouse comes up, the pool will keep failing repeatedly with tls: first record does not look like a TLS handshake.

The proposed remediation is to just allow explicitly disabling TLS via configuration.

Alternative approach would be to derive this property by looking at port numbers:

  • 9000 -> no TLS
  • 9440 -> TLS

of course being very verbose in logs about Quesma's choice.

@mieciu mieciu requested a review from a team as a code owner July 18, 2024 16:59
@mieciu mieciu added this pull request to the merge queue Jul 18, 2024
Merged via the queue into main with commit 6cf3fc0 Jul 18, 2024
5 checks passed
@mieciu mieciu deleted the disable-tls branch July 18, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants