Fix for servers being returned that are less performant than the closest

cmd/db/genaccessors.go

@@ -2,7 +2,7 @@ package main
 
 import (
 	"fmt"
-	"github.com/armbian/redirector/db"
+	"github.com/armbian/redirector/geo"
 	"github.com/samber/lo"
 	"reflect"
 	"strings"
@@ -15,11 +15,11 @@ var (
 // This is a VERY messy way to generate static recursive field getters, which transform rule strings
 // into the field value
 func main() {
-	var asn db.ASN
+	var asn geo.ASN
 
 	accessors(asn)
 
-	var city db.City
+	var city geo.City
 
 	accessors(city)
 	accessors(city.Continent)

cmd/main.go

@@ -25,7 +25,8 @@ func main() {
 
 	viper.SetDefault("bind", ":8080")
 	viper.SetDefault("cacheSize", 1024)
-	viper.SetDefault("topChoices", 3)
+	viper.SetDefault("topChoices", 1)
+	viper.SetDefault("maxDeviation", 50*1000) // 50 kilometers
 	viper.SetDefault("reloadKey", util.RandomSequence(32))
 
 	viper.SetConfigName("dlrouter")        // name of config file (without extension)

config.go

@@ -10,9 +10,8 @@
 	"sync"
 	"time"
 
-	"github.com/armbian/redirector/db"
+	"github.com/armbian/redirector/geo"
 	lru "github.com/hashicorp/golang-lru"
-	"github.com/oschwald/maxminddb-golang"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -47,8 +46,9 @@
 	// CheckURL is the url used to verify mirror versions
 	CheckURL string `mapstructure:"checkUrl"`
 
-	// SameCityThreshold is the parameter used to specify a threshold between mirrors and the client
-	SameCityThreshold float64 `mapstructure:"sameCityThreshold"`
+	// MaxDeviation is used when we have multiple servers that could be used,
+	// but one or more may be too far from the others
+	MaxDeviation float64 `mapstructure:"maxDeviation"`
 
 	// ServerList is a list of ServerConfig structs, which gets parsed into servers.
 	ServerList []ServerConfig `mapstructure:"servers"`
@@ -82,7 +82,7 @@
 	}
 }
 
 func Remove[V comparable](collection []V, value V) []V {
 	return lo.Filter(collection, func(item V, _ int) bool {
 		return item != value
 	})
@@ -95,31 +95,17 @@
 	var err error
 
 	// Load maxmind database
-	if r.db != nil {
-		err = r.db.Close()
+	if r.geo != nil {
+		err = r.geo.Close()
 		if err != nil {
 			return errors.Wrap(err, "Unable to close database")
 		}
 	}
 
-	if r.asnDB != nil {
-		err = r.asnDB.Close()
-		if err != nil {
-			return errors.Wrap(err, "Unable to close asn database")
-		}
-	}
+	r.geo, err = geo.NewMaxmindProvider(r.config.GeoDBPath, r.config.ASNDBPath)
 
-	// db can be hot-reloaded if the file changed
-	r.db, err = maxminddb.Open(r.config.GeoDBPath)
 	if err != nil {
-		return errors.Wrap(err, "Unable to open database")
-	}
-
-	if r.config.ASNDBPath != "" {
-		r.asnDB, err = maxminddb.Open(r.config.ASNDBPath)
-		if err != nil {
-			return errors.Wrap(err, "Unable to open asn database")
-		}
+		return err
 	}
 
 	// Refresh server cache if size changed
@@ -163,11 +149,6 @@
 		r.config.TopChoices = len(r.servers)
 	}
 
-    // Check if on the config is declared or use default logic
-	if r.config.SameCityThreshold == 0 {
-		r.config.SameCityThreshold = 200000.0
-	}
-
 	// Force check
 	go r.servers.Check(r, r.checks)
 
@@ -230,7 +211,7 @@
 					"path":      u.Path,
 					"latitude":  s.Latitude,
 					"longitude": s.Longitude,
-					"country": s.Country,
+					"country":   s.Country,
 				}).Info("Added server")
 			}
 		}(i, server, u)
@@ -287,8 +268,9 @@
 		}).Warning("Could not resolve address")
 		return nil, err
 	}
-	var city db.City
-	err = r.db.Lookup(ips[0], &city)
+
+	city, err := r.geo.City(ips[0])
+
 	if err != nil {
 		log.WithFields(log.Fields{
 			"error":  err,
@@ -298,13 +280,16 @@
 		return nil, err
 	}
 	s.Country = city.Country.IsoCode
+
 	if s.Continent == "" {
 		s.Continent = city.Continent.Code
 	}
+
 	if s.Latitude == 0 && s.Longitude == 0 {
 		s.Latitude = city.Location.Latitude
 		s.Longitude = city.Location.Longitude
 	}
+
 	return s, nil
 }
 

dlrouter.yaml

@@ -3,7 +3,8 @@ geodb: GeoLite2-City.mmdb
 asndb: GeoLite2-ASN.mmdb
 dl_map: userdata.csv
 
-sameCityThreshold: 200000.0
+# Max distance to allow for other "nearby" servers
+maxDeviation: 50000
 
 checkUrl: https://imola.armbian.com/apt/.control
 

db/accessors.go → geo/accessors.go

@@ -1,4 +1,4 @@
-package db
+package geo
 
 import (
 	"regexp"

geo/geolocation.go

@@ -0,0 +1,11 @@
+package geo
+
+import (
+	"net"
+)
+
+type Provider interface {
+	City(ip net.IP) (*City, error)
+	ASN(ip net.IP) (*ASN, error)
+	Close() error
+}

geo/maxmind.go

@@ -0,0 +1,76 @@
+package geo
+
+import (
+	"fmt"
+	"github.com/oschwald/maxminddb-golang"
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
+	"net"
+)
+
+var ErrNoASN = errors.New("no asn database loaded")
+
+type MaxmindProvider struct {
+	db    *maxminddb.Reader
+	asnDB *maxminddb.Reader
+}
+
+func (m *MaxmindProvider) City(ip net.IP) (*City, error) {
+	var city City
+
+	if err := m.db.Lookup(ip, &city); err != nil {
+		return nil, err
+	}
+
+	return &city, nil
+}
+
+func (m *MaxmindProvider) ASN(ip net.IP) (*ASN, error) {
+	if m.asnDB == nil {
+		return nil, ErrNoASN
+	}
+
+	var asn ASN
+
+	if err := m.asnDB.Lookup(ip, &asn); err != nil {
+		log.WithError(err).Warning("Unable to load ASN information")
+		return nil, err
+	}
+
+	return &asn, nil
+}
+
+func (m *MaxmindProvider) Close() error {
+	if m.db != nil {
+		_ = m.db.Close()
+	}
+
+	if m.asnDB != nil {
+		_ = m.asnDB.Close()
+	}
+
+	return nil
+}
+
+func NewMaxmindProvider(geoPath, asnPath string) (Provider, error) {
+	// db can be hot-reloaded if the file changed
+	db, err := maxminddb.Open(geoPath)
+
+	if err != nil {
+		return nil, fmt.Errorf("unable to open geo database: %w", err)
+	}
+
+	var asnDB *maxminddb.Reader
+
+	if asnPath != "" {
+		asnDB, err = maxminddb.Open(asnPath)
+		if err != nil {
+			return nil, fmt.Errorf("unable to open asn database: %w", err)
+		}
+	}
+
+	return &MaxmindProvider{
+		db:    db,
+		asnDB: asnDB,
+	}, nil
+}

geo/mock.go

@@ -0,0 +1,34 @@
+package geo
+
+import (
+	"github.com/stretchr/testify/mock"
+	"net"
+)
+
+type MockProvider struct {
+	mock.Mock
+}
+
+func (m *MockProvider) City(ip net.IP) (*City, error) {
+	args := m.Mock.Called(ip)
+
+	if v := args.Get(0); v != nil {
+		return v.(*City), args.Error(1)
+	}
+
+	return nil, args.Error(1)
+}
+
+func (m *MockProvider) ASN(ip net.IP) (*ASN, error) {
+	args := m.Mock.Called(ip)
+
+	if v := args.Get(0); v != nil {
+		return v.(*ASN), args.Error(1)
+	}
+
+	return nil, args.Error(1)
+}
+
+func (m *MockProvider) Close() error {
+	return nil
+}

db/structs.go → geo/structs.go

@@ -1,4 +1,4 @@
-package db
+package geo
 
 // City represents a MaxmindDB city.
 // This used to only be used on load, but is now used with rules as well.

go.mod

@@ -17,12 +17,14 @@ require (
 	github.com/sirupsen/logrus v1.9.3
 	github.com/sourcegraph/conc v0.3.0
 	github.com/spf13/viper v1.18.2
+	github.com/stretchr/testify v1.8.4
 	golang.org/x/text v0.14.0
 )
 
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
+	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-logr/logr v1.3.0 // indirect
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
@@ -32,6 +34,7 @@ require (
 	github.com/magiconair/properties v1.8.7 // indirect
 	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/pelletier/go-toml/v2 v2.1.0 // indirect
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.46.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
@@ -40,6 +43,7 @@ require (
 	github.com/spf13/afero v1.11.0 // indirect
 	github.com/spf13/cast v1.6.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
+	github.com/stretchr/objx v0.5.0 // indirect
 	github.com/subosito/gotenv v1.6.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect

go.sum

@@ -93,6 +93,7 @@ github.com/spf13/viper v1.18.2 h1:LUXCnvUvSM6FXAsj6nnfc8Q2tp1dIgUfY9Kc8GsSOiQ=
 github.com/spf13/viper v1.18.2/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMVB+yk=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
 github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=

http.go

@@ -10,7 +10,6 @@ import (
 	"path"
 	"strings"
 
-	"github.com/armbian/redirector/db"
 	"github.com/jmcvetta/randutil"
 	log "github.com/sirupsen/logrus"
 )
@@ -205,8 +204,7 @@ func (r *Redirector) geoIPHandler(w http.ResponseWriter, req *http.Request) {
 
 	ip := net.ParseIP(ipStr)
 
-	var city db.City
-	err = r.db.Lookup(ip, &city)
+	city, err := r.geo.City(ip)
 
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)

redirector.go

@@ -1,11 +1,11 @@
 package redirector
 
 import (
+	"github.com/armbian/redirector/geo"
 	"github.com/armbian/redirector/middleware"
 	"github.com/chi-middleware/logrus-logger"
 	"github.com/go-chi/chi/v5"
 	lru "github.com/hashicorp/golang-lru"
-	"github.com/oschwald/maxminddb-golang"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -28,8 +28,7 @@ var (
 // Redirector is our application instance.
 type Redirector struct {
 	config      *Config
-	db          *maxminddb.Reader
-	asnDB       *maxminddb.Reader
+	geo         geo.Provider
 	servers     ServerList
 	regionMap   map[string][]*Server
 	hostMap     map[string]*Server