-
Notifications
You must be signed in to change notification settings - Fork 126
Update geoip databases to include data about documentation ranges #2450
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
Conversation
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#12962 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to use "real" cities here in the new entries. And for that, I got the data about them (geo codes or latitute/longitude values) using https://www.geonames.org/
The assignation of this ranges to those cities is faked and it is just for testing purposes in elastic-package
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you see any issue of using that data here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine to use real cities.
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#12962 |
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#12983 |
Include documentation about these GeoLite2 databases and how they should be generated.
88211de
to
906b3a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is fine to use real cities.
|
||
Once applied all these changes, you can build the `write-test-data` tool: | ||
```shell | ||
git clone https://github.com/maxmind/MaxMind-DB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point the repository should have been already cloned, and patched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the latest changes, this section has been updated and now it just refers to the new tool.
tools/geoipdatabases/writer.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In https://github.com/maxmind/MaxMind-DB, this file and geoip2.go
are located in another module (pkg/writer
).
I thought to keep it simple here and just locate everything in just one package main
.
@@ -4,3 +4,4 @@ | |||
89.160.20.156 - - [26/Dec/2016:18:23:35 +0200] "GET / HTTP/1.1" 200 45 | |||
89.160.20.156 - - [26/Dec/2016:18:23:41 +0200] "GET /notfound HTTP/1.1" 404 206 | |||
89.160.20.156 - - [26/Dec/2016:18:23:45 +0200] "GET /hmm HTTP/1.1" 404 201 | |||
192.0.2.100 - - [26/Dec/2016:18:23:45 +0200] "GET /hmm HTTP/1.1" 404 201 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New ip from the address block of documentation (192.0.2.100/24)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
tools/geoipdatabases/main.go
Outdated
source := flag.String("source", "", "Source data directory") | ||
target := flag.String("target", "", "Destination directory for the generated mmdb files") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. We could hard-code these paths to the paths we have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in f6799e4
and updated README accordingly.
💚 Build Succeeded
History
cc @mrodm |
Closes #2414
Follows #2408
This PR includes new GeoIP databases including some entries for the documentation prefixes.
Added entries for the following address blocks for documentation in the GeoLite2 JSON files:
I've tried to use other "real" cities here (Las Vegas, Amsterdam and Madrid) as well as Greenwich (as mentioned in this blog post), and get the data about them using https://www.geonames.org/
This data set in those JSON files is faked just for testing purposes in elastic-package. This could affect system and pipeline tests.
Data added to the databases can be reviewed in this commit: 906b3a6
Moreover, some tests have been added to the
isAllowedIPValue
function as well as it has been used an IP of the documentation range in the test packageapache
.Author's checklist
How to test this PR locally