Merge pull request #5180 from vespa-engine/freva/docker-auth

Docker auth for image pull

6 files changed, 81 insertions, 51 deletions

diff --git a/docker-api/pom.xml b/docker-api/pom.xml
index fc3407d08be..e2ddd8dbcc9 100644
--- a/docker-api/pom.xml
+++ b/docker-api/pom.xml
@@ -69,7 +69,6 @@
                     <groupId>org.apache.httpcomponents</groupId>
                     <artifactId>httpcore</artifactId>
                 </exclusion>
-
                 <exclusion>
                     <groupId>org.apache.httpcomponents</groupId>
                     <artifactId>httpclient</artifactId>
@@ -77,22 +76,28 @@
             </exclusions>
         </dependency>
         <dependency>
+            <groupId>commons-codec</groupId>
+            <artifactId>commons-codec</artifactId>
+            <version>1.10</version>
+            <scope>compile</scope>
+        </dependency>
+        <dependency>
             <groupId>net.jpountz.lz4</groupId>
             <artifactId>lz4</artifactId>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpcore</artifactId>
-            <!-- We explicitly specify the version of httpcore to be used by 
-            docker-java so the dependency is declared closer to the root of maven and 
+            <!-- We explicitly specify the version of httpcore to be used by
+            docker-java so the dependency is declared closer to the root of maven and
             more likely be the version that is finally being used. -->
             <version>4.4.1</version>
         </dependency>
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>
-             <!-- We explicitly specify the version of httpclient to be used by 
-            docker-java so the dependency is declared closer to the root of maven and 
+             <!-- We explicitly specify the version of httpclient to be used by
+            docker-java so the dependency is declared closer to the root of maven and
             more likely be the version that is finally being used. -->
             <version>4.5</version>
         </dependency>
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
index 04d4628d576..2039d0adfc9 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/Docker.java
@@ -1,7 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.dockerapi;
 
-import java.io.File;
 import java.net.InetAddress;
 import java.util.List;
 import java.util.Map;
@@ -61,8 +60,6 @@ public interface Docker {
 
     void connectContainerToNetwork(ContainerName containerName, String networkName);
 
-    void copyArchiveToContainer(String sourcePath, ContainerName destinationContainer, String destinationPath);
-
     List<Container> getAllContainersManagedBy(String manager);
 
     List<ContainerName> listAllContainersManagedBy(String manager);
@@ -80,15 +77,13 @@ public interface Docker {
 
     void deleteImage(DockerImage dockerImage);
 
-    void buildImage(File dockerfile, DockerImage dockerImage);
-
     /**
      * Deletes the local images that are currently not in use by any container and not recently used.
      */
     void deleteUnusedDockerImages();
 
     /**
-     * Execute a command in docker container as "yahoo". Will block until the command is finished.
+     * Execute a command in docker container as $VESPA_USER. Will block until the command is finished.
      *
      * @param containerName The name of the container
      * @param command The command with arguments to run.
@@ -120,4 +115,10 @@ public interface Docker {
     ProcessResult executeInContainerAsRoot(ContainerName containerName, Long timeoutSeconds, String... command);
 
     String getGlobalIPv6Address(ContainerName name);
+
+    /**
+     * If set, the supplier will we called every time before a pull/push request is made to get the credentials
+     */
+    void setDockerRegistryCredentialsSupplier(DockerRegistryCredentialsSupplier dockerRegistryCredentialsSupplier);
+
 }
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
index 2da18e12e40..f6588512e2d 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerImpl.java
@@ -8,9 +8,11 @@ import com.github.dockerjava.api.command.InspectContainerCmd;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectExecResponse;
 import com.github.dockerjava.api.command.InspectImageResponse;
+import com.github.dockerjava.api.command.PullImageCmd;
 import com.github.dockerjava.api.exception.DockerClientException;
 import com.github.dockerjava.api.exception.NotFoundException;
 import com.github.dockerjava.api.exception.NotModifiedException;
+import com.github.dockerjava.api.model.AuthConfig;
 import com.github.dockerjava.api.model.Image;
 import com.github.dockerjava.api.model.Network;
 import com.github.dockerjava.api.model.Statistics;
@@ -18,18 +20,17 @@ import com.github.dockerjava.core.DefaultDockerClientConfig;
 import com.github.dockerjava.core.DockerClientConfig;
 import com.github.dockerjava.core.DockerClientImpl;
 import com.github.dockerjava.core.async.ResultCallbackTemplate;
-import com.github.dockerjava.core.command.BuildImageResultCallback;
 import com.github.dockerjava.core.command.ExecStartResultCallback;
 import com.github.dockerjava.core.command.PullImageResultCallback;
 import com.github.dockerjava.jaxrs.JerseyDockerCmdExecFactory;
 import com.google.inject.Inject;
+import com.yahoo.log.LogLevel;
 import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper;
 import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 
 import javax.annotation.concurrent.GuardedBy;
 import java.io.ByteArrayOutputStream;
-import java.io.File;
 import java.io.IOException;
 import java.net.Inet6Address;
 import java.net.InetAddress;
@@ -68,8 +69,9 @@ public class DockerImpl implements Docker {
     @GuardedBy("monitor")
     private final Set<DockerImage> scheduledPulls = new HashSet<>();
 
-    // Exposed for testing.
-    DockerClient dockerClient;
+    private volatile Optional<DockerRegistryCredentialsSupplier> dockerRegistryCredentialsSupplier = Optional.empty();
+
+    private DockerClient dockerClient;
 
     @Inject
     public DockerImpl(DockerConfig config, MetricReceiverWrapper metricReceiverWrapper) {
@@ -145,18 +147,6 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public void copyArchiveToContainer(String sourcePath, ContainerName destinationContainer, String destinationPath) {
-        try {
-            dockerClient.copyArchiveToContainerCmd(destinationContainer.asString())
-                    .withHostResource(sourcePath).withRemotePath(destinationPath).exec();
-        } catch (RuntimeException e) {
-            numberOfDockerDaemonFails.add();
-            throw new DockerException("Failed to copy container " + sourcePath + " to " +
-                    destinationContainer + ":" + destinationPath, e);
-        }
-    }
-
-    @Override
     public boolean pullImageAsyncIfNeeded(final DockerImage image) {
         try {
             synchronized (monitor) {
@@ -164,7 +154,17 @@ public class DockerImpl implements Docker {
                 if (imageIsDownloaded(image)) return false;
 
                 scheduledPulls.add(image);
-                dockerClient.pullImageCmd(image.asString()).exec(new ImagePullCallback(image));
+                PullImageCmd pullImageCmd = dockerClient.pullImageCmd(image.asString());
+
+                dockerRegistryCredentialsSupplier
+                        .flatMap(credentialsSupplier -> credentialsSupplier.getCredentials(image))
+                        .map(credentials -> new AuthConfig()
+                                .withRegistryAddress(credentials.registry.toString())
+                                .withUsername(credentials.username)
+                                .withPassword(credentials.password))
+                        .ifPresent(pullImageCmd::withAuthConfig);
+
+                pullImageCmd.exec(new ImagePullCallback(image));
                 return true;
             }
         } catch (RuntimeException e) {
@@ -378,6 +378,11 @@ public class DockerImpl implements Docker {
         return cmd.exec().getNetworkSettings().getGlobalIPv6Address();
     }
 
+    @Override
+    public void setDockerRegistryCredentialsSupplier(DockerRegistryCredentialsSupplier dockerRegistryCredentialsSupplier) {
+        this.dockerRegistryCredentialsSupplier = Optional.of(dockerRegistryCredentialsSupplier);
+    }
+
     private Stream<Container> asContainer(String container) {
         return inspectContainerCmd(container)
                 .map(response ->
@@ -434,17 +439,6 @@ public class DockerImpl implements Docker {
     }
 
     @Override
-    public void buildImage(File dockerfile, DockerImage image) {
-        try {
-            dockerClient.buildImageCmd(dockerfile).withTags(Collections.singleton(image.asString()))
-                    .exec(new BuildImageResultCallback()).awaitImageId();
-        } catch (RuntimeException e) {
-            numberOfDockerDaemonFails.add();
-            throw new DockerException("Failed to build image " + image.asString(), e);
-        }
-    }
-
-    @Override
     public void deleteUnusedDockerImages() {
         if (!dockerImageGC.isPresent()) return;
 
@@ -464,7 +458,7 @@ public class DockerImpl implements Docker {
         @Override
         public void onError(Throwable throwable) {
             removeScheduledPoll(dockerImage);
-            throw new DockerClientException("Could not download image: " + dockerImage);
+            logger.log(LogLevel.ERROR, "Could not download image " + dockerImage.asString(), throwable);
         }
 
 
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentials.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentials.java
new file mode 100644
index 00000000000..c9603e9e53a
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentials.java
@@ -0,0 +1,19 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi;
+
+import java.net.URI;
+
+/**
+ * @author freva
+ */
+public class DockerRegistryCredentials {
+    public final URI registry;
+    public final String username;
+    public final String password;
+
+    public DockerRegistryCredentials(URI registry, String username, String password) {
+        this.registry = registry;
+        this.username = username;
+        this.password = password;
+    }
+}
diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentialsSupplier.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentialsSupplier.java
new file mode 100644
index 00000000000..6f16a6cd545
--- /dev/null
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/DockerRegistryCredentialsSupplier.java
@@ -0,0 +1,16 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.hosted.dockerapi;
+
+import java.util.Optional;
+
+/**
+ * @author freva
+ */
+public interface DockerRegistryCredentialsSupplier {
+
+    /**
+     * Returns credentials to docker registry needed to be able to pull/push given
+     * docker image.
+     */
+    Optional<DockerRegistryCredentials> getCredentials(DockerImage dockerImage);
+}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
index 6494037ec3e..6c9df440826 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerMock.java
@@ -6,9 +6,9 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.DockerImage;
+import com.yahoo.vespa.hosted.dockerapi.DockerRegistryCredentialsSupplier;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
 
-import java.io.File;
 import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -59,11 +59,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public void copyArchiveToContainer(String sourcePath, ContainerName destinationContainer, String destinationPath) {
-
-    }
-
-    @Override
     public List<Container> getAllContainersManagedBy(String manager) {
         synchronized (monitor) {
             return new ArrayList<>(containersByContainerName.values());
@@ -135,11 +130,6 @@ public class DockerMock implements Docker {
     }
 
     @Override
-    public void buildImage(File dockerfile, DockerImage dockerImage) {
-
-    }
-
-    @Override
     public void deleteUnusedDockerImages() {
 
     }
@@ -174,6 +164,11 @@ public class DockerMock implements Docker {
         return "2001:db8:1:2:0:242:ac13:2";
     }
 
+    @Override
+    public void setDockerRegistryCredentialsSupplier(DockerRegistryCredentialsSupplier dockerRegistryCredentialsSupplier) {
+
+    }
+
     public static class StartContainerCommandMock implements CreateContainerCommand {
         @Override
         public CreateContainerCommand withLabel(String name, String value) {