Fix for issue #1074 - add IsDocumentation function #2408
Conversation
Used in isAllowedIPValue()
|
Hey @colin-stubbs, thanks! We were actually discussing these days about this issue, with the aim of replacing all public IPs in tests with IPs in the documentation range. This consists on three parts:
|
|
/test |
| // IsDocumentation reports whether ip is a reserved address for documentation, | ||
| // according to RFC 5737 (IPv4 Address Blocks Reserved for Documentation) and | ||
| // RFC 3849 (IPv6 Address Prefix Reserved for Documentation). | ||
| func IsDocumentation(ip net.IP) bool { |
There was a problem hiding this comment.
Nit.
| // IsDocumentation reports whether ip is a reserved address for documentation, | |
| // according to RFC 5737 (IPv4 Address Blocks Reserved for Documentation) and | |
| // RFC 3849 (IPv6 Address Prefix Reserved for Documentation). | |
| func IsDocumentation(ip net.IP) bool { | |
| // isDocumentationIP reports whether ip is a reserved address for documentation, | |
| // according to RFC 5737 (IPv4 Address Blocks Reserved for Documentation) and | |
| // RFC 3849 (IPv6 Address Prefix Reserved for Documentation). | |
| func isDocumentationIP(ip net.IP) bool { |
| // IsDocumentation reports whether ip is a reserved address for documentation, | ||
| // according to RFC 5737 (IPv4 Address Blocks Reserved for Documentation) and | ||
| // RFC 3849 (IPv6 Address Prefix Reserved for Documentation). | ||
| func IsDocumentation(ip net.IP) bool { |
There was a problem hiding this comment.
Nit. Consider adding some test case in validate_test.go and/or in some test package under test/packages.
Yeap... I do see the value in having a dummy geoip database (or dummy code that always provides geoip data regardless of IP) so that if geoip ingest processors are used it'll return something that will wind up on documents regardless of what the IP is. This would better ensure validity of ingest processor usage and field definitions for integrations. I'll take a look and see if I can't work out how to do it. |
|
Also, in terms of tooling to en masse replace any IP's currently in test/sample files... something like this? |
Great, thanks, in any case this can be handled as a separate change, we can go on with this PR without this.
Yes, something like this :D |
|
@jsoriano is there an existing issue or somewhere else that I can dump this info? I've worked out how to generate dummy GeoIP Lite2 .mmdb files that will respond to ANY IP with the same ASN/City/Country info. This would mean that if a geoip ingest processor is used as part of package testing it will always get valid GeoIP data back to attach to the document. e.g. this is what it looks like after replacing the .mmdb files that elastic-package injects into the elasticsearch container, e.g. with the package I'm currently working on, with 203.0.113.0/24 TEST-NET-3 IP's... here's some of the new fields that get attached to test events. The actual City name, Country details, location coords etc could be dummied further to not refer to a real place at all. |
|
@colin-stubbs how are you creating this database? It looks great, do you think we could add some different locations?
We have an internal meta issue that includes this, I have just created a public one: #2414 |
Fixes #1074
Adds an IsDocumentation function to check if IP's are reserved IP's used for documentation, these should be permitted in sample/test events in integrations.
IsDocumentation() is then used within isAllowedIPValue() to allow documentation IP's in the a similar way that elastic-package allows private RFC-1918, loopback, multicast, link-local and unspecified addresses via go net package provided functions.
Unfortunatly the go net package does not currently contain any kind of similar function so it needs to be local.
Without this addition package developers have to waste time substituting their lab/test/already replaced IP's - which are already documentation IP's! - for IP's that conform to the random set of real public Internet IP's provided in allowed_geo_ips.txt which makes no sense whatsoever.
e.g. they get this kind of noise back from elastic-package when running tests,
I've rebuilt and testing my locally built elastic-package using this diff, I no longer get noise.