fix(ecr): Properly check if host-persistent-path is the name of a Docker volume (#678)

Author: cfranzen

src/main/java/io/github/hectorvent/floci/core/common/docker/ContainerLifecycleManager.java

@@ -5,6 +5,7 @@
 import com.github.dockerjava.api.command.CreateContainerCmd;
 import com.github.dockerjava.api.command.CreateContainerResponse;
 import com.github.dockerjava.api.command.InspectContainerResponse;
+import com.github.dockerjava.api.exception.DockerException;
 import com.github.dockerjava.api.exception.NotFoundException;
 import com.github.dockerjava.api.model.Bind;
 import com.github.dockerjava.api.model.Container;
@@ -302,6 +303,42 @@ public DockerClient getDockerClient() {
         return dockerClient;
     }
 
+    /**
+     * Returns {@code true} if the container runtime (Docker, Moby, or Podman) has a volume
+     * with the given name. The volume does not need to be attached to the current container.
+     * <p>
+     * This method uses the Docker Engine API ({@code /volumes/{name}}) which is supported
+     * by Docker, Moby, and Podman runtimes on all operating systems.
+     *
+     * @param name the volume name to look up
+     * @return {@code true} if the volume exists, {@code false} otherwise
+     */
+    public boolean volumeExists(String name) {
+        if (name == null || name.isBlank()) {
+            return false;
+        }
+        // Is a Unix absolute or relative path (e.g. "/var/lib/data", "./data", "../data")
+        if (name.startsWith("/") || name.startsWith(".")) {
+            return false;
+        }
+        // Is a Windows absolute path (e.g. "C:\Users\data", "D:/sources/data")
+        if (name.length() >= 3 && Character.isLetter(name.charAt(0))
+                && name.charAt(1) == ':' && (name.charAt(2) == '\\' || name.charAt(2) == '/')) {
+            return false;
+        }
+        try {
+            dockerClient.inspectVolumeCmd(name).exec();
+            LOG.debugv("Volume ''{0}'' exists in the container runtime", name);
+            return true;
+        } catch (NotFoundException e) {
+            LOG.debugv("Volume ''{0}'' not found in the container runtime", name);
+            return false;
+        } catch (DockerException e) {
+            LOG.warnv("Failed to inspect volume ''{0}'': {1}", name, e.getMessage());
+            return false;
+        }
+    }
+
     private HostConfig buildHostConfig(ContainerSpec spec) {
         HostConfig hostConfig = HostConfig.newHostConfig();
 

src/main/java/io/github/hectorvent/floci/services/ecr/registry/EcrRegistryManager.java

@@ -180,8 +180,7 @@ public synchronized void ensureStarted() {
     private void addPersistenceMounts(ContainerBuilder.Builder specBuilder, List<String> env) {
         String hostPersistentPath = config.storage().hostPersistentPath();
         boolean inContainer = containerDetector.isRunningInContainer();
-        boolean isExplicitVolumeName = !hostPersistentPath.startsWith("/")
-                && !hostPersistentPath.startsWith(".");
+        boolean isExplicitVolume = lifecycleManager.volumeExists(hostPersistentPath);
         boolean isRelativeDefault = hostPersistentPath.startsWith(".");
 
         if (inContainer && isRelativeDefault) {
@@ -190,10 +189,10 @@ private void addPersistenceMounts(ContainerBuilder.Builder specBuilder, List<Str
             LOG.infov("Floci in container with relative host-persistent-path ({0}); "
                     + "using named volume {1} for ECR registry data",
                     hostPersistentPath, NAMED_VOLUME);
-        } else if (isExplicitVolumeName) {
+        } else if (isExplicitVolume) {
             // User set hostPersistentPath to a Docker named-volume name
             String internalMountPath = "/app/data";
-            specBuilder.withBind(hostPersistentPath, internalMountPath);
+            specBuilder.withNamedVolume(hostPersistentPath, internalMountPath);
             env.add("REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY=" + internalMountPath + "/ecr/registry");
         } else {
             // Host path bind-mount.

src/test/java/io/github/hectorvent/floci/core/common/docker/ContainerLifecycleManagerVolumeTest.java

@@ -0,0 +1,103 @@
+package io.github.hectorvent.floci.core.common.docker;
+
+import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.command.InspectVolumeCmd;
+import com.github.dockerjava.api.command.InspectVolumeResponse;
+import com.github.dockerjava.api.exception.DockerException;
+import com.github.dockerjava.api.exception.NotFoundException;
+import io.github.hectorvent.floci.services.lambda.launcher.ImageCacheService;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+@ExtendWith(MockitoExtension.class)
+class ContainerLifecycleManagerVolumeTest {
+
+    @Mock
+    private DockerClient dockerClient;
+
+    @Mock
+    private ImageCacheService imageCacheService;
+
+    @Mock
+    private ContainerDetector containerDetector;
+
+    @Mock
+    private PortAllocator portAllocator;
+
+    private ContainerLifecycleManager manager;
+
+    @BeforeEach
+    void setUp() {
+        manager = new ContainerLifecycleManager(dockerClient, imageCacheService, containerDetector, portAllocator);
+    }
+
+    @Test
+    void volumeExists_returnsTrue_whenVolumeExists() {
+        InspectVolumeCmd cmd = mock(InspectVolumeCmd.class);
+        when(dockerClient.inspectVolumeCmd("my-volume")).thenReturn(cmd);
+        when(cmd.exec()).thenReturn(mock(InspectVolumeResponse.class));
+
+        assertTrue(manager.volumeExists("my-volume"));
+    }
+
+    @Test
+    void volumeExists_returnsFalse_whenVolumeNotFound() {
+        InspectVolumeCmd cmd = mock(InspectVolumeCmd.class);
+        when(dockerClient.inspectVolumeCmd("nonexistent")).thenReturn(cmd);
+        when(cmd.exec()).thenThrow(new NotFoundException("No such volume"));
+
+        assertFalse(manager.volumeExists("nonexistent"));
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forNullName() {
+        assertFalse(manager.volumeExists(null));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forBlankName() {
+        assertFalse(manager.volumeExists("  "));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forAbsolutePath() {
+        assertFalse(manager.volumeExists("/var/lib/data"));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forRelativePath() {
+        assertFalse(manager.volumeExists("./data"));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forWindowsAbsolutePathBackslash() {
+        assertFalse(manager.volumeExists("C:\\Users\\data"));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_forWindowsAbsolutePathForwardSlash() {
+        assertFalse(manager.volumeExists("D:/sources/data"));
+        verifyNoInteractions(dockerClient);
+    }
+
+    @Test
+    void volumeExists_returnsFalse_onDockerException() {
+        InspectVolumeCmd cmd = mock(InspectVolumeCmd.class);
+        when(dockerClient.inspectVolumeCmd("some-volume")).thenReturn(cmd);
+        when(cmd.exec()).thenThrow(new DockerException("Connection refused", 500));
+
+        assertFalse(manager.volumeExists("some-volume"));
+    }
+}
+