-
Notifications
You must be signed in to change notification settings - Fork 312
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
Fix the bug #6022 #6075
Fix the bug #6022 #6075
Conversation
Signed-off-by: Anton Litvinov <[email protected]>
Signed-off-by: Anton Litvinov <[email protected]>
Signed-off-by: Anton Litvinov <[email protected]>
Signed-off-by: Anton Litvinov <[email protected]>
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #6075 +/- ##
==========================================
- Coverage 37.92% 28.47% -9.45%
==========================================
Files 371 520 +149
Lines 20715 29987 +9272
==========================================
+ Hits 7856 8540 +684
- Misses 12075 20624 +8549
- Partials 784 823 +39 ☔ View full report in Codecov by Sentry. |
ok = "failed" | ||
} | ||
log.Println("Test result:", ok) | ||
// dev.Down() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be cleaned.
}, | ||
} | ||
|
||
resp, err := client.Get("http://107.173.23.19:8080/test") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a part of the local environment, we cannot depend on external service in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came to a conclusion that a local address can not be used (for the endpoint), it must be an external address,
b/c we test the path: wireguard client->wireguard server->gvisor tcp/ip stack->external web server.
This test turnes out to be an integrational test by its nature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will try to transform into a integrational test and run using docker compose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@soffokl
fixed!
|
||
func startServer(t *testing.T, privKey, pubClinet wgtypes.Key) { | ||
tun, _, _, err := CreateNetTUNWithStack( | ||
[]netip.Addr{netip.MustParseAddr("192.168.4.1")}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a static IP, or it can be anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
192.168.4.1 - wireguard "server" in gvisor net
192.168.4.100 - wireguard "client"
They can be , but must have a common subnet, of course
878347a
to
97f3db6
Compare
Signed-off-by: Anton Litvinov <[email protected]>
Fixes #6022