-
Notifications
You must be signed in to change notification settings - Fork 712
Change default for GuestIPMustBeZero when GuestIP is 0.0.0.0 #4221
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
Conversation
63eac3c to
1eb9e5e
Compare
It now defaults to `true`. The user must explicitly set it to false to match any interface and not just 0.0.0.0. The existing tests have been amended to explicitly set GuestIPMustBeZero to `false` to continue testing the old behaviour. This change is not backwards compatible! Signed-off-by: Jan Dubois <jan.dubois@suse.com>
1eb9e5e to
4d04ff3
Compare
|
When the guestIP is not specified, the bind to the loopback or unspecified can be accepted, but the binding to the other is rejected. Is it better to make it clear in the test? Is it like this when they are covered? $ git diff
diff --git a/hack/test-port-forwarding.pl b/hack/test-port-forwarding.pl
index bbd9eab2..4c586dbf 100755
--- a/hack/test-port-forwarding.pl
+++ b/hack/test-port-forwarding.pl
@@ -347,14 +347,41 @@ portForwards:
# forward: :: 3032 → ipv4 2032
# forward: ::1 3033 → ipv4 2033
-- guestPortRange: [300, 309]
+- guestPortRange: [300, 304]
- # forward: 127.0.0.1 300 → 127.0.0.1 300
+ # forward: 127.0.0.1 300 → 127.0.0.1 300
+ # forward: 0.0.0.0 301 → 127.0.0.1 301
+ # forward: :: 302 → 127.0.0.1 302
+ # forward: ::1 303 → 127.0.0.1 303
+ # ignore: 192.168.5.15 304 → 127.0.0.1 304
-- guestPortRange: [310, 319]
+- guestPortRange: [305, 309]
+ guestIPMustBeZero: false
+
+ # forward: 127.0.0.1 325 → 127.0.0.1 325
+ # forward: 0.0.0.0 326 → 127.0.0.1 326
+ # forward: :: 327 → 127.0.0.1 327
+ # forward: ::1 328 → 127.0.0.1 328
+ # ignore: 192.168.5.15 329 → 127.0.0.1 329
+
+- guestPortRange: [310, 314]
+ hostIP: 0.0.0.0
+
+ # forward: 127.0.0.1 310 → 0.0.0.0 310
+ # forward: 0.0.0.0 311 → 0.0.0.0 311
+ # forward: :: 312 → 0.0.0.0 312
+ # forward: ::1 313 → 0.0.0.0 313
+ # ignore: 192.168.5.15 314 → 0.0.0.0 314
+
+- guestPortRange: [315, 319]
+ guestIPMustBeZero: false
hostIP: 0.0.0.0
- # forward: 127.0.0.1 310 → 0.0.0.0 310
+ # forward: 127.0.0.1 315 → 0.0.0.0 315
+ # forward: 0.0.0.0 316 → 0.0.0.0 316
+ # forward: :: 317 → 0.0.0.0 317
+ # forward: ::1 318 → 0.0.0.0 318
+ # ignore: 192.168.5.15 319 → 0.0.0.0 319
# Things we can't test:
# - Accessing a forward from a different interface (e.g. connect to ipv4 to connect to 0.0.0.0)
@@ -419,4 +446,21 @@ portForwards:
- guestPort: 5000
hostSocket: port5000.sock
- # forward: 127.0.0.1 5000 → sockDir/port5000.sock
+ # forward: 127.0.0.1 5000 → sockDir/port5000.sock
+
+- guestPort: 5001
+ hostSocket: port5001.sock
+
+ # ignore: 192.168.5.15 5001 → sockDir/port5001.sock
+
+- guestPort: 5002
+ guestIPMustBeZero: false
+ hostSocket: port5002.sock
+
+ # forward: 127.0.0.1 5002 → sockDir/port5002.sock
+
+- guestPort: 5003
+ guestIPMustBeZero: false
+ hostSocket: port5003.sock
+
+ # ignore: 192.168.5.15 5003 → sockDir/port5003.sockMaybe each intention should be written in some comments. 🤔 It may be necessary to make the rule parser possible for unit testing. |
Suggested in lima-vm#4221 (comment) Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Yes, they all pass. I've added them to the PR, although it gets a bit tedious to manually verify each one of them. One day we should convert all of them into BATS tests (but not today). |
Suggested in lima-vm#4221 (comment) Signed-off-by: Jan Dubois <jan.dubois@suse.com>
8f9976d to
bdd8ed5
Compare
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.
LGTM! 👍🏻
It now defaults to
true. The user must explicitly set it to false to match any interface and not just 0.0.0.0.This change is not backwards compatible!
Closes #4193