Please add support for pushgateways protected with basic authentication

Currently there doesn't seem to be any way to provide basic authentication credentials for talking to pushgateways (urllib2 doesn't allow putting them in the URL). Would be great if this could be added.

Just noticed that the URL is hardcoded to begin with "http://", so ideally HTTPS should be supported as well.

I'll need to think on this a little as there's lots of different options a user may want to specify in this area. How would a function that returned the content and path work, that you could then adjust and send on work for you?

Yes, I think this solution would be quite OK for me.
Thanks for looking into this!

We're hitting this as well. I setup a HTTPS nginx instance to receive metrics and forward them to the pushgateway. Specifying https doesn't work since the http is hardcoded. A redirect doesn't work (and according to the RFC shouldn't work for PUT requests anyway):
Traceback (most recent call last):
File "testpushd.py", line 12, in
push_to_gateway('prom-push.tst.local', job='batchA', registry=registry)
File "/home/geertn/Envs/promtest/lib/python2.7/site-packages/prometheus_client/exposition.py", line 88, in push_to_gateway
_use_gateway('PUT', gateway, job, registry, grouping_key, timeout)
File "/home/geertn/Envs/promtest/lib/python2.7/site-packages/prometheus_client/exposition.py", line 122, in _use_gateway
resp = build_opener(HTTPHandler).open(request, timeout=timeout)
File "/usr/lib64/python2.7/urllib2.py", line 410, in open
response = meth(req, response)
File "/usr/lib64/python2.7/urllib2.py", line 523, in http_response
'http', request, response, code, msg, hdrs)
File "/usr/lib64/python2.7/urllib2.py", line 442, in error
result = self._call_chain(_args)
File "/usr/lib64/python2.7/urllib2.py", line 382, in _call_chain
result = func(_args)
File "/usr/lib64/python2.7/urllib2.py", line 608, in http_error_302
new = self.redirect_request(req, fp, code, msg, headers, newurl)
File "/usr/lib64/python2.7/urllib2.py", line 569, in redirect_request
raise HTTPError(req.get_full_url(), code, msg, headers, fp)
urllib2.HTTPError: HTTP Error 301: Moved Permanently

Basically for us it would be handy to be able to specify the full URL including HTTPS.

Is this still desired, and would somebody who needs the functionality be able/willing to contribute it?

Rough draft for comment. Tested on python 2.7.9, 3.4.2, 3.5.2.

n.b. older pythons won't enforce https certificate checking (PEP 476 etc.).

Also re previous comments, https urls already worked (but https://user:pwd@host:port form didn't, and still don't with this patch applied either).

python_prometheus_client_http_basicauth_support.patch.txt

Minimally tested with:

push_to_gateway('https://pushgw.example.com', job='batchA', registry=registry, username='foo', password='bar')

... with haproxy doing basic auth and https reverse proxying on the other end.

Cheers,

Tim.

That's cover us for basic auth, but I expect someone will want bearer token and client cert at some point.

@brian-brazil

True @brian-brazil , but on the other hand, even though other features are needed in the future, this patch does seem to fix the specific issue reported in this, well, issue.

Using basic auth with https for internal services does seem like a reasonable solution for internal systems and a real blocker for adoption according to the original reporter.

Just my €.02

This patch sets a pattern where we'll end up with 30-40 arguments to this function as all the various client auth options are added. I appreciate that users need this, but that's not a sane API direction to set.

I'll see if I can knock up something so allow the option of passing in a handler...

OK, I've got something which seems to work, and allow both https client certificates and basic auth to work by passing in a handler to push_to_gateway() etc..

I don't write much Python, so excuse me if I've done something daft...

python3 POC:

import urllib
import http

from prometheus_client import CollectorRegistry, Gauge, push_to_gateway
registry = CollectorRegistry()
g = Gauge('some_metric_too', 'Last time a batch job successfully finished', registry=registry)
g.set_to_current_time()

class HTTPSClientAuthHandler(urllib.request.HTTPSHandler):
    def __init__(self, key_file, cert_file):
        urllib.request.HTTPSHandler.__init__(self)
        self.key_file = key_file
        self.cert_file= cert_file

    def https_open(self, req):
        return self.do_open(self.getConnection, req)

    def getConnection(self, host, timeout):
        return http.client.HTTPSConnection(host, key_file=self.key_file,
                cert_file=self.cert_file)

url='https://pushgw.example.org'

p = urllib.request.HTTPPasswordMgrWithDefaultRealm()
p.add_password(realm=None,
        uri=url,
        user='admin',
        passwd='shh-dont-tell-anyone')
handler = urllib.request.HTTPBasicAuthHandler(p)

#handler = HTTPSClientAuthHandler(key_file=r'x509-client-key.pem', cert_file=r'x509-client.pem')

push_to_gateway(url, job='batchA', registry=registry, handler=handler)

Whilst I realise you don't want to turn the API into a swiss army knife, using basic auth seems like a pretty common use case (I'd guess at least an order of magnitude more common than client certs, bearer tokens etc.), I think I'd argue for adding username, password, anyway, as well as a handler argument, but I don't mind too much either way.

Which additional argument(s) would you like me to implement?

. username, password
. username, password, handler
. handler

and would you prefer it as a diff or a PR?

Cheers,

Tim.

Let's not tie this to urllib2, it's a bad API that we're only using because requests a) isn't in the standard library and b) has significant undocumented API changes between versions.

I'm thinking more a handler that takes the content, content-type, path and host and goes from there. That should be enough flexibility.

How about adding support for passing in a function to push_to_getway (and related functions) which takes the following args:

url, request_method, timeout, content_type, content

and should raise an IOError on failure?

That's along the lines I was thinking. Probably want headers rather than just content_type, there might be more in future.

Just got back on to this.

Quick Q:

The gateway arg to push_to_gateway(gateway, ....) is documented as:

"the url for your push gateway. Either of the form 'http://pushgateway.local', or 'pushgateway.local'. Scheme defaults to 'http' if none is provided"

If a handler is also passed to push_to_gateway() then perhaps the gateway shouldn't be altered, but instead passed unmodified to form the leading part of the URL, in case someone wants to use a scheme other than http:// or https:// and implement this in their own handler?

Don't mind either way, but in either case, it seems like there is some tension between this behaviour, and giving the option to pass your own handler in.

Thoughts?

Offhand I'd always add in the default scheme, as depending on which libraries the user is using there's a good chance they'd have to do it themselves anyway. That still works with other schemes.

This should be resolved now in latest release.

Navigation Menu

Repository navigation

Please add support for pushgateways protected with basic authentication #60

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Please add support for pushgateways protected with basic authentication #60

Description

Activity

kkoppel commented on Oct 15, 2015

brian-brazil commented on Oct 15, 2015

kkoppel commented on Oct 19, 2015

geertn commented on Jan 14, 2016

rud commented on Sep 27, 2016

tim-seoss commented on Oct 26, 2016

brian-brazil commented on Oct 26, 2016

rud commented on Oct 27, 2016

brian-brazil commented on Oct 27, 2016

tim-seoss commented on Oct 27, 2016

tim-seoss commented on Oct 27, 2016

brian-brazil commented on Oct 28, 2016

tim-seoss commented on Nov 2, 2016

brian-brazil commented on Nov 2, 2016

tim-seoss commented on Nov 14, 2016

brian-brazil commented on Nov 14, 2016

rossigee commented on Mar 2, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions