Skip to content

Conversation

robin-rexstedt-stretch
Copy link

@robin-rexstedt-stretch robin-rexstedt-stretch commented Oct 2, 2025

This adds a way to add a client cert to both TcpSender and HttpSender via SenderOptions, without also setting the tls_roots.
This is needed in our case where we use Traefik as a https proxy in front of QuestDB.

Absolutely willing to adjust the implementation details if needed to adhere to QuestDB standards if this is something you are willing to consider merging.

The cert being provided in X509Certificate2 format instead of a file path is intentional, as it allows much greater freedom in cert types (pem+pem, pfx...) and also where it is read from - disk, base64 string, key vaults, IConfiguration, etc.

@CLAassistant
Copy link

CLAassistant commented Oct 2, 2025

CLA assistant check
All committers have signed the CLA.

@ideoma
Copy link
Collaborator

ideoma commented Oct 6, 2025

Hey @robin-rexstedt-stretch, thanks for the contribution!

Few questions: is X509Certificate2 instance thread-safe to be reused in multiple client instances? I cannot find relevant documentation.

To complete the PR, please add a test that uses an X509Certificate2 instance. Add the certificate to the mock server and verify that the client works correctly with the certificates and does not work without them.

@robin-rexstedt-stretch
Copy link
Author

Few questions: is X509Certificate2 instance thread-safe to be reused in multiple client instances? I cannot find relevant documentation.

As long as it is not disposed its entirely immutable these days.

To complete the PR, please add a test that uses an X509Certificate2 instance. Add the certificate to the mock server and verify that the client works correctly with the certificates and does not work without them.

Okay, I'll add that to the PR. Thanks for the feedback!

@robin-rexstedt-stretch
Copy link
Author

Hi @ideoma, I have now added the requested unit tests. Please review and if you have any further questions or change requests, let me know.

@ideoma
Copy link
Collaborator

ideoma commented Oct 9, 2025

I tried running the new test and got this

QuestDB.Utils.IngressError : ServerFlushError : Cannot connect to `localhost:29474`
   at QuestDB.Senders.HttpSender.SendWithRetries(CancellationToken ct, Func`1 requestFactory, TimeSpan retryTimeout) in /Users/alpel/src/net-questdb-client/src/net-questdb-client/Senders/HttpSender.cs:line 435
   at QuestDB.Senders.HttpSender.Build() in /Users/alpel/src/net-questdb-client/src/net-questdb-client/Senders/HttpSender.cs:line 152
   at QuestDB.Senders.HttpSender..ctor(SenderOptions options) in /Users/alpel/src/net-questdb-client/src/net-questdb-client/Senders/HttpSender.cs:line 66
   at QuestDB.Sender.New(SenderOptions options) in /Users/alpel/src/net-questdb-client/src/net-questdb-client/Sender.cs:line 78
   at QuestDB.Utils.SenderOptions.Build() in /Users/alpel/src/net-questdb-client/src/net-questdb-client/Utils/SenderOptions.cs:line 660
   at net_questdb_client_tests.HttpTests.SendWithCert() in /Users/alpel/src/net-questdb-client/src/net-questdb-client-tests/HttpTests.cs:line 1615
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.<>c__DisplayClass4_0.<PerformWork>b__0()
   at NUnit.Framework.Internal.ContextUtils.<>c__DisplayClass1_0`1.<DoIsolated>b__0(Object _)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated(ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated[T](Func`1 func)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.PerformWork()


@robin-rexstedt-stretch
Copy link
Author

I've only tried it on my local dev machine so far, which runs Windows 11 (where I get the error described in #50 due to system culture differences). I'll try to find some other machines to run it on, see if I can reproduce your above error. Certificate loading can be pretty OS dependent, so that might be the issue. The error message you get is exactly what is expected for when a certificate isn't being sent.

I'll figure out a way to reproduce and then fix it.

@ideoma
Copy link
Collaborator

ideoma commented Oct 9, 2025

I'm using MacOS

@ideoma
Copy link
Collaborator

ideoma commented Oct 9, 2025

If you wait a bit, I will be able to set up Azure CI that will run the tests on Mac, Linux, and Windows, and you can test this PR on all platforms

@ideoma
Copy link
Collaborator

ideoma commented Oct 10, 2025

Hey @robin-rexstedt-stretch, I added an Azure pipeline; it should be triggered on PRs automatically. Please merge the latest from main

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.

3 participants