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 828f61f89dc8..57a85c570949 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 @@ -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; diff --git a/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java b/services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/template/UploadManagerImplTest.java new file mode 100644 index 000000000000..045e1df83a8f --- /dev/null +++ b/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 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)); + } +} diff --git a/utils/src/main/java/com/cloud/utils/FileUtil.java b/utils/src/main/java/com/cloud/utils/FileUtil.java index eea7c0f85617..8a1d3d32ec19 100644 --- a/utils/src/main/java/com/cloud/utils/FileUtil.java +++ b/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 entries = Files.list(path)) { + List list = entries.collect(Collectors.toList()); + for (Path entry : list) { + if (!deleteRecursively(entry)) { + return false; + } + } + } + } + return Files.deleteIfExists(path); + } }