From 2315a73a209790da17579249e7f31368a2a5a81e Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Wed, 19 Jun 2024 16:17:43 +0530 Subject: [PATCH] User friendly name of Downloaded Templates Volumes and ISOs (#9252) --- .../CreateEntityDownloadURLCommand.java | 27 ++++++++---- .../subsystem/api/storage/DataObject.java | 2 + .../CloudStackImageStoreDriverImpl.java | 44 ++++++++++++++----- .../storage/upload/UploadMonitorImpl.java | 4 +- .../diagnostics/to/DiagnosticsDataObject.java | 5 +++ .../storage/template/UploadManagerImpl.java | 6 ++- 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateEntityDownloadURLCommand.java b/core/src/main/java/com/cloud/agent/api/storage/CreateEntityDownloadURLCommand.java index c84eceb0ca2..33403580b6f 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/CreateEntityDownloadURLCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/CreateEntityDownloadURLCommand.java @@ -23,18 +23,19 @@ import com.cloud.agent.api.to.DataTO; public class CreateEntityDownloadURLCommand extends AbstractDownloadCommand { - public CreateEntityDownloadURLCommand(String parent, String installPath, String uuid, DataTO data) { // this constructor is for creating template download url + public CreateEntityDownloadURLCommand(String parent, String installPath, String fileName, String filePath, DataTO data) { // this constructor is for creating template download url super(); this.parent = parent; // parent is required as not the template can be child of one of many parents this.installPath = installPath; - this.extractLinkUUID = uuid; + this.filenameInExtractURL = fileName; + this.filepathInExtractURL = filePath; this.data = data; } - public CreateEntityDownloadURLCommand(String installPath, String uuid) { + public CreateEntityDownloadURLCommand(String installPath, String filename) { super(); this.installPath = installPath; - this.extractLinkUUID = uuid; + this.filenameInExtractURL = filename; } public CreateEntityDownloadURLCommand() { @@ -42,7 +43,8 @@ public class CreateEntityDownloadURLCommand extends AbstractDownloadCommand { private String installPath; private String parent; - private String extractLinkUUID; + private String filenameInExtractURL; + private String filepathInExtractURL; public DataTO getData() { return data; @@ -75,12 +77,19 @@ public class CreateEntityDownloadURLCommand extends AbstractDownloadCommand { this.parent = parent; } - public String getExtractLinkUUID() { - return extractLinkUUID; + public String getFilenameInExtractURL() { + return filenameInExtractURL; } - public void setExtractLinkUUID(String extractLinkUUID) { - this.extractLinkUUID = extractLinkUUID; + public void setFilenameInExtractURL(String filenameInExtractURL) { + this.filenameInExtractURL = filenameInExtractURL; } + public String getFilepathInExtractURL() { + return filepathInExtractURL; + } + + public void setFilepathInExtractURL(String filepathInExtractURL) { + this.filepathInExtractURL = filepathInExtractURL; + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataObject.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataObject.java index 091c09d7a4d..fe052f01606 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataObject.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataObject.java @@ -50,4 +50,6 @@ public interface DataObject { void decRefCount(); Long getRefCount(); + + String getName(); } diff --git a/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java b/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java index 71fa2e91bcb..fca6db26169 100644 --- a/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java +++ b/plugins/storage/image/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackImageStoreDriverImpl.java @@ -25,6 +25,7 @@ import javax.inject.Inject; import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand; import com.cloud.host.dao.HostDao; import com.cloud.storage.Upload; +import com.cloud.utils.StringUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; @@ -64,20 +65,43 @@ public class CloudStackImageStoreDriverImpl extends NfsImageStoreDriverImpl { return nfsTO; } + private String createObjectNameForExtractUrl(String installPath, ImageFormat format, DataObject dataObject) { + String objectNameInUrl = dataObject.getName(); + try { + objectNameInUrl = cleanObjectName(objectNameInUrl); + } catch (Exception e) { + objectNameInUrl = UUID.randomUUID().toString(); + } + + if (format != null) { + objectNameInUrl = objectNameInUrl + "." + format.getFileExtension(); + } else if (installPath.lastIndexOf(".") != -1) { + objectNameInUrl = objectNameInUrl + "." + installPath.substring(installPath.lastIndexOf(".") + 1); + } + + return objectNameInUrl; + } + + private String cleanObjectName(String objectName) { + if (StringUtils.isEmpty(objectName)) { + throw new IllegalArgumentException("Object name is empty or null"); + } + return objectName.trim() + .replaceAll("[^a-zA-Z0-9]+", "-") + .replaceAll("-{2,}", "-") + .replaceAll("^-|-$", ""); + } + @Override public String createEntityExtractUrl(DataStore store, String installPath, ImageFormat format, DataObject dataObject) { // find an endpoint to send command EndPoint ep = _epSelector.select(store); // Create Symlink at ssvm String path = installPath; - String uuid = UUID.randomUUID().toString(); - if (format != null) { - uuid = uuid + "." + format.getFileExtension(); - } else if (path.lastIndexOf(".") != -1) { - uuid = uuid + "." + path.substring(path.lastIndexOf(".") + 1); - } + String objectNameInUrl = createObjectNameForExtractUrl(path, format, dataObject); + String objectPathInUrl = UUID.randomUUID().toString(); CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)store).getMountPoint(), - path, uuid, dataObject == null ? null: dataObject.getTO()); + path, objectNameInUrl, objectPathInUrl, dataObject == null ? null: dataObject.getTO()); Answer ans = null; if (ep == null) { String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; @@ -92,10 +116,10 @@ public class CloudStackImageStoreDriverImpl extends NfsImageStoreDriverImpl { throw new CloudRuntimeException(errorString); } // Construct actual URL locally now that the symlink exists at SSVM - return generateCopyUrl(ep.getPublicAddr(), uuid); + return generateCopyUrl(ep.getPublicAddr(), objectNameInUrl, objectPathInUrl); } - private String generateCopyUrl(String ipAddress, String uuid) { + private String generateCopyUrl(String ipAddress, String fileName, String filePath) { String hostname = ipAddress; String scheme = "http"; @@ -118,7 +142,7 @@ public class CloudStackImageStoreDriverImpl extends NfsImageStoreDriverImpl { } scheme = "https"; } - return scheme + "://" + hostname + "/userdata/" + uuid; + return scheme + "://" + hostname + "/userdata/" + filePath + "/" + fileName; } @Override diff --git a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java index 64ada6dc309..2d4f8eb8085 100644 --- a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java +++ b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java @@ -261,7 +261,7 @@ public class UploadMonitorImpl extends ManagerBase implements UploadMonitor { // Create Symlink at ssvm String path = vmTemplateHost.getInstallPath(); String uuid = UUID.randomUUID().toString() + "." + template.getFormat().getFileExtension(); // adding "." + vhd/ova... etc. - CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)store).getMountPoint(), path, uuid, null); + CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)store).getMountPoint(), path, uuid, null, null); Answer ans = ep.sendMessage(cmd); if (ans == null || !ans.getResult()) { errorString = "Unable to create a link for " + type + " id:" + template.getId() + "," + (ans == null ? "" : ans.getDetails()); @@ -317,7 +317,7 @@ public class UploadMonitorImpl extends ManagerBase implements UploadMonitor { throw new CloudRuntimeException(errorString); } - CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)secStore).getMountPoint(), path, uuid, null); + CreateEntityDownloadURLCommand cmd = new CreateEntityDownloadURLCommand(((ImageStoreEntity)secStore).getMountPoint(), path, uuid, null, null); Answer ans = ep.sendMessage(cmd); if (ans == null || !ans.getResult()) { errorString = "Unable to create a link for " + type + " id:" + entityId + "," + (ans == null ? "" : ans.getDetails()); diff --git a/server/src/main/java/org/apache/cloudstack/diagnostics/to/DiagnosticsDataObject.java b/server/src/main/java/org/apache/cloudstack/diagnostics/to/DiagnosticsDataObject.java index ebe1d7fbc73..de66ad4d5e6 100644 --- a/server/src/main/java/org/apache/cloudstack/diagnostics/to/DiagnosticsDataObject.java +++ b/server/src/main/java/org/apache/cloudstack/diagnostics/to/DiagnosticsDataObject.java @@ -99,4 +99,9 @@ public class DiagnosticsDataObject implements DataObject { public Long getRefCount() { return null; } + + @Override + public String getName() { + return dataStore.getName(); + } } diff --git a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java index 5c589b6e35c..9c8587b6954 100644 --- a/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java +++ b/services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java @@ -268,6 +268,8 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager { } // Create the directory structure so that its visible under apache server root String extractDir = "/var/www/html/userdata/"; + extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator; + s_logger.info("HARI " + extractDir); Script command = new Script("/bin/su", s_logger); command.add("-s"); command.add("/bin/bash"); @@ -290,11 +292,11 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager { } // Create a random file under the directory for security reasons. - String uuid = cmd.getExtractLinkUUID(); + String filename = cmd.getFilenameInExtractURL(); // Create a symbolic link from the actual directory to the template location. The entity would be directly visible under /var/www/html/userdata/cmd.getInstallPath(); command = new Script("/bin/bash", s_logger); command.add("-c"); - command.add("ln -sf /mnt/SecStorage/" + cmd.getParent() + File.separator + cmd.getInstallPath() + " " + extractDir + uuid); + command.add("ln -sf /mnt/SecStorage/" + cmd.getParent() + File.separator + cmd.getInstallPath() + " " + extractDir + filename); result = command.execute(); if (result != null) { String errorString = "Error in linking err=" + result;