Skip to content
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

[FEAT] host-specific ssh keys for cockpit container #20904

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

benniekiss
Copy link

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

@martinpitt
Copy link
Member

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
Copy link
Author

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants