Add firewall support to http based alertmanager receiver integrations #4085
Conversation
|
LGTM. I think we should also provide a way to configure global HTTP proxy with authentication, that would be injected into all user-configured receivers, as mentioned in #2036, but that is for a separate PR. |
bboreham
changed the title
pracucci
force-pushed
the
add-firewal-support-to-http-based-alertmanager-receiver-integrations
branch
from
1de0471 to
e6c74da
Compare
pkg/alertmanager/firewall.go
Outdated
| } | ||
|
|
||
| type FirewallHostsSpec struct { | ||
| CIDRs flagext.CIDRSliceCSV `yaml:"cidrs"` |
There was a problem hiding this comment.
CIDRNetworks / CIDRRanges might be a better name
pkg/alertmanager/firewall.go
Outdated
|
|
||
| type FirewallHostsSpec struct { | ||
| CIDRs flagext.CIDRSliceCSV `yaml:"cidrs"` | ||
| Private bool `yaml:"private_addresses"` |
There was a problem hiding this comment.
Is there a reason why we don't use PrivateAddresses in the golang struct?
…n receiver integrations Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
pracucci
force-pushed
the
add-firewal-support-to-http-based-alertmanager-receiver-integrations
branch
from
a5f2d3a to
d1f64ac
Compare
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
There was a problem hiding this comment.
LGTM overall. I think it would be useful to explicitly enumerate IP networks that are blocked by -alertmanager.receivers-firewall.block.private-addresses, because it may not be obvious from the description and without reading the code. User needs this information to know whether to add extra IP ranges or not.
Signed-off-by: Marco Pracucci <[email protected]>
Makes sense to me. I think enumerating all address ranges would be quite difficult (there's IPv6 too), so I've picked the path to mention the RFCs and which classes are blocked. |
|
Thanks for updating the description. |
What this PR does:
In this PR I'm proposing a solution to introduce a sort a firewall for HTTP-based alertmanager receiver integrations to block private/local addresses and a set of configurable CIDRs.
This PR doesn't address the email receiver (would be a follow up work).
Which issue(s) this PR fixes:
Fixes #2036
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]