consoles: Don't use listening address to connect to VNC #1982
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mvollmer
force-pushed
the
consoles-correct-connection
branch
4 times, most recently
from
02946bd to
887cde1
Compare
mvollmer
force-pushed
the
consoles-correct-connection
branch
2 times, most recently
from
235c6fd to
4b2534b
Compare
| if (app.startsWith("cockpit+=")) { | ||
| address = app.substr(9); |
There was a problem hiding this comment.
Hmm, this should run...
| } | ||
| } | ||
|
|
||
| domainDesktopConsole({ name: vm.name, consoleDetail: Object.assign({}, consoleDetail, { address }) }); |
There was a problem hiding this comment.
{...consoleDetail, address} reads a bit nicer IMHO, but up to you.
test/check-machines-consoles
Outdated
| class TestMultiMachinesConsoles(ConsoleCase): | ||
| provision = { # noqa: RUF012 | ||
| "machine": {"address": "10.111.113.1/20"}, | ||
| } |
There was a problem hiding this comment.
This is still only one machine -- seems you could get this more cheaply with the already running nondestructive machine? 172.27.0.15 is reliable, a couple of cockpit tests refer to it as well.
There was a problem hiding this comment.
Right, I thought I'd seen something like that... Will that work also with TF?
There was a problem hiding this comment.
I have made the new test destructive and use that address.
| b.open(f"/={my_ip}/machines") | ||
| b.set_val('#login-user-input', "admin") | ||
| b.set_val('#login-password-input', "foobar") | ||
| b.click("#login-button") |
There was a problem hiding this comment.
That should be self.login_and_go("/machines", host=my_ip). I wonder why you don't' have to m.start_cockpit() here, it's usually required?
There was a problem hiding this comment.
That should be self.login_and_go("/machines", host=my_ip)
The host parameter gives you a "/@{host}" prefix, but I want a "/={host}" prefix.
There was a problem hiding this comment.
I wonder why you don't' have to m.start_cockpit() here, it's usually required?
Good question... I guess cockpit is enabled by default somewhere already?
There was a problem hiding this comment.
I have added a call to "start_cockpit".
| # Login from Shell via SSH | ||
|
|
There was a problem hiding this comment.
Is that actually something different? I thought transport-wise bastion host and shell host selector would be the same?
There was a problem hiding this comment.
The new code in the PR handles that differently.. maybe it doesn't have to.
Bastion host has:
- cockpit.transport.host == "localhost"
- application == "cockpit+=address"
where "address" is the string typed into the login page, and visible in the browser URL as "/=address". But it's not in the frame URL.
SSH from the Shell has:
- cockpit.transport.host == "address"
where "address" is the string typed into the "add remote host" dialog.
There was a problem hiding this comment.
(I don't really understand our "application" business, tbh.)
mvollmer
force-pushed
the
consoles-correct-connection
branch
3 times, most recently
from
06e86cb to
9c6e8b6
Compare
The listening address is most likely 127.0.0.1 (libvirt default) or 0.0.0.0 (yolo config). Neither of these works remotely. Let's tell the remote viewer some address that has a chance of working. More complicated setups, like a bastion host or connecting via SSH inside Cockpit, might not work because of networking routing challanges, but at least we get better error messages like "No route to host" etc. Also, with the libvirt default of only listening on 127.0.0.1, no remote connection will work. But again, we now at least get the expected "Connection refused" error message.
mvollmer
force-pushed
the
consoles-correct-connection
branch
from
9c6e8b6 to
5ac4425
Compare
| if (app.startsWith("cockpit+=")) { | ||
| address = app.substr(9); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test.
There was a problem hiding this comment.
I don't understand this... these lines clearly run.
There was a problem hiding this comment.
Yeah, coverage collection must be buggy somehow... grr.
The listening address is most likely 127.0.0.1 (libvirt default) or 0.0.0.0 (yolo config). Neither of these works remotely.
Let's tell the remote viewer some address that has a chance of working. More complicated setups, like a bastion host or connecting via SSH inside Cockpit, might not work because of networking routing challanges, but at least we get better error messages like "No route to host" etc.
Also, with the libvirt default of only listening on 127.0.0.1, no remote connection will work. But again, we now at least get the expected "Connection refused" error message.
Demo: https://youtu.be/zjQlyt3SzG4
Machines: "Launch viewer" now uses correct address
The "Launch viewer" button in the "Console" card of a virtual machine would tell the remote viewer application to connect to the listening address of the VNC server. This is wrong when the server listens on 127.0.0.1 (libvirt default) or 0.0.0.0 (allow connections on all interfaces). Now Cockpit will tell the remote viewer application to connect to the same domain name or IP address that Cockpit itself is using to reach the virtual machine host.