test(connlib): simulate IO in state machine tests #4728
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Terraform Cloud Plan Output |
Performance Test ResultsTCP
UDP
|
thomaseizinger
force-pushed
the
chore/connlib/tests-send-transmit
branch
from
19e7358
to
846b05c
Compare
thomaseizinger
force-pushed
the
chore/connlib/tests-send-transmit
branch
from
1beedd2
to
abf84f0
Compare
thomaseizinger
changed the title
github-actions
bot
added
the
kind/test
thomaseizinger
changed the title
This abstraction is no longer needed after the refactoring of `progress`.
…nough in our loop as part of the transition
thomaseizinger
force-pushed
the
chore/connlib/tests-send-transmit
branch
from
ea0b736
to
1d9760f
Compare
| @@ -5871,8 +5862,8 @@ checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" | |||
|
|
|||
| [[package]] | |||
| name = "str0m" | |||
| version = "0.5.0" | |||
| source = "git+https://github.com/firezone/str0m?branch=main#44b616945145033ade739f30cdce004728011619" | |||
There was a problem hiding this comment.
This PR updates str0m in to include algesten/str0m#515. Because main of str0m also includes algesten/str0m#508 which caused problems (#4826) when we integrated it into firezone, I've reverted that PR in our fork in firezone/str0m@07c4fb0.
Thus, our current diff to upsteam str0m looks like this: algesten/str0m@main...firezone:str0m:main
rust/connlib/tunnel/src/client.rs
Outdated
There was a problem hiding this comment.
In here, we update the APIs of ClientState to be easier to call from the tests.
rust/connlib/tunnel/src/gateway.rs
Outdated
There was a problem hiding this comment.
Same thing for the gateway. These updates make it easier / possible to call the necessary APIs from the proptest.
| } | ||
| Transition::SendICMPPacketToIp4Resource { src, r_idx } => { | ||
| Transition::SendICMPPacketToIp4Resource { r_idx } => { |
There was a problem hiding this comment.
Something good to add in a later PR would be sending packets the other way, to test the encapsulate on the gateway-side and decapsulate on the client-side
There was a problem hiding this comment.
I'll record it as a follow-up issue!
| let weight_ip4 = if num_ip4_resources == 0 { 0 } else { 3 }; | ||
| let weight_ip6 = if num_ip6_resources == 0 { 0 } else { 3 }; |
There was a problem hiding this comment.
we use 3 here to represent that it's more likely to have packets flowing than resources changing?
There was a problem hiding this comment.
And more likely to generate packets for resources than non-resources.
|
@ReactorScram I am merging based on @conectado 's review and to move my stacked PRs forward. I'll implement any feedback in follow-up PRs! |
| fn preconditions(state: &Self::State, transition: &Self::Transition) -> bool { | ||
| match transition { | ||
| Transition::AddCidrResource(r) => { | ||
| // TODO: PRODUCTION CODE DOES NOT HANDLE THIS! |
There was a problem hiding this comment.
Production does not handle a gateway getting a resource that it doesn't have connectivity for?
| } | ||
|
|
||
| /// Implementation of our reference state machine. | ||
| /// Stub implementation of the portal. |
There was a problem hiding this comment.
It would be really cool if I could use all this to test the Windows Clients in CI without having access to Docker / Elixir / a staging portal. but the SimPortal couldn't hook up to a real Client tunnel yet, right?
I think we had it as a long-term goal but it would take a while?
There was a problem hiding this comment.
These tests are running in Windows CI already! But they don't do any platform-specific stuff so unless the Rust compiler mis-compiles some of the state machine stuff, I'd be surprised to find a windows-specific bug with these tests.
Or what exactly were you thinking of running in CI here?
There was a problem hiding this comment.
I mean I'd like to boot up the Client, open the GUI stuff, open wintun and the virtual network interface, and exercise all the platform-specific code with only a fake Portal and fake Gateway, since those don't run easily in Windows and are hard to hook into the runners.
Like sort of tell connlib "I know there is no portal but just connect everything anyway"
This implements feedback from @ReactorScram on #4728.
This is similar to #4097 and #4585 but for the entire
ClientStateandGatewayState. We also do it in the context of a property-based test with the vision that we can deterministically explore a large space of state transitions and see where our main property breaks: Being able to send an ICMP packet from the client to the gateway.In other words, we now correctly pass all the
Transmits back and forth between the components as if they would receive it from the network. Due to the nature of property-based tests, this already exercises a very large input space. For example, if the client does not have an IPv6 socket and the gateway doesn't have an IPv4 socket, this test already checks whether we then correctly fall back to using a relay (because the allocation we make on the relay is the only network path where the STUN requests pass through).What this does not (yet) do is set up a proper network topology. The
dispatch_transmitfunction will happily "route" aTransmitfrom e.g. the client to the gateway even if they are in different subnets. In other words, these tests assume that the actual network itself works and we can exchange UDP packets between the components.For now, we only send ICMPs to CIDR resources. As a next step, we can extend this to DNS resources by sending DNS queries for our DNS resources and then sending an ICMP to the resolved IP.