Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add fallback domain names for openvpn experiment #1654

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ainghazal
Copy link
Contributor

while working on this, I also gave more priority to possible oonirun descriptors passed in the command line.

  • Related: #2805

Checklist

  • I have read the contribution guidelines
  • reference issue for this pull request: add fallback domains for openvpn test probe#2805
  • if you changed anything related to how experiments work and you need to reflect these changes in the ooni/spec repository, please link to the related ooni/spec pull request:
  • if you changed code inside an experiment, make sure you bump its version number

Description

Add fallback domains for openvpn default endpoints to be probed.

while working on this, I also gave more priority to possible oonirun
descriptors passed in the command line.

- Related: #2805
@ainghazal ainghazal changed the title feat: add fallback domain names feat: add fallback domain names for openvpn experiment Oct 8, 2024
@ainghazal
Copy link
Contributor Author

changed the logic after 1:1 with @hellais, ready for review

}
return defaultOONITargetURL(ip) + "&transport=tcp", nil
return nil, fmt.Errorf("cannot find any usable endpoint")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this result in a hard failure of the test? Is there some way we can ensure that's not the case? For example we might want for certain runs in particular countries to not run the vpn tests and it would be ideal if that results in a noop as opposed to the test being considered a failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this function returns error, we simply return an empty targets list, see:

if inputs, err := pickOONIOpenVPNTargets(addrs); err == nil {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the comment below

@ainghazal ainghazal force-pushed the feat/add-alternative-endpoints branch from e5411af to 78ac661 Compare October 24, 2024 18:27
@ainghazal
Copy link
Contributor Author

ainghazal commented Oct 24, 2024

To clarify, I've added a few more comments to the docstring of the Load() function. An error is ignored (logged now); returning the empty list (in case of all bogons) is equivalent of passing an empty input list to the experiment.

@hellais: here I've pointed both domains to localhost in /etc/hosts, see:

[      0.328261] <info> Looking up your location; please be patient...
[      0.328277] <info> iplookup: using cloudflare
[      0.328407] <info> sessionresolver: lookup www.cloudflare.com using https://wikimedia-dns.org/dns-query... started
[      0.376407] <info> sessionresolver: lookup www.cloudflare.com using https://wikimedia-dns.org/dns-query... ok
[      0.485202] <info> - country: NO
[      0.485209] <info> - network: FOOBAR (AS0)
[      0.485213] <info> - resolver's IP: 172.253.5.153
[      0.485216] <info> - resolver's network: Google LLC (AS15169)
[      0.485338] <warn> Picking from default OpenVPN endpoints
[      0.485343] <warn> No targets loaded from default endpoints
[      0.485351] <info> experiment: recv   0.00  byte, sent   0.00  byte
[      0.485519] <info> whole session: recv   3.86 kbyte, sent   1.46 kbyte

To trace the execution path, and reason about it, I think the right place to go is the the call to newInputProcessor, which passes a targetList (the targetList this experiment provides, in this case).

In turn, inputprocessor.Run calls inputprocessor.run, which loops over the array of inputs. I think this ensures that an empty array runs the experiment zero times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants