Improve Docker options configuration and forward from mavenExecute (#123)

documentation/docs/steps/dockerExecute.md

@@ -17,7 +17,7 @@ Proxy environment variables defined on the Jenkins machine are also available in
 
 * `dockerImage` Name of the docker image that should be used. If empty, Docker is not used.
 * `dockerEnvVars` Environment variables to set in the container, e.g. [http_proxy:'proxy:8080']
-* `dockerOptions` Docker options to be set when starting the container.
+* `dockerOptions` Docker options to be set when starting the container. It can be a list or a string.
 * `dockerVolumeBind` Volumes that should be mounted into the container.
 
 ## Step configuration

test/groovy/DockerExecuteTest.groovy

@@ -17,7 +17,7 @@ import static org.junit.Assert.assertFalse
 
 class DockerExecuteTest extends BasePipelineTest {
     private DockerMock docker
-    
+
     private JenkinsLoggingRule jlr = new JenkinsLoggingRule(this)
     private JenkinsStepRule jsr = new JenkinsStepRule(this)
     private JenkinsEnvironmentRule jer = new JenkinsEnvironmentRule(this)
@@ -27,7 +27,7 @@ class DockerExecuteTest extends BasePipelineTest {
         .getCommonRules(this)
         .around(jlr)
         .around(jsr)
-    
+
     int whichDockerReturnValue = 0
     def bodyExecuted
 
@@ -51,7 +51,7 @@ class DockerExecuteTest extends BasePipelineTest {
 
         assertEquals('maven:3.5-jdk-8-alpine', docker.getImageName())
         assertTrue(docker.isImagePulled())
-        assertEquals(' --env http_proxy --env https_proxy --env no_proxy --env HTTP_PROXY --env HTTPS_PROXY --env NO_PROXY', docker.getParameters())
+        assertEquals('--env http_proxy --env https_proxy --env no_proxy --env HTTP_PROXY --env HTTPS_PROXY --env NO_PROXY', docker.getParameters().trim())
         assertTrue(bodyExecuted)
     }
 
@@ -65,13 +65,27 @@ class DockerExecuteTest extends BasePipelineTest {
                       dockerEnvVars: ['http_proxy': 'http://proxy:8000']) {
             bodyExecuted = true
         }
-        assertTrue(docker.getParameters().contains(' --env https_proxy '))
-        assertTrue(docker.getParameters().contains(' --env http_proxy=http://proxy:8000'))
-        assertTrue(docker.getParameters().contains(' -it'))
-        assertTrue(docker.getParameters().contains(' --volume my_vol:/my_vol'))
+        assertTrue(docker.getParameters().contains('--env https_proxy '))
+        assertTrue(docker.getParameters().contains('--env http_proxy=http://proxy:8000'))
+        assertTrue(docker.getParameters().contains('-it'))
+        assertTrue(docker.getParameters().contains('--volume my_vol:/my_vol'))
         assertTrue(bodyExecuted)
     }
 
+    @Test
+    void testExecuteDockerWithDockerOptionsList() throws Exception {
+        jsr.step.call(script: [commonPipelineEnvironment: jer.env],
+            dockerImage: 'maven:3.5-jdk-8-alpine',
+            dockerOptions: ['-it', '--network=my-network'],
+            dockerEnvVars: ['http_proxy': 'http://proxy:8000']) {
+            bodyExecuted = true
+        }
+        assertTrue(docker.getParameters().contains('--env https_proxy '))
+        assertTrue(docker.getParameters().contains('--env http_proxy=http://proxy:8000'))
+        assertTrue(docker.getParameters().contains('-it'))
+        assertTrue(docker.getParameters().contains('--network=my-network'))
+    }
+
     @Test
     void testDockerNotInstalledResultsInLocalExecution() throws Exception {
 

test/groovy/MavenExecuteTest.groovy

@@ -66,4 +66,13 @@ class MavenExecuteTest extends BasePipelineTest {
         String mvnCommand = "mvn --global-settings 'globalSettingsFile.xml' -Dmaven.repo.local='m2Path' --settings 'projectSettingsFile.xml' --file 'pom.xml' -o clean install -Dmaven.tests.skip=true"
         assertTrue(jscr.shell.contains(mvnCommand))
     }
+
+    @Test
+    void testMavenCommandForwardsDockerOptions() throws Exception {
+
+        mavenExecuteScript.call(script: [commonPipelineEnvironment: cpe], goals: 'clean install')
+        assertEquals('maven:3.5-jdk-7', dockerParameters.dockerImage)
+
+        assert jscr.shell[0] == 'mvn clean install'
+    }
 }

vars/dockerExecute.groovy

@@ -6,10 +6,10 @@ def call(Map parameters = [:], body) {
     def PLUGIN_ID_DOCKER_WORKFLOW = 'docker-workflow'
 
     handlePipelineStepErrors(stepName: STEP_NAME, stepParameters: parameters){
-        def dockerImage = parameters.get('dockerImage', '')
-        Map dockerEnvVars = parameters.get('dockerEnvVars', [:])
-        def dockerOptions = parameters.get('dockerOptions', '')
-        Map dockerVolumeBind = parameters.get('dockerVolumeBind', [:])
+        def dockerImage = parameters.dockerImage ?: ''
+        Map dockerEnvVars = parameters.dockerEnvVars ?: [:]
+        def dockerOptions = parameters.dockerOptions ?: ''
+        Map dockerVolumeBind = parameters.dockerVolumeBind ?: [:]
 
         if(dockerImage) {
 
@@ -60,27 +60,34 @@ private getDockerOptions(Map dockerEnvVars, Map dockerVolumeBind, def dockerOpti
         'HTTPS_PROXY',
         'NO_PROXY'
     ]
-    def options = ""
+    def options = []
     if (dockerEnvVars) {
         for (String k : dockerEnvVars.keySet()) {
-            options += " --env ${k}=" + dockerEnvVars[k].toString()
+            options.add("--env ${k}=${dockerEnvVars[k].toString()}")
         }
     }
 
     for (String envVar : specialEnvironments) {
         if (dockerEnvVars == null || !dockerEnvVars.containsKey(envVar)) {
-            options += " --env ${envVar}"
+            options.add("--env ${envVar}")
         }
     }
 
     if (dockerVolumeBind) {
         for (String k : dockerVolumeBind.keySet()) {
-            options += " --volume ${k}:" + dockerVolumeBind[k].toString()
+            options.add("--volume ${k}:${dockerVolumeBind[k].toString()}")
         }
     }
 
-    if (dockerOptions) {
-        options += " ${dockerOptions}"
+    if (dockerOptions instanceof CharSequence) {
+        options.add(dockerOptions.toString())
+    } else if (dockerOptions instanceof List) {
+        for (String option : dockerOptions) {
+            options.add "${option}"
+        }
+    } else {
+        throw new IllegalArgumentException("Unexpected type for dockerOptions. Expected was either a list or a string. Actual type was: '${dockerOptions.getClass()}'")
     }
-    return options
+
+    return options.join(' ')
 }

vars/mavenExecute.groovy

@@ -10,6 +10,7 @@ def call(Map parameters = [:]) {
 
         Set parameterKeys = [
             'dockerImage',
+            'dockerOptions',
             'globalSettingsFile',
             'projectSettingsFile',
             'pomPath',
@@ -74,7 +75,7 @@ def call(Map parameters = [:]) {
             command += " ${defines}"
         }
 
-        dockerExecute(dockerImage: configuration.dockerImage) { sh command }
+        dockerExecute(dockerImage: configuration.dockerImage, dockerOptions: configuration.dockerOptions) { sh command }
     }
 }