-
Notifications
You must be signed in to change notification settings - Fork 243
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
Support WSL2 distros accessing private (apiserver, ssh, cockpit) services #3337
Conversation
Hi @GingerGeek. Thanks for your PR. I'm waiting for a code-ready member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @anjannath |
/ok-to-test |
This changes behaviour when WSL2 is installed on a target machine. I would say this should NOT be the default option. Especially your caveat indicates that a problem exists with the hostnames, this can therefore not be considered a reliable solution for regular use. Can this be behind a switch, like |
@gbraad From a "work-outs-the-box" perspective I would suggest this being enabled by default without an additional flag, with potentially an override flag to disable it (although I can't think of why you would need to). Using WSL2 for development, and having all code repos, toolings, etc., within it, is becoming a more typical pattern for developers using Windows as a daily driver. Especially with integrations of VSCode etc. From a usability perspective, I would think a developer trying out OKD would want to be able Although there is a behaviour change, it's not a "visible" one to a user since you can still access all services as you would before from Windows. The change in January (dcae454) was made to ensure apiserver, cockpit and SSH were not exposed publically; this effectively retains that as the WSL2 LAN is private to the machine. The only potential break would be if someone is referencing the cluster directly via 127.0.0.1 within their application or scripts, but given the default configuration of Openshift/OKD (ie routes based off hostname), I think that's unlikely (all docs also refer to hostnames). On the DNS caveat... To clarify this only exists within WSL2. The behaviour from the Windows perspective remains stable. I've been looking into this and trying to figure out when/how WSL2 updates DNS configuration. I perhaps was being too pessimistic in my original PR. WSL2 relies on a service on the Windows machine for DNS, this of course picks up changes instantly from the Windows hosts file. However, the changes won't get reflected in This would mean there is a potential for removed routes' DNS records to linger within WSL2, but I don't think that's a show-stopping issue. |
8c4af78
to
cb43922
Compare
/ok-to-test |
/retest |
We can't change behaviour as people might have functionality that relies on this. This is practice that people use when DNS is not working as expected or using a VPN route-all setup. NOTE: the |
I understand, but this is something that needs additional messaging/feedback or documentation, otherwise we will see people file issues about broken DNS when using this. Does ICS need a restart? Perhaps if you detect WSL to be available (hence the flag to enable so you don't have to detect), you perform a preflight check to see if DNS is operational; |
Not ideal... we are called out for doing this on We remove records using
Perhaps, we can check for impact over time. (non-default for now?) |
Note: flag can be a config option too... as long as this can be 'enabled'/'disabled', though default might no be the right approach now. Perhaps at a later time. We are still working on some issues around WSL2 and Podman before we commit to this. |
@code-ready/crc-team adding peer review required as this needs to be tested with a route-all VPN setup. I am sure this will fail as addresses will be resolved on a non-localhost address, and this will not be possible in that case. |
I've done some more testing on DNS, this seems to be the state of play: For static hosts declaration within the distribution:
For DNS resolution within the distribution:
This leads to the following behaviour under default settings:
If static hosts updating/syncing is disabled, then the resolution will still function within the WSL2 environment as long as the ICS DNS server is still in use. If a user configures custom nameservers within the WSL2 distribution, then DNS resolution for CRC may fail - although I think that's out of scope to support. TL;DR: Will work "out-the-box" unless user has some advanced customisations on the WSL2 network setup. There is sometimes a delay in removing hosts from WSL2 but I would view it as a non-issue. I think a WSL health check would be a good addition, I note currently when I run
I will look to implement some basic health checks |
Right, but this is therefore not much of our responsibility. Good... let's not consider this. We are however looking into #2992, so that should 'resolve' and clean automagically.
We used to rely on a catch-all, but this might be an issue (we use dnsmasq for this). |
On the route-all VPN... I think this will vary from vendor to vendor, however many do now have specific support for the internal WSL2 network (e.g Cisco AnyConnect). At the very least, since the IP we bind to is the machine's own, I believe most VPN clients would allow connections to it (but not the wider network)? This is not really something I have too much experience with. The exception of course is if there is a routing overlap between VPN networks and WSL2 internal network. There are several open issues around this within microsoft/WSL (e.g microsoft/WSL#5782 microsoft/WSL#4210). I found this blog around it as well I've tested this using Tailscale and an exit node-enabled (with local network access disabled). It's working fine from both the WSL2 side and Windows side. I can probably also have a test done with an OpenVPN setup but I don't have easy access to the more corporate-y VPNs like AnyConnect etc |
General todo/issues/thoughts:
|
So is the consensus I should pull this out to be behind a config option? Something along the lines of
I would then suggest
|
i've rebased the PR branch and made some modifications to the way the WSL vswitch ip is fetched here: anjannath@bedcc6f tested it with the podman preset and its working as expected, need to also test with a VPN and the openshift and microshift presets |
|
the detection is not an issue, but the |
1b92c43
to
e091eaf
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
bedcc6f
to
de9bd8c
Compare
de9bd8c
to
9076983
Compare
6d66c12
to
9076983
Compare
@gbraad @praveenkumar Updated the PR:
|
Thanks for picking this up, sorry I've not been able to give this any more time |
@GingerGeek no worries. life can get in the way of things; it happens to all of us. |
|
||
// API, Cockpit and Internal SSH are all bound to a private address, usually 127.0.0.1 | ||
// On Windows, where WSL2 is active, it's bound to the Windows machine's address within the private WSL2 network | ||
privateIP := VsockPrivateAddress() |
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 also be behind the config option, now on windows it is always binding the port forwards to the WSL vswitch ip address
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 is handled now in e0ef98d
@@ -89,7 +89,7 @@ func (vm *virtualMachine) State() (state.State, error) { | |||
|
|||
func (vm *virtualMachine) IP() (string, error) { | |||
if vm.vsock { | |||
return "127.0.0.1", nil | |||
return VsockPrivateAddress(), nil |
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.
similar to https://github.com/crc-org/crc/pull/3337/files#r1302624090 this should also report the WSL vswitch ip address only when wsl-host-access
is enabled, but currently its not considering the config option
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.
It'd be easier to toggle this with an env variable instead of the config option, so i suggest instead of having a config option wsl-host-access
we can depend on a env variable CRC_WSL_HOST_ACCESS
which should toggle this behavior, wdyt @gbraad ?
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 don't know from where to where the connections would be allowed, but crc has a host-network-access Allow TCP/IP connections from the CRC VM to services running on the host
config option, let's try to get the naming of the new option consistent/not confusing with this preexisting option.
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.
The comm. is to be allowed from WSL to crc VM, so that the oc
commands work from the WSL terminal
i mistakenly made the config key wsl-host-access
but previously wsl2-network-access
was suggested in #3337 (comment)
pkg/crc/config/settings.go
Outdated
cfg.AddSetting(EnableSharedDirs, false, validateSmbSharedDirs, SuccessfullyApplied, | ||
"Mounts host's user profile folder at '/' in the CRC VM (true/false, default: false)") | ||
// Setting to enable access to CRC vm from wsl | ||
cfg.AddSetting(WSLHostAccess, false, ValidateBool, SuccessfullyApplied, | ||
"Enable acess to CRC VM within WSL environment (true/false, default: false)") |
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.
access
pkg/crc/config/settings.go
Outdated
@@ -35,6 +35,7 @@ const ( | |||
IngressHTTPPort = "ingress-http-port" | |||
IngressHTTPSPort = "ingress-https-port" | |||
EmergencyLogin = "enable-emergency-login" | |||
WSLHostAccess = "wsl-host-access" |
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.
The title is: "add 'wsl-network-access'" ?
pkg/crc/config/settings.go
Outdated
cfg.AddSetting(EnableSharedDirs, false, validateSmbSharedDirs, SuccessfullyApplied, | ||
"Mounts host's user profile folder at '/' in the CRC VM (true/false, default: false)") | ||
// Setting to enable access to CRC vm from wsl | ||
cfg.AddSetting(WSLHostAccess, false, ValidateBool, SuccessfullyApplied, |
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 network access or host access?
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.
should be WSLNetworkAccess
updated now
…ices Bind Windows host-side vSock ports to the Window's machines IP on the WSL2 network allowing both Windows host and WSL2 to connect to VM services that aren't shared publically.
9076983
to
8fe4a6d
Compare
VsockPrivateAddress should be dependent on the wsl-network-access config setting and return the fallback address when the config is false
8fe4a6d
to
e0ef98d
Compare
@GingerGeek: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@gbraad should we close this now, we haven't seen users requesting this feature much, can reopen again later if needed |
There is a Closing it is OK |
this is the link to the article about mirrored |
Addresses: Issue #374
Solution/Idea
Bind Windows host-side vSock ports to the Window's machines IP on the WSL2 network, allowing both Windows host and WSL2 to connect to VM services that aren't shared publically.
Proposed changes
Adds decision-making on the "local" address for API Server, SSH server and DNS resolution of routes when these are using vsock as transport on windows.
In addition, the injected DNS routes point to the WSL2 host address, as these additions are automatically* (see caveats) added into WSL2 so that the domains work across both Windows and WSL2.
Testing
After successfully running
start
andsetup
you are able to run tooling from within WSL 2.Caveats
Changes to the Window's hosts files are not always automatically picked up by WSL2 distributions. After a
crc start
, if the DNS entries were not already present, WSL2 may need to be restarted withwsl --shutdown
.