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

Add ip restriction for the prometheus_client output #4431

Merged

Conversation

dupondje
Copy link
Contributor

Sometimes you want to protect the exported /metrics path from the prometheus_client.
It was already possible to secure it with a password.

This patch adds the option to allow only defined CIDR ranges to access the /metrics.

@dupondje dupondje force-pushed the prometheus_client_ip_restriction branch from dcc7d1c to c3a2feb Compare July 17, 2018 09:24
@danielnelson
Copy link
Contributor

@dupondje The code here is very good, but I feel like this addition won't be in very wide demand, and I don't want to support too many connection limitations as they are endless. Why not just set this up in your system firewall instead?

@dupondje
Copy link
Contributor Author

@danielnelson: You could be correct that this isn't very wide demanded.

But this can be used in cases where firewalling isn't possible for example. Or when you want to have some additional protection mechanism.

Also I don't think there are endless ways to limit connections (that are supported by Prometheus itself).
Prometheus can only send credentials to the endpoint (this is already implemented).
Protecting via IP can be done without Prometheus needing to know this limitation.

Next to that, I think the patch is rather straightforward and small. So it doesn't add a lot of complexity to the code :)
And if you don't enable it, behaviour is unchanged.

Anyway, your call if you merge it or not!

@danielnelson danielnelson requested a review from glinton July 27, 2018 00:59
remoteIPs, _, _ := net.SplitHostPort(r.RemoteAddr)
remoteIP := net.ParseIP(remoteIPs)
for _, iprange := range p.IPRange {
_, ipNet, _ := net.ParseCIDR(iprange)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle the error so the next line doesn't panic if user passes bad configuration

Sometimes you want to protect the exported /metrics path from the prometheus_client.
It was already possible to secure it with a password.

This patch adds the option to allow only defined CIDR ranges to access the /metrics.
@dupondje dupondje force-pushed the prometheus_client_ip_restriction branch from c3a2feb to 5ee3ec5 Compare August 1, 2018 08:16
@dupondje
Copy link
Contributor Author

dupondje commented Aug 1, 2018

Adjusted the code like requested.
Also put everything inside a if len(p.IPRange) > 0 {, cause we don't need to parse the RemoteIP if we don't set a ip_range restriction ofcourse. Saves some cycles :)

@glinton glinton modified the milestones: 1.7.3, 1.8.0 Aug 1, 2018
@glinton glinton merged commit e1160c2 into influxdata:master Aug 1, 2018
@dupondje dupondje deleted the prometheus_client_ip_restriction branch August 10, 2018 08:46
rgitzel pushed a commit to rgitzel/telegraf that referenced this pull request Oct 17, 2018
otherpirate pushed a commit to otherpirate/telegraf that referenced this pull request Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants