refactor(firezone-tunnel): move routes and DNS control out of connlib and up to the Client #5111
Conversation
…zone into refactor/debug-ipc-service
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Terraform Cloud Plan Output |
…o refactor/move-linux-dns-control
ReactorScram
changed the base branch from
refactor/debug-ipc-service
to
refactor/dedupe-ipc-clients
Or, if you construct |
| let mut signals = Signals::new()?; | ||
|
|
||
| loop { | ||
| match future::select(pin!(signals.recv()), pin!(on_disconnect_rx.recv())).await { | ||
| match future::select(pin!(signals.recv()), pin!(cb_rx.recv())).await { |
There was a problem hiding this comment.
is pin needed here? aren't these Unpin?
There was a problem hiding this comment.
🤷♀️ If I try to remove either of those pin!() macros it won't compile.
I don't know how pinning works so I could be overlooking something.
We could just construct the |
| pub struct InterfaceManager { | ||
| // This gets lazy-initialized when the interface is first configured | ||
| connection: Option<Connection>, | ||
| dns_control_method: Option<DnsControlMethod>, |
There was a problem hiding this comment.
Do we have to keep DNS in here? How we control DNS doesn't really have anything to do with the TUN device.
There was a problem hiding this comment.
I was using it for RAII but yeah looks like that can be split out into its own struct, so I will
| connlib | ||
| .set_dns(client::resolvers::get().unwrap_or_default()) | ||
| .await?; |
There was a problem hiding this comment.
We can delete this because we will just do it on the first tick of the timer?
There was a problem hiding this comment.
Maybe. I don't think I have a test to prove that yet, so I'll leave it. If DNS control is up in the Client now, maybe I can try to enforce in the Client that the "Firezone connected" notification or the systemd ready notification only fires after we definitely control DNS. Maybe in another PR
rust/connlib/tunnel/src/gateway.rs
Outdated
There was a problem hiding this comment.
Nice work!
Move 👏 more 👏 config 👏 to 👏 the 👏 clients 👏
Refs #3636 (This pays down some of the technical debt from Linux DNS)
Refs #4473 (This partially fulfills it)
Refs #5068 (This is needed to make
FIREZONE_DNS_CONTROLmandatory)As of dd6421:
on_set_interface_config) both move to the Clienttun_windows.rs. Route setting in Windows requires us to know the interface index, which we don't know in the Client code. If we could pass opaque platform-specific data between the tunnel and the Client it would be easy.workertask intun_linux.rsBefore merging / notes
main? I don't see where it hooks up. I think I only set up DNS inTun::new(Yes, theTungets recreated every time we reconfigure the device)FIREZONE_DNS_CONTROLmandatory #5068)Add DNS control test(failed)