-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Avoid unnecessary SSH key regeneration in developer mode #12059
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
base: main
Are you sure you want to change the base?
Conversation
…hey already existed.
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
There was a problem hiding this 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 aims to fix unnecessary regeneration of SSH keys in developer mode when they already exist. The change adds a conditional check to skip key generation if the key files are already present on the filesystem.
- Adds file existence check before SSH key generation
- Targets developer mode to prevent overwriting existing SSH keys
- Reduces unnecessary file system operations during startup
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| boolean onWindows = isOnWindows(); | ||
| if(!onWindows) { | ||
| Script.runSimpleBashScript("if [ -f " + privkeyfile + " ]; then rm -f " + privkeyfile + "; fi; ssh-keygen -t ecdsa -m PEM -N '' -f " + privkeyfile + " -q 2>/dev/null || ssh-keygen -t ecdsa -N '' -f " + privkeyfile + " -q"); | ||
| if (!privkeyfile.exists() && !pubkeyfile.exists()) { |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition !privkeyfile.exists() && !pubkeyfile.exists() uses AND logic, which will prevent SSH key generation if only one of the key files exists. This could leave the system in an inconsistent state. Consider using OR logic (||) instead, or check if BOTH files exist before skipping generation:
if (privkeyfile.exists() && pubkeyfile.exists()) {
// Both keys exist, skip generation
} else {
// At least one key is missing, regenerate both
Script.runSimpleBashScript("if [ -f " + privkeyfile + " ]; then rm -f " + privkeyfile + "; fi; ssh-keygen -t ecdsa -m PEM -N '' -f " + privkeyfile + " -q 2>/dev/null || ssh-keygen -t ecdsa -N '' -f " + privkeyfile + " -q");
}This ensures that both keys are always present and consistent when the code proceeds to read them at lines 628-638.
| if (!privkeyfile.exists() && !pubkeyfile.exists()) { | |
| if (!privkeyfile.exists() || !pubkeyfile.exists()) { |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12059 +/- ##
============================================
+ Coverage 17.55% 17.56% +0.01%
- Complexity 15537 15538 +1
============================================
Files 5910 5910
Lines 529336 529352 +16
Branches 64654 64656 +2
============================================
+ Hits 92904 92977 +73
+ Misses 425975 425917 -58
- Partials 10457 10458 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This PR addresses an issue #12055 where SSH keys were being unnecessarily regenerated in developer mode even when valid keys already existed on disk.
When both the public and private key files are present at the expected file paths but missing from the database, CloudStack will now reuse the existing keys on disk instead of regenerating them. The existing keys are injected into the database, ensuring consistency without overwriting the developer’s configured keys.
Impact
Prevents loss of existing SSH keys in developer environments.
Ensures smoother setup by reusing valid keys already present on disk.
No impact on normal operation or production deployments.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Verified behavior in developer mode with pre-existing SSH key files but no DB entries.
Confirmed that no new keys are generated and the existing keys are properly persisted in the database.
Before Fix
After Fix
How did you try to break this feature and the system with this change?