Get rid of the blacklisted_syslog_ranges feature

[Benjamintf1]

I don't think anyone actually uses it, and even if they tried, I think it would be hard to make that feature actually useful. We should remove this functionality and the code associated with it.

[mkocher]

This feature dates back to publicly accessible CF instances to prevent users from ddos'ing random IPs. I'm not sure if we want to say that's not a valid use case for CF?

[Benjamintf1]

CF users always have had and continue to be able to ddos random ips if they wanted to.

The explaination I heard was to prevent exfiltration of logs/metrics to undesired ip addresses(which seems to me like being able to remove addresses rather then ip ranges would be easier to do in many cases?, or perhaps removing all external ip addresses or so on would be a much more functional usecase)

The biggest thing is I think that nobody is utilizing this feature at all.

[ctlong]

It does not seem like anyone is using this feature from some GitHub searching. Additionally, we received a complaint from an operator that the consistent DNS lookups performed by Syslog Agents for this feature – need to convert hostnames in drains to IPs in order to compare them to the blacklist – is resulting in a heavy load on their DNS system.

I think we should rip this out in a major version.

[ctlong]

One thing to be wary of is how this changes the metrics emitted by the Syslog Agent. At the very least, the invalid drains metric and blacklisted drains metric may change or be removed.

[chombium]

I've checked the code and it shouldn't have any problems with the invalid_drains metric. There are still checks if the Syslog drain URL is valid and similar things, the only check where the metric is increased is if there are domains defined in the Syslog drain URLs which cannot be resolved to ip addresses. If we remove this check, we'll still get an error when the the Syslog Agent creates the Syslog drain and tries to connect to the given URL. The question only is if we want to pull out the metric so that is propagated to the place where the connection to the external Syslog server is created, so that we can increment the invalid drains counter when we cannot open a connection to the external Syslog server. Even if we decide to remove the dns resolution we have logs about failed connections, hence the data is there, but in another form ;)

The removal of the blacklisted_drains metric shouldn't be a problem at all.

[ctlong]

The removal of the blacklisted_drains metric shouldn't be a problem at all.

I agree with this assertion. Since the feature is not used as far as I can tell, I don't see why this metric would be valuable at all.

[ctlong]

We don't think that it's possible to remove the DNS resolution check from the binding sync loop unless it's as a breaking change. Unlike blacklisted_drains it seems like invalid_drains may be observed by operators, so removing it or changing it significantly would be a breaking change. We think it's possible to have the Syslog Agent continue to emit the invalid_drains metric by capturing the state of the individual drain writers. However, that zeros out the value of invalid_drains in VMs that do not emit app envelopes, as they would never create writers for syslog app bindings.

Since a breaking change seems like more trouble than this gain is worth, we're gonna set this back down for now. Maybe we'll come back to it later. Happy to review PRs for it in the meantime though :)