From e7a33160f36e527cb130fcf3b1f32d7261fc279e Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 23 Feb 2025 04:47:41 -0500 Subject: [PATCH 1/3] [bugfix] Avoid far servers when choosing - Provides better documentation on the topChoices field - Changes default to only return the best server instead of top 3 - Avoids distant servers (>50km by default) from the top choice when topChoices is enabled, while still keeping similar servers (same location, etc) - Reverts localized servers due to issues with large countries and limited server selection --- cmd/main.go | 3 +- config.go | 9 +- servers.go | 257 ++++++++++++++++++++++++++-------------------------- 3 files changed, 136 insertions(+), 133 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 5cbdefc..f7dfa27 100644 --- a/cmd/main.go +++ b/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) diff --git a/config.go b/config.go index 5c5792e..e72a5b4 100644 --- a/config.go +++ b/config.go @@ -47,8 +47,9 @@ type Config struct { // 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"` @@ -163,7 +164,7 @@ func (r *Redirector) ReloadConfig() error { r.config.TopChoices = len(r.servers) } - // Check if on the config is declared or use default logic + // Check if on the config is declared or use default logic if r.config.SameCityThreshold == 0 { r.config.SameCityThreshold = 200000.0 } @@ -230,7 +231,7 @@ func (r *Redirector) reloadServers() error { "path": u.Path, "latitude": s.Latitude, "longitude": s.Longitude, - "country": s.Country, + "country": s.Country, }).Info("Added server") } }(i, server, u) diff --git a/servers.go b/servers.go index a9e40d4..044ad97 100644 --- a/servers.go +++ b/servers.go @@ -196,134 +196,135 @@ type ComputedDistance struct { // it is selected deterministically; otherwise, a weighted selection is used. // If no local servers exist, it falls back to a weighted selection among all valid servers. func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, float64, error) { - cacheKey := scheme + "_" + ip.String() - - if cached, exists := r.serverCache.Get(cacheKey); exists { - if comp, ok := cached.(ComputedDistance); ok { - log.Infof("Cache hit: %s", comp.Server.Host) - return comp.Server, comp.Distance, nil - } - r.serverCache.Remove(cacheKey) - } - - var city db.City - if err := r.db.Lookup(ip, &city); err != nil { - log.WithError(err).Warning("Unable to lookup client location") - return nil, -1, err - } - clientCountry := city.Country.IsoCode - - var asn db.ASN - if r.asnDB != nil { - if err := r.asnDB.Lookup(ip, &asn); err != nil { - log.WithError(err).Warning("Unable to load ASN information") - return nil, -1, err - } - } - - ruleInput := RuleInput{ - IP: ip.String(), - ASN: asn, - Location: city, - } - - validServers := lo.Filter(s, func(server *Server, _ int) bool { - if !server.Available || !lo.Contains(server.Protocols, scheme) { - return false - } - if len(server.Rules) > 0 && !server.checkRules(ruleInput) { - log.WithField("host", server.Host).Debug("Skipping server due to rules") - return false - } - return true - }) - - if len(validServers) < 2 { - validServers = s - } - - localServers := lo.Filter(validServers, func(server *Server, _ int) bool { - return server.Country == clientCountry - }) - - if len(localServers) > 0 { - computedLocal := lo.Map(localServers, func(server *Server, _ int) ComputedDistance { - d := Distance(city.Location.Latitude, city.Location.Longitude, server.Latitude, server.Longitude) - return ComputedDistance{ - Server: server, - Distance: d, - } - }) - - sort.Slice(computedLocal, func(i, j int) bool { - return computedLocal[i].Distance < computedLocal[j].Distance - }) - - if computedLocal[0].Distance < r.config.SameCityThreshold { - chosen := computedLocal[0] - r.serverCache.Add(cacheKey, chosen) - return chosen.Server, chosen.Distance, nil - } - - choiceCount := r.config.TopChoices - if len(computedLocal) < choiceCount { - choiceCount = len(computedLocal) - } - - choices := make([]randutil.Choice, choiceCount) - for i, item := range computedLocal[:choiceCount] { - choices[i] = randutil.Choice{ - Weight: item.Server.Weight, - Item: item, - } - } - - choice, err := randutil.WeightedChoice(choices) - if err != nil { - log.WithError(err).Warning("Unable to choose a weighted choice") - return nil, -1, err - } - - dist := choice.Item.(ComputedDistance) - r.serverCache.Add(cacheKey, dist) - return dist.Server, dist.Distance, nil - } - - // Fallback: if no local servers exist, simply select the nearest server among all valid servers. - computed := lo.Map(validServers, func(server *Server, _ int) ComputedDistance { - d := Distance(city.Location.Latitude, city.Location.Longitude, server.Latitude, server.Longitude) - return ComputedDistance{ - Server: server, - Distance: d, - } - }) - - sort.Slice(computed, func(i, j int) bool { - return computed[i].Distance < computed[j].Distance - }) - - choiceCount := r.config.TopChoices - if len(computed) < choiceCount { - choiceCount = len(computed) - } - - choices := make([]randutil.Choice, choiceCount) - for i, item := range computed[:choiceCount] { - choices[i] = randutil.Choice{ - Weight: item.Server.Weight, - Item: item, - } - } - - choice, err := randutil.WeightedChoice(choices) - if err != nil { - log.WithError(err).Warning("Unable to choose a weighted choice") - return nil, -1, err - } - - dist := choice.Item.(ComputedDistance) - r.serverCache.Add(cacheKey, dist) - return dist.Server, dist.Distance, nil + cacheKey := scheme + "_" + ip.String() + + cached, exists := r.serverCache.Get(cacheKey) + + var choices []randutil.Choice + + if exists { + choices = cached.([]randutil.Choice) + + // Fast path for single servers + if len(choices) == 1 { + closest := choices[0].Item.(ComputedDistance) + + // Validate server is available to our health checks + // Since Server is a pointer, this will always be up to date. + if closest.Server.Available { + return closest.Server, closest.Distance, nil + } + + // Server went offline during the cache period, re-evaluate choices + exists = false + } + } + + // This isn't an else statement as exists can be set to false when the closest is offline + if !exists { + var city db.City + + if err := r.db.Lookup(ip, &city); err != nil { + log.WithError(err).Warning("Unable to lookup client location") + return nil, -1, err + } + + var asn db.ASN + + if r.asnDB != nil { + if err := r.asnDB.Lookup(ip, &asn); err != nil { + log.WithError(err).Warning("Unable to load ASN information") + return nil, -1, err + } + } + + // TODO: Use a provider pattern for the db.City and db.ASN fields, allowing for testing/mocking + // This would also allow the use of HTTP APIs for lookups instead of just local databases + + ruleInput := RuleInput{ + IP: ip.String(), + ASN: asn, + Location: city, + } + + validServers := lo.Filter(s, func(server *Server, _ int) bool { + if !server.Available || !lo.Contains(server.Protocols, scheme) { + return false + } + + if len(server.Rules) > 0 && !server.checkRules(ruleInput) { + log.WithField("host", server.Host).Debug("Skipping server due to rules") + return false + } + + return true + }) + + if len(validServers) < 2 { + validServers = s + } + + computed := lo.Map(validServers, func(server *Server, _ int) ComputedDistance { + d := Distance(city.Location.Latitude, city.Location.Longitude, server.Latitude, server.Longitude) + return ComputedDistance{ + Server: server, + Distance: d, + } + }) + + sort.Slice(computed, func(i, j int) bool { + return computed[i].Distance < computed[j].Distance + }) + + choiceCount := r.config.TopChoices + + if len(computed) < choiceCount { + choiceCount = len(computed) + } + + // Save our closest distance to compare others to ensure a good experience + closestDistance := computed[0].Distance + + choices = make([]randutil.Choice, 0) + + for _, item := range computed[:choiceCount] { + // Skip servers which are further away so we avoid a situation where + // we have a very close server, but also two somewhat far (500 km/miles+) servers + if item.Distance-closestDistance > r.config.MaxDeviation { + continue + } + + choices = append(choices, randutil.Choice{ + Weight: item.Server.Weight, + Item: item, + }) + } + } + + // Cache all choices for a round-robin approach + // This is disabled by default, and can be enabled by using TopChoices > 1 + // There is code to specifically bypass the random selection when this is the case + r.serverCache.Add(cacheKey, choices) + + // Fast path to avoid weighted selection + if len(choices) == 1 { + dist := choices[0].Item.(ComputedDistance) + return dist.Server, dist.Distance, nil + } + + // Choose a "random" but influenced decision on the server list. + // With the MaxDeviation code, this would prefer things like faster (10Gb) networks and other + // factors, like potentially bandwidth providers, + // Ideally, this would also be influenced by distance with the closest being weighted higher? + choice, err := randutil.WeightedChoice(choices) + + if err != nil { + log.WithError(err).Warning("Unable to choose a weighted choice") + return nil, -1, err + } + + dist := choice.Item.(ComputedDistance) + return dist.Server, dist.Distance, nil } // haversin(θ) function From 4f737c571e6b3a581ac3ad5465d29cdfc4f1db1c Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 23 Feb 2025 04:53:02 -0500 Subject: [PATCH 2/3] [chore] Prepare code for unit tests - Prepare code for unit tests - Mock unit tests in ideal cases --- servers.go | 55 +++++++++++++++++++++++++++++-------------------- servers_test.go | 22 ++++++++++++++++++++ 2 files changed, 55 insertions(+), 22 deletions(-) create mode 100644 servers_test.go diff --git a/servers.go b/servers.go index 044ad97..7298712 100644 --- a/servers.go +++ b/servers.go @@ -196,6 +196,35 @@ type ComputedDistance struct { // it is selected deterministically; otherwise, a weighted selection is used. // If no local servers exist, it falls back to a weighted selection among all valid servers. func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, float64, error) { + choices, err := s.Choices(r, scheme, ip) + + if err != nil { + return nil, -1, err + } + + // Fast path to avoid weighted selection + if len(choices) == 1 { + dist := choices[0].Item.(ComputedDistance) + return dist.Server, dist.Distance, nil + } + + // Choose a "random" but influenced decision on the server list. + // With the MaxDeviation code, this would prefer things like faster (10Gb) networks and other + // factors, like potentially bandwidth providers, + // Ideally, this would also be influenced by distance with the closest being weighted higher? + choice, err := randutil.WeightedChoice(choices) + + if err != nil { + log.WithError(err).Warning("Unable to choose a weighted choice") + return nil, -1, err + } + + dist := choice.Item.(ComputedDistance) + return dist.Server, dist.Distance, nil +} + +// Choices is the internal logic of Closest, allowing us to compare all choices +func (s ServerList) Choices(r *Redirector, scheme string, ip net.IP) ([]randutil.Choice, error) { cacheKey := scheme + "_" + ip.String() cached, exists := r.serverCache.Get(cacheKey) @@ -212,7 +241,7 @@ func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, f // Validate server is available to our health checks // Since Server is a pointer, this will always be up to date. if closest.Server.Available { - return closest.Server, closest.Distance, nil + return choices, nil } // Server went offline during the cache period, re-evaluate choices @@ -226,7 +255,7 @@ func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, f if err := r.db.Lookup(ip, &city); err != nil { log.WithError(err).Warning("Unable to lookup client location") - return nil, -1, err + return nil, err } var asn db.ASN @@ -234,7 +263,7 @@ func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, f if r.asnDB != nil { if err := r.asnDB.Lookup(ip, &asn); err != nil { log.WithError(err).Warning("Unable to load ASN information") - return nil, -1, err + return nil, err } } @@ -306,25 +335,7 @@ func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, f // There is code to specifically bypass the random selection when this is the case r.serverCache.Add(cacheKey, choices) - // Fast path to avoid weighted selection - if len(choices) == 1 { - dist := choices[0].Item.(ComputedDistance) - return dist.Server, dist.Distance, nil - } - - // Choose a "random" but influenced decision on the server list. - // With the MaxDeviation code, this would prefer things like faster (10Gb) networks and other - // factors, like potentially bandwidth providers, - // Ideally, this would also be influenced by distance with the closest being weighted higher? - choice, err := randutil.WeightedChoice(choices) - - if err != nil { - log.WithError(err).Warning("Unable to choose a weighted choice") - return nil, -1, err - } - - dist := choice.Item.(ComputedDistance) - return dist.Server, dist.Distance, nil + return choices, nil } // haversin(θ) function diff --git a/servers_test.go b/servers_test.go new file mode 100644 index 0000000..5179823 --- /dev/null +++ b/servers_test.go @@ -0,0 +1,22 @@ +package redirector + +import ( + . "github.com/onsi/ginkgo/v2" +) + +var _ = Describe("Servers", func() { + It("Should successfully return the closest server", func() { + + }) + Context("Round-Robin Balancing", func() { + It("Should successfully return the closest server, when other servers are too far", func() { + + }) + It("Should successfully return the top servers if they are all in the same location/city", func() { + + }) + It("Should successfully return the top servers if they are all within reasonable distance", func() { + + }) + }) +}) From ba200dd04e81afbd8fd1b479d35494367ac028c9 Mon Sep 17 00:00:00 2001 From: Tyler Date: Tue, 25 Feb 2025 01:57:04 -0500 Subject: [PATCH 3/3] [feature] Add server tests - Changes geolocation to a provider for use of either API or Mocking - Add server tests for testing closest/choices - Adds test for deviation checking --- cmd/db/genaccessors.go | 6 +- config.go | 38 ++---- dlrouter.yaml | 3 +- {db => geo}/accessors.go | 2 +- geo/geolocation.go | 11 ++ geo/maxmind.go | 76 ++++++++++++ geo/mock.go | 34 ++++++ {db => geo}/structs.go | 2 +- go.mod | 4 + go.sum | 1 + http.go | 4 +- redirector.go | 5 +- servers.go | 47 +++++--- servers_test.go | 245 ++++++++++++++++++++++++++++++++++++++- util/util.go | 6 +- 15 files changed, 426 insertions(+), 58 deletions(-) rename {db => geo}/accessors.go (99%) create mode 100644 geo/geolocation.go create mode 100644 geo/maxmind.go create mode 100644 geo/mock.go rename {db => geo}/structs.go (99%) diff --git a/cmd/db/genaccessors.go b/cmd/db/genaccessors.go index a0a737f..cf23cfa 100644 --- a/cmd/db/genaccessors.go +++ b/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) diff --git a/config.go b/config.go index e72a5b4..6a8a376 100644 --- a/config.go +++ b/config.go @@ -10,9 +10,8 @@ import ( "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" @@ -96,31 +95,17 @@ func (r *Redirector) ReloadConfig() error { 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 @@ -164,11 +149,6 @@ func (r *Redirector) ReloadConfig() error { 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) @@ -288,8 +268,9 @@ func (r *Redirector) addServer(server ServerConfig, u *url.URL) (*Server, error) }).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, @@ -299,13 +280,16 @@ func (r *Redirector) addServer(server ServerConfig, u *url.URL) (*Server, error) 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 } diff --git a/dlrouter.yaml b/dlrouter.yaml index 8330145..b214b1f 100644 --- a/dlrouter.yaml +++ b/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 diff --git a/db/accessors.go b/geo/accessors.go similarity index 99% rename from db/accessors.go rename to geo/accessors.go index 1f0b4bb..af32f0d 100644 --- a/db/accessors.go +++ b/geo/accessors.go @@ -1,4 +1,4 @@ -package db +package geo import ( "regexp" diff --git a/geo/geolocation.go b/geo/geolocation.go new file mode 100644 index 0000000..46a7911 --- /dev/null +++ b/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 +} diff --git a/geo/maxmind.go b/geo/maxmind.go new file mode 100644 index 0000000..ddfef2d --- /dev/null +++ b/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 +} diff --git a/geo/mock.go b/geo/mock.go new file mode 100644 index 0000000..c64dd62 --- /dev/null +++ b/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 +} diff --git a/db/structs.go b/geo/structs.go similarity index 99% rename from db/structs.go rename to geo/structs.go index 182e18b..73c8dd8 100644 --- a/db/structs.go +++ b/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. diff --git a/go.mod b/go.mod index 588cfff..bd9e194 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 54d9aaf..e9a5895 100644 --- a/go.sum +++ b/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= diff --git a/http.go b/http.go index 3e5ce09..c67ad07 100644 --- a/http.go +++ b/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) diff --git a/redirector.go b/redirector.go index f71d701..39bdc00 100644 --- a/redirector.go +++ b/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 diff --git a/servers.go b/servers.go index 7298712..e6c751b 100644 --- a/servers.go +++ b/servers.go @@ -1,8 +1,9 @@ package redirector import ( + "errors" "fmt" - "github.com/armbian/redirector/db" + "github.com/armbian/redirector/geo" "github.com/armbian/redirector/util" "github.com/jmcvetta/randutil" "github.com/prometheus/client_golang/prometheus" @@ -179,9 +180,9 @@ func (s ServerList) Check(r *Redirector, checks []ServerCheck) { // RuleInput is a set of fields used for rule checks type RuleInput struct { - IP string `json:"ip"` - ASN db.ASN `json:"asn"` - Location db.City `json:"location"` + IP string `json:"ip"` + ASN *geo.ASN `json:"asn"` + Location *geo.City `json:"location"` } // ComputedDistance is a wrapper that contains a Server and Distance. @@ -208,6 +209,10 @@ func (s ServerList) Closest(r *Redirector, scheme string, ip net.IP) (*Server, f return dist.Server, dist.Distance, nil } + log.WithFields(log.Fields{ + "choices": choices, + }).Debug("Found potential choices") + // Choose a "random" but influenced decision on the server list. // With the MaxDeviation code, this would prefer things like faster (10Gb) networks and other // factors, like potentially bandwidth providers, @@ -251,25 +256,22 @@ func (s ServerList) Choices(r *Redirector, scheme string, ip net.IP) ([]randutil // This isn't an else statement as exists can be set to false when the closest is offline if !exists { - var city db.City + city, err := r.geo.City(ip) - if err := r.db.Lookup(ip, &city); err != nil { + if err != nil { log.WithError(err).Warning("Unable to lookup client location") return nil, err } - var asn db.ASN + asn, err := r.geo.ASN(ip) - if r.asnDB != nil { - if err := r.asnDB.Lookup(ip, &asn); err != nil { + if err != nil { + if !errors.Is(err, geo.ErrNoASN) { log.WithError(err).Warning("Unable to load ASN information") return nil, err } } - // TODO: Use a provider pattern for the db.City and db.ASN fields, allowing for testing/mocking - // This would also allow the use of HTTP APIs for lookups instead of just local databases - ruleInput := RuleInput{ IP: ip.String(), ASN: asn, @@ -289,6 +291,10 @@ func (s ServerList) Choices(r *Redirector, scheme string, ip net.IP) ([]randutil return true }) + log.WithFields(log.Fields{ + "validServers": len(validServers), + }).Debug("Filtered to only valid servers") + if len(validServers) < 2 { validServers = s } @@ -305,6 +311,14 @@ func (s ServerList) Choices(r *Redirector, scheme string, ip net.IP) ([]randutil return computed[i].Distance < computed[j].Distance }) + for i, choice := range computed { + log.WithFields(log.Fields{ + "index": i, + "host": choice.Server.Host, + "distance": choice.Distance, + }).Debug("Initial distances computed") + } + choiceCount := r.config.TopChoices if len(computed) < choiceCount { @@ -319,7 +333,14 @@ func (s ServerList) Choices(r *Redirector, scheme string, ip net.IP) ([]randutil for _, item := range computed[:choiceCount] { // Skip servers which are further away so we avoid a situation where // we have a very close server, but also two somewhat far (500 km/miles+) servers - if item.Distance-closestDistance > r.config.MaxDeviation { + if r.config.MaxDeviation != 0 && item.Distance-closestDistance > r.config.MaxDeviation { + log.WithFields(log.Fields{ + "host": item.Server.Host, + "maxDeviation": r.config.MaxDeviation, + "closestDistance": closestDistance, + "distance": item.Distance, + "deviation": item.Distance - closestDistance, + }).Debug("Skipping server due to being too far") continue } diff --git a/servers_test.go b/servers_test.go index 5179823..2c209b7 100644 --- a/servers_test.go +++ b/servers_test.go @@ -1,22 +1,261 @@ package redirector import ( + "encoding/json" + "github.com/armbian/redirector/geo" + lru "github.com/hashicorp/golang-lru" . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + log "github.com/sirupsen/logrus" + "net" ) +// server1.example.com -> San Francisco, CA, USA (37.8749, -122.3194) +// server2.example.ca -> Ottawa, ON, Canada (45.5215, -75.5972) +// server3.example.com -> New York, NY, USA (40.8128, -73.906) +// server4.example.ca -> Vancouver, BC, Canada (49.3827, -123.0207) +// server5.example.com -> Los Angeles, CA, USA (34.1522, -118.1437) +// server6.example.ca -> Calgary, AB, Canada (51.1447, -114.1719) +// server7.example.com -> Chicago, IL, USA (41.9781, -87.5298) +// server8.example.ca -> Edmonton, AB, Canada (53.6461, -113.3938) +// server9.example.com -> Houston, TX, USA (29.8604, -95.2698) +// server10.example.ca -> Toronto, ON, Canada (43.7532, -79.2832) +// server11.example.com -> Chicago, IL, USA (42.1781, -87.7298) ~20km variation +// server12.example.com -> Chicago, IL, USA (42.5781, -87.9298) ~50km variation +// server13.example.com -> Detroit, MI, USA (42.3314, -83.0458) +// server14.example.com -> Detroit, MI, USA (42.5314, -83.2458) ~20km variation +const serverTestJson = `[ + { + "available": true, + "host": "server1.example.com", + "latitude": 37.7749, + "longitude": -122.4194, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server2.example.ca", + "latitude": 45.4215, + "longitude": -75.6972, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server3.example.com", + "latitude": 40.7128, + "longitude": -74.006, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server4.example.ca", + "latitude": 49.2827, + "longitude": -123.1207, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server5.example.com", + "latitude": 34.0522, + "longitude": -118.2437, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server6.example.ca", + "latitude": 51.0447, + "longitude": -114.0719, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server7.example.com", + "latitude": 41.8781, + "longitude": -87.6298, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server8.example.ca", + "latitude": 53.5461, + "longitude": -113.4938, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server9.example.com", + "latitude": 29.7604, + "longitude": -95.3698, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server10.example.ca", + "latitude": 43.6532, + "longitude": -79.3832, + "weight": 10, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server11.example.com", + "latitude": 42.1781, + "longitude": -87.7298, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server12.example.com", + "latitude": 42.5781, + "longitude": -87.9298, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server13.example.com", + "latitude": 42.3314, + "longitude": -83.0458, + "protocols": ["http", "https"] + }, + { + "available": true, + "host": "server14.example.com", + "latitude": 42.5314, + "longitude": -83.2458, + "protocols": ["http", "https"] + } +] +` + var _ = Describe("Servers", func() { - It("Should successfully return the closest server", func() { + var ( + r *Redirector + mockProvider *geo.MockProvider + ) + + BeforeEach(func() { + log.SetLevel(log.DebugLevel) + mockProvider = &geo.MockProvider{} + r = New(&Config{}) + r.geo = mockProvider + r.serverCache, _ = lru.New(10) + + err := json.Unmarshal([]byte(serverTestJson), &r.servers) + + Expect(err).To(BeNil()) + }) + + Context("Single server", func() { + BeforeEach(func() { + // Single server returns + r.config.TopChoices = 1 + }) + It("Should successfully return the closest server to San Francisco, CA", func() { + ip := net.IPv4(1, 2, 3, 4) + // This geolocations to San Francisco, CA, USA + mockProvider.On("City", ip).Return(&geo.City{ + Location: geo.Location{Latitude: 37.8749, Longitude: -122.3194}, + }, nil) + mockProvider.On("ASN", ip).Return(nil, geo.ErrNoASN) + + closest, distance, err := r.servers.Closest(r, "https", ip) + + Expect(err).To(BeNil()) + + // Expect the closest server to be server1.example.com with a distance of around 14185 + Expect(closest.Host).To(Equal("server1.example.com")) + + // Distance is calculated beforehand and should be the same no matter how many runs due to presets + Expect(int(distance)).To(Equal(14185)) + }) }) Context("Round-Robin Balancing", func() { - It("Should successfully return the closest server, when other servers are too far", func() { + BeforeEach(func() { + // Multi server returns + r.config.TopChoices = 5 + r.config.MaxDeviation = 50000 // 50km + }) + It("Should successfully return a server in the closest area, when other servers are too far", func() { + ip := net.IPv4(4, 3, 2, 1) + + // Ottawa, CA + mockProvider.On("City", ip).Return(&geo.City{ + Location: geo.Location{Latitude: 45.5215, Longitude: -75.5972}, + }, nil) + mockProvider.On("ASN", ip).Return(nil, geo.ErrNoASN) + closest, distance, err := r.servers.Closest(r, "https", ip) + + Expect(err).To(BeNil()) + + // This should ONLY return our Ottawa server due to the deviation setting + Expect(closest.Host).To(Equal("server2.example.ca")) + + // This is the distance we expect + Expect(int(distance)).To(Equal(13596)) }) - It("Should successfully return the top servers if they are all in the same location/city", func() { + It("Should successfully return a close server and not one from outside our deviation range", func() { + ip := net.IPv4(4, 3, 2, 1) + + // Ottawa, CA + mockProvider.On("City", ip).Return(&geo.City{ + // Near Ann Arbor, MI - outside of Detroit + Location: geo.Location{Latitude: 42.2819, Longitude: -83.7538}, + }, nil) + mockProvider.On("ASN", ip).Return(nil, geo.ErrNoASN) + + choices, err := r.servers.Choices(r, "https", ip) + + Expect(err).To(BeNil()) + + // Expect only the Detroit servers + Expect(len(choices)).To(Equal(2)) + for _, choice := range choices { + item := choice.Item.(ComputedDistance) + + Expect(item.Server.Host).To(BeElementOf("server13.example.com", "server14.example.com")) + Expect(item.Distance).To(BeNumerically("<", 60000)) + } }) It("Should successfully return the top servers if they are all within reasonable distance", func() { + ip := net.IPv4(4, 3, 2, 1) + + // Ottawa, CA + mockProvider.On("City", ip).Return(&geo.City{ + // Grand Rapids, MI - in between Detroit and Chicago + Location: geo.Location{Latitude: 42.9657, Longitude: -85.6774}, + }, nil) + mockProvider.On("ASN", ip).Return(nil, geo.ErrNoASN) + + choices, err := r.servers.Choices(r, "https", ip) + + Expect(err).To(BeNil()) + + Expect(len(choices)).To(Equal(r.config.TopChoices)) + + for _, choice := range choices { + item := choice.Item.(ComputedDistance) + + Expect(item.Server.Host).To(BeElementOf( + "server7.example.com", // Chicago, IL, USA + "server11.example.com", // Chicago, IL, USA + "server12.example.com", // Chicago, IL, USA + "server13.example.com", // Detroit, MI + "server14.example.com", // Detroit, MI + )) + } }) }) }) diff --git a/util/util.go b/util/util.go index 0691646..9a7e2f2 100644 --- a/util/util.go +++ b/util/util.go @@ -5,7 +5,7 @@ import ( "reflect" "strings" - "github.com/armbian/redirector/db" + "github.com/armbian/redirector/geo" ) var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -26,7 +26,7 @@ var ( func GetValue(val any, key string) (any, bool) { // Bypass reflection for known types if strings.HasPrefix(key, "asn") || strings.HasPrefix(key, "city") { - return db.GetValue(val, key) + return geo.GetValue(val, key) } // Fallback to reflection @@ -35,7 +35,7 @@ func GetValue(val any, key string) (any, bool) { // GetValue is a reflection helper like Laravel's data_get // It lets us use the syntax of some.field.nested in rules. -// This is the slow path, see db.GetValue for the faster (somewhat generated) path +// This is the slow path, see geo.GetValue for the faster (somewhat generated) path func getValueReflect(val any, key string) (any, bool) { v := reflect.ValueOf(val)