Skip to content

refactor: expose public API #70

Merged
ainghazal merged 3 commits intoooni:mainfrom
ainghazal:feat/public-tun
Mar 6, 2024
Merged

refactor: expose public API #70
ainghazal merged 3 commits intoooni:mainfrom
ainghazal:feat/public-tun

Conversation

@ainghazal
Copy link
Copy Markdown
Collaborator

@ainghazal ainghazal commented Feb 21, 2024 •
edited
Loading

Expose config and tunnel as a public API.

This refactor tries to expose all the useful functions to instantiate an OpenVPN tunnel (paving the way to usage from probe-cli).

Checklist

  • I have read the contribution guidelines
  • Iff you changed code related to services, or inter-service communication, make sure you update the diagrams in ARCHITECTURE.md.
  • Reference issue for this pull request:

@ainghazal
Copy link
Copy Markdown
Collaborator Author

ainghazal commented Feb 21, 2024

Deferring review until we merge #68 (which is included in this branch right now).

@ainghazal ainghazal mentioned this pull request Feb 21, 2024
Merged
3 tasks
@ainghazal ainghazal self-assigned this Mar 6, 2024
@ainghazal ainghazal requested a review from hellais March 6, 2024 00:41
hellais
hellais approved these changes Mar 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@hellais hellais left a comment

Choose a reason for hiding this comment

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

This looks good to me.

From an inspection it looks like the only thing that's really changing are the import paths for model and tun, which are moved out of the internal package.

In light of this some code is also moved around a little bit, where the most noticeable difference is the tunnel.Start method which takes the code which used to live inside of main.

The only question I have is why the logger_test and tracer_test were removed? Is it because this new layout makes testing of those packages harder?

All in all, this looks good to merge for me!

@ainghazal
Copy link
Copy Markdown
Collaborator Author

ainghazal commented Mar 6, 2024

The only question I have is why the logger_test and tracer_test were removed? Is it because this new layout makes testing of those packages harder?

it was a bit of a cleanup:

  • logger_test was providing a test logger for other tests to use; this was moved to model.mocks.TestLogger
  • similarly, tracer_test provided only a no-op implementation of model.HandshakeTracer; I decided that I can pass model.DummyTracer to all tests that need to receive a tracer.

@ainghazal ainghazal merged commit 41e4e0b into ooni:main Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hellais hellais hellais approved these changes

Assignees

@ainghazal ainghazal

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ainghazal @hellais