* feat: add support for system resolver in measurexlite
* more tests for coverage
* Apply suggestions from code review
Co-authored-by: decfox <decfox@github.com>
Co-authored-by: Simone Basso <bassosimone@gmail.com>
Until OONI Run v2 has support for repeating the measurement with a schedule, introduce a command line flag requested by users to repeat a measurement every given number of seconds.
Part of https://github.com/ooni/probe/issues/2184
This diff adds support for running OONIRun v1 links.
Run with `miniooni` using:
```
./miniooni -i LINK oonirun
```
Part of https://github.com/ooni/probe/issues/2184
This diff refactors the ./internal/cmd/miniooni pkg and moves the code
for running experiments inside of the ./internal/oonirun pkg.
It's the first concrete step towards https://github.com/ooni/probe/issues/2184.
The integration test that was broken was:
```
--- FAIL: TestCreateInvalidExperiment (0.35s)
experiment_integration_test.go:192: expected a nil builder here
```
While there improve the documentation of the ExperimentSession
and see there's a method that we are not using.
This diff is a cleanup that I come up with while working
on https://github.com/ooni/probe/issues/2184.
This option has been disabled for a long time and we said in the
codebase we were going to remove it after 2021-11-01.
So, it feels okay to remove it.
This diff is a cleanup in preparation for https://github.com/ooni/probe/issues/2184.
This diff modifies the engine package to make Experiment and
ExperimentBuilder interfaces rather than structs.
The previosuly existing structs are now named experiment{,Builder}.
This diff helps https://github.com/ooni/probe/issues/2184
because it allows us to write unit tests more easily.
There should be no functional change.
While there, I removed a bunch of deprecated functions, which were
unnecessarily complicate the implementation and could be easily
replaced by passing them a context.Context or context.Background().
This diff refactors how we set options for experiments to accept
in input an any value or a map[string]any, depending on which method
we choose to actually set options.
There should be no functional change, except that now we're not
guessing the type and then attempting to set the value of the selected
field: now, instead, we match the provided type and the field's type
as part of the same function (i.e., SetOptionAny).
This diff is functional to https://github.com/ooni/probe/issues/2184,
because it will allow us to load options from a map[string]any,
which will be part of the OONI Run v2 JSON descriptor.
If we didn't apply this change, we would only have been to set options
from a map[string]string, which is good enough as a solution for the
CLI but is definitely clumsy when you have to write stuff like:
```JSON
{
"options": {
"HTTP3Enabled": "true"
}
}
```
when you could instead more naturally write:
```JSON
{
"options": {
"HTTP3Enabled": true
}
}
```
This diff makes the implementation of the engine package more
abstract by changing HTTPClient() to return a model.HTTPClient
as opposed to returning an *http.Client.
Part of https://github.com/ooni/probe/issues/2184
In https://github.com/ooni/probe-cli/pull/832's initial diff, I
mentioned it would be cool to flatten oohelperd's hier.
I'm doing this now, and just for the master branch.
This diff is mostly a mechanical refactoring with very light
and apparently rather safe manual changes.
This diff modifies the implementation of oohelperd in the master branch
to always use throw-away HTTPClient, Dialer, and Resolver.
The rationale of this change is to ensure we're not hitting limits of the
HTTPClient regarding the max number of connections per host.
This issue is described at https://github.com/ooni/probe/issues/2182.
While there, it feels more correct to use throw-away Dialer and Resolver.
We have a different patch for the release/3.15 branch because of
netx-related refactorings: https://github.com/ooni/probe-cli/pull/832.
## Checklist
- [x] I have read the [contribution guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request: https://github.com/ooni/probe/issues/2158
- [x] if you changed anything related how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request: https://github.com/ooni/spec/pull/250
## Description
This diff refactors the codebase to reimplement tlsping and tcpping
to use the step-by-step measurements style.
See docs/design/dd-003-step-by-step.md for more information on the
step-by-step measurement style.
I am not 100% sure I was able to fix all the cases in which we
need higher permissions than the strict default.
At least, I tried.
It may be reasonable to make an interim release to check whether I
successfully fixed all the cases.
Ref issue: https://github.com/ooni/probe/issues/2154
This bug is one of these bugs that definitely help one to stay
humble and focused on improving the codebase.
Of course I `<facepalmed>` when I understood the root cause.
We did not move the annotations below the `if` which is checking
whether the measurement was successful when we refactored the
codebase to support returning multiple measurements per run, which
happened in https://github.com/ooni/probe-cli/pull/527.
While I am not going to whip myself too much because of this, it's
clearly a bummer that we didn't notice this bug back then. On top
of this, it's also quite sad it took us so much time to notice that
there was this bug inside the tree.
The lesson (hopefully) learned is probably that we need to be more
careful when we refactor and we should always ask the question of
whether, not only we have tests, but whether these tests could maybe
be improved to give us even more confidence about correctness.
The reference issue is https://github.com/ooni/probe/issues/2173.
This pull request publishes the step-by-step design document that I have been discussing with @hellais and @DecFox recently. Compared to the document that was approved, this one has been edited for readability.
While there, I figured it was also be beneficial to publish the few ooni/probe-cli related design documents we produced in the past, because they probably help someone to get acquainted with the codebase.
Reference issue for this pull request: https://github.com/ooni/probe/issues/2148
This diff addresses the following points of https://github.com/ooni/probe/issues/2135:
- [x] the `childResolver` type is useless and we can use `model.Resolver` directly;
- [x] we should use `model/mocks` instead of custom fakes;
- [x] we should not use `log.Log` rather we should use `model.DiscardLogger`;
- [x] make `timeLimitedLookup` easier to test with a `-short` tests;
- [x] ensure `timeLimitedLookup` returns as soon as its context expires regardless of the child resolver;
Subsequent diffs will address more points mentioned in there.
The oohelperd implementation did not actually need using netx because
it was just constructing default types with logging, which is what
netxlite already does. Hence, let's avoid using netx here.
See https://github.com/ooni/probe/issues/2121
The oohelper does not need to use netx and it's enough to use
netxlite, hence let us apply this refactor.
The original code used DoT but the explanatory comment said we were
using DoT because of unclear issues inside GitHub actions.
We are now using DoH and this is fine as well. The comment implied
that any encrypted transport would do.
See https://github.com/ooni/probe/issues/2121
This diff forward ports 261d1a4cdc88522f6a8f63d6c540f51054566b28 to master
whose original commit message follows:
- - -
It's not working for me from a couple of places and also it does not
seem to be documented upstream, see:
https://docs.namebase.io/guides-1/resolving-handshake-1/hdns.io
This diff WILL need to be forwardported to master.
This diff refactors netx and netxlite to ensure we're not using
netxlite legacy names inside of netx.
To this end, we're cheating a bit. We're exposing a new factory to
get an unwrapped stdlib resolver rather than defining a legacy name
to export the private name of the same factory.
This is actually a fine place to stop, for now, the next and
netxlite refactoring at https://github.com/ooni/probe/issues/2121.
Before finishing the ongoing refactoring and leaving whatever
is left of netx in tree, I would like to restructure it so that
we'll have an easy time next time we need to modify it.
Currently, every functionality lives into the `netx.go` file and
we have a support file called `httptransport.go`.
I would like to reorganize by topic, instead. This would allow
future me to more easily perform topic-specific changes.
While there, improve `netx`'s documentation and duplicate some of
this documentation inside `internal/README.md` to provide pointers
to previous documentation, historical context, and some help to
understand the logic architecture of network extensions (aka `netx`).
Part of https://github.com/ooni/probe-cli/pull/396
Now that we have properly refactored the caching resolvers we can
move them into netxlite as optional resolvers created using the
proper abstract factories we just added.
This diff reduces the complexity and the code size of netx.
See https://github.com/ooni/probe/issues/2121.
For testability, replace most if-based construction logic with
calls to well-tested factories living in other packages.
While there, acknowledge that a bunch of types could now be private
and make them private, modifying the code to call the public
factories allowing to construct said types instead.
Part of https://github.com/ooni/probe/issues/2121
This diff modifies netx to stop using most netxlite resolver internals
but the internal function that creates a new, unwrapped system resolver,
which will be dealt with in a subsequent pull request.
See https://github.com/ooni/probe/issues/2121
1. Use the netxlite.NewHTTPTransport factory for creating a new
HTTP2 (and HTTP1) transport;
2. Recognize the netxlite.NewOOHTTPTransport has now become
an implementation detail so make it private;
3. Recognize that netxlite.NewHTTP3Transport should call
netxlite.WrapTransport so it returns the same typechain
returned by netxlite.NewHTTPTransport (modulo, of course,
the real underlying transport), so ensure that we are
calling netxlite.WrapTransport in NewHTTP3Transport;
4. Recognize that the table based constructor inside of
netx needs a logger to create HTTPTransport instances using
either netxlite.NewHTTP{,3}Transport so pass this argument
along and ensure it's not nil using a constructor inside
model that guarantees that;
5. Cleanup netx's tests to avoid type asserting on the
typechains returned by netxlite since we already test
that inside netxlite;
6. Recognize that now we can make more legacy names inside
of netxlite private because we don't need to use them
inside tests anymore (because of previous point).
Reference issue: https://github.com/ooni/probe/issues/2121
