Skip to content

Conversation

alexandr-san4ez
Copy link
Contributor

Change summary

During upgrade, the script now checks if any known_hosts files exist. If so, it prompts the user to save these SSH fingerprints, and upon confirmation, copies the files to the new image persistence directory.

Would you like to copy SSH host keys? [Y/n]
Copying SSH host keys
Would you like to save the SSH known hosts (fingerprints) from your current configuration? [Y/n]
Copying SSH known hosts
Copying system image files

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

How to test / Smoketest result

Ensure known_hosts files exist for root or users, then run the upgrade and confirm you’re prompted to migrate them. After choosing Yes, verify the files are copied; with No or no files, they should not be migrated or prompted.

vyos@vyos:~$ touch /home/vyos/.ssh/known_hosts
vyos@vyos:~$ sudo touch /root/.ssh/known_hosts
vyos@vyos:~$ add system image vyos-1.5-rolling-202508251355-generic-amd64.iso
Validating image compatibility
Validating image checksums
What would you like to name this image? (Default: 1.5-rolling-202508251355) test1
Would you like to set the new image as the default one for boot? [Y/n] Y
An active configuration was found. Would you like to copy it to the new image? [Y/n] Y
Copying configuration directory
Would you like to copy SSH host keys? [Y/n] Y
Copying SSH host keys
Would you like to save the SSH known hosts (fingerprints) from your current configuration? [Y/n] Y
Copying SSH known hosts
Copying system image files
Cleaning up
Unmounting target filesystems
Removing temporary files
vyos@vyos:~$ sudo reboot now
...
vyos@vyos:~$ find /etc /home /root -name known_hosts 2>/dev/null
/home/vyos/.ssh/known_hosts
/root/.ssh/known_hosts

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Copy link

github-actions bot commented Aug 25, 2025

👍
No issues in PR Title / Commit Title

@alexandr-san4ez alexandr-san4ez changed the title image: T5455: Add migration of SSH known_hosts files during image upgrade image: T5455: Add migration of SSH known_hosts files during upgrade Aug 25, 2025
@sever-sever sever-sever requested a review from Copilot August 26, 2025 07:35
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds functionality to migrate SSH known_hosts files during VyOS system image upgrades. The migration preserves SSH fingerprints from the current configuration to the new image, preventing the need to re-verify SSH connections after upgrade.

  • Implements detection and migration of known_hosts files for root and all users
  • Adds user prompts during upgrade to confirm migration of SSH known hosts
  • Integrates the migration process into the existing image installation workflow

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@c-po
Copy link
Member

c-po commented Aug 28, 2025

Other then the tree minor changes in placing imports on their own line - code looks good.

@c-po c-po added the bp/circinus Create automatic backport for circinus label Aug 28, 2025
@alexandr-san4ez alexandr-san4ez requested a review from c-po August 28, 2025 15:23
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All requested changes added. Change looks good.

Copy link
Contributor

@jestabro jestabro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see one problem with this: by implicitly creating the home directories in the persistence directory
/usr/lib/live/mount/persistence/boot/my-new-image-name/rw
during add system image, the useradd in system_login.py on first boot into the new image will avoid copying skeleton files. The warning is not recorded, but an analogous situation is, say:

vyos@vyos:~$ sudo mkdir /home/bob
vyos@vyos:~$ sudo useradd --create-home --home /home/bob bob
useradd: warning: the home directory /home/bob already exists.
useradd: Not copying any file from skel directory into it.

The result in my test is that, on booting into the new image, one sees:

vyos@vyos:~$ show version

  Invalid command: [show]

vyos@vyos:~$ pwd
/home/vyos
vyos@vyos:~$ ls -la
total 16
drwxr-xr-x 3 vyos root  4096 Aug 29 17:56 .
drwxr-xr-x 1 root root  4096 Aug 29 17:41 ..
-rw------- 1 vyos users  224 Aug 29 17:56 .bash_history
drwxr-xr-x 2 vyos root  4096 Aug 29 17:47 .ssh
-rw-r--r-- 1 vyos users    0 Aug 29 17:47 .sudo_as_admin_successful

so, no .bashrc etc, hence no completions.

Updating the image with this PR but without copying the known_hosts files returns normal behavior in the new image.

So, I don't have a concrete suggestion yet, but do want to point out the issue.

@alexandr-san4ez
Copy link
Contributor Author

@jestabro thanks for finding this because I missed it. I will prepare fix for this PR.

Copy link

github-actions bot commented Sep 3, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…pgrade

During upgrade, the script now checks if any `known_hosts` files exist.
If so, it prompts the user to save these SSH fingerprints, and upon
confirmation, copies the files to the new image persistence directory.
@alexandr-san4ez
Copy link
Contributor Author

@jestabro thanks for finding this because I missed it. I will prepare fix for this PR.

Fix: Preserve existing home directory when adding user by backing up and restoring its contents after useradd to prevent data loss.

@github-actions github-actions bot removed the conflicts label Sep 3, 2025
Copy link

github-actions bot commented Sep 3, 2025

Conflicts have been resolved. A maintainer will review the pull request shortly.

@jestabro
Copy link
Contributor

jestabro commented Sep 3, 2025

Re-running failed jobs, as a check; all passed in a local test. Review pending.

Copy link

github-actions bot commented Sep 3, 2025

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests VPP 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • Config tests VPP 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@jestabro jestabro requested a review from c-po September 3, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bp/circinus Create automatic backport for circinus current
Development

Successfully merging this pull request may close these issues.

3 participants