Skip to content

Fix infrastructure leak in template from volume creation error message #12650

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.

Sign up for GitHub

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

Open
erikbocks wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −2
Open

Fix infrastructure leak in template from volume creation error message #12650

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,8 @@ public DataStore getImageStore(String storeUuid, Long zoneId, VolumeVO volume) {
}

if (imageStore == null) {
throw new CloudRuntimeException(String.format("Cannot find an image store for zone [%s].", zoneId));
logger.error("Cannot find an image store for zone [{}].", zoneId);
throw new CloudRuntimeException("Failed to create template. Please contact the cloud administrator.");
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

getImageStore is used for both template creation and volume upload (e.g., VolumeApiServiceImpl.uploadVolume calls _tmpltMgr.getImageStore(...)). Throwing "Failed to create template..." here will surface an incorrect error message to volume-upload callers. Consider using an operation-neutral message (e.g., "Failed to find an image store. Please contact the cloud administrator."), or passing a caller-specific context string into getImageStore so each API reports the right high-level failure without exposing the zone ID.

Suggested change
throw new CloudRuntimeException("Failed to create template. Please contact the cloud administrator.");
throw new CloudRuntimeException("Failed to find an image store. Please contact the cloud administrator.");

Copilot uses AI. Check for mistakes.
Comment on lines +479 to +480
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

There are existing unit tests for getImageStore in TemplateManagerImplTest, but they currently only assert that an exception is thrown. Since this change is specifically about preventing infrastructure/zone ID leakage via exception messages, please add assertions that the thrown CloudRuntimeException message does not include the zone ID (and matches the new sanitized message).

Copilot uses AI. Check for mistakes.
}

return imageStore;
Expand Down Expand Up @@ -1702,7 +1703,8 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t
}
DataStore store = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId);
if (store == null) {
throw new CloudRuntimeException("cannot find an image store for zone " + zoneId);
logger.error("Cannot find an image store for zone [{}].", zoneId);
throw new CloudRuntimeException("Failed to create template. Please contact the cloud administrator.");
Comment on lines 1705 to +1707
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

The same log+throw pattern and user-facing message is now duplicated here and in getImageStore. To avoid the two sites drifting (especially if you later tweak the sanitized message), consider factoring this into a small helper (e.g., throwImageStoreNotFound(zoneId, operation)), which logs the detailed message and throws the sanitized exception.

Copilot uses AI. Check for mistakes.
}
AsyncCallFuture<TemplateApiResult> future = null;

Expand Down
Loading