Added unit test and refactored a few things

Author: arthurvanduynhoven

README.md

@@ -26,6 +26,49 @@ thereby keeping configuration more [DRY](http://en.wikipedia.org/wiki/Don%27t_re
 
 See [Docker Pipeline Plugin](https://plugins.jenkins.io/docker-workflow) for the typical usage.
 
+### Multiple Private/Authenticated Registries
+
+In some scenarios you need to authenticate between multiple registries from within a single pipeline step/command. Below
+are some examples to allow this:
+
+*Scripted*
+```groovy
+node('docker') {
+  docker.withRegistry('private-repo1-url', 'id-for-a-docker-cred-repo1') {
+    docker.withRegistry('private-repo2-url', 'id-for-a-docker-cred-repo2') {
+      writeFile file: 'Dockerfile', text: '''
+          FROM private-repo1-url/image
+          COPY someFile /
+          ENTRYPOINT /someFile'''
+      sh 'docker build --tag private-repo2-url/myapp  .'
+    }
+  }
+}
+```
+
+*Declarative*
+```groovy
+pipeline {
+  agent docker
+  stages {
+    stage('foo') {
+      steps {
+        docker.withRegistry('private-repo1-url', 'id-for-a-docker-cred-repo1') {
+            docker.withRegistry('private-repo2-url', 'id-for-a-docker-cred-repo2') {
+              writeFile file: 'Dockerfile', text: '''
+                  FROM private-repo1-url/image
+                  COPY someFile /
+                  ENTRYPOINT /someFile'''
+              sh 'docker build --tag private-repo2-url/myapp .'
+            }
+          }
+      }
+    }
+  }
+}
+```
+
+
 ## Declarative pipeline example
 
 An example on how to bind Docker host/daemon credentials in a declarative pipeline: 

src/main/java/org/jenkinsci/plugins/docker/commons/impl/RegistryKeyMaterialFactory.java

@@ -28,6 +28,7 @@
 import java.io.IOException;
 import java.net.URL;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
 
 import javax.annotation.Nonnull;
 
@@ -54,9 +55,7 @@ public class RegistryKeyMaterialFactory extends KeyMaterialFactory {
     private static final String DOCKER_CONFIG_FILENAME = "config.json";
     private static final String BLACKLISTED_PROPERTY_CREDS_STORE = "credsStore";
     private static final String BLACKLISTED_PROPERTY_AUTHS = "auths";
-    private static final String BLACKLISTED_PROPERTY_PROXIES = "proxies";
     private static final String[] BLACKLISTED_PROPERTIES = { BLACKLISTED_PROPERTY_AUTHS, BLACKLISTED_PROPERTY_CREDS_STORE };
-    private static final String[] BLACKLISTED_NESTED_PROPERTIES = { BLACKLISTED_PROPERTY_CREDS_STORE, BLACKLISTED_PROPERTY_PROXIES };
 
     private final @Nonnull String username;
     private final @Nonnull String password;
@@ -80,20 +79,25 @@ public RegistryKeyMaterialFactory(@Nonnull String username, @Nonnull String pass
     public KeyMaterial materialize() throws IOException, InterruptedException {
         FilePath dockerConfig = createSecretsDirectory();
 
-        // read the existing docker config file, which might hold some important settings (e.b. proxies)
+        // read the user's home dir docker config file, which might hold some important settings (e.b. proxies)
         FilePath configJsonPath = FilePath.getHomeDirectory(this.launcher.getChannel()).child(".docker").child(DOCKER_CONFIG_FILENAME);
-        dockerConfig = UpdateDockerConfigFromSource(dockerConfig, configJsonPath, BLACKLISTED_PROPERTIES);
-
-        // Read the existing docker config file from a nested config block, will probably hold some previous credentials
-        String existingDockerSecretConfigPath = this.env.get("DOCKER_CONFIG");
-        if (StringUtils.isNotBlank(existingDockerSecretConfigPath)) {
-            // Can't use FilePath(File) yet as not supported till later versions of jenkins..
-            //FilePath existingDockerConfig = FilePath(new File(existingDockerSecretConfigPath, DOCKER_CONFIG_FILENAME));
-            FilePath baseDir = getContext().getBaseDir();
-            // Need to get tmp dir - get base dir length and increase by 1 to include the path separator
-            String existingTmpConfigDir = existingDockerSecretConfigPath.substring(baseDir.getRemote().length() + 1);
-            FilePath existingDockerConfigPath = baseDir.child(existingTmpConfigDir).child(DOCKER_CONFIG_FILENAME);
-            dockerConfig = updateDockerConfigFromSource(dockerConfig, existingDockerConfigPath, BLACKLISTED_NESTED_PROPERTIES);
+        // read the current docker config which might hold some existing settings (e.b. credentials)
+        FilePath existingDockerConfigPath = new FilePath(this.launcher.getChannel(),
+                    Paths.get(this.env.get("DOCKER_CONFIG"), DOCKER_CONFIG_FILENAME).toString());
+
+        String dockerConfigJson = "";
+        if (existingDockerConfigPath.exists()) {
+            this.launcher.getListener().getLogger().print("Reading the existing DOCKER_CONFIG '" +
+                    existingDockerConfigPath + "' docker config file.\n");
+            dockerConfigJson = existingDockerConfigPath.readToString();
+        } else if (configJsonPath.exists()) {
+            this.launcher.getListener().getLogger().print("Reading the existing user's home '" +
+                    configJsonPath + "' docker config file.\n");
+            dockerConfigJson = removeBlacklistedProperties(configJsonPath.readToString(), BLACKLISTED_PROPERTIES);
+        }
+
+        if (StringUtils.isNotBlank(dockerConfigJson)) {
+            dockerConfig.child(DOCKER_CONFIG_FILENAME).write(dockerConfigJson, StandardCharsets.UTF_8.name());
         }
 
         try {
@@ -114,32 +118,19 @@ public KeyMaterial materialize() throws IOException, InterruptedException {
         return new RegistryKeyMaterial(dockerConfig, new EnvVars("DOCKER_CONFIG", dockerConfig.getRemote()));
     }
 
-    /**
-     * Copy docker config source data to another docker config
-     * @param dockerConfig
-     * @param dockerConfigSourcePath
-     * @param blacklistedProperties
-     * @return FilePath dockerConfig
-     */
-    private FilePath updateDockerConfigFromSource(@Nonnull FilePath dockerConfig, @Nonnull FilePath dockerConfigSourcePath, @Nonnull String[] blacklistedProperties) throws IOException, InterruptedException {
-        // Make sure config exists
-        if (dockerConfigSourcePath.exists()) {
-            String configJson = dockerConfigSourcePath.readToString();
-            if (StringUtils.isNotBlank(configJson)) {
-                this.launcher.getListener().getLogger().print("Using the existing docker config file.");
-
-                JSONObject json = JSONObject.fromObject(configJson);
-                for (String property : blacklistedProperties) {
-                    Object value = json.remove(property);
-                    if (value != null) {
-                        this.launcher.getListener().getLogger().print("Removing blacklisted property: " + property);
-                    }
+    private String removeBlacklistedProperties(@Nonnull String json, @Nonnull String[] blacklistedProperties) {
+        String jsonString = "";
+        if (StringUtils.isNotBlank(json)) {
+            JSONObject jsonObject = JSONObject.fromObject(json);
+            for (String property : blacklistedProperties) {
+                Object value = jsonObject.remove(property);
+                if (value != null) {
+                    this.launcher.getListener().getLogger().print("Removing blacklisted property: " + property + "\n");
                 }
-
-                dockerConfig.child(DOCKER_CONFIG_FILENAME).write(json.toString(), StandardCharsets.UTF_8.name());
             }
+            jsonString = jsonObject.toString();
         }
-        return dockerConfig;
+        return jsonString;
     }
 
     private static class RegistryKeyMaterial extends KeyMaterial {

src/test/java/org/jenkinsci/plugins/docker/commons/impl/RegistryKeyMaterialFactoryTest.java

@@ -27,6 +27,7 @@
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.emptyArray;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
@@ -41,13 +42,15 @@
 import org.jenkinsci.plugins.docker.commons.credentials.KeyMaterialContext;
 import org.jenkinsci.plugins.docker.commons.credentials.KeyMaterialFactory;
 import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.FakeLauncher;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.PretendSlave;
+import net.sf.json.JSONObject;
 
 import hudson.EnvVars;
 import hudson.FilePath;
@@ -66,6 +69,7 @@ public class RegistryKeyMaterialFactoryTest {
     @Rule
     public TemporaryFolder tempFolder = new TemporaryFolder();
 
+    private EnvVars env = new EnvVars();
     private KeyMaterialFactory factory;
 
     @Before
@@ -98,13 +102,17 @@ public <V, T extends Throwable> V call(final Callable<V, T> callable) throws T {
 	};
 
 	URL endpoint = new DockerRegistryEndpoint(null, null).getEffectiveUrl();
-	EnvVars env = new EnvVars();
 	String dockerExecutable = DockerTool.getExecutable(null, null, listener, env);
 
 	factory = new RegistryKeyMaterialFactory("username", "password", endpoint, launcher, env, listener,
 		dockerExecutable).contextualize(new KeyMaterialContext(new FilePath(tempFolder.newFolder())));
     }
 
+    @After
+	public void reset() {
+		env = new EnvVars();
+	}
+
     @Test
     public void materialize_userConfigFileNotPresent_notCreated() throws Exception {
 	// act
@@ -232,4 +240,34 @@ public void materialize_userConfigFileWithCredStoreAndHttpHeaders_createdWithHea
 	assertEquals("{\"HttpHeaders\":{\"User-Agent\":\"Docker-Client\"}}", FileUtils.readFileToString(jsonFile));
     }
 
+	@Test
+	public void materialize_existingConfigFile_updatedWithCredentials() throws Exception {
+
+		// arrange
+		File existingCfgFile = new File(new File(tempFolder.getRoot(), ".docker"), "config.json");
+		String existingCfgPath = new File(existingCfgFile.getAbsolutePath()).getParent();
+		String updatedConfigJson = "{" + "\"auths\":{\"localhost2:5001\":{\"auth\":\"whatever2\",\"email\":\"\"}}," +
+				"\"proxies\":{\"default\":{\"httpProxy\":\"proxy\",\"noProxy\":\"something\"}}" + "}";
+		FileUtils.write(existingCfgFile, updatedConfigJson);
+		assertNotNull(existingCfgPath);
+		env.put("DOCKER_CONFIG", existingCfgPath);
+		assertNotNull(env.get("DOCKER_CONFIG", null));
+
+		// act
+		KeyMaterial material = factory.materialize();
+
+		// assert
+		String cfgFolderPath = material.env().get("DOCKER_CONFIG", null);
+		assertNotNull(cfgFolderPath);
+		assertNotEquals(cfgFolderPath, existingCfgPath);
+
+		File dockerCfgFolder = new File(cfgFolderPath);
+		assertTrue(dockerCfgFolder.exists());
+
+		String[] existingFiles = dockerCfgFolder.list();
+		assertThat(existingFiles, arrayContaining("config.json"));
+
+		File jsonFile = new File(dockerCfgFolder, "config.json");
+		assertEquals(updatedConfigJson, FileUtils.readFileToString(jsonFile));
+	}
 }