diff --git a/.github/workflows/alltests.yml b/.github/workflows/alltests.yml index 34ddc2e..be88cdd 100644 --- a/.github/workflows/alltests.yml +++ b/.github/workflows/alltests.yml @@ -13,5 +13,4 @@ jobs: with: go-version: "1.16" - uses: actions/checkout@v2 - - run: go run ./internal/cmd/getresources - run: go test -race -tags shaping ./... diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 2399cb5..b242c76 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -15,7 +15,6 @@ jobs: with: go-version: "${{ matrix.go }}" - uses: actions/checkout@v2 - - run: go run ./internal/cmd/getresources - run: go test -short -race -tags shaping -coverprofile=probe-cli.cov ./... - uses: shogo82148/actions-goveralls@v1 with: diff --git a/.github/workflows/shorttests.yml b/.github/workflows/shorttests.yml index ffcdb09..3ade3d5 100644 --- a/.github/workflows/shorttests.yml +++ b/.github/workflows/shorttests.yml @@ -15,5 +15,4 @@ jobs: with: go-version: "${{ matrix.go }}" - uses: actions/checkout@v2 - - run: go run ./internal/cmd/getresources - run: go test -short -race -tags shaping ./... diff --git a/E2E/miniooni.bash b/E2E/miniooni.bash index 01fb975..36b7ce5 100755 --- a/E2E/miniooni.bash +++ b/E2E/miniooni.bash @@ -1,6 +1,5 @@ #!/bin/bash set -e -go run ./internal/cmd/getresources go build -v ./internal/cmd/miniooni probeservices=() probeservices+=( "https://ps1.ooni.io" ) diff --git a/QA/pyrun.sh b/QA/pyrun.sh index 2c8db8a..76f3f28 100755 --- a/QA/pyrun.sh +++ b/QA/pyrun.sh @@ -1,7 +1,6 @@ #!/bin/sh set -ex export GOPATH=/jafar/QA/GOPATH GOCACHE=/jafar/QA/GOCACHE GO111MODULE=on -go run ./internal/cmd/getresources go build -v ./internal/cmd/miniooni go build -v ./internal/cmd/jafar sudo ./QA/$1.py ./miniooni diff --git a/Readme.md b/Readme.md index 8520de3..acd0838 100644 --- a/Readme.md +++ b/Readme.md @@ -24,15 +24,7 @@ Every top-level directory contains an explanatory README file. ## Development setup Be sure you have golang >= 1.16 and a C compiler (when developing for Windows, you -need Mingw-w64 installed). - -You need to download assets first using: - -```bash -go run ./internal/cmd/getresources -``` - -Then you can build using: +need Mingw-w64 installed). You can build using: ```bash go build -v ./cmd/ooniprobe diff --git a/build-android.bash b/build-android.bash index 2b5e9ab..161edb7 100755 --- a/build-android.bash +++ b/build-android.bash @@ -28,5 +28,4 @@ export PATH=$(go env GOPATH)/bin:$PATH go get -u golang.org/x/mobile/cmd/gomobile gomobile init output=MOBILE/android/oonimkall.aar -go run ./internal/cmd/getresources gomobile bind -target=android -o $output -ldflags="-s -w" ./pkg/oonimkall diff --git a/build-ios.bash b/build-ios.bash index 70962a5..a2cc5a6 100755 --- a/build-ios.bash +++ b/build-ios.bash @@ -6,5 +6,4 @@ export PATH=$(go env GOPATH)/bin:$PATH go get -u golang.org/x/mobile/cmd/gomobile gomobile init output=MOBILE/ios/oonimkall.framework -go run ./internal/cmd/getresources gomobile bind -target=ios -o $output -ldflags="-s -w" ./pkg/oonimkall diff --git a/build-miniooni.sh b/build-miniooni.sh index 85d1610..18b2434 100755 --- a/build-miniooni.sh +++ b/build-miniooni.sh @@ -2,12 +2,10 @@ set -e case $1 in macos|darwin) - go run ./internal/cmd/getresources export GOOS=darwin GOARCH=amd64 go build -o ./CLI/darwin/amd64 -ldflags="-s -w" ./internal/cmd/miniooni echo "Binary ready at ./CLI/darwin/amd64/miniooni";; linux) - go run ./internal/cmd/getresources export GOOS=linux GOARCH=386 go build -o ./CLI/linux/386 -tags netgo -ldflags='-s -w -extldflags "-static"' ./internal/cmd/miniooni echo "Binary ready at ./CLI/linux/386/miniooni" @@ -21,7 +19,6 @@ case $1 in go build -o ./CLI/linux/arm64 -tags netgo -ldflags='-s -w -extldflags "-static"' ./internal/cmd/miniooni echo "Binary ready at ./CLI/linux/arm64/miniooni";; windows) - go run ./internal/cmd/getresources export GOOS=windows GOARCH=386 go build -o ./CLI/windows/386 -ldflags="-s -w" ./internal/cmd/miniooni echo "Binary ready at ./CLI/windows/386/miniooni.exe" diff --git a/build.sh b/build.sh index a8196ac..456f28e 100755 --- a/build.sh +++ b/build.sh @@ -12,7 +12,6 @@ case $1 in ;; windows_amd64) - go run ./internal/cmd/getresources # Note! This assumes we've installed the mingw-w64 compiler. GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc \ go build -ldflags='-s -w' ./cmd/ooniprobe @@ -23,7 +22,6 @@ case $1 in ;; windows_386) - go run ./internal/cmd/getresources # Note! This assumes we've installed the mingw-w64 compiler. GOOS=windows GOARCH=386 CGO_ENABLED=1 CC=i686-w64-mingw32-gcc \ go build -ldflags='-s -w' ./cmd/ooniprobe @@ -40,7 +38,6 @@ case $1 in ;; linux_amd64) - go run ./internal/cmd/getresources docker pull --platform linux/amd64 golang:1.16-alpine docker run --platform linux/amd64 -v`pwd`:/ooni -w/ooni golang:1.16-alpine ./build.sh _alpine tar -cvzf ooniprobe_${v}_linux_amd64.tar.gz LICENSE.md Readme.md ooniprobe @@ -48,7 +45,6 @@ case $1 in ;; linux_386) - go run ./internal/cmd/getresources docker pull --platform linux/386 golang:1.16-alpine docker run --platform linux/386 -v`pwd`:/ooni -w/ooni golang:1.16-alpine ./build.sh _alpine tar -cvzf ooniprobe_${v}_linux_386.tar.gz LICENSE.md Readme.md ooniprobe @@ -64,7 +60,6 @@ case $1 in macos|darwin) set -x - go run ./internal/cmd/getresources # Note! The following line _assumes_ you have a working C compiler. If you # have Xcode command line tools installed, you are fine. go build -ldflags='-s -w' ./cmd/ooniprobe diff --git a/cmd/ooniprobe/internal/ooni/ooni.go b/cmd/ooniprobe/internal/ooni/ooni.go index ab9c065..7dfb079 100644 --- a/cmd/ooniprobe/internal/ooni/ooni.go +++ b/cmd/ooniprobe/internal/ooni/ooni.go @@ -14,6 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/enginex" "github.com/ooni/probe-cli/v3/cmd/ooniprobe/internal/utils" engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" "github.com/pkg/errors" "upper.io/db.v3/lib/sqlbuilder" ) @@ -177,6 +178,14 @@ func (p *Probe) Init(softwareName, softwareVersion string) error { } p.db = db + // We cleanup the assets files used by versions of ooniprobe + // older than v3.9.0, where we started embedding the assets + // into the binary and use that directly. This cleanup doesn't + // remove the whole directory but only known files inside it + // and then the directory itself, if empty. We explicitly discard + // the return value as it does not matter to us here. + _, _ = assetsdir.Cleanup(utils.AssetsDir(p.home)) + tempDir, err := ioutil.TempDir("", "ooni") if err != nil { return errors.Wrap(err, "creating TempDir") @@ -199,7 +208,6 @@ func (p *Probe) NewSession() (*engine.Session, error) { return nil, errors.Wrap(err, "creating engine's kvstore") } return engine.NewSession(engine.SessionConfig{ - AssetsDir: utils.AssetsDir(p.home), KVStore: kvstore, Logger: enginex.Logger, SoftwareName: p.softwareName, diff --git a/go.mod b/go.mod index f8bd676..77327f2 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect github.com/miekg/dns v1.1.41 github.com/montanaflynn/stats v0.6.5 + github.com/ooni/probe-assets v0.0.0-20210401100648-90ed7b6dff90 github.com/ooni/psiphon v0.6.0 github.com/oschwald/geoip2-golang v1.5.0 github.com/pborman/getopt/v2 v2.1.0 diff --git a/go.sum b/go.sum index b601407..1fe72ff 100644 --- a/go.sum +++ b/go.sum @@ -335,6 +335,8 @@ github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1Cpa github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1 h1:o0+MgICZLuZ7xjH7Vx6zS/zcu93/BEp1VwkIW1mEXCE= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= +github.com/ooni/probe-assets v0.0.0-20210401100648-90ed7b6dff90 h1:G7urihGBD88p5iU1bg7cFAqX/38qIPZH03wCTmyUAXI= +github.com/ooni/probe-assets v0.0.0-20210401100648-90ed7b6dff90/go.mod h1:N0PyNM3aadlYDDCFXAPzs54HC54+MZA/4/xnCtd9EAo= github.com/ooni/psiphon v0.6.0 h1:TWXFSlXBOFUwfGblLKaLXSoI7UL6dreoPXHW2uQTHlc= github.com/ooni/psiphon v0.6.0/go.mod h1:i1v6JweJtxDKaI0i1aEw2/Fr/CUi5BoQ75GYz5KmKwU= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= diff --git a/internal/cmd/getresources/getresources.go b/internal/cmd/getresources/getresources.go index 3f6ba7d..c8eebb1 100644 --- a/internal/cmd/getresources/getresources.go +++ b/internal/cmd/getresources/getresources.go @@ -2,50 +2,13 @@ package main import ( - "crypto/sha256" - "errors" - "fmt" - "io/ioutil" "log" - "net/http" - "net/url" - "path/filepath" - - "github.com/ooni/probe-cli/v3/internal/engine/resources" + "time" ) func main() { - for name, ri := range resources.All { - if err := getit(name, &ri); err != nil { - log.Fatal(err) - } - } -} - -func getit(name string, ri *resources.ResourceInfo) error { - workDir := filepath.Join("internal", "engine", "resourcesmanager") - URL, err := url.Parse(resources.BaseURL) - if err != nil { - return err - } - URL.Path = ri.URLPath - log.Println("fetching", URL.String()) - resp, err := http.Get(URL.String()) - if err != nil { - return err - } - defer resp.Body.Close() - if resp.StatusCode != 200 { - return errors.New("http request failed") - } - data, err := ioutil.ReadAll(resp.Body) - if err != nil { - return err - } - checksum := fmt.Sprintf("%x", sha256.Sum256(data)) - if checksum != ri.GzSHA256 { - return errors.New("sha256 mismatch") - } - fullpath := filepath.Join(workDir, name+".gz") - return ioutil.WriteFile(fullpath, data, 0644) + log.Printf("This command is no longer needed. We will keep it into") + log.Printf("the repository until the end of 2021, so you have\n") + log.Printf("time to adjust your build workflow.\n") + time.Sleep(5 * time.Second) } diff --git a/internal/cmd/miniooni/libminiooni.go b/internal/cmd/miniooni/libminiooni.go index a126cf4..25b3a65 100644 --- a/internal/cmd/miniooni/libminiooni.go +++ b/internal/cmd/miniooni/libminiooni.go @@ -18,6 +18,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/humanizex" + "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/selfcensor" "github.com/ooni/probe-cli/v3/internal/version" @@ -303,9 +304,18 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { homeDir := gethomedir(currentOptions.HomeDir) fatalIfFalse(homeDir != "", "home directory is empty") miniooniDir := path.Join(homeDir, ".miniooni") + err = os.MkdirAll(miniooniDir, 0700) + fatalOnError(err, "cannot create $HOME/.miniooni directory") + + // We cleanup the assets files used by versions of ooniprobe + // older than v3.9.0, where we started embedding the assets + // into the binary and use that directly. This cleanup doesn't + // remove the whole directory but only known files inside it + // and then the directory itself, if empty. We explicitly discard + // the return value as it does not matter to us here. assetsDir := path.Join(miniooniDir, "assets") - err = os.MkdirAll(assetsDir, 0700) - fatalOnError(err, "cannot create assets directory") + _, _ = assetsdir.Cleanup(assetsDir) + log.Debugf("miniooni state directory: %s", miniooniDir) consentFile := path.Join(miniooniDir, "informed") @@ -324,7 +334,6 @@ func MainWithConfiguration(experimentName string, currentOptions Options) { fatalOnError(err, "cannot create kvstore2 directory") config := engine.SessionConfig{ - AssetsDir: assetsDir, KVStore: kvstore, Logger: logger, ProxyURL: proxyURL, diff --git a/internal/engine/experiment.go b/internal/engine/experiment.go index 0ba160c..cd6ad89 100644 --- a/internal/engine/experiment.go +++ b/internal/engine/experiment.go @@ -7,7 +7,6 @@ import ( "errors" "net/http" "os" - "strconv" "time" "github.com/ooni/probe-cli/v3/internal/engine/geolocate" @@ -17,7 +16,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" - "github.com/ooni/probe-cli/v3/internal/engine/resources" "github.com/ooni/probe-cli/v3/internal/version" ) @@ -168,7 +166,6 @@ func (e *Experiment) newMeasurement(input string) *model.Measurement { TestStartTime: e.testStartTime, TestVersion: e.testVersion, } - m.AddAnnotation("assets_version", strconv.FormatInt(resources.Version, 10)) m.AddAnnotation("engine_name", "ooniprobe-engine") m.AddAnnotation("engine_version", version.Version) m.AddAnnotation("platform", platform.Name()) diff --git a/internal/engine/experiment/dnscheck/dnscheck.go b/internal/engine/experiment/dnscheck/dnscheck.go index 1a17b3e..6e8906c 100644 --- a/internal/engine/experiment/dnscheck/dnscheck.go +++ b/internal/engine/experiment/dnscheck/dnscheck.go @@ -169,7 +169,7 @@ func (m *Measurer) Run( ResolveSaver: evsaver, }) addrs, err := m.lookupHost(ctx, URL.Hostname(), resolver) - queries := archival.NewDNSQueriesList(begin, evsaver.Read(), sess.ASNDatabasePath()) + queries := archival.NewDNSQueriesList(begin, evsaver.Read()) tk.BootstrapFailure = archival.NewFailure(err) if len(queries) > 0 { // We get no queries in case we are resolving an IP address, since diff --git a/internal/engine/experiment/fbmessenger/fbmessenger_test.go b/internal/engine/experiment/fbmessenger/fbmessenger_test.go index e6640bc..ab6438e 100644 --- a/internal/engine/experiment/fbmessenger/fbmessenger_test.go +++ b/internal/engine/experiment/fbmessenger/fbmessenger_test.go @@ -222,7 +222,6 @@ func TestComputeEndpointStatsDNSIsLying(t *testing.T) { func newsession(t *testing.T) model.ExperimentSession { sess, err := engine.NewSession(engine.SessionConfig{ - AssetsDir: "../../testdata", AvailableProbeServices: []model.Service{{ Address: "https://ams-pg-test.ooni.org", Type: "https", diff --git a/internal/engine/experiment/hhfm/hhfm_test.go b/internal/engine/experiment/hhfm/hhfm_test.go index 63ab715..1cd1504 100644 --- a/internal/engine/experiment/hhfm/hhfm_test.go +++ b/internal/engine/experiment/hhfm/hhfm_test.go @@ -559,7 +559,6 @@ func TestTransactCannotReadBody(t *testing.T) { func newsession(t *testing.T) model.ExperimentSession { sess, err := engine.NewSession(engine.SessionConfig{ - AssetsDir: "../../testdata", AvailableProbeServices: []model.Service{{ Address: "https://ams-pg-test.ooni.org", Type: "https", diff --git a/internal/engine/experiment/stunreachability/stunreachability.go b/internal/engine/experiment/stunreachability/stunreachability.go index 4d5ba5b..b4b2211 100644 --- a/internal/engine/experiment/stunreachability/stunreachability.go +++ b/internal/engine/experiment/stunreachability/stunreachability.go @@ -107,7 +107,7 @@ func (tk *TestKeys) run( tk.NetworkEvents, archival.NewNetworkEventsList(begin, events)..., ) tk.Queries = append( - tk.Queries, archival.NewDNSQueriesList(begin, events, sess.ASNDatabasePath())..., + tk.Queries, archival.NewDNSQueriesList(begin, events)..., ) return err } diff --git a/internal/engine/experiment/urlgetter/getter.go b/internal/engine/experiment/urlgetter/getter.go index 6ec1dd6..9957efa 100644 --- a/internal/engine/experiment/urlgetter/getter.go +++ b/internal/engine/experiment/urlgetter/getter.go @@ -58,10 +58,7 @@ func (g Getter) Get(ctx context.Context) (TestKeys, error) { tk.FailedOperation = archival.NewFailedOperation(err) tk.Failure = archival.NewFailure(err) events := saver.Read() - tk.Queries = append( - tk.Queries, archival.NewDNSQueriesList( - g.Begin, events, g.Session.ASNDatabasePath())..., - ) + tk.Queries = append(tk.Queries, archival.NewDNSQueriesList(g.Begin, events)...) tk.NetworkEvents = append( tk.NetworkEvents, archival.NewNetworkEventsList(g.Begin, events)..., ) diff --git a/internal/engine/experiment/webconnectivity/control.go b/internal/engine/experiment/webconnectivity/control.go index 4d189b0..f3cd6b3 100644 --- a/internal/engine/experiment/webconnectivity/control.go +++ b/internal/engine/experiment/webconnectivity/control.go @@ -78,7 +78,7 @@ func (dns *ControlDNSResult) FillASNs(sess model.ExperimentSession) { 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(sess.ASNDatabasePath(), ip) + asn, _, _ := geolocate.LookupASN(ip) dns.ASNs = append(dns.ASNs, int64(asn)) } } diff --git a/internal/engine/experiment/webconnectivity/control_test.go b/internal/engine/experiment/webconnectivity/control_test.go index dc704ad..e33ff61 100644 --- a/internal/engine/experiment/webconnectivity/control_test.go +++ b/internal/engine/experiment/webconnectivity/control_test.go @@ -16,15 +16,6 @@ func TestFillASNsEmpty(t *testing.T) { } } -func TestFillASNsNoDatabase(t *testing.T) { - dns := new(webconnectivity.ControlDNSResult) - dns.Addrs = []string{"8.8.8.8", "1.1.1.1"} - dns.FillASNs(new(mockable.Session)) - if diff := cmp.Diff(dns.ASNs, []int64{0, 0}); diff != "" { - t.Fatal(diff) - } -} - func TestFillASNsSuccess(t *testing.T) { sess := newsession(t, false) dns := new(webconnectivity.ControlDNSResult) diff --git a/internal/engine/experiment/webconnectivity/webconnectivity_test.go b/internal/engine/experiment/webconnectivity/webconnectivity_test.go index f61ba0b..13a6bfa 100644 --- a/internal/engine/experiment/webconnectivity/webconnectivity_test.go +++ b/internal/engine/experiment/webconnectivity/webconnectivity_test.go @@ -202,7 +202,6 @@ func TestMeasureWithNoAvailableTestHelpers(t *testing.T) { func newsession(t *testing.T, lookupBackends bool) model.ExperimentSession { sess, err := engine.NewSession(engine.SessionConfig{ - AssetsDir: "../../testdata", AvailableProbeServices: []model.Service{{ Address: "https://ams-pg-test.ooni.org", Type: "https", diff --git a/internal/engine/geolocate/geolocate.go b/internal/engine/geolocate/geolocate.go index 15378cb..ec75df6 100644 --- a/internal/engine/geolocate/geolocate.go +++ b/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 diff --git a/internal/engine/geolocate/geolocate_test.go b/internal/engine/geolocate/geolocate_test.go index cabf2e2..b7d8fbf 100644 --- a/internal/engine/geolocate/geolocate_test.go +++ b/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" { diff --git a/internal/engine/geolocate/mmdblookup.go b/internal/engine/geolocate/mmdblookup.go index dc33158..bb02acf 100644 --- a/internal/engine/geolocate/mmdblookup.go +++ b/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 } diff --git a/internal/engine/geolocate/mmdblookup_test.go b/internal/engine/geolocate/mmdblookup_test.go index e66fbe8..7b9c055 100644 --- a/internal/engine/geolocate/mmdblookup_test.go +++ b/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") } diff --git a/internal/engine/inputloader_network_test.go b/internal/engine/inputloader_network_test.go index 3d10957..97c5e24 100644 --- a/internal/engine/inputloader_network_test.go +++ b/internal/engine/inputloader_network_test.go @@ -19,7 +19,6 @@ func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { Address: "https://ams-pg-test.ooni.org/", Type: "https", }}, - AssetsDir: "testdata", KVStore: kvstore.NewMemoryKeyValueStore(), Logger: log.Log, SoftwareName: "miniooni", diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 7287c46..764796c 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -237,7 +237,6 @@ func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { func TestInputLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing.T) { sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", KVStore: kvstore.NewMemoryKeyValueStore(), Logger: log.Log, SoftwareName: "miniooni", diff --git a/internal/engine/internal/mockable/mockable.go b/internal/engine/internal/mockable/mockable.go index 032e24d..7c606b2 100644 --- a/internal/engine/internal/mockable/mockable.go +++ b/internal/engine/internal/mockable/mockable.go @@ -18,7 +18,6 @@ import ( // Session allows to mock sessions. type Session struct { - MockableASNDatabasePath string MockableTestHelpers map[string][]model.Service MockableHTTPClient *http.Client MockableLogger model.Logger @@ -39,11 +38,6 @@ type Session struct { MockableUserAgent string } -// ASNDatabasePath implements ExperimentSession.ASNDatabasePath -func (sess *Session) ASNDatabasePath() string { - return sess.MockableASNDatabasePath -} - // GetTestHelpersByName implements ExperimentSession.GetTestHelpersByName func (sess *Session) GetTestHelpersByName(name string) ([]model.Service, bool) { services, okay := sess.MockableTestHelpers[name] diff --git a/internal/engine/internal/psiphonx/psiphonx_test.go b/internal/engine/internal/psiphonx/psiphonx_test.go index b5fc1be..427820b 100644 --- a/internal/engine/internal/psiphonx/psiphonx_test.go +++ b/internal/engine/internal/psiphonx/psiphonx_test.go @@ -17,7 +17,6 @@ func TestStartWithCancelledContext(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() sess, err := engine.NewSession(engine.SessionConfig{ - AssetsDir: "../../testdata", Logger: log.Log, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", @@ -39,7 +38,6 @@ func TestStartStop(t *testing.T) { t.Skip("skip test in short mode") } sess, err := engine.NewSession(engine.SessionConfig{ - AssetsDir: "../../testdata", Logger: log.Log, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", diff --git a/internal/engine/legacy/assetsdir/assetsdir.go b/internal/engine/legacy/assetsdir/assetsdir.go new file mode 100644 index 0000000..9a517a6 --- /dev/null +++ b/internal/engine/legacy/assetsdir/assetsdir.go @@ -0,0 +1,62 @@ +// Package assetsdir contains code to cleanup the assets dir. We removed +// the assetsdir in the 3.9.0 development cycle. +package assetsdir + +import ( + "errors" + "os" + "path/filepath" +) + +// ErrEmptyDir indicates that you passed to Cleanup an empty dir. +var ErrEmptyDir = errors.New("empty assets directory") + +// Result is the result of a Cleanup run. +type Result struct { + // ASNDatabaseErr is the error of deleting the + // file containing the old ASN database. + ASNDatabaseErr error + + // CABundleErr is the error of deleting the file + // containing the old CA bundle. + CABundleErr error + + // CountryDatabaseErr is the error of deleting the + // file containing the old country database. + CountryDatabaseErr error + + // RmdirErr is the error of deleting the supposedly + // empty directory that contained assets. + RmdirErr error +} + +// Cleanup removes data from the assetsdir. This function will +// try to delete the known assets inside of dir. It then also +// tries to delete the directory. If the directory is not empty, +// this operation will fail. That means the user has put some +// extra data in there and we don't want to remove it. +// +// Returns the Result of cleaning up the assets on success and +// an error on failure. The only cause of error is passing to +// this function an empty directory. The Result data structure +// contains the result of each individual remove operation. +func Cleanup(dir string) (*Result, error) { + return fcleanup(dir, os.Remove) +} + +// fcleanup is a version of Cleanup where we can mock the real function +// used for removing files and dirs, so we can write unit tests. +func fcleanup(dir string, remove func(name string) error) (*Result, error) { + if dir == "" { + return nil, ErrEmptyDir + } + r := &Result{} + asndb := filepath.Join(dir, "asn.mmdb") + r.ASNDatabaseErr = os.Remove(asndb) + cabundle := filepath.Join(dir, "ca-bundle.pem") + r.CABundleErr = os.Remove(cabundle) + countrydb := filepath.Join(dir, "country.mmdb") + r.CountryDatabaseErr = os.Remove(countrydb) + r.RmdirErr = os.Remove(dir) + return r, nil +} diff --git a/internal/engine/legacy/assetsdir/assetsdir_test.go b/internal/engine/legacy/assetsdir/assetsdir_test.go new file mode 100644 index 0000000..3fb8d0a --- /dev/null +++ b/internal/engine/legacy/assetsdir/assetsdir_test.go @@ -0,0 +1,40 @@ +package assetsdir + +import ( + "errors" + "strings" + "testing" +) + +func TestCleanupNormalUsage(t *testing.T) { + result, err := Cleanup("testdata") + if err != nil { + t.Fatal(err) + } + // we expect a bunch of ENOENT because the directory does not exist. + isExpectedErr := func(err error) bool { + return err != nil && strings.HasSuffix(err.Error(), "no such file or directory") + } + if !isExpectedErr(result.ASNDatabaseErr) { + t.Fatal("unexpected error", result.ASNDatabaseErr) + } + if !isExpectedErr(result.CABundleErr) { + t.Fatal("unexpected error", result.CABundleErr) + } + if !isExpectedErr(result.CountryDatabaseErr) { + t.Fatal("unexpected error", result.CountryDatabaseErr) + } + if !isExpectedErr(result.RmdirErr) { + t.Fatal("unexpected error", result.RmdirErr) + } +} + +func TestCleanupWithEmptyInput(t *testing.T) { + result, err := Cleanup("") + if !errors.Is(err, ErrEmptyDir) { + t.Fatal("unexpected error", err) + } + if result != nil { + t.Fatal("expected nil result") + } +} diff --git a/internal/engine/model/experiment.go b/internal/engine/model/experiment.go index 18e5f32..25e39e1 100644 --- a/internal/engine/model/experiment.go +++ b/internal/engine/model/experiment.go @@ -24,7 +24,6 @@ type ExperimentOrchestraClient interface { // ExperimentSession is the experiment's view of a session. type ExperimentSession interface { - ASNDatabasePath() string GetTestHelpersByName(name string) ([]Service, bool) DefaultHTTPClient() *http.Client Logger() Logger diff --git a/internal/engine/netx/archival/archival.go b/internal/engine/netx/archival/archival.go index 67ef5f8..11fd776 100644 --- a/internal/engine/netx/archival/archival.go +++ b/internal/engine/netx/archival/archival.go @@ -397,7 +397,7 @@ type DNSQueryEntry struct { type dnsQueryType string // NewDNSQueriesList returns a list of DNS queries. -func NewDNSQueriesList(begin time.Time, events []trace.Event, dbpath string) []DNSQueryEntry { +func NewDNSQueriesList(begin time.Time, events []trace.Event) []DNSQueryEntry { // TODO(bassosimone): add support for CNAME lookups. var out []DNSQueryEntry for _, ev := range events { @@ -409,7 +409,7 @@ func NewDNSQueriesList(begin time.Time, events []trace.Event, dbpath string) []D for _, addr := range ev.Addresses { if qtype.ipoftype(addr) { entry.Answers = append( - entry.Answers, qtype.makeanswerentry(addr, dbpath)) + entry.Answers, qtype.makeanswerentry(addr)) } } if len(entry.Answers) <= 0 && ev.Err == nil { @@ -431,16 +431,16 @@ func NewDNSQueriesList(begin time.Time, events []trace.Event, dbpath string) []D func (qtype dnsQueryType) ipoftype(addr string) bool { switch qtype { case "A": - return strings.Contains(addr, ":") == false + return !strings.Contains(addr, ":") case "AAAA": - return strings.Contains(addr, ":") == true + return strings.Contains(addr, ":") } return false } -func (qtype dnsQueryType) makeanswerentry(addr string, dbpath string) DNSAnswerEntry { +func (qtype dnsQueryType) makeanswerentry(addr string) DNSAnswerEntry { answer := DNSAnswerEntry{AnswerType: string(qtype)} - asn, org, _ := geolocate.LookupASN(dbpath, addr) + asn, org, _ := geolocate.LookupASN(addr) answer.ASN = int64(asn) answer.ASOrgName = org switch qtype { diff --git a/internal/engine/netx/archival/archival_internal_test.go b/internal/engine/netx/archival/archival_internal_test.go new file mode 100644 index 0000000..e927f9f --- /dev/null +++ b/internal/engine/netx/archival/archival_internal_test.go @@ -0,0 +1,41 @@ +package archival + +import "testing" + +func TestDNSQueryIPOfType(t *testing.T) { + type expectation struct { + qtype dnsQueryType + ip string + output bool + } + var expectations = []expectation{{ + qtype: "A", + ip: "8.8.8.8", + output: true, + }, { + qtype: "A", + ip: "2a00:1450:4002:801::2004", + output: false, + }, { + qtype: "AAAA", + ip: "8.8.8.8", + output: false, + }, { + qtype: "AAAA", + ip: "2a00:1450:4002:801::2004", + output: true, + }, { + qtype: "ANTANI", + ip: "2a00:1450:4002:801::2004", + output: false, + }, { + qtype: "ANTANI", + ip: "8.8.8.8", + output: false, + }} + for _, exp := range expectations { + if exp.qtype.ipoftype(exp.ip) != exp.output { + t.Fatalf("failure for %+v", exp) + } + } +} diff --git a/internal/engine/netx/archival/archival_test.go b/internal/engine/netx/archival/archival_test.go index df38a3e..5172ed6 100644 --- a/internal/engine/netx/archival/archival_test.go +++ b/internal/engine/netx/archival/archival_test.go @@ -16,7 +16,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" - "github.com/ooni/probe-cli/v3/internal/engine/resourcesmanager" ) func TestNewTCPConnectList(t *testing.T) { @@ -285,15 +284,10 @@ func TestNewRequestList(t *testing.T) { } func TestNewDNSQueriesList(t *testing.T) { - err := (&resourcesmanager.CopyWorker{DestDir: "../../testdata"}).Ensure() - if err != nil { - t.Fatal(err) - } begin := time.Now() type args struct { begin time.Time events []trace.Event - dbpath string } tests := []struct { name string @@ -334,9 +328,13 @@ func TestNewDNSQueriesList(t *testing.T) { }, want: []archival.DNSQueryEntry{{ Answers: []archival.DNSAnswerEntry{{ + ASN: 15169, + ASOrgName: "Google LLC", AnswerType: "A", IPv4: "8.8.8.8", }, { + ASN: 15169, + ASOrgName: "Google LLC", AnswerType: "A", IPv4: "8.8.4.4", }}, @@ -357,27 +355,6 @@ func TestNewDNSQueriesList(t *testing.T) { Time: begin.Add(200 * time.Millisecond), }}, }, - want: []archival.DNSQueryEntry{{ - Answers: []archival.DNSAnswerEntry{{ - AnswerType: "AAAA", - IPv6: "2001:4860:4860::8888", - }}, - Hostname: "dns.google.com", - QueryType: "AAAA", - T: 0.2, - }}, - }, { - name: "run with ASN DB", - args: args{ - begin: begin, - events: []trace.Event{{ - Addresses: []string{"2001:4860:4860::8888"}, - Hostname: "dns.google.com", - Name: "resolve_done", - Time: begin.Add(200 * time.Millisecond), - }}, - dbpath: "../../testdata/asn.mmdb", - }, want: []archival.DNSQueryEntry{{ Answers: []archival.DNSAnswerEntry{{ ASN: 15169, @@ -399,7 +376,6 @@ func TestNewDNSQueriesList(t *testing.T) { Name: "resolve_done", Time: begin.Add(200 * time.Millisecond), }}, - dbpath: "../../testdata/asn.mmdb", }, want: []archival.DNSQueryEntry{{ Answers: nil, @@ -419,9 +395,9 @@ func TestNewDNSQueriesList(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := archival.NewDNSQueriesList( - tt.args.begin, tt.args.events, tt.args.dbpath); !reflect.DeepEqual(got, tt.want) { - t.Error(cmp.Diff(got, tt.want)) + got := archival.NewDNSQueriesList(tt.args.begin, tt.args.events) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Fatal(diff) } }) } @@ -1009,13 +985,6 @@ func TestNewFailure(t *testing.T) { } } -func TestDNSQueryTypeInvalidIPOfType(t *testing.T) { - qtype := archival.DNSQueryType("ANTANI") - if qtype.IPOfType("8.8.8.8") != false { - t.Fatal("unexpected return value") - } -} - func TestNewFailedOperation(t *testing.T) { type args struct { err error diff --git a/internal/engine/netx/archival/archival_test_internal.go b/internal/engine/netx/archival/archival_test_internal.go deleted file mode 100644 index 976ba3f..0000000 --- a/internal/engine/netx/archival/archival_test_internal.go +++ /dev/null @@ -1,8 +0,0 @@ -package archival - -// DNSQueryType allows to access dnsQueryType from unit tests -type DNSQueryType = dnsQueryType - -func (qtype dnsQueryType) IPOfType(addr string) bool { - return qtype.ipoftype(addr) -} diff --git a/internal/engine/resources/assets.go b/internal/engine/resources/assets.go deleted file mode 100644 index 6e6d6ed..0000000 --- a/internal/engine/resources/assets.go +++ /dev/null @@ -1,42 +0,0 @@ -package resources - -const ( - // Version contains the assets version. - Version = 20210303114512 - - // ASNDatabaseName is the ASN-DB file name - ASNDatabaseName = "asn.mmdb" - - // CountryDatabaseName is country-DB file name - CountryDatabaseName = "country.mmdb" - - // BaseURL is the asset's repository base URL - BaseURL = "https://github.com/" -) - -// ResourceInfo contains information on a resource. -type ResourceInfo struct { - // URLPath is the resource's URL path. - URLPath string - - // GzSHA256 is used to validate the downloaded file. - GzSHA256 string - - // SHA256 is used to check whether the assets file - // stored locally is still up-to-date. - SHA256 string -} - -// All contains info on all known assets. -var All = map[string]ResourceInfo{ - "asn.mmdb": { - URLPath: "/ooni/probe-assets/releases/download/20210303114512/asn.mmdb.gz", - GzSHA256: "efafd5a165c5a4e6bf6258d87ed685254a2660669eb4557e25c5ed72e48d039a", - SHA256: "675dbaec3fa1e6f12957c4e4ddee03f50f5192507b5095ccb9ed057468c2441b", - }, - "country.mmdb": { - URLPath: "/ooni/probe-assets/releases/download/20210303114512/country.mmdb.gz", - GzSHA256: "7f1db0e2903271258319834f26bbcdedd2d0641457a8c0a63b048a985b7d6e7b", - SHA256: "19e4d2c5cd31789da1a67baf883995f2ea03c4b8ba7342b69ef8ae2c2aa8409c", - }, -} diff --git a/internal/engine/resources/doc.go b/internal/engine/resources/doc.go deleted file mode 100644 index a694698..0000000 --- a/internal/engine/resources/doc.go +++ /dev/null @@ -1,3 +0,0 @@ -// Package resources contains info on resources. See also -// the resourcesmanager package. -package resources diff --git a/internal/engine/resourcesmanager/.gitignore b/internal/engine/resourcesmanager/.gitignore deleted file mode 100644 index de9444a..0000000 --- a/internal/engine/resourcesmanager/.gitignore +++ /dev/null @@ -1,3 +0,0 @@ -/asn.mmdb.gz -/country.mmdb.gz -/testdata diff --git a/internal/engine/resourcesmanager/resourcesmanager.go b/internal/engine/resourcesmanager/resourcesmanager.go deleted file mode 100644 index d7a891e..0000000 --- a/internal/engine/resourcesmanager/resourcesmanager.go +++ /dev/null @@ -1,157 +0,0 @@ -// Package resourcesmanager contains the resources manager. -package resourcesmanager - -import ( - "compress/gzip" - "crypto/sha256" - "embed" - "errors" - "fmt" - "io" - "io/fs" - "io/ioutil" - "os" - "path/filepath" - - "github.com/ooni/probe-cli/v3/internal/engine/resources" -) - -// Errors returned by this package. -var ( - ErrDestDirEmpty = errors.New("resources: DestDir is empty") - ErrSHA256Mismatch = errors.New("resources: sha256 mismatch") -) - -// CopyWorker ensures that resources are current. You always need to set -// the DestDir attribute. All the rest is optional. -type CopyWorker struct { - DestDir string // mandatory - Different func(left, right string) bool // optional - Equal func(left, right string) bool // optional - MkdirAll func(path string, perm os.FileMode) error // optional - NewReader func(r io.Reader) (io.ReadCloser, error) // optional - Open func(path string) (fs.File, error) // optional - ReadAll func(r io.Reader) ([]byte, error) // optional - ReadFile func(filename string) ([]byte, error) // optional - WriteFile func(filename string, data []byte, perm fs.FileMode) error // optional -} - -// If you arrive here because of this error: -// -// internal/engine/resourcesmanager/resourcesmanager.go:39:12: pattern *.mmdb.gz: no matching files found -// internal/engine/resourcesmanager/resourcesmanager.go:39:12: pattern *.mmdb.gz: no matching files found -// -// then your problem is that you need to fetch resources _before_ compiling -// ooniprobe. See Readme.md for instructions on how to do that. - -//go:embed *.mmdb.gz -var efs embed.FS - -func (cw *CopyWorker) mkdirAll(path string, perm os.FileMode) error { - if cw.MkdirAll != nil { - return cw.MkdirAll(path, perm) - } - return os.MkdirAll(path, perm) -} - -// Ensure ensures that the resources on disk are current. -func (cw *CopyWorker) Ensure() error { - if cw.DestDir == "" { - return ErrDestDirEmpty - } - if err := cw.mkdirAll(cw.DestDir, 0700); err != nil { - return err - } - for name, resource := range resources.All { - if err := cw.ensureFor(name, &resource); err != nil { - return err - } - } - return nil -} - -func (cw *CopyWorker) readFile(path string) ([]byte, error) { - if cw.ReadFile != nil { - return cw.ReadFile(path) - } - return ioutil.ReadFile(path) -} - -func (cw *CopyWorker) equal(left, right string) bool { - if cw.Equal != nil { - return cw.Equal(left, right) - } - return left == right -} - -func (cw *CopyWorker) different(left, right string) bool { - if cw.Different != nil { - return cw.Different(left, right) - } - return left != right -} - -func (cw *CopyWorker) open(path string) (fs.File, error) { - if cw.Open != nil { - return cw.Open(path) - } - return efs.Open(path) -} - -func (cw *CopyWorker) newReader(r io.Reader) (io.ReadCloser, error) { - if cw.NewReader != nil { - return cw.NewReader(r) - } - return gzip.NewReader(r) -} - -func (cw *CopyWorker) readAll(r io.Reader) ([]byte, error) { - if cw.ReadAll != nil { - return cw.ReadAll(r) - } - return ioutil.ReadAll(r) -} - -func (cw *CopyWorker) writeFile(filename string, data []byte, perm fs.FileMode) error { - if cw.WriteFile != nil { - return cw.WriteFile(filename, data, perm) - } - return ioutil.WriteFile(filename, data, perm) -} - -func (cw *CopyWorker) sha256sum(data []byte) string { - return fmt.Sprintf("%x", sha256.Sum256(data)) -} - -func (cw *CopyWorker) allGood(rpath string, resource *resources.ResourceInfo) bool { - data, err := cw.readFile(rpath) - if err != nil { - return false - } - return cw.equal(cw.sha256sum(data), resource.SHA256) -} - -func (cw *CopyWorker) ensureFor(name string, resource *resources.ResourceInfo) error { - rpath := filepath.Join(cw.DestDir, name) - if cw.allGood(rpath, resource) { - return nil - } - filep, err := cw.open(name + ".gz") - if err != nil { - return err - } - defer filep.Close() - gzfilep, err := cw.newReader(filep) - if err != nil { - return err - } - defer gzfilep.Close() - data, err := cw.readAll(gzfilep) - if err != nil { - return err - } - if cw.different(cw.sha256sum(data), resource.SHA256) { - return ErrSHA256Mismatch - } - return cw.writeFile(rpath, data, 0600) -} diff --git a/internal/engine/resourcesmanager/resourcesmanager_test.go b/internal/engine/resourcesmanager/resourcesmanager_test.go deleted file mode 100644 index 5101e5f..0000000 --- a/internal/engine/resourcesmanager/resourcesmanager_test.go +++ /dev/null @@ -1,142 +0,0 @@ -package resourcesmanager - -import ( - "errors" - "io" - "io/fs" - "os" - "testing" -) - -func TestAllGood(t *testing.T) { - // make sure we start from scratch - if err := os.RemoveAll("testdata"); err != nil { - t.Fatal(err) - } - // first iteration should copy the resources - cw := &CopyWorker{DestDir: "testdata"} - if err := cw.Ensure(); err != nil { - t.Fatal(err) - } - // second iteration should just ensure they're there - if err := cw.Ensure(); err != nil { - t.Fatal(err) - } -} - -func TestEmptyDestDir(t *testing.T) { - cw := &CopyWorker{DestDir: ""} - if err := cw.Ensure(); !errors.Is(err, ErrDestDirEmpty) { - t.Fatal("not the error we expected", err) - } -} - -func TestMkdirAllFailure(t *testing.T) { - errMocked := errors.New("mocked error") - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return errMocked - }, - } - if err := cw.Ensure(); !errors.Is(err, errMocked) { - t.Fatal("not the error we expected", err) - } -} - -func TestOpenFailure(t *testing.T) { - errMocked := errors.New("mocked error") - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return nil - }, - ReadFile: func(path string) ([]byte, error) { - return []byte(`fake`), nil - }, - Equal: func(left, right string) bool { - return false - }, - Open: func(path string) (fs.File, error) { - return nil, errMocked - }, - } - if err := cw.Ensure(); !errors.Is(err, errMocked) { - t.Fatal("not the error we expected", err) - } -} - -func TestNewReaderFailure(t *testing.T) { - errMocked := errors.New("mocked error") - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return nil - }, - Equal: func(left, right string) bool { - return false - }, - NewReader: func(r io.Reader) (io.ReadCloser, error) { - return nil, errMocked - }, - } - if err := cw.Ensure(); !errors.Is(err, errMocked) { - t.Fatal("not the error we expected", err) - } -} - -func TestReadAllFailure(t *testing.T) { - errMocked := errors.New("mocked error") - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return nil - }, - Equal: func(left, right string) bool { - return false - }, - ReadAll: func(r io.Reader) ([]byte, error) { - return nil, errMocked - }, - } - if err := cw.Ensure(); !errors.Is(err, errMocked) { - t.Fatal("not the error we expected", err) - } -} - -func TestSHA256Mismatch(t *testing.T) { - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return nil - }, - Equal: func(left, right string) bool { - return false - }, - Different: func(left, right string) bool { - return true - }, - } - if err := cw.Ensure(); !errors.Is(err, ErrSHA256Mismatch) { - t.Fatal("not the error we expected", err) - } -} - -func TestWriteFileFailure(t *testing.T) { - errMocked := errors.New("mocked error") - cw := &CopyWorker{ - DestDir: "testdata", - MkdirAll: func(path string, perm os.FileMode) error { - return nil - }, - Equal: func(left, right string) bool { - return false - }, - WriteFile: func(filename string, data []byte, perm fs.FileMode) error { - return errMocked - }, - } - if err := cw.Ensure(); !errors.Is(err, errMocked) { - t.Fatal("not the error we expected", err) - } -} diff --git a/internal/engine/session.go b/internal/engine/session.go index 862e14a..af165e1 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -8,7 +8,6 @@ import ( "net/http" "net/url" "os" - "path/filepath" "sync" "github.com/ooni/probe-cli/v3/internal/engine/atomicx" @@ -21,14 +20,11 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/bytecounter" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" - "github.com/ooni/probe-cli/v3/internal/engine/resources" - "github.com/ooni/probe-cli/v3/internal/engine/resourcesmanager" "github.com/ooni/probe-cli/v3/internal/version" ) // SessionConfig contains the Session config type SessionConfig struct { - AssetsDir string AvailableProbeServices []model.Service KVStore KVStore Logger model.Logger @@ -40,9 +36,11 @@ type SessionConfig struct { TorBinary string } -// Session is a measurement session. +// Session is a measurement session. It contains shared information +// required to run a measurement session, and it controls the lifecycle +// of such resources. It is not possible to reuse a Session. You MUST +// NOT attempt to use a Session again after Session.Close. type Session struct { - assetsDir string availableProbeServices []model.Service availableTestHelpers map[string][]model.Service byteCounter *bytecounter.Counter @@ -64,6 +62,9 @@ type Session struct { tunnelName string tunnel tunnel.Tunnel + // closeOnce allows us to call Close just once. + closeOnce sync.Once + // mu provides mutual exclusion. mu sync.Mutex @@ -93,9 +94,6 @@ type sessionProbeServicesClientForCheckIn interface { // NewSession creates a new session or returns an error func NewSession(config SessionConfig) (*Session, error) { - if config.AssetsDir == "" { - return nil, errors.New("AssetsDir is empty") - } if config.Logger == nil { return nil, errors.New("Logger is empty") } @@ -117,7 +115,6 @@ func NewSession(config SessionConfig) (*Session, error) { return nil, err } sess := &Session{ - assetsDir: config.AssetsDir, availableProbeServices: config.AvailableProbeServices, byteCounter: bytecounter.New(), kvStore: config.KVStore, @@ -147,12 +144,6 @@ func NewSession(config SessionConfig) (*Session, error) { return sess, nil } -// ASNDatabasePath returns the path where the ASN database path should -// be if you have called s.FetchResourcesIdempotent. -func (s *Session) ASNDatabasePath() string { - return filepath.Join(s.assetsDir, resources.ASNDatabaseName) -} - // KibiBytesReceived accounts for the KibiBytes received by the HTTP clients // managed by this session so far, including experiments. func (s *Session) KibiBytesReceived() float64 { @@ -255,19 +246,19 @@ func (s *Session) newProbeServicesClientForCheckIn( // cause memory leaks in your application because of open idle connections, // as well as excessive usage of disk space. func (s *Session) Close() error { - // TODO(bassosimone): introduce a sync.Once to make this method idempotent. + s.closeOnce.Do(s.doClose) + return nil +} + +// doClose implements Close. This function is called just once. +func (s *Session) doClose() { s.httpDefaultTransport.CloseIdleConnections() s.resolver.CloseIdleConnections() s.logger.Infof("%s", s.resolver.Stats()) if s.tunnel != nil { s.tunnel.Stop() } - return os.RemoveAll(s.tempDir) -} - -// CountryDatabasePath is like ASNDatabasePath but for the country DB path. -func (s *Session) CountryDatabasePath() string { - return filepath.Join(s.assetsDir, resources.CountryDatabaseName) + _ = os.RemoveAll(s.tempDir) } // GetTestHelpersByName returns the available test helpers that @@ -546,11 +537,6 @@ func (s *Session) UserAgent() (useragent string) { return } -// MaybeUpdateResources updates the resources if needed. -func (s *Session) MaybeUpdateResources(ctx context.Context) error { - return (&resourcesmanager.CopyWorker{DestDir: s.assetsDir}).Ensure() -} - // getAvailableProbeServicesUnlocked returns the available probe // services. This function WILL NOT acquire the mu mutex, therefore, // you MUST ensure you are using it from a locked context. @@ -629,7 +615,6 @@ func (s *Session) LookupLocationContext(ctx context.Context) (*geolocate.Results EnableResolverLookup: s.proxyURL == nil, Logger: s.Logger(), Resolver: s.resolver, - ResourcesManager: s, UserAgent: s.UserAgent(), })) return task.Run(ctx) diff --git a/internal/engine/session_integration_test.go b/internal/engine/session_integration_test.go index f007637..08aa198 100644 --- a/internal/engine/session_integration_test.go +++ b/internal/engine/session_integration_test.go @@ -46,27 +46,19 @@ func TestNewSessionBuilderChecks(t *testing.T) { t.Run("with no settings", func(t *testing.T) { newSessionMustFail(t, SessionConfig{}) }) - t.Run("with only assets dir", func(t *testing.T) { - newSessionMustFail(t, SessionConfig{ - AssetsDir: "testdata", - }) - }) t.Run("with also logger", func(t *testing.T) { newSessionMustFail(t, SessionConfig{ - AssetsDir: "testdata", - Logger: model.DiscardLogger, + Logger: model.DiscardLogger, }) }) t.Run("with also software name", func(t *testing.T) { newSessionMustFail(t, SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", }) }) t.Run("with software version and wrong tempdir", func(t *testing.T) { newSessionMustFail(t, SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", @@ -97,7 +89,6 @@ func TestSessionTorArgsTorBinary(t *testing.T) { t.Skip("skip test in short mode") } sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", AvailableProbeServices: []model.Service{{ Address: "https://ams-pg-test.ooni.org", Type: "https", @@ -130,7 +121,6 @@ func TestSessionTorArgsTorBinary(t *testing.T) { func newSessionForTestingNoLookupsWithProxyURL(t *testing.T, URL *url.URL) *Session { sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", AvailableProbeServices: []model.Service{{ Address: "https://ams-pg-test.ooni.org", Type: "https", @@ -357,40 +347,11 @@ func TestSessionCloseCancelsTempDir(t *testing.T) { } } -func TestSessionDownloadResources(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - tmpdir, err := ioutil.TempDir("", "test-download-resources-idempotent") - if err != nil { - t.Fatal(err) - } - ctx := context.Background() - sess := newSessionForTestingNoLookups(t) - defer sess.Close() - sess.SetAssetsDir(tmpdir) - err = sess.MaybeUpdateResources(ctx) - if err != nil { - t.Fatal(err) - } - readfile := func(path string) (err error) { - _, err = ioutil.ReadFile(path) - return - } - if err := readfile(sess.ASNDatabasePath()); err != nil { - t.Fatal(err) - } - if err := readfile(sess.CountryDatabasePath()); err != nil { - t.Fatal(err) - } -} - func TestGetAvailableProbeServices(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", @@ -411,7 +372,6 @@ func TestMaybeLookupBackendsFailure(t *testing.T) { t.Skip("skip test in short mode") } sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", @@ -433,7 +393,6 @@ func TestMaybeLookupTestHelpersIdempotent(t *testing.T) { t.Skip("skip test in short mode") } sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", @@ -459,7 +418,6 @@ func TestAllProbeServicesUnsupported(t *testing.T) { t.Skip("skip test in short mode") } sess, err := NewSession(SessionConfig{ - AssetsDir: "testdata", Logger: model.DiscardLogger, SoftwareName: "ooniprobe-engine", SoftwareVersion: "0.0.1", diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go index b90b6fc..e13e60b 100644 --- a/internal/engine/session_internal_test.go +++ b/internal/engine/session_internal_test.go @@ -11,10 +11,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" ) -func (s *Session) SetAssetsDir(assetsDir string) { - s.assetsDir = assetsDir -} - func (s *Session) GetAvailableProbeServices() []model.Service { return s.getAvailableProbeServicesUnlocked() } diff --git a/pkg/oonimkall/internal/tasks/runner.go b/pkg/oonimkall/internal/tasks/runner.go index 34345e2..ccdc9a9 100644 --- a/pkg/oonimkall/internal/tasks/runner.go +++ b/pkg/oonimkall/internal/tasks/runner.go @@ -75,7 +75,6 @@ func (r *Runner) newsession(logger *ChanLogger) (*engine.Session, error) { return nil, err } config := engine.SessionConfig{ - AssetsDir: r.settings.AssetsDir, KVStore: kvstore, Logger: logger, SoftwareName: r.settings.Options.SoftwareName, diff --git a/pkg/oonimkall/session.go b/pkg/oonimkall/session.go index 6458946..537339b 100644 --- a/pkg/oonimkall/session.go +++ b/pkg/oonimkall/session.go @@ -9,6 +9,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/engine/atomicx" + "github.com/ooni/probe-cli/v3/internal/engine/legacy/assetsdir" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" "github.com/ooni/probe-cli/v3/internal/engine/runtimex" @@ -54,6 +55,9 @@ type ExperimentCallbacks interface { type SessionConfig struct { // AssetsDir is the mandatory directory where to store assets // required by a Session, e.g. MaxMind DB files. + // + // This field is currently deprecated and unused. We will + // remove it when we'll bump the major number. AssetsDir string // Logger is the optional logger that will receive all the @@ -116,6 +120,15 @@ func NewSession(config *SessionConfig) (*Session, error) { if err != nil { return nil, err } + + // We cleanup the assets files used by versions of ooniprobe + // older than v3.9.0, where we started embedding the assets + // into the binary and use that directly. This cleanup doesn't + // remove the whole directory but only known files inside it + // and then the directory itself, if empty. We explicitly discard + // the return value as it does not matter to us here. + _, _ = assetsdir.Cleanup(config.AssetsDir) + var availableps []model.Service if config.ProbeServicesURL != "" { availableps = append(availableps, model.Service{ @@ -124,7 +137,6 @@ func NewSession(config *SessionConfig) (*Session, error) { }) } engineConfig := engine.SessionConfig{ - AssetsDir: config.AssetsDir, AvailableProbeServices: availableps, KVStore: kvstore, Logger: newLogger(config.Logger, config.Verbose), @@ -212,15 +224,10 @@ type GeolocateResults struct { Org string } -// MaybeUpdateResources ensures that resources are up to date. This function -// could perform network activity when we need to update resources. -// -// This function locks the session until it's done. That is, no other operation -// can be performed as long as this function is pending. +// MaybeUpdateResources is a legacy stub. It does nothing. We will +// remove it when we're ready to bump the major number. func (sess *Session) MaybeUpdateResources(ctx *Context) error { - sess.mtx.Lock() - defer sess.mtx.Unlock() - return sess.sessp.MaybeUpdateResources(ctx.ctx) + return nil } // Geolocate performs a geolocate operation and returns the results. diff --git a/pkg/oonimkall/session_integration_test.go b/pkg/oonimkall/session_integration_test.go index 9d9b7ff..2569047 100644 --- a/pkg/oonimkall/session_integration_test.go +++ b/pkg/oonimkall/session_integration_test.go @@ -47,25 +47,9 @@ func TestNewSessionWithInvalidStateDir(t *testing.T) { } } -func TestNewSessionWithMissingSoftwareName(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := oonimkall.NewSession(&oonimkall.SessionConfig{ - StateDir: "../testdata/oonimkall/state", - }) - if err == nil || err.Error() != "AssetsDir is empty" { - t.Fatal("not the error we expected") - } - if sess != nil { - t.Fatal("expected a nil Session here") - } -} - func TestMaybeUpdateResourcesWithCancelledContext(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } + // Note that MaybeUpdateResources is now a deprecated stub that + // does nothing. We will remove it when we bump major. dir, err := ioutil.TempDir("", "xx") if err != nil { t.Fatal(err) diff --git a/pkg/oonimkall/task_integration_test.go b/pkg/oonimkall/task_integration_test.go index e8e81de..bfb149f 100644 --- a/pkg/oonimkall/task_integration_test.go +++ b/pkg/oonimkall/task_integration_test.go @@ -172,38 +172,6 @@ func TestEmptyStateDir(t *testing.T) { } } -func TestEmptyAssetsDir(t *testing.T) { - task, err := oonimkall.StartTask(`{ - "log_level": "DEBUG", - "name": "Example", - "options": { - "software_name": "oonimkall-test", - "software_version": "0.1.0" - }, - "state_dir": "../testdata/oonimkall/state", - "version": 1 - }`) - if err != nil { - t.Fatal(err) - } - var seen bool - for !task.IsDone() { - eventstr := task.WaitForNextEvent() - var event eventlike - if err := json.Unmarshal([]byte(eventstr), &event); err != nil { - t.Fatal(err) - } - if event.Key == "failure.startup" { - if strings.Contains(eventstr, "AssetsDir is empty") { - seen = true - } - } - } - if !seen { - t.Fatal("did not see failure.startup") - } -} - func TestUnknownExperiment(t *testing.T) { task, err := oonimkall.StartTask(`{ "assets_dir": "../testdata/oonimkall/assets",