Copilot commented on code in PR #12059:
URL: https://github.com/apache/cloudstack/pull/12059#discussion_r2522136422
##########
server/src/main/java/com/cloud/server/ConfigurationServerImpl.java:
##########
@@ -617,7 +617,9 @@ public void updateKeyPairs() {
// FIXME: take a global database lock here for safety.
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()) {
Review Comment:
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:
```java
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.
```suggestion
if (!privkeyfile.exists() || !pubkeyfile.exists()) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
