containers/ws: Support host-specific ssh keys

[benniekiss]

This PR adds the ability to set host-specific ssh keys via env-var in the cockpit-ws container.

Users can pass environment variables in the format of COCKPIT_SSH_KEY_PATH_{HOSTNAME}, where HOSTNAME is the hostname used in the Connect to field of the cockpit login page, and cockpit will use the configured key to login to the host. If no host-specific key is set, it falls back to using the COCKPIT_SSH_KEY_PATH environment variable.

I still need to make a unit test, but I wanted to ask for suggestions as to the best route. The cockpit bastion test could be replicated, or it could be refactored to test host-specific keys. I wasn't sure what the best approach would be, and would appreciate any advice before proceeding!

This functionality will enable users to login to multiple hosts via ssh keys once the host-switcher is deprecated, providing a solution to user concerns such as #20901

cockpit/ws container: Support host specific SSH keys

The cockpit/ws container now supports adding multiple SSH private keys. In addition to the existing $COCKPIT_SSH_KEY_PATH environment variable, you can now set host specific $COCKPIT_SSH_KEY_PATH_{HOSTNAME} variables, where {HOSTNAME} is the host name (in capital letters) used in the Connect to: field of the login page.

Thanks to benniekiss for contributing this feature!

[martinpitt]

Thanks @benniekiss ! This is a pretty cool feature! It does need an integration test, yes. I suggest you factorize that test a bit, with a do_test_key_login(options: str), and then call this several times in testKeyLogin() with the scenarios (only COCKPIT_SSH_KEY_PATH, the per-hostname ones, and the fallback). You can use invalid keys for the ones that you don't expect to get picked of course, so you don't need multiple VMs/SSH targets.

[benniekiss]

Thank you! I'll start working on the test soon.

FWIW, I've been running this patch personally, and it's worked well.

[benniekiss]

I refactored the test to account for setting a host-specific key var and the fallback. I was not able to run these tests locally. I was having a lot of trouble getting the test infrastructure up on my aarch64 system -- M2 macbook on asahi. I tried following the docs in HACKING.md and in the test/README.md, but I struggled to get it working.

[martinpitt]

Cheers! This looks pretty good, I'd just like to adjust the documentation. Please also squash the commits together into a "containers/ws: Support host-specific ssh keys".

I triggered the fedora-coreos tests (the only relevant ones) to see how it fares.

[martinpitt]

The test failed in the second round, presumably as it tries to regenerate the RSA key a second time when it already exists. This needs to be taken out of the loop.

[martinpitt]

@benniekiss as previously: Please tell me when you are fed up and want me to take over 😁

[benniekiss]

rebased on main and squashed!

I moved the ssh-keygen calls and the self.addCleanup calls outside of the loop, and I updated the documentation as suggested.

If the tests don't pass, I'd appreciate if you'd be able to takeover and see this through! Just because I still haven't been able to get the tests running locally

[benniekiss]

fixed a misplaced comment

[martinpitt]

@benniekiss Still unhappy.. I'll take over, thank you!

[benniekiss]

You're welcome! And thank you, too

[martinpitt]

Fixed ; it was missing the .upper() for the hostname and some cleanup.

Thanks!

[martinpitt]

@benniekiss I added a release note to the description. Proof-reading much appreciated!

[benniekiss]

Release note looks good overall! I cleaned up some grammar.

This markdown formatting is a little hard to read, but I wasn't sure if there was a clearer way to write it.

$COCKPIT_SSH_KEY_PATH_HOSTNAME

I had used COCKPIT_SSH_KEY_PATH_{HOSTNAME} previously, so that would be my suggestion.

[martinpitt]

@benniekiss you are right, I changed it to {}. Cheers!