Add netemx tests for webconnectivitylte #1185

kelmenhorst · 2023-07-18T14:00:43Z

Checklist

I have read the contribution guidelines
reference issue for this pull request: engine: rewrite most important experiment tests using netem probe#2461

Description

This diff implements netemx testing for webconnectivitylte by doing the following high-level things:

Move most code in internal/cmd/oohelperd to internal/oohelperd such that this library can be used by our tests.
Refactor netxlite w.r.t. the underlying network (this change enables us to run multiple stacks in the QAEnv)
- stdlib dialers/resolvers/handshakers now optionally own an instance of model.UnderlyingNetwork that, if set, is used instead of tproxySingleton()
- internal/netxlite/net.go now contains the high-level struct Net which wraps an UnderlyingNetwork. Its methods are constructors that mimic constructors in netxlite, e.g. NewDialerWithResolver. These constructor methods return dialers/resolvers/handshakers which use this Net's UnderlyingNetwork.
Add a success case and DPI case as integration tests for webconnectivitylte. Note that we can now run both the probe as well as the testhelper as separate stacks in the testing environment.

…locking handling

… network

…to add handler after environment creation

… webconnectivitylte fields

This diff has been extracted from #1185 and modifies netxlite/filtering to export the DNSComposeResponse function. The reference issue is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

bassosimone · 2023-08-24T10:53:42Z

internal/netemx/qaenv.go

- ipAddr := net.ParseIP(addr)
- runtimex.Assert(ipAddr != nil, "invalid IP addr")
+ handler := factory.NewHandler(stack)
+ closables = env.serverListen(stack, handler, addr)


I think here we should append to the closables rather than assigning.

This diff factors the function used for listening out of the loop where we create all the servers stacks. The same diff is part of the original PR that introduced these changes: #1185. The intent of pushing this commit is to make sure the diff we're merging next is easier to understand because there's no refactoring noise. Reference issue: ooni/probe#2461.

This diff factors the function used for listening out of the loop where we create all the servers stacks. The same diff is part of the original PR that introduced these changes: #1185. The intent of pushing this commit is to make sure the diff we're merging next is easier to understand because there's no refactoring noise. Reference issue: ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

The oohelperd `http.Handler` constructor needs a reference to a `model.UnderlyingNetwork` that will be used for fetching websites and verifying that they're accessible. This diff changes how `netemx` works such that we can construct an `http.Handler` that depends on a `model.UnderlyingNetwork`, thus enabling this use case. This diff has been adapted from #1185. The reference issue is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff implements the subset of the OONI API required for testing Web Connectivity LTE using netemx. Diff adapted from: #1185. Reference issue: ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff extends netemx to add support for simulating the oohelperd using netem. We extracted this diff from #1185. The reference issue is ooni/probe#2461. We're now well positioned to write netemx-based tests for Web Connectivity 🥳 🥳 🥳 🥳 ! --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff introduces code to emulate a large-scale scenario with several servers. This scenario is suitable for writing Web Connectivity integration tests because it includes a test helper, the OONI API, a GeoIP service, etc. This work derives from #1185 and is part of ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

bassosimone · 2023-08-24T20:30:34Z

I have split this PR in several smaller PRs and integrated the code in master.

This diff has been extracted from ooni#1185 and modifies netxlite/filtering to export the DNSComposeResponse function. The reference issue is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

Code in netxlite is currently tightly coupled with the tproxy singleton (i.e., with a `model.UnderlyingNetwork`). Unfortunately, to write netemx tests for Web Connectivity v0.5, we need to break this coupling. In fact, we need the test helper's code to use a different underlying network that uses a different IP address and is not censored, unlike the network used by the probe. To this end, in this diff we're introducing a nil-safe provider for a tproxy. When the provider struct pointer is nil or the tproxy field is nil, the code behaves as before and uses the tproxy singleton. Otherwise, the code will use the given tproxy. Then we modify fundamental types to explicitly depend on this nil-safe provider rather than implicitly depending on the tproxy singleton. We're soon going to take advantage of this diff and introduce support for constructing common netxlite types using a custom tproxy that is not a singleton but rather is a field of a concrete "net" structure. This work has been extracted and adapted from ooni#1185. The reference issue for this pull request is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

) This diff introduces the netxlite.Netx type. This type depends on a model.UnderlyingNetwork. When the underlying network is nil, using this type is equivalent to using netxlite toplevel functions directly. To simplify testing this diff, let's move the netem->netxlite UnderlyingNetworkAdapter from netemx and let's use netem to write tests, so we can have confidence specifically about the most common use case of overriding the underlying network. Originally part of ooni#1185. The reference issue is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

The ooni#1185 PR originally defined this server in the `internal/netemx` package, but I chose to move this functionality into the much-lower-level `testingx` package. The rationale for moving this functionality is that a DNS-over-HTTPS server is useful in several test cases, so it should be as general as possible. This work is part of ooni/probe#2461 --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff implements the Ubuntu GeoIP resolver service inside of the testingx package. Original code: ooni#1185. Reference issue: ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff factors the function used for listening out of the loop where we create all the servers stacks. The same diff is part of the original PR that introduced these changes: ooni#1185. The intent of pushing this commit is to make sure the diff we're merging next is easier to understand because there's no refactoring noise. Reference issue: ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

The oohelperd `http.Handler` constructor needs a reference to a `model.UnderlyingNetwork` that will be used for fetching websites and verifying that they're accessible. This diff changes how `netemx` works such that we can construct an `http.Handler` that depends on a `model.UnderlyingNetwork`, thus enabling this use case. This diff has been adapted from ooni#1185. The reference issue is ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff implements the subset of the OONI API required for testing Web Connectivity LTE using netemx. Diff adapted from: ooni#1185. Reference issue: ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff extends netemx to add support for simulating the oohelperd using netem. We extracted this diff from ooni#1185. The reference issue is ooni/probe#2461. We're now well positioned to write netemx-based tests for Web Connectivity 🥳 🥳 🥳 🥳 ! --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

This diff introduces code to emulate a large-scale scenario with several servers. This scenario is suitable for writing Web Connectivity integration tests because it includes a test helper, the OONI API, a GeoIP service, etc. This work derives from ooni#1185 and is part of ooni/probe#2461. --------- Co-authored-by: kelmenhorst <k.elmenhorst@mailbox.org>

kelmenhorst added 14 commits July 7, 2023 10:06

(chore): export webconnectivitylte fields to use them in test package

252dc80

move cmd/oohelperd lib code to internal/oohelperd

c8c91f5

filtering: expose DNSComposeResponse

6cbe601

feat: webconnectivitylte success test using netemx

65dc454

renamed webconnectivitylte test

daeea0d

split webconnectivitylte tests

4e53b8f

webconnectivitylte: adapt measurer_test to new netemx QAEnv

7f46267

webconnectivity: add (failing) DPITarget test

8c608e2

feat: add optional underlying network to netxlite stdlib structs

24401f0

webconnectivitylte: analysiscore: comment for legacy analysisFlagTLSB…

457117e

…locking handling

feat(netxlite): Net: constructors for operations on custom underlying…

0f26164

… network

feat(qaenv): make the environment's server stacks accessible, option …

a65d6a5

…to add handler after environment creation

feat(adapter): wrap given netem.UnderlyingNetwork

24439d6

test(webconnectivitylte): use testhelper on concurrent server stack

c5e6740

kelmenhorst self-assigned this Jul 18, 2023

kelmenhorst requested review from bassosimone and hellais as code owners July 18, 2023 14:00

kelmenhorst and others added 10 commits July 21, 2023 10:27

Merge remote-tracking branch 'origin/master' into test-webconnectivity

3fe3dd0

fix: add QAEnv.serverStacks

ca42b77

make underlying network mandatory for system dialers/resolver/handshaker

66cff41

remove package webconnectivitylte: use mocks.Session, revert exposing…

617a529

… webconnectivitylte fields

netemx geoip: change mock geoip service's response to public server

a4f9e92

QAEnvHTTPHandlerFactory: change construction pattern for http.Handlers

16afba7

Merge remote-tracking branch 'origin/master' into test-webconnectivity

c383749

sync

05648fe

Merge branch 'master' into test-webconnectivity

281850b

x

b02ecc3

This was referenced Aug 23, 2023

netxlite/filtering: export DNSComposeResponse #1195

Merged

webconnectivity: write A/B testing tool using netem ooni/probe#2525

Closed

bassosimone reviewed Aug 24, 2023

View reviewed changes

bassosimone mentioned this pull request Aug 24, 2023

refactor(netemx): factor function for listening #1200

Merged

bassosimone mentioned this pull request Aug 24, 2023

refactor(netemx): allow running the oohelperd using netem #1201

Merged

bassosimone added 3 commits August 24, 2023 13:37

Merge branch 'master' into test-webconnectivity

c48f1fe

Merge remote-tracking branch 'origin/master' into test-webconnectivity

bf4443c

x

697aa89

bassosimone mentioned this pull request Aug 24, 2023

feat(netemx): implement subset of the OONI API #1203

Merged

Merge branch 'master' into test-webconnectivity

8d05817

bassosimone mentioned this pull request Aug 24, 2023

feat(netemx): support simulating the oohelperd #1204

Merged

Merge branch 'master' into test-webconnectivity

e740cb6

bassosimone mentioned this pull request Aug 24, 2023

feat(netemx): create "internet" netem-based emulation #1205

Merged

Merge branch 'master' into test-webconnectivity

6ed061d

bassosimone closed this Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add netemx tests for webconnectivitylte #1185

Add netemx tests for webconnectivitylte #1185

kelmenhorst commented Jul 18, 2023

bassosimone Aug 24, 2023

bassosimone commented Aug 24, 2023

Add netemx tests for webconnectivitylte #1185

Add netemx tests for webconnectivitylte #1185

Conversation

kelmenhorst commented Jul 18, 2023

Checklist

Description

bassosimone Aug 24, 2023

Choose a reason for hiding this comment

bassosimone commented Aug 24, 2023