This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new b56bbc22934 SSVM: entity download URL creation: ensure userdata base
dir exists (#14447)
b56bbc22934 is described below
commit b56bbc22934d8aecc2fe507d0b2e757ae634512b
Author: dahn <[email protected]>
AuthorDate: Tue Nov 10 10:23:08 2026 +0100
SSVM: entity download URL creation: ensure userdata base dir exists (#14447)
Co-authored-by: waterWang <[email protected]>
---
.../storage/template/UploadManagerImpl.java | 32 ++++++++++++++-
.../storage/template/UploadManagerImplTest.java | 46 ++++++++++++++++++++++
2 files changed, 77 insertions(+), 1 deletion(-)
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 aacca9926f7..a57dc18c600 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
@@ -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 class UploadManagerImpl extends ManagerBase
implements UploadManager {
// 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 class UploadManagerImpl extends ManagerBase
implements UploadManager {
}
+ /**
+ * 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) {
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
index 9038098e235..4e86a7f1f52 100644
---
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
@@ -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 @@ public class UploadManagerImplTest {
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);
+ }
+ }
}
