secondary-storage: delete temp directory while deleting entity download url

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java

@@ -16,7 +16,10 @@
 // under the License.
 package org.apache.cloudstack.storage.template;
 
+import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+
 import java.io.File;
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
@@ -30,10 +33,10 @@
 
 import javax.naming.ConfigurationException;
 
-import com.cloud.agent.api.Answer;
-
 import org.apache.cloudstack.storage.resource.SecondaryStorageResource;
+import org.apache.commons.lang3.StringUtils;
 
+import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLAnswer;
 import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
 import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
@@ -48,15 +51,17 @@
 import com.cloud.storage.template.TemplateUploader;
 import com.cloud.storage.template.TemplateUploader.Status;
 import com.cloud.storage.template.TemplateUploader.UploadCompleteCallback;
+import com.cloud.utils.FileUtil;
 import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.UuidUtils;
 import com.cloud.utils.component.ManagerBase;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 
-import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
-
 public class UploadManagerImpl extends ManagerBase implements UploadManager {
 
+    protected static final String BASE_EXTRACT_DIR = "/var/www/html/userdata/";
+
     public class Completion implements UploadCompleteCallback {
         private final String jobId;
 
@@ -266,7 +271,7 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
             return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
         }
         // Create the directory structure so that its visible under apache server root
-        String extractDir = "/var/www/html/userdata/";
+        String extractDir = BASE_EXTRACT_DIR;
         extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator;
         Script command = new Script("/bin/su", logger);
         command.add("-s");
@@ -330,12 +335,15 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
         String extractUrl = cmd.getExtractUrl();
         String result;
         if (extractUrl != null) {
-            command.add("unlink /var/www/html/userdata/" + extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1));
+            String linkPath = extractUrl.substring(extractUrl.lastIndexOf(File.separator) + 1);
+            command.add("unlink " + BASE_EXTRACT_DIR + linkPath);
             result = command.execute();
             if (result != null) {
                 // FIXME - Ideally should bail out if you can't delete symlink. Not doing it right now.
                 // This is because the ssvm might already be destroyed and the symlinks do not exist.
                 logger.warn("Error in deleting symlink :" + result);
+            } else {
+                deleteEntitySymlinkRootPathIfNeeded(cmd, linkPath);
             }
         }
 
@@ -356,6 +364,30 @@ public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLComman
         return new Answer(cmd, true, "");
     }
 
+    protected void deleteEntitySymlinkRootPathIfNeeded(DeleteEntityDownloadURLCommand cmd, String linkPath) {
+        if (StringUtils.isEmpty(linkPath)) {
+            return;
+        }
+        String[] parts = linkPath.split("/");
+        if (parts.length == 0) {
+            return;
+        }
+        String rootDir = parts[0];
+        if (StringUtils.isEmpty(rootDir) || !UuidUtils.isUuid(rootDir)) {
+            return;
+        }
+        logger.info("Deleting symlink root directory: {} for {}", rootDir, cmd.getExtractUrl());
+        Path rootDirPath = Path.of(BASE_EXTRACT_DIR + rootDir);
+        String failMsg = "Failed to delete symlink root directory: {} for {}";
+        try {
+            if (!FileUtil.deleteRecursively(rootDirPath)) {
+                logger.warn(failMsg, rootDir, cmd.getExtractUrl());
+            }
+        } catch (IOException e) {
+            logger.warn(failMsg, rootDir, cmd.getExtractUrl(), e);
+        }
+    }
+
     private String getInstallPath(String jobId) {
         // TODO Auto-generated method stub
         return null;

services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java

@@ -0,0 +1,78 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.template;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+
+import java.nio.file.Path;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
+import com.cloud.utils.FileUtil;
+
+@RunWith(MockitoJUnitRunner.class)
+public class UploadManagerImplTest {
+
+    @InjectMocks
+    UploadManagerImpl uploadManager;
+
+    MockedStatic<FileUtil> fileUtilMock;
+
+    @Before
+    public void setup() {
+        fileUtilMock = mockStatic(FileUtil.class, Mockito.CALLS_REAL_METHODS);
+        fileUtilMock.when(() -> FileUtil.deleteRecursively(any(Path.class))).thenReturn(true);
+    }
+
+    @After
+    public void tearDown() {
+        fileUtilMock.close();
+    }
+
+    @Test
+    public void doesNotDeleteWhenLinkPathIsEmpty() {
+        String emptyLinkPath = "";
+        uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), emptyLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
+    }
+
+    @Test
+    public void doesNotDeleteWhenRootDirIsNotUuid() {
+        String invalidLinkPath = "invalidRootDir/file";
+        uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), invalidLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), never());
+    }
+
+    @Test
+    public void deletesSymlinkRootDirectoryWhenValidUuid() {
+        String validLinkPath = "123e4567-e89b-12d3-a456-426614174000/file";
+        uploadManager.deleteEntitySymlinkRootPathIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath);
+        fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1));
+    }
+}

utils/src/main/java/com/cloud/utils/FileUtil.java

@@ -160,4 +160,19 @@ public static boolean writeToFile(String fileName, String content) {
     public static String readResourceFile(String resource) throws IOException {
         return IOUtils.toString(Objects.requireNonNull(Thread.currentThread().getContextClassLoader().getResourceAsStream(resource)), com.cloud.utils.StringUtils.getPreferredCharset());
     }
+
+    public static boolean deleteRecursively(Path path) throws IOException {
+        LOGGER.debug("Deleting path: {}", path);
+        if (Files.isDirectory(path)) {
+            try (Stream<Path> entries = Files.list(path)) {
+                List<Path> list = entries.collect(Collectors.toList());
+                for (Path entry : list) {
+                    if (!deleteRecursively(entry)) {
+                        return false;
+                    }
+                }
+            }
+        }
+        return Files.deleteIfExists(path);
+    }
 }