fabric8io · stromnet · Feb 5, 2018
diff --git a/src/main/java/io/fabric8/maven/docker/access/ContainerCreateConfig.java b/src/main/java/io/fabric8/maven/docker/access/ContainerCreateConfig.java
@@ -66,6 +66,9 @@ public ContainerCreateConfig environment(String envPropsFile, Map<String, String
             addPropertiesFromFile(envPropsFile, envProps);
         }
 
+        // Props from Maven environment takes highest precedence.
+        addEnvironmentFromMavenProperties(envProps, mavenProps);
+
         if (envProps.size() > 0) {
             addEnvironment(envProps);
         }
@@ -168,4 +171,10 @@ private void addPropertiesFromFile(String envPropsFile, Properties envProps) {
             throw new IllegalArgumentException(String.format("Error while loading environment properties: %s", e.getMessage()), e);
         }
     }
+
+    private void addEnvironmentFromMavenProperties(Map envProps, Map mavenProps) {
+        String argPrefix = "docker.runEnv";
+        EnvUtil.extractMavenAndSystemProperties(argPrefix, mavenProps, false, envProps);
+    }
+
 }
diff --git a/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java b/src/main/java/io/fabric8/maven/docker/config/handler/property/PropertyConfigHandler.java
@@ -314,7 +314,7 @@ private List<String> listWithPrefix(String prefix, ConfigKey key, Properties pro
     }
 
     private Map<String, String> mapWithPrefix(String prefix, ConfigKey key, Properties properties) {
-        return extractFromPropertiesAsMap(key.asPropertyKey(prefix), properties);
+        return extractFromPropertiesAsMap(key.asPropertyKey(prefix), properties, true);
     }
 
     private String withPrefix(String prefix, ConfigKey key, Properties properties) {

diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildService.java b/src/main/java/io/fabric8/maven/docker/service/BuildService.java
@@ -163,29 +163,14 @@ private String doBuildImage(String imageName, File dockerArchive, BuildOptions o
     }
 
     private Map<String, String> addBuildArgs(BuildContext buildContext) {
-        Map<String, String> buildArgsFromProject = addBuildArgsFromProperties(buildContext.getMojoParameters().getProject().getProperties());
-        Map<String, String> buildArgsFromSystem = addBuildArgsFromProperties(System.getProperties());
-        return ImmutableMap.<String, String>builder()
-                .putAll(buildContext.getBuildArgs() != null ? buildContext.getBuildArgs() : Collections.<String, String>emptyMap())
-                .putAll(buildArgsFromProject)
-                .putAll(buildArgsFromSystem)
-                .build();
-    }
-
-    private Map<String, String> addBuildArgsFromProperties(Properties properties) {
-        String argPrefix = "docker.buildArg.";
+        String argPrefix = "docker.buildArg";
         Map<String, String> buildArgs = new HashMap<>();
-        for (Object keyObj : properties.keySet()) {
-            String key = (String) keyObj;
-            if (key.startsWith(argPrefix)) {
-                String argKey = key.replaceFirst(argPrefix, "");
-                String value = properties.getProperty(key);
-
-                if (!isEmpty(value)) {
-                    buildArgs.put(argKey, value);
-                }
-            }
-        }
+
+        if(buildContext.getBuildArgs() != null)
+            buildArgs.putAll(buildContext.getBuildArgs());
+
+        EnvUtil.extractMavenAndSystemProperties(argPrefix, buildContext.getMojoParameters().getProject().getProperties(), false, buildArgs);
+
         log.debug("Build args set %s", buildArgs);
         return buildArgs;
     }
@@ -247,11 +232,6 @@ private boolean checkForNocache(ImageConfiguration imageConfig) {
         }
     }
 
-    private boolean isEmpty(String str) {
-        return str == null || str.isEmpty();
-    }
-
-
     // ===========================================
 
 

diff --git a/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java b/src/main/java/io/fabric8/maven/docker/util/EnvUtil.java
@@ -197,26 +197,58 @@ public static String stringJoin(List list, String separator) {
         return ret.toString();
     }
 
+    /**
+     * Extract any properties matching the given prefix from a given Map, and join with any System properites.
+     *
+     * Intended for use to get Maven properties and any overrides from mvn -D flags.
+     * Intentionally not using String generics so it can be used with Properties instances.
+     *
+     * @param prefix prefix which specifies the part which should be extracted as map (without trailing dot)
+     * @param mavenProperties source properties to extract from
+     * @param includeEmpty If false, only add fields for which the value is non-null and non-empty
+     * @param target A Map which to put any found values in.
+     */
+    public static void extractMavenAndSystemProperties(String prefix, Map mavenProperties, boolean includeEmpty, Map target) {
+        EnvUtil.extractFromMap(prefix, mavenProperties, includeEmpty, target);
+        EnvUtil.extractFromMap(prefix, System.getProperties(), includeEmpty, target);
+    }
+
     /**
      * Extract part of given properties as a map. The given prefix is used to find the properties,
      * the rest of the property name is used as key for the map.
      *
-     * @param prefix prefix which specifies the part which should be extracted as map
+     * @param prefix prefix which specifies the part which should be extracted as map (without trailing dot)
      * @param properties properties to extract from
-     * @return the extracted map or null if no such map exists
+     * @param includeEmpty If false, only add fields for which the value is non-null and non-empty.
+     * @return the extracted map or null if no matching entries exists
      */
-    public static Map<String, String> extractFromPropertiesAsMap(String prefix, Properties properties) {
+    public static Map<String, String> extractFromPropertiesAsMap(String prefix, Properties properties, boolean includeEmpty) {
         Map<String, String> ret = new HashMap<>();
-        Enumeration names = properties.propertyNames();
+        extractFromMap(prefix, properties, includeEmpty, ret);
+        return ret.size() > 0 ? ret : null;
+    }
+
+    /**
+     * Extract part of given map to another map. The given prefix is used to find the matching string key,
+     * the rest of the string key name is used as key for the map.
+     *
+     * Intentionally not using String generics so it can be used with Properties instances.
+     *
+     * @param prefix prefix which specifies the part which should be extracted as map (without trailing dot)
+     * @param src Map to extract from
+     * @param includeEmpty If false, only add fields for which the value is non-null and non-empty.
+     * @param target A Map which to put any found values in.
+     */
+    public static void extractFromMap(String prefix, Map src, boolean includeEmpty, Map target) {
         String prefixP = prefix + ".";
-        while (names.hasMoreElements()) {
-            String propName = (String) names.nextElement();
-            if (propMatchesPrefix(prefixP, propName)) {
-                String mapKey = propName.substring(prefixP.length());
-                ret.put(mapKey, properties.getProperty(propName));
+        for (Object propName : src.keySet()) {
+            if (propName instanceof String && propMatchesPrefix(prefixP, (String)propName)) {
+                String mapKey = ((String)propName).substring(prefixP.length());
+                Object value = src.get(propName);
+                if(value instanceof String && (includeEmpty || !isEmpty((String)value)))
+                    target.put(mapKey, value);
             }
         }
-        return ret.size() > 0 ? ret : null;
     }
 
     /**
@@ -328,6 +360,9 @@ public static String formatDurationTill(long start) {
 
     // ======================================================================================================
 
+    private static boolean isEmpty(String str) {
+        return str == null || str.isEmpty();
+    }
     private static boolean propMatchesPrefix(String prefix, String key) {
         return key.startsWith(prefix) && key.length() >= prefix.length();
     }

diff --git a/src/test/java/io/fabric8/maven/docker/access/ContainerCreateConfigTest.java b/src/test/java/io/fabric8/maven/docker/access/ContainerCreateConfigTest.java
@@ -7,6 +7,7 @@
 import org.apache.commons.io.FileUtils;
 import org.json.JSONArray;
 import org.json.JSONObject;
+import org.junit.After;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -34,6 +35,11 @@
  */
 public class ContainerCreateConfigTest {
 
+    @After
+    public void tearDown() throws Exception {
+        System.clearProperty("docker.runEnv.otherProp");
+    }
+
     @Test
     public void testEnvironment() throws Exception {
         ContainerCreateConfig cc = new ContainerCreateConfig("testImage");
@@ -97,6 +103,39 @@ public void testNoPropFile() {
         cc.environment("/not/really/a/file",null,Collections.<String, String>emptyMap());
     }
 
+    @Test
+    public void testEnvFromMavenAndSystemProperties() throws Exception {
+        ContainerCreateConfig cc = new ContainerCreateConfig("testImage");
+
+        Map<String, String> envMap = getEnvMap();
+        Map<String, String> mavenProps = new HashMap<>();
+        mavenProps.put("docker.runEnv.http_proxy", "http://proxy.example.com:8080");
+        mavenProps.put("docker.buildArg.https_proxy", "ignored");
+
+        cc.environment(copyPropsToFile(), envMap, mavenProps);
+
+        JSONArray env = getEnvArray(cc);
+        assertNotNull(env);
+        assertEquals(4, env.length());
+        List<String> envAsString = convertToList(env);
+        assertTrue(envAsString.contains("JAVA_OPTS=-Xmx512m"));
+        assertTrue(envAsString.contains("TEST_SERVICE=SECURITY"));
+        assertTrue(envAsString.contains("EXTERNAL_ENV=TRUE"));
+        assertTrue(envAsString.contains("http_proxy=http://proxy.example.com:8080"));
+
+        System.setProperty("docker.runEnv.otherProp", "testvalue");
+        cc.environment(copyPropsToFile(), envMap, mavenProps);
+
+        env = getEnvArray(cc);
+        assertNotNull(env);
+        assertEquals(5, env.length());
+        envAsString = convertToList(env);
+        assertTrue(envAsString.contains("JAVA_OPTS=-Xmx512m"));
+        assertTrue(envAsString.contains("TEST_SERVICE=SECURITY"));
+        assertTrue(envAsString.contains("EXTERNAL_ENV=TRUE"));
+        assertTrue(envAsString.contains("http_proxy=http://proxy.example.com:8080"));
+        assertTrue(envAsString.contains("otherProp=testvalue"));
+    }
 
     private JSONArray getEnvArray(ContainerCreateConfig cc) {
         JSONObject config = new JSONObject(cc.toJson());

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,13 +99,73 @@ public void extractMapFromProperties() {
         Properties props = getTestProperties(
                 "bla.hello","world",
                 "bla.max","morlock",
-                "blub.not","aMap");
-        Map<String,String> result = EnvUtil.extractFromPropertiesAsMap("bla", props);
+                "blub.not","aMap",
+                "bla.empty", "");
+
+        Map<String,String> result = EnvUtil.extractFromPropertiesAsMap("bla", props, true);
+        assertEquals(3,result.size());
+        assertEquals("world",result.get("hello"));
+        assertEquals("morlock",result.get("max"));
+        assertEquals("",result.get("empty"));
+
+        result = EnvUtil.extractFromPropertiesAsMap("bla", props, false);
         assertEquals(2,result.size());
         assertEquals("world",result.get("hello"));
         assertEquals("morlock",result.get("max"));
     }
 
+    @Test
+    public void extractFromMap() {
+        Map<String, String> src = getTestMap(
+                "bla.hello","world",
+                "bla.max","morlock",
+                "blub.not","aMap",
+                "bla.empty", "");
+
+        Map<String, String> result = new HashMap<>();
+        EnvUtil.extractFromMap("bla", src, true, result);
+        assertEquals(3,result.size());
+        assertEquals("world",result.get("hello"));
+        assertEquals("morlock",result.get("max"));
+        assertEquals("",result.get("empty"));
+
+        // test sending to Properties
+        Properties resultP = new Properties();
+        EnvUtil.extractFromMap("bla", src, false, resultP);
+        assertEquals(2,resultP.size());
+        assertEquals("world",resultP.get("hello"));
+        assertEquals("morlock",resultP.get("max"));
+    }
+
+    @Test
+    public void extractMavenAndSystemProperties() {
+        Properties src = getTestProperties(
+                "bla.hello","world",
+                "bla.max","morlock",
+                "blub.not","aMap",
+                "bla.empty", "");
+
+        System.setProperty("bla.sysprop", "someSysProp");
+        System.setProperty("bla.emptyprop", "");
+
+        Map<String, String> result = new HashMap<>();
+        EnvUtil.extractMavenAndSystemProperties("bla", src, true, result);
+        assertEquals(5,result.size());
+        assertEquals("world",result.get("hello"));
+        assertEquals("morlock",result.get("max"));
+        assertEquals("someSysProp",result.get("sysprop"));
+        assertEquals("",result.get("emptyprop"));
+        assertEquals("",result.get("empty"));
+
+        // test properties alternative, which is also Map
+        Properties resultP = new Properties();
+        EnvUtil.extractMavenAndSystemProperties("bla", src, false, resultP);
+        assertEquals(3,resultP.size());
+        assertEquals("world",resultP.get("hello"));
+        assertEquals("morlock",resultP.get("max"));
+        assertEquals("someSysProp",result.get("sysprop"));
+    }
+
     @Test
     public void extractListFromProperties() {
         Properties props = getTestProperties(
@@ -189,5 +249,13 @@ private Properties getTestProperties(String ... vals) {
         return ret;
     }
 
+    private Map<String, String> getTestMap(String ... vals) {
+        Map<String, String> ret = new HashMap();
+        for (int i = 0; i < vals.length; i+=2) {
+            ret.put(vals[i],vals[i+1]);
+        }
+        return ret;
+    }
+
     private Object $(Object ... o) { return o; }
 }