Skip to content

plugin/geoip: Upgrade the geoip2-golang dependency to v2#7732

Merged
snebel29 merged 2 commits into
coredns:masterfrom
case:geoip2-golang-v2
Dec 8, 2025
Merged

plugin/geoip: Upgrade the geoip2-golang dependency to v2#7732
snebel29 merged 2 commits into
coredns:masterfrom
case:geoip2-golang-v2

Conversation

@case

@case case commented Nov 30, 2025

Copy link
Copy Markdown
Contributor

1. Why is this pull request needed and what does it do?

While working on #7730, I noticed that the GeoIP plugin is using v1 of the oschwald/geoip2-golang library, which recently bumped to v2. (The v2 beta series started on 2025-06-22, and shipped to v2.0.0 on 2025-10-19)

Notable changes in v2, from the geoip2-golang readme:

  • 56% fewer allocations and 34% less memory usage
  • Uses netip.Addr instead of net.IP for better performance
  • All result structs now include Network and IPAddress fields
  • New HasData() method to easily check if data was found
  • Replaced map[string]string with typed Names struct for better performance
  • Uses omitzero JSON tags to match MaxMind database behavior

I followed the v1 -> v2 migration guide; here are a few notes from the updates:

  • plugin/geoip/city.go:
    • Names changed from map[string]string to the typed struct with language fields
    • IsoCode renamed to ISOCode
    • Latitude/Longitude changed from float64 to *float64 to distinguish "value is 0.0" from "value not present in database"
  • plugin/geoip/geoip.go: netip.ParseAddr makes errors explicit
  • plugin/geoip/setup_test.go:
    • netip.Addr is a value type, needs IsValid() instead of a nil check
    • Removed a v1 test that is no longer needed for v2 - in v2, netip.Addr{} (zero value) doesn't cause an error from the geoip2 library - it simply returns empty data.

2. Which issues (if any) are related?

No, I went ahead and just created this PR instead of filing an issue. Happy to file one if needed.

3. Which documentation changes (if any) need to be made?

None needed.

4. Does this introduce a backward incompatible change or deprecation?

No

@snebel29

Copy link
Copy Markdown
Collaborator

@case Thank you for your contribution, I will have a full look during the week and come back with feedback.

@case

case commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@case Thank you for your contribution, I will have a full look during the week and come back with feedback.

Thanks @snebel29! It isn't urgent, just something I noticed when digging into GeoIP stuff over the past week.

Cheers!

Comment thread plugin/geoip/geoip.go
Comment thread plugin/geoip/city.go
Comment thread plugin/geoip/city.go
Comment thread plugin/geoip/geoip.go
@case case force-pushed the geoip2-golang-v2 branch 3 times, most recently from 51687c6 to d56e8a0 Compare December 2, 2025 17:43
yongtang added a commit to yongtang/coredns that referenced this pull request Dec 8, 2025
This PR add a deprecation noticed for geoip plugin's upcoming
behavior change of 0. Please see PR coredns#7732 for details.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang added a commit to yongtang/coredns that referenced this pull request Dec 8, 2025
This PR add a deprecation noticed for geoip plugin's upcoming
behavior change of 0. Please see PR coredns#7732 for details.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Comment thread plugin/geoip/README.md Outdated

## Notable changes

- In CoreDNS **vFIXME**, released **2025-12-FIXME-release-date**, the `geoip` plugin was upgraded to use [`oschwald/geoip2-golang/v2`](https://github.com/oschwald/geoip2-golang/blob/main/MIGRATION.md), the Go library that reads and parses [`.mmdb`](https://maxmind.github.io/MaxMind-DB/) databases. It has a small, but possibly-breaking change, where the `Location.Latitude` and `Location.Longitude` structs changed from value types to pointers (`float64` → `*float64`). In `oschwald/geoip2-golang` v1, missing coordinates returned "0" (which is a [valid location](https://en.wikipedia.org/wiki/Null_Island)), and in v2 they now return an empty string "".

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@case based on https://github.com/coredns/coredns/pull/7740/files look like this change will finally be in version 1.13.2, can you update this README text?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure thing, this is done.

Comment thread plugin/geoip/city.go
Comment thread go.mod
github.com/prometheus/client_model v0.6.2
github.com/prometheus/common v0.67.4
github.com/quic-go/quic-go v0.57.0
github.com/stretchr/testify v1.11.1

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this direct dependency being added here? At first glance I do not see testify being directly used anywhere in this change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I noticed that too, according to go mod graph | grep testify it's a transitive dependency of github.com/oschwald/geoip2-golang/v2 and github.com/oschwald/maxminddb-golang/v2

A bunch of other deps use it as well, but v1.11.1 appears to now be the MVS result - https://go.dev/ref/mod#minimal-version-selection

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you resolve conflicts on go.mod?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep will do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

go.mod conflict is resolved.

Signed-off-by: Eric Case <eric.case@gmail.com>
@case case force-pushed the geoip2-golang-v2 branch from d56e8a0 to e8aad77 Compare December 8, 2025 16:22

@snebel29 snebel29 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Eric Case <eric.case@gmail.com>
@case case force-pushed the geoip2-golang-v2 branch from 6691767 to 66c49c4 Compare December 8, 2025 19:45
@snebel29 snebel29 merged commit 8f48704 into coredns:master Dec 8, 2025
11 checks passed
@case

case commented Dec 8, 2025

Copy link
Copy Markdown
Contributor Author

Thanks again for your help with this PR, @snebel29! 🙏

@snebel29

snebel29 commented Dec 8, 2025

Copy link
Copy Markdown
Collaborator

Thanks for your contributions @case !

thevilledev pushed a commit that referenced this pull request Dec 9, 2025
This PR add a deprecation noticed for geoip plugin's upcoming
behavior change of 0. Please see PR #7732 for details.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants