Skip to content

Commit

Permalink
refactor: spin geoipx off geolocate (#893)
Browse files Browse the repository at this point in the history
A bunch of packages (including oohelperd) just need the ability to
use MaxMind-like databases. They don't need the additional functionality
implemented by the geolocate package. Such a package, in fact, is
mostly (if not only) needed by the engine package.

Therefore, move code to query MaxMind-like databases to a separate
package, and avoid depending on geolocate in all the packages for
which it's sufficient to use geoipx.

Part of ooni/probe#2240
  • Loading branch information
bassosimone authored Aug 28, 2022
1 parent 1e7384d commit 110a118
Show file tree
Hide file tree
Showing 23 changed files with 224 additions and 211 deletions.
4 changes: 2 additions & 2 deletions internal/cmd/oohelperd/ipinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"strings"

"github.com/ooni/probe-cli/v3/internal/engine/experiment/webconnectivity"
"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
"github.com/ooni/probe-cli/v3/internal/geoipx"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)

Expand All @@ -36,7 +36,7 @@ func newIPInfo(creq *ctrlRequest, addrs []string) map[string]*webconnectivity.Co
if netxlite.IsBogon(addr) { // note: we already excluded non-IP addrs above
flags |= webconnectivity.ControlIPInfoFlagIsBogon
}
asn, _, _ := geolocate.LookupASN(addr) // AS0 on failure
asn, _, _ := geoipx.LookupASN(addr) // AS0 on failure
ipinfo[addr] = &webconnectivity.ControlIPInfo{
ASN: int64(asn),
Flags: flags,
Expand Down
3 changes: 1 addition & 2 deletions internal/engine/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"time"

"github.com/ooni/probe-cli/v3/internal/bytecounter"
"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
"github.com/ooni/probe-cli/v3/internal/engine/probeservices"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/version"
Expand Down Expand Up @@ -199,7 +198,7 @@ func (e *experiment) newMeasurement(input string) *model.Measurement {
Input: model.MeasurementTarget(input),
MeasurementStartTime: utctimenow.Format(dateFormat),
MeasurementStartTimeSaved: utctimenow,
ProbeIP: geolocate.DefaultProbeIP,
ProbeIP: model.DefaultProbeIP,
ProbeASN: e.session.ProbeASNString(),
ProbeCC: e.session.ProbeCC(),
ProbeNetworkName: e.session.ProbeNetworkName(),
Expand Down
6 changes: 2 additions & 4 deletions internal/engine/experiment/webconnectivity/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package webconnectivity
import (
"context"

"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
"github.com/ooni/probe-cli/v3/internal/geoipx"
"github.com/ooni/probe-cli/v3/internal/httpx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
Expand Down Expand Up @@ -122,9 +122,7 @@ func Control(
func (dns *ControlDNSResult) FillASNs(sess model.ExperimentSession) {
dns.ASNs = []int64{}
for _, ip := range dns.Addrs {
// TODO(bassosimone): this would be more efficient if we'd open just
// once the database and then reuse it for every address.
asn, _, _ := geolocate.LookupASN(ip)
asn, _, _ := geoipx.LookupASN(ip)
dns.ASNs = append(dns.ASNs, int64(asn))
}
}
2 changes: 1 addition & 1 deletion internal/engine/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestExperimentHonoursSharingDefaults(t *testing.T) {
name: "probeIP",
locationInfo: &geolocate.Results{ProbeIP: "8.8.8.8"},
expect: func(m *model.Measurement) bool {
return m.ProbeIP == geolocate.DefaultProbeIP
return m.ProbeIP == model.DefaultProbeIP
},
}, {
name: "probeASN",
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/geolocate/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func cloudflareIPLookup(
UserAgent: model.HTTPHeaderUserAgent,
}).WithBodyLogging().Build().FetchResource(ctx, "/cdn-cgi/trace")
if err != nil {
return DefaultProbeIP, err
return model.DefaultProbeIP, err
}
r := regexp.MustCompile("(?:ip)=(.*)")
ip := strings.Trim(string(r.Find(data)), "ip=")
Expand Down
45 changes: 7 additions & 38 deletions internal/engine/geolocate/geolocate.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,37 +10,6 @@ import (
"github.com/ooni/probe-cli/v3/internal/version"
)

const (
// DefaultProbeASN is the default probe ASN as a number.
DefaultProbeASN uint = 0

// DefaultProbeCC is the default probe CC.
DefaultProbeCC = "ZZ"

// DefaultProbeIP is the default probe IP.
DefaultProbeIP = model.DefaultProbeIP

// DefaultProbeNetworkName is the default probe network name.
DefaultProbeNetworkName = ""

// DefaultResolverASN is the default resolver ASN.
DefaultResolverASN uint = 0

// DefaultResolverIP is the default resolver IP.
DefaultResolverIP = "127.0.0.2"

// DefaultResolverNetworkName is the default resolver network name.
DefaultResolverNetworkName = ""
)

var (
// DefaultProbeASNString is the default probe ASN as a string.
DefaultProbeASNString = fmt.Sprintf("AS%d", DefaultProbeASN)

// DefaultResolverASNString is the default resolver ASN as a string.
DefaultResolverASNString = fmt.Sprintf("AS%d", DefaultResolverASN)
)

// Results contains geolocate results.
type Results struct {
// ASN is the autonomous system number.
Expand Down Expand Up @@ -139,13 +108,13 @@ type Task struct {
func (op Task) Run(ctx context.Context) (*Results, error) {
var err error
out := &Results{
ASN: DefaultProbeASN,
CountryCode: DefaultProbeCC,
NetworkName: DefaultProbeNetworkName,
ProbeIP: DefaultProbeIP,
ResolverASN: DefaultResolverASN,
ResolverIP: DefaultResolverIP,
ResolverNetworkName: DefaultResolverNetworkName,
ASN: model.DefaultProbeASN,
CountryCode: model.DefaultProbeCC,
NetworkName: model.DefaultProbeNetworkName,
ProbeIP: model.DefaultProbeIP,
ResolverASN: model.DefaultResolverASN,
ResolverIP: model.DefaultResolverIP,
ResolverNetworkName: model.DefaultResolverNetworkName,
}
ip, err := op.probeIPLookupper.LookupProbeIP(ctx)
if err != nil {
Expand Down
46 changes: 24 additions & 22 deletions internal/engine/geolocate/geolocate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"errors"
"testing"

"github.com/ooni/probe-cli/v3/internal/model"
)

type taskProbeIPLookupper struct {
Expand All @@ -25,25 +27,25 @@ func TestLocationLookupCannotLookupProbeIP(t *testing.T) {
if !errors.Is(err, expected) {
t.Fatalf("not the error we expected: %+v", err)
}
if out.ASN != DefaultProbeASN {
if out.ASN != model.DefaultProbeASN {
t.Fatal("invalid ASN value")
}
if out.CountryCode != DefaultProbeCC {
if out.CountryCode != model.DefaultProbeCC {
t.Fatal("invalid CountryCode value")
}
if out.NetworkName != DefaultProbeNetworkName {
if out.NetworkName != model.DefaultProbeNetworkName {
t.Fatal("invalid NetworkName value")
}
if out.ProbeIP != DefaultProbeIP {
if out.ProbeIP != model.DefaultProbeIP {
t.Fatal("invalid ProbeIP value")
}
if out.ResolverASN != DefaultResolverASN {
if out.ResolverASN != model.DefaultResolverASN {
t.Fatal("invalid ResolverASN value")
}
if out.ResolverIP != DefaultResolverIP {
if out.ResolverIP != model.DefaultResolverIP {
t.Fatal("invalid ResolverIP value")
}
if out.ResolverNetworkName != DefaultResolverNetworkName {
if out.ResolverNetworkName != model.DefaultResolverNetworkName {
t.Fatal("invalid ResolverNetworkName value")
}
}
Expand All @@ -69,25 +71,25 @@ func TestLocationLookupCannotLookupProbeASN(t *testing.T) {
if !errors.Is(err, expected) {
t.Fatalf("not the error we expected: %+v", err)
}
if out.ASN != DefaultProbeASN {
if out.ASN != model.DefaultProbeASN {
t.Fatal("invalid ASN value")
}
if out.CountryCode != DefaultProbeCC {
if out.CountryCode != model.DefaultProbeCC {
t.Fatal("invalid CountryCode value")
}
if out.NetworkName != DefaultProbeNetworkName {
if out.NetworkName != model.DefaultProbeNetworkName {
t.Fatal("invalid NetworkName value")
}
if out.ProbeIP != "1.2.3.4" {
t.Fatal("invalid ProbeIP value")
}
if out.ResolverASN != DefaultResolverASN {
if out.ResolverASN != model.DefaultResolverASN {
t.Fatal("invalid ResolverASN value")
}
if out.ResolverIP != DefaultResolverIP {
if out.ResolverIP != model.DefaultResolverIP {
t.Fatal("invalid ResolverIP value")
}
if out.ResolverNetworkName != DefaultResolverNetworkName {
if out.ResolverNetworkName != model.DefaultResolverNetworkName {
t.Fatal("invalid ResolverNetworkName value")
}
}
Expand Down Expand Up @@ -116,7 +118,7 @@ func TestLocationLookupCannotLookupProbeCC(t *testing.T) {
if out.ASN != 1234 {
t.Fatal("invalid ASN value")
}
if out.CountryCode != DefaultProbeCC {
if out.CountryCode != model.DefaultProbeCC {
t.Fatal("invalid CountryCode value")
}
if out.NetworkName != "1234.com" {
Expand All @@ -125,13 +127,13 @@ func TestLocationLookupCannotLookupProbeCC(t *testing.T) {
if out.ProbeIP != "1.2.3.4" {
t.Fatal("invalid ProbeIP value")
}
if out.ResolverASN != DefaultResolverASN {
if out.ResolverASN != model.DefaultResolverASN {
t.Fatal("invalid ResolverASN value")
}
if out.ResolverIP != DefaultResolverIP {
if out.ResolverIP != model.DefaultResolverIP {
t.Fatal("invalid ResolverIP value")
}
if out.ResolverNetworkName != DefaultResolverNetworkName {
if out.ResolverNetworkName != model.DefaultResolverNetworkName {
t.Fatal("invalid ResolverNetworkName value")
}
}
Expand Down Expand Up @@ -173,13 +175,13 @@ func TestLocationLookupCannotLookupResolverIP(t *testing.T) {
if out.didResolverLookup != true {
t.Fatal("invalid DidResolverLookup value")
}
if out.ResolverASN != DefaultResolverASN {
if out.ResolverASN != model.DefaultResolverASN {
t.Fatal("invalid ResolverASN value")
}
if out.ResolverIP != DefaultResolverIP {
if out.ResolverIP != model.DefaultResolverIP {
t.Fatal("invalid ResolverIP value")
}
if out.ResolverNetworkName != DefaultResolverNetworkName {
if out.ResolverNetworkName != model.DefaultResolverNetworkName {
t.Fatal("invalid ResolverNetworkName value")
}
}
Expand Down Expand Up @@ -213,13 +215,13 @@ func TestLocationLookupCannotLookupResolverNetworkName(t *testing.T) {
if out.didResolverLookup != true {
t.Fatal("invalid DidResolverLookup value")
}
if out.ResolverASN != DefaultResolverASN {
if out.ResolverASN != model.DefaultResolverASN {
t.Fatalf("invalid ResolverASN value: %+v", out.ResolverASN)
}
if out.ResolverIP != "4.3.2.1" {
t.Fatalf("invalid ResolverIP value: %+v", out.ResolverIP)
}
if out.ResolverNetworkName != DefaultResolverNetworkName {
if out.ResolverNetworkName != model.DefaultResolverNetworkName {
t.Fatal("invalid ResolverNetworkName value")
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal/engine/geolocate/iplookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ func (c ipLookupClient) doWithCustomFunc(
defer clnt.CloseIdleConnections()
ip, err := fn(ctx, clnt, c.Logger, c.UserAgent)
if err != nil {
return DefaultProbeIP, err
return model.DefaultProbeIP, err
}
if net.ParseIP(ip) == nil {
return DefaultProbeIP, fmt.Errorf("%w: %s", ErrInvalidIPAddress, ip)
return model.DefaultProbeIP, fmt.Errorf("%w: %s", ErrInvalidIPAddress, ip)
}
c.Logger.Debugf("iplookup: IP: %s", ip)
return ip, nil
Expand All @@ -110,5 +110,5 @@ func (c ipLookupClient) LookupProbeIP(ctx context.Context) (string, error) {
}
union.Add(err)
}
return DefaultProbeIP, union
return model.DefaultProbeIP, union
}
4 changes: 2 additions & 2 deletions internal/engine/geolocate/iplookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestIPLookupAllFailed(t *testing.T) {
if !errors.Is(err, context.Canceled) {
t.Fatal("expected an error here")
}
if ip != DefaultProbeIP {
if ip != model.DefaultProbeIP {
t.Fatal("expected the default IP here")
}
}
Expand All @@ -51,7 +51,7 @@ func TestIPLookupInvalidIP(t *testing.T) {
if !errors.Is(err, ErrInvalidIPAddress) {
t.Fatal("expected an error here")
}
if ip != DefaultProbeIP {
if ip != model.DefaultProbeIP {
t.Fatal("expected the default IP here")
}
}
49 changes: 5 additions & 44 deletions internal/engine/geolocate/mmdblookup.go
Original file line number Diff line number Diff line change
@@ -1,54 +1,15 @@
package geolocate

import (
"net"

"github.com/ooni/probe-assets/assets"
"github.com/oschwald/geoip2-golang"
"github.com/ooni/probe-cli/v3/internal/geoipx"
)

type mmdbLookupper struct{}

func (mmdbLookupper) LookupASN(ip string) (asn uint, org string, err error) {
asn, org = DefaultProbeASN, DefaultProbeNetworkName
db, err := geoip2.FromBytes(assets.ASNDatabaseData())
if err != nil {
return
}
defer db.Close()
record, err := db.ASN(net.ParseIP(ip))
if err != nil {
return
}
asn = record.AutonomousSystemNumber
if record.AutonomousSystemOrganization != "" {
org = record.AutonomousSystemOrganization
}
return
}

// LookupASN returns the ASN and the organization associated with the
// given IP address.
func LookupASN(ip string) (asn uint, org string, err error) {
return (mmdbLookupper{}).LookupASN(ip)
func (mmdbLookupper) LookupASN(ip string) (uint, string, error) {
return geoipx.LookupASN(ip)
}

func (mmdbLookupper) LookupCC(ip string) (cc string, err error) {
cc = DefaultProbeCC
db, err := geoip2.FromBytes(assets.CountryDatabaseData())
if err != nil {
return
}
defer db.Close()
record, err := db.Country(net.ParseIP(ip))
if err != nil {
return
}
// With MaxMind DB we used record.RegisteredCountry.IsoCode but that does
// not seem to work with the db-ip.com database. The record is empty, at
// least for my own IP address in Italy. --Simone (2020-02-25)
if record.Country.IsoCode != "" {
cc = record.Country.IsoCode
}
return
func (mmdbLookupper) LookupCC(ip string) (string, error) {
return geoipx.LookupCC(ip)
}
Loading

0 comments on commit 110a118

Please sign in to comment.