SSVM: entity download URL creation: ensure userdata base dir exists

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

@@ -64,6 +64,7 @@ public class UploadManagerImpl extends ManagerBase implements UploadManager {
 
     protected static final String EXTRACT_USERDATA_DIR = "userdata";
     protected static final String BASE_EXTRACT_PATH = String.format("/var/www/html/%s/", EXTRACT_USERDATA_DIR);
+    protected static final String APACHE_USER = "www-data";
 
     public class Completion implements UploadCompleteCallback {
         private final String jobId;
@@ -276,13 +277,19 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
         // Create the directory structure so that its visible under apache server root
         String extractDir = BASE_EXTRACT_PATH;
         extractDir = extractDir + cmd.getFilepathInExtractURL() + File.separator;
+        String result = ensureBaseExtractDirectory();
+        if (result != null) {
+            String errorString = String.format("Error in creating base extract directory %s owned by %s: %s", BASE_EXTRACT_PATH, APACHE_USER, result);
+            logger.error(errorString);
+            return new CreateEntityDownloadURLAnswer(errorString, CreateEntityDownloadURLAnswer.RESULT_FAILURE);
+        }
         Script command = new Script("/bin/su", logger);
         command.add("-s");
         command.add("/bin/bash");
         command.add("-c");
         command.add("mkdir -p " + extractDir);
         command.add("www-data");
-        String result = command.execute();
+        result = command.execute();
         if (result != null) {
             String errorString = "Error in creating directory =" + result;
             logger.error(errorString);
@@ -325,6 +332,29 @@ public CreateEntityDownloadURLAnswer handleCreateEntityURLCommand(CreateEntityDo
 
     }
 
+    /**
+     * Makes sure the base extract directory exists and is owned by the apache user, so that the apache user
+     * can create the per-entity subdirectory inside it. Stock systemvm templates do not ship this directory.
+     * @return null on success, the error output otherwise
+     */
+    protected String ensureBaseExtractDirectory() {
+        if (isBaseExtractDirectoryReady(Path.of(BASE_EXTRACT_PATH))) {
+            return null;
+        }
+        Script command = new Script("install", logger);
+        command.add("-d", "-o", APACHE_USER, "-g", APACHE_USER, "-m", "755", BASE_EXTRACT_PATH);
+        return command.execute();
+    }
+
+    protected boolean isBaseExtractDirectoryReady(Path baseDir) {
+        try {
+            return Files.isDirectory(baseDir) && APACHE_USER.equals(Files.getOwner(baseDir).getName());
+        } catch (IOException e) {
+            logger.debug("Unable to determine owner of {}: {}", baseDir, e.getMessage());
+            return false;
+        }
+    }
+
     @Override
     public Answer handleDeleteEntityDownloadURLCommand(DeleteEntityDownloadURLCommand cmd) {
 

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

@@ -16,29 +16,39 @@
 // under the License.
 package org.apache.cloudstack.storage.template;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockConstruction;
 import static org.mockito.Mockito.mockStatic;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.MockedConstruction;
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
+import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
 import com.cloud.utils.FileUtil;
+import com.cloud.utils.script.Script;
 
 @RunWith(MockitoJUnitRunner.class)
 public class UploadManagerImplTest {
 
+    @Spy
     @InjectMocks
     UploadManagerImpl uploadManager;
 
@@ -82,4 +92,40 @@
         uploadManager.deleteEntitySymlinkRootDirectoryIfNeeded(mock(DeleteEntityDownloadURLCommand.class), validLinkPath);
         fileUtilMock.verify(() -> FileUtil.deleteRecursively(any(Path.class)), times(1));
     }
+
+    @Test
+    public void baseExtractDirectoryIsNotReadyWhenMissing() {
+        assertFalse(uploadManager.isBaseExtractDirectoryReady(Path.of("/nonexistent/cloudstack/userdata/")));
+    }
+
+    @Test
+    public void baseExtractDirectoryIsNotReadyWhenNotOwnedByApacheUser() throws Exception {
+        Path dir = Files.createTempDirectory("userdata");
+        try {
+            assertFalse(uploadManager.isBaseExtractDirectoryReady(dir));
+        } finally {
+            Files.delete(dir);
+        }
+    }
+
+    @Test
+    public void ensureBaseExtractDirectorySkipsInstallWhenReady() {
+        doReturn(true).when(uploadManager).isBaseExtractDirectoryReady(any(Path.class));
+        try (MockedConstruction<Script> scripts = mockConstruction(Script.class)) {
+            assertNull(uploadManager.ensureBaseExtractDirectory());
+            Assert.assertTrue(scripts.constructed().isEmpty());
+        }
+    }
+
+    @Test
+    public void ensureBaseExtractDirectoryInstallsWhenNotReady() {
+        doReturn(false).when(uploadManager).isBaseExtractDirectoryReady(any(Path.class));
+        try (MockedConstruction<Script> scripts = mockConstruction(Script.class,
+                (script, context) -> Mockito.when(script.execute()).thenReturn("Permission denied"))) {
+            Assert.assertEquals("Permission denied", uploadManager.ensureBaseExtractDirectory());
+            Assert.assertEquals(1, scripts.constructed().size());
+            Mockito.verify(scripts.constructed().get(0)).add("-d", "-o", UploadManagerImpl.APACHE_USER, "-g", UploadManagerImpl.APACHE_USER,
+                    "-m", "755", UploadManagerImpl.BASE_EXTRACT_PATH);
+        }
+    }
 }