User friendly name of Downloaded Templates Volumes and ISOs (apache#9252)

harikrishna-patnala · dhslove · commit b22116fc0887 · 2024-06-24T14:11:29.000+09:00
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
@@ -23,26 +23,28 @@
 
 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() {
     }
 
     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 void setParent(String parent) {
         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
@@ -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
@@ -25,6 +25,7 @@
 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.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
@@ -62,20 +63,43 @@ public DataStoreTO getStoreTO(DataStore store) {
         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?";
@@ -90,10 +114,10 @@ public String createEntityExtractUrl(DataStore store, String installPath, ImageF
             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";
@@ -116,7 +140,7 @@ private String generateCopyUrl(String ipAddress, String uuid) {
             }
             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
@@ -259,7 +259,7 @@ else if ((token != null) && (token.length == 5) && (token[2].startsWith(hostname
             // 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());
@@ -315,7 +315,7 @@ public void createVolumeDownloadURL(Long entityId, String path, Type type, Long
                 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
@@ -99,4 +99,9 @@ public void decRefCount() {
     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
@@ -288,11 +288,11 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
         }
 
         // 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", 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;

Original file line number	Diff line number	Diff line change
`@@ -50,4 +50,6 @@ public interface DataObject {`
`50`	`50`	`void decRefCount();`
`51`	`51`
`52`	`52`	`Long getRefCount();`
	`53`	`+`
	`54`	`+ String getName();`
`53`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -99,4 +99,9 @@ public void decRefCount() {`
`99`	`99`	`public Long getRefCount() {`
`100`	`100`	`return null;`
`101`	`101`	`}`
	`102`	`+`
	`103`	`+ @Override`
	`104`	`+ public String getName() {`
	`105`	`+ return dataStore.getName();`
	`106`	`+ }`
`102`	`107`	`}`