#462 : Fix read only mounting

Should now work more flexibly (and still on windows when using paths in the format 'c:\...'
fabric8io · May 30, 2016 · 7b8739a · 7b8739a
1 parent ff82767
commit 7b8739a
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 16 deletions.
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -1,5 +1,9 @@
 # ChangeLog
 
+* **0.15-SNAPSHOT**
+  - Update dependencies: Apache HttpClient 4.5.2, JMockit 1.23, ...
+  - Fix read-only bindings (#462) 
+
 * **0.15.3** (2016-05-27)
   - Add duration information when pulling, building and pushing images ([#313](https://github.com/fabric8io/docker-maven-plugin/issues/313))
   - Fixed logging to always use format strings ([#457](https://github.com/fabric8io/docker-maven-plugin/issues/457))

diff --git a/src/main/java/io/fabric8/maven/docker/access/ContainerCreateConfig.java b/src/main/java/io/fabric8/maven/docker/access/ContainerCreateConfig.java
@@ -3,6 +3,7 @@
 import java.io.*;
 import java.util.*;
 
+import io.fabric8.maven.docker.util.EnvUtil;
 import org.apache.commons.lang3.text.StrSubstitutor;
 import io.fabric8.maven.docker.config.Arguments;
 import org.json.JSONArray;
@@ -137,13 +138,14 @@ private ContainerCreateConfig add(String name, Object value) {
     }
 
     private String extractContainerPath(String volume) {
-        if (volume.contains(":")) {
-            String[] parts = volume.split(":");
+        String path  = EnvUtil.fixupPath(volume);
+        if (path.contains(":")) {
+            String[] parts = path.split(":");
             if (parts.length > 1) {
-                return parts[parts.length - 1];
+                return parts[1];
             }
         }
-        return volume;
+        return path;
     }
 
     private void addEnvironment(Properties envProps) {

diff --git a/src/main/java/io/fabric8/maven/docker/access/ContainerHostConfig.java b/src/main/java/io/fabric8/maven/docker/access/ContainerHostConfig.java
@@ -7,6 +7,7 @@
 import java.util.Map;
 
 import io.fabric8.maven.docker.config.LogConfiguration;
+import io.fabric8.maven.docker.util.EnvUtil;
 import org.json.JSONArray;
 import org.json.JSONObject;
 
@@ -21,10 +22,9 @@ public ContainerHostConfig binds(List<String> bind) {
             JSONArray binds = new JSONArray();
 
             for (String volume : bind) {
+                volume = EnvUtil.fixupPath(volume);
+
                 if (volume.contains(":")) {
-                    // Hack-fix for mounting on Windows where the ${projectDir} variable and other
-                    // contain backslashes and what not. Related to #188
-                    volume = volume.replace("\\", "/").replaceAll("^(?i:C:)", "/c");
                     binds.put(volume);
                 }
             }

diff --git a/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java b/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java
@@ -198,7 +198,23 @@ public static String extractMavenPropertyName(String propName) {
         }
     }
 
-
+    /**
+     * Fix path on Windows machines, i.e. convert 'c:\...\' to '/c/..../'
+     *
+     * @param path path to fix
+     * @return the fixed path
+     */
+    public static String fixupPath(String path) {
+        // Hack-fix for mounting on Windows where the ${projectDir} variable and other
+        // contain backslashes and what not. Related to #188
+        Pattern pattern = Pattern.compile("^(?i)([A-Z]):(.*)$");
+        Matcher matcher = pattern.matcher(path);
+        if (matcher.matches()) {
+            String result = "/" + matcher.group(1).toLowerCase() + matcher.group(2);
+            return result.replace("\\","/");
+        }
+        return path;
+    }
 
     /**
      * Calculate the duration between now and the given time

diff --git a/src/test/java/io/fabric8/maven/docker/access/ContainerCreateConfigTest.java b/src/test/java/io/fabric8/maven/docker/access/ContainerCreateConfigTest.java
@@ -60,7 +60,8 @@ public void testEnvironmentEmptyPropertiesFile() {
     public void testBind() {
         String[] testData = new String[] {
             "c:\\this\\is\\my\\path:/data", "/data",
-            "/home/user:/user", "/user"};
+            "/home/user:/user", "/user",
+            "c:\\this\\too:/data:ro", "/data"};
         for (int i = 0; i < testData.length; i += 2) {
             ContainerCreateConfig cc = new ContainerCreateConfig("testImage");
             cc.binds(Arrays.asList(testData[i]));

diff --git a/src/test/java/io/fabric8/maven/docker/access/ContainerHostConfigTest.java b/src/test/java/io/fabric8/maven/docker/access/ContainerHostConfigTest.java
@@ -8,6 +8,7 @@
 import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.Test;
+import org.junit.experimental.theories.DataPoints;
 import org.skyscreamer.jsonassert.JSONAssert;
 import org.skyscreamer.jsonassert.JSONParser;
 
@@ -38,13 +39,19 @@ public void testMapExtraHosts() {
 
     @Test
     public void testBinds() throws Exception {
-        ContainerHostConfig hc = new ContainerHostConfig();
-        JSONObject result = (JSONObject) hc.binds(Arrays.asList("c:\\Users\\roland\\sample:/sample")).toJsonObject();
-        JSONObject expected = new JSONObject();
-        JSONArray binds = new JSONArray();
-        binds.put("/c/Users/roland/sample:/sample");
-        expected.put("Binds",binds);
-        JSONAssert.assertEquals(expected,result,false);
+        String[] data = {
+            "c:\\Users\\roland\\sample:/sample", "/c/Users/roland/sample:/sample",
+            "M:\\Users\\roland\\sample:/sample:ro", "/m/Users/roland/sample:/sample:ro"
+        };
+        for (int i = 0; i < data.length; i+=2) {
+            ContainerHostConfig hc = new ContainerHostConfig();
+            JSONObject result = (JSONObject) hc.binds(Arrays.asList(data[i])).toJsonObject();
+            JSONObject expected = new JSONObject();
+            JSONArray binds = new JSONArray();
+            binds.put(data[i+1]);
+            expected.put("Binds",binds);
+            JSONAssert.assertEquals(expected,result,false);
+        }
     }
 
 

diff --git a/src/test/java/io/fabric8/maven/docker/util/EnvUtilTest.java b/src/test/java/io/fabric8/maven/docker/util/EnvUtilTest.java
@@ -99,6 +99,22 @@ public void minimalVersion() {
             assertEquals(data[i+2],EnvUtil.extractLargerVersion(data[i],data[i+1]));
         }
     }
+
+    @Test
+    public void fixupPath() throws Exception {
+        String[] data = new String[] {
+            "my/regular/path", "my/regular/path",
+            "c:\\windows\\path", "/c/windows/path",
+            "Z:\\yet another\\path", "/z/yet another/path"
+        };
+
+        for (int i = 0; i < data.length; i+=2) {
+            assertEquals(data[i+1], EnvUtil.fixupPath(data[i]));
+        }
+
+
+    }
+
     private Properties getTestProperties(String ... vals) {
         Properties ret = new Properties();
         for (int i = 0; i < vals.length; i+=2) {
@@ -108,4 +124,6 @@ private Properties getTestProperties(String ... vals) {
     }
 
 
+
+
 }