client_python icon indicating copy to clipboard operation
client_python copied to clipboard

Add function to use TLS when pushing to gateway

Open Roymprog opened this issue 2 years ago • 5 comments

Closes https://github.com/prometheus/client_python/issues/982

Roymprog avatar Jan 15 '24 09:01 Roymprog

We could, the reason I didn't do that was the name of the function. The name tls_auth_handler implies it does authentication. The function becomes confusing if you allow to not provide your own TLS certificate and only use it to verify the certificate of the server.

I'd propose keeping one function called tls_handler, but that would be a breaking change.

Roymprog avatar Feb 02 '24 14:02 Roymprog

What would this function provide over just using an https scheme in push_to_gateway? I guess the use case is for if you need to specify the cafile and verify mode?

That can be done. It's how I'm doing it right now. If you want to go that route, it would be better to make _make_handler a public function. Otherwise users would need to copy the boilerplate code from that function to create the handler. HTTPShandler cannot be passed to push_to_gateway directly, as it expects a callable (which HTTPShandler) is not.

Roymprog avatar Feb 05 '24 07:02 Roymprog

I might be missing something as well, but perhaps this is a bug? I believe push_to_gateway is supposed to just work for https, and you are saying that you need to pass a custom handler to make it work? I wonder if we just need to use the scheme of the url to choose HTTP vs HTTPS in default_handler?

csmarchbanks avatar Feb 13 '24 17:02 csmarchbanks

The default_handler works with HTTPS by default. The issue I have is that I need to use a custom SSL context because I need to load a custom trust location. I cannot do that through the default_handler. So, it's unnecessary to specify the scheme. What would be nice in that case is to have an optional SSL context argument for the default_handler.

Roymprog avatar Feb 20 '24 11:02 Roymprog

That makes sense, thank you.

Adding a custom context to default_handler as you suggest seems like it could be a good, and more general solution? That way users could customize the context as they need without us supporting every argument.

I am still thinking about it some and happy to hear other opinions, or advantages/disadvantages of that approach to what you have here. My main concern with this PR is that having two tls related handlers will be confusing.

csmarchbanks avatar Feb 22 '24 03:02 csmarchbanks