system-reinstall-bootc: SSH key handling via cloud-init detection#2163
system-reinstall-bootc: SSH key handling via cloud-init detection#2163hone wants to merge 1 commit intobootc-dev:mainfrom
Conversation
If cloud-init is detected in the target image: 1. Check the host for existing root SSH keys at /root/.ssh/authorized_keys. 2. If host keys exist, automatically configure inheritance via /target/root/.ssh/authorized_keys. 3. If no host keys exist, proceed without manual interaction, trusting cloud-init in the new system to handle key provisioning. If cloud-init is not detected, maintain the existinginteractive behavior to prevent user lock-out. Ref: bootc-dev#1300 Signed-off-by: Terence Lee <hone02@gmail.com>
github-actions
Bot
added
the
area/system-reinstall-bootc
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automatic SSH key inheritance when cloud-init is detected in the target image, allowing the system to reuse host keys instead of prompting the user. It adds a new check for cloud-init within the container and updates the command generation logic to handle optional SSH keys. Feedback suggests moving the cloud-init check before the spinner is cleared to improve the user experience during the container run. Additionally, it is recommended to expand the key inheritance logic to include keys from other users on the host, ensuring consistency with existing behavior and preventing potential lockouts.
| let has_clean = podman::bootc_has_clean(&opts.image)?; | ||
| spinner.finish_and_clear(); | ||
|
|
||
| let ssh_key_file = tempfile::NamedTempFile::new()?; | ||
| let ssh_key_file_path = ssh_key_file | ||
| .path() | ||
| .to_str() | ||
| .ok_or_else(|| anyhow::anyhow!("unable to create authorized_key temp file"))?; | ||
| let mut _ssh_key_tempfile = None; | ||
| let mut ssh_key_file_path = None; | ||
|
|
||
| if podman::image_has_cloud_init(&opts.image)? { |
There was a problem hiding this comment.
The podman::image_has_cloud_init check involves running a container, which can take a few seconds. It should be performed while the spinner is active to provide better feedback to the user and avoid a perceived hang in the UI.
| let has_clean = podman::bootc_has_clean(&opts.image)?; | |
| spinner.finish_and_clear(); | |
| let ssh_key_file = tempfile::NamedTempFile::new()?; | |
| let ssh_key_file_path = ssh_key_file | |
| .path() | |
| .to_str() | |
| .ok_or_else(|| anyhow::anyhow!("unable to create authorized_key temp file"))?; | |
| let mut _ssh_key_tempfile = None; | |
| let mut ssh_key_file_path = None; | |
| if podman::image_has_cloud_init(&opts.image)? { | |
| let has_clean = podman::bootc_has_clean(&opts.image)?; | |
| let has_cloud_init = podman::image_has_cloud_init(&opts.image)?; | |
| spinner.finish_and_clear(); | |
| let mut _ssh_key_tempfile = None; | |
| let mut ssh_key_file_path = None; | |
| if has_cloud_init { |
| let host_root_keys = std::path::Path::new("/root/.ssh/authorized_keys"); | ||
| if host_root_keys.exists() { |
There was a problem hiding this comment.
The automatic inheritance logic only checks for keys in /root/.ssh/authorized_keys. This is a departure from the existing interactive behavior in get_ssh_keys, which scans all users on the host system. If a user is running this tool via sudo but has their keys in their own home directory (e.g., /home/user/.ssh/authorized_keys), they will not be inherited automatically, potentially leading to a lockout if the image relies on these keys and cloud-init has no other datasource. Consider using the existing multi-user key detection logic or at least checking the current user's keys.
1 participant
If cloud-init is detected in the target image:
If cloud-init is not detected, maintain the existinginteractive behavior to prevent user lock-out.
Ref: #1300