refactor: redesign how we import assets (#260)

* fix(pkg.go.dev): import a subpackage containing the assets

We're trying to fix this issue that pkg.go.dev does not build.

Thanks to @hellais for this very neat idea! Let's keep our
fingers crossed and see whether it fixes!

* feat: use embedded geoip databases

Closes https://github.com/ooni/probe/issues/1372.

Work done as part of https://github.com/ooni/probe/issues/1369.

* fix(assetsx): add tests

* feat: simplify and just vendor uncompressed DBs

* remove tests that seems not necessary anymore

* fix: run go mod tidy

* Address https://github.com/ooni/probe-cli/pull/260/files#r605181364

* rewrite a test in a better way

* fix: gently cleanup the legacy assetsdir

Do not remove the whole directory with brute force. Just zap the
files whose name we know. Then attempt to delete the legacy directory
as well. If not empty, just fail. This is fine because it means the
user has stored other files inside the directory.

* fix: create .miniooni if missing

internal/engine/geolocate/geolocate.go

@@ -3,7 +3,6 @@ package geolocate
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
@@ -43,12 +42,6 @@ var (
 	DefaultResolverASNString = fmt.Sprintf("AS%d", DefaultResolverASN)
 )
 
-var (
-	// ErrMissingResourcesManager indicates that no resources
-	// manager has been configured inside of Config.
-	ErrMissingResourcesManager = errors.New("geolocate: ResourcesManager is nil")
-)
-
 // Logger is the definition of Logger used by this package.
 type Logger interface {
 	Debug(msg string)
@@ -96,30 +89,17 @@ type probeIPLookupper interface {
 }
 
 type asnLookupper interface {
-	LookupASN(path string, ip string) (asn uint, network string, err error)
+	LookupASN(ip string) (asn uint, network string, err error)
 }
 
 type countryLookupper interface {
-	LookupCC(path string, ip string) (cc string, err error)
+	LookupCC(ip string) (cc string, err error)
 }
 
 type resolverIPLookupper interface {
 	LookupResolverIP(ctx context.Context) (addr string, err error)
 }
 
-// ResourcesManager manages the required resources.
-type ResourcesManager interface {
-	// ASNDatabasePath returns the path of the ASN database.
-	ASNDatabasePath() string
-
-	// CountryDatabasePath returns the path of the country database.
-	CountryDatabasePath() string
-
-	// MaybeUpdateResources ensures that the required resources
-	// have been downloaded and are current.
-	MaybeUpdateResources(ctx context.Context) error
-}
-
 // Resolver is a DNS resolver.
 type Resolver interface {
 	LookupHost(ctx context.Context, domain string) ([]string, error)
@@ -142,10 +122,6 @@ type Config struct {
 	// use a logger that discards all messages.
 	Logger Logger
 
-	// ResourcesManager is the mandatory resources manager. If not
-	// set, we will not be able to perform any lookup.
-	ResourcesManager ResourcesManager
-
 	// UserAgent is the user agent to use. If not set, then
 	// we will use a default user agent.
 	UserAgent string
@@ -162,9 +138,6 @@ func NewTask(config Config) (*Task, error) {
 	if config.Logger == nil {
 		config.Logger = model.DiscardLogger
 	}
-	if config.ResourcesManager == nil {
-		return nil, ErrMissingResourcesManager
-	}
 	if config.UserAgent == "" {
 		config.UserAgent = fmt.Sprintf("ooniprobe-engine/%s", version.Version)
 	}
@@ -183,7 +156,6 @@ func NewTask(config Config) (*Task, error) {
 		probeASNLookupper:    mmdbLookupper{},
 		resolverASNLookupper: mmdbLookupper{},
 		resolverIPLookupper:  resolverLookupClient{},
-		resourcesManager:     config.ResourcesManager,
 	}, nil
 }
 
@@ -196,7 +168,6 @@ type Task struct {
 	probeASNLookupper    asnLookupper
 	resolverASNLookupper asnLookupper
 	resolverIPLookupper  resolverIPLookupper
-	resourcesManager     ResourcesManager
 }
 
 // Run runs the task.
@@ -211,23 +182,18 @@ func (op Task) Run(ctx context.Context) (*Results, error) {
 		ResolverIP:          DefaultResolverIP,
 		ResolverNetworkName: DefaultResolverNetworkName,
 	}
-	if err := op.resourcesManager.MaybeUpdateResources(ctx); err != nil {
-		return out, fmt.Errorf("MaybeUpdateResource failed: %w", err)
-	}
 	ip, err := op.probeIPLookupper.LookupProbeIP(ctx)
 	if err != nil {
 		return out, fmt.Errorf("lookupProbeIP failed: %w", err)
 	}
 	out.ProbeIP = ip
-	asn, networkName, err := op.probeASNLookupper.LookupASN(
-		op.resourcesManager.ASNDatabasePath(), out.ProbeIP)
+	asn, networkName, err := op.probeASNLookupper.LookupASN(out.ProbeIP)
 	if err != nil {
 		return out, fmt.Errorf("lookupASN failed: %w", err)
 	}
 	out.ASN = asn
 	out.NetworkName = networkName
-	cc, err := op.countryLookupper.LookupCC(
-		op.resourcesManager.CountryDatabasePath(), out.ProbeIP)
+	cc, err := op.countryLookupper.LookupCC(out.ProbeIP)
 	if err != nil {
 		return out, fmt.Errorf("lookupProbeCC failed: %w", err)
 	}
@@ -244,7 +210,7 @@ func (op Task) Run(ctx context.Context) (*Results, error) {
 		}
 		out.ResolverIP = resolverIP
 		resolverASN, resolverNetworkName, err := op.resolverASNLookupper.LookupASN(
-			op.resourcesManager.ASNDatabasePath(), out.ResolverIP,
+			out.ResolverIP,
 		)
 		if err != nil {
 			return out, nil

internal/engine/geolocate/geolocate_test.go

@@ -6,57 +6,6 @@ import (
 	"testing"
 )
 
-type taskResourcesManager struct {
-	asnDatabasePath     string
-	countryDatabasePath string
-	err                 error
-}
-
-func (c taskResourcesManager) ASNDatabasePath() string {
-	return c.asnDatabasePath
-}
-
-func (c taskResourcesManager) CountryDatabasePath() string {
-	return c.countryDatabasePath
-}
-
-func (c taskResourcesManager) MaybeUpdateResources(ctx context.Context) error {
-	return c.err
-}
-
-func TestLocationLookupCannotUpdateResources(t *testing.T) {
-	expected := errors.New("mocked error")
-	op := Task{
-		resourcesManager: taskResourcesManager{err: expected},
-	}
-	ctx := context.Background()
-	out, err := op.Run(ctx)
-	if !errors.Is(err, expected) {
-		t.Fatalf("not the error we expected: %+v", err)
-	}
-	if out.ASN != DefaultProbeASN {
-		t.Fatal("invalid ASN value")
-	}
-	if out.CountryCode != DefaultProbeCC {
-		t.Fatal("invalid CountryCode value")
-	}
-	if out.NetworkName != DefaultProbeNetworkName {
-		t.Fatal("invalid NetworkName value")
-	}
-	if out.ProbeIP != DefaultProbeIP {
-		t.Fatal("invalid ProbeIP value")
-	}
-	if out.ResolverASN != DefaultResolverASN {
-		t.Fatal("invalid ResolverASN value")
-	}
-	if out.ResolverIP != DefaultResolverIP {
-		t.Fatal("invalid ResolverIP value")
-	}
-	if out.ResolverNetworkName != DefaultResolverNetworkName {
-		t.Fatal("invalid ResolverNetworkName value")
-	}
-}
-
 type taskProbeIPLookupper struct {
 	ip  string
 	err error
@@ -69,7 +18,6 @@ func (c taskProbeIPLookupper) LookupProbeIP(ctx context.Context) (string, error)
 func TestLocationLookupCannotLookupProbeIP(t *testing.T) {
 	expected := errors.New("mocked error")
 	op := Task{
-		resourcesManager: taskResourcesManager{},
 		probeIPLookupper: taskProbeIPLookupper{err: expected},
 	}
 	ctx := context.Background()
@@ -106,14 +54,13 @@ type taskASNLookupper struct {
 	name string
 }
 
-func (c taskASNLookupper) LookupASN(path string, ip string) (uint, string, error) {
+func (c taskASNLookupper) LookupASN(ip string) (uint, string, error) {
 	return c.asn, c.name, c.err
 }
 
 func TestLocationLookupCannotLookupProbeASN(t *testing.T) {
 	expected := errors.New("mocked error")
 	op := Task{
-		resourcesManager:  taskResourcesManager{},
 		probeIPLookupper:  taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper: taskASNLookupper{err: expected},
 	}
@@ -150,14 +97,13 @@ type taskCCLookupper struct {
 	cc  string
 }
 
-func (c taskCCLookupper) LookupCC(path string, ip string) (string, error) {
+func (c taskCCLookupper) LookupCC(ip string) (string, error) {
 	return c.cc, c.err
 }
 
 func TestLocationLookupCannotLookupProbeCC(t *testing.T) {
 	expected := errors.New("mocked error")
 	op := Task{
-		resourcesManager:  taskResourcesManager{},
 		probeIPLookupper:  taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper: taskASNLookupper{asn: 1234, name: "1234.com"},
 		countryLookupper:  taskCCLookupper{cc: "US", err: expected},
@@ -202,7 +148,6 @@ func (c taskResolverIPLookupper) LookupResolverIP(ctx context.Context) (string,
 func TestLocationLookupCannotLookupResolverIP(t *testing.T) {
 	expected := errors.New("mocked error")
 	op := Task{
-		resourcesManager:     taskResourcesManager{},
 		probeIPLookupper:     taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper:    taskASNLookupper{asn: 1234, name: "1234.com"},
 		countryLookupper:     taskCCLookupper{cc: "IT"},
@@ -243,7 +188,6 @@ func TestLocationLookupCannotLookupResolverIP(t *testing.T) {
 func TestLocationLookupCannotLookupResolverNetworkName(t *testing.T) {
 	expected := errors.New("mocked error")
 	op := Task{
-		resourcesManager:     taskResourcesManager{},
 		probeIPLookupper:     taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper:    taskASNLookupper{asn: 1234, name: "1234.com"},
 		countryLookupper:     taskCCLookupper{cc: "IT"},
@@ -284,7 +228,6 @@ func TestLocationLookupCannotLookupResolverNetworkName(t *testing.T) {
 
 func TestLocationLookupSuccessWithResolverLookup(t *testing.T) {
 	op := Task{
-		resourcesManager:     taskResourcesManager{},
 		probeIPLookupper:     taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper:    taskASNLookupper{asn: 1234, name: "1234.com"},
 		countryLookupper:     taskCCLookupper{cc: "IT"},
@@ -325,7 +268,6 @@ func TestLocationLookupSuccessWithResolverLookup(t *testing.T) {
 
 func TestLocationLookupSuccessWithoutResolverLookup(t *testing.T) {
 	op := Task{
-		resourcesManager:     taskResourcesManager{},
 		probeIPLookupper:     taskProbeIPLookupper{ip: "1.2.3.4"},
 		probeASNLookupper:    taskASNLookupper{asn: 1234, name: "1234.com"},
 		countryLookupper:     taskCCLookupper{cc: "IT"},
@@ -364,13 +306,8 @@ func TestLocationLookupSuccessWithoutResolverLookup(t *testing.T) {
 }
 
 func TestSmoke(t *testing.T) {
-	maybeFetchResources(t)
 	config := Config{
 		EnableResolverLookup: true,
-		ResourcesManager: taskResourcesManager{
-			asnDatabasePath:     asnDBPath,
-			countryDatabasePath: countryDBPath,
-		},
 	}
 	task := Must(NewTask(config))
 	result, err := task.Run(context.Background())
@@ -384,16 +321,6 @@ func TestSmoke(t *testing.T) {
 	// value is okay for all codepaths.
 }
 
-func TestNewTaskWithNoResourcesManager(t *testing.T) {
-	task, err := NewTask(Config{})
-	if !errors.Is(err, ErrMissingResourcesManager) {
-		t.Fatal("not the error we expected")
-	}
-	if task != nil {
-		t.Fatal("expected nil task here")
-	}
-}
-
 func TestASNStringWorks(t *testing.T) {
 	r := Results{ASN: 1234}
 	if r.ASNString() != "AS1234" {

internal/engine/geolocate/mmdblookup.go

@@ -3,14 +3,15 @@ package geolocate
 import (
 	"net"
 
+	"github.com/ooni/probe-assets/assets"
 	"github.com/oschwald/geoip2-golang"
 )
 
 type mmdbLookupper struct{}
 
-func (mmdbLookupper) LookupASN(path, ip string) (asn uint, org string, err error) {
+func (mmdbLookupper) LookupASN(ip string) (asn uint, org string, err error) {
 	asn, org = DefaultProbeASN, DefaultProbeNetworkName
-	db, err := geoip2.Open(path)
+	db, err := geoip2.FromBytes(assets.ASNDatabaseData())
 	if err != nil {
 		return
 	}
@@ -28,13 +29,13 @@ func (mmdbLookupper) LookupASN(path, ip string) (asn uint, org string, err error
 
 // LookupASN returns the ASN and the organization associated with the
 // given ip using the ASN database at path.
-func LookupASN(path, ip string) (asn uint, org string, err error) {
-	return (mmdbLookupper{}).LookupASN(path, ip)
+func LookupASN(ip string) (asn uint, org string, err error) {
+	return (mmdbLookupper{}).LookupASN(ip)
 }
 
-func (mmdbLookupper) LookupCC(path, ip string) (cc string, err error) {
+func (mmdbLookupper) LookupCC(ip string) (cc string, err error) {
 	cc = DefaultProbeCC
-	db, err := geoip2.Open(path)
+	db, err := geoip2.FromBytes(assets.CountryDatabaseData())
 	if err != nil {
 		return
 	}

internal/engine/geolocate/mmdblookup_test.go

@@ -1,27 +1,11 @@
 package geolocate
 
-import (
-	"testing"
+import "testing"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/resourcesmanager"
-)
-
-const (
-	asnDBPath     = "../testdata/asn.mmdb"
-	countryDBPath = "../testdata/country.mmdb"
-	ipAddr        = "35.204.49.125"
-)
-
-func maybeFetchResources(t *testing.T) {
-	c := &resourcesmanager.CopyWorker{DestDir: "../testdata/"}
-	if err := c.Ensure(); err != nil {
-		t.Fatal(err)
-	}
-}
+const ipAddr = "35.204.49.125"
 
 func TestLookupASN(t *testing.T) {
-	maybeFetchResources(t)
-	asn, org, err := LookupASN(asnDBPath, ipAddr)
+	asn, org, err := LookupASN(ipAddr)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -33,23 +17,8 @@ func TestLookupASN(t *testing.T) {
 	}
 }
 
-func TestLookupASNInvalidFile(t *testing.T) {
-	maybeFetchResources(t)
-	asn, org, err := LookupASN("/nonexistent", ipAddr)
-	if err == nil {
-		t.Fatal("expected an error here")
-	}
-	if asn != DefaultProbeASN {
-		t.Fatal("expected a zero ASN")
-	}
-	if org != DefaultProbeNetworkName {
-		t.Fatal("expected an empty org")
-	}
-}
-
 func TestLookupASNInvalidIP(t *testing.T) {
-	maybeFetchResources(t)
-	asn, org, err := LookupASN(asnDBPath, "xxx")
+	asn, org, err := LookupASN("xxx")
 	if err == nil {
 		t.Fatal("expected an error here")
 	}
@@ -62,8 +31,7 @@ func TestLookupASNInvalidIP(t *testing.T) {
 }
 
 func TestLookupCC(t *testing.T) {
-	maybeFetchResources(t)
-	cc, err := (mmdbLookupper{}).LookupCC(countryDBPath, ipAddr)
+	cc, err := (mmdbLookupper{}).LookupCC(ipAddr)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -72,20 +40,8 @@ func TestLookupCC(t *testing.T) {
 	}
 }
 
-func TestLookupCCInvalidFile(t *testing.T) {
-	maybeFetchResources(t)
-	cc, err := (mmdbLookupper{}).LookupCC("/nonexistent", ipAddr)
-	if err == nil {
-		t.Fatal("expected an error here")
-	}
-	if cc != DefaultProbeCC {
-		t.Fatal("expected an empty cc")
-	}
-}
-
 func TestLookupCCInvalidIP(t *testing.T) {
-	maybeFetchResources(t)
-	cc, err := (mmdbLookupper{}).LookupCC(asnDBPath, "xxx")
+	cc, err := (mmdbLookupper{}).LookupCC("xxx")
 	if err == nil {
 		t.Fatal("expected an error here")
 	}