plugin/geoip: Upgrade to geoip2-golang v2 (#7732)

Signed-off-by: Eric Case <eric.case@gmail.com>

plugin/geoip/README.md

@@ -156,3 +156,7 @@ A limited set of fields will be exported as labels, all values are stored using
 | NA    | North America  |
 | OC    | Oceania        |
 | SA    | South America  |
+
+## Notable changes
+
+- In CoreDNS v1.13.2, 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 "".

plugin/geoip/asn.go

@@ -6,7 +6,7 @@ import (
 
 	"github.com/coredns/coredns/plugin/metadata"
 
-	"github.com/oschwald/geoip2-golang"
+	"github.com/oschwald/geoip2-golang/v2"
 )
 
 func (g GeoIP) setASNMetadata(ctx context.Context, data *geoip2.ASN) {

plugin/geoip/city.go

@@ -7,27 +7,25 @@ import (
 
 	"github.com/coredns/coredns/plugin/metadata"
 
-	"github.com/oschwald/geoip2-golang"
+	"github.com/oschwald/geoip2-golang/v2"
 )
 
-const defaultLang = "en"
-
 func (g GeoIP) setCityMetadata(ctx context.Context, data *geoip2.City) {
 	// Set labels for city, country and continent names.
-	cityName := data.City.Names[defaultLang]
+	cityName := data.City.Names.English
 	metadata.SetValueFunc(ctx, pluginName+"/city/name", func() string {
 		return cityName
 	})
-	countryName := data.Country.Names[defaultLang]
+	countryName := data.Country.Names.English
 	metadata.SetValueFunc(ctx, pluginName+"/country/name", func() string {
 		return countryName
 	})
-	continentName := data.Continent.Names[defaultLang]
+	continentName := data.Continent.Names.English
 	metadata.SetValueFunc(ctx, pluginName+"/continent/name", func() string {
 		return continentName
 	})
 
-	countryCode := data.Country.IsoCode
+	countryCode := data.Country.ISOCode
 	metadata.SetValueFunc(ctx, pluginName+"/country/code", func() string {
 		return countryCode
 	})
@@ -37,7 +35,7 @@ func (g GeoIP) setCityMetadata(ctx context.Context, data *geoip2.City) {
 	// a comma separated string, with the exact values provided by the database, even if those were empty strings.
 	subdivisionCodes := make([]string, 0, len(data.Subdivisions))
 	for _, sub := range data.Subdivisions {
-		subdivisionCodes = append(subdivisionCodes, sub.IsoCode)
+		subdivisionCodes = append(subdivisionCodes, sub.ISOCode)
 	}
 	metadata.SetValueFunc(ctx, pluginName+"/subdivisions/code", func() string {
 		return strings.Join(subdivisionCodes, ",")
@@ -52,11 +50,17 @@ func (g GeoIP) setCityMetadata(ctx context.Context, data *geoip2.City) {
 		return continentCode
 	})
 
-	latitude := strconv.FormatFloat(data.Location.Latitude, 'f', -1, 64)
+	var latitude string
+	if data.Location.Latitude != nil {
+		latitude = strconv.FormatFloat(*data.Location.Latitude, 'f', -1, 64)
+	}
 	metadata.SetValueFunc(ctx, pluginName+"/latitude", func() string {
 		return latitude
 	})
-	longitude := strconv.FormatFloat(data.Location.Longitude, 'f', -1, 64)
+	var longitude string
+	if data.Location.Longitude != nil {
+		longitude = strconv.FormatFloat(*data.Location.Longitude, 'f', -1, 64)
+	}
 	metadata.SetValueFunc(ctx, pluginName+"/longitude", func() string {
 		return longitude
 	})

plugin/geoip/geoip.go

@@ -4,7 +4,7 @@ package geoip
 import (
 	"context"
 	"fmt"
-	"net"
+	"net/netip"
 	"path/filepath"
 
 	"github.com/coredns/coredns/plugin"
@@ -12,7 +12,7 @@ import (
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
-	"github.com/oschwald/geoip2-golang"
+	"github.com/oschwald/geoip2-golang/v2"
 )
 
 var log = clog.NewWithPlugin(pluginName)
@@ -37,7 +37,7 @@ const (
 	asn
 )
 
-var probingIP = net.ParseIP("127.0.0.1")
+var probingIP = netip.MustParseAddr("127.0.0.1")
 
 func newGeoIP(dbPath string, edns0 bool) (*GeoIP, error) {
 	reader, err := geoip2.Open(dbPath)
@@ -80,13 +80,22 @@ func (g GeoIP) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 // Metadata implements the metadata.Provider Interface in the metadata plugin, and is used to store
 // the data associated with the source IP of every request.
 func (g GeoIP) Metadata(ctx context.Context, state request.Request) context.Context {
-	srcIP := net.ParseIP(state.IP())
+	srcIP, err := netip.ParseAddr(state.IP())
+	if err != nil {
+		log.Debugf("Failed to parse source IP %q: %v", state.IP(), err)
+		return ctx
+	}
 
 	if g.edns0 {
 		if o := state.Req.IsEdns0(); o != nil {
 			for _, s := range o.Option {
 				if e, ok := s.(*dns.EDNS0_SUBNET); ok {
-					srcIP = e.Address
+					// e.Address is still a net.IP type
+					if addr, ok := netip.AddrFromSlice(e.Address); ok {
+						srcIP = addr
+					} else {
+						log.Debugf("Failed to parse EDNS0 subnet address %v", e.Address)
+					}
 					break
 				}
 			}

plugin/geoip/setup_test.go

@@ -2,7 +2,6 @@ package geoip
 
 import (
 	"fmt"
-	"net"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -19,7 +18,7 @@ var (
 )
 
 func TestProbingIP(t *testing.T) {
-	if probingIP == nil {
+	if !probingIP.IsValid() {
 		t.Fatalf("Invalid probing IP: %q", probingIP)
 	}
 }
@@ -96,19 +95,4 @@ func TestGeoIPParse(t *testing.T) {
 			t.Errorf("Test %d: expected db type %d not found, database file provides %d", i, test.expectedDBType, geoIP.db.provides)
 		}
 	}
-
-	// Set nil probingIP to test unexpected validate error()
-	defer func(ip net.IP) { probingIP = ip }(probingIP)
-	probingIP = nil
-
-	c = caddy.NewTestController("dns", fmt.Sprintf("%s %s\n", pluginName, cityDBPath))
-	_, err := geoipParse(c)
-	if err != nil {
-		expectedErr := "unexpected failure looking up database"
-		if !strings.Contains(err.Error(), expectedErr) {
-			t.Errorf("expected error to contain: %s", expectedErr)
-		}
-	} else {
-		t.Errorf("with a nil probingIP test is expected to fail")
-	}
 }