-
Notifications
You must be signed in to change notification settings - Fork 549
Adds support for RegistryAuthSupplier #749
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,7 @@ | |
import com.spotify.docker.client.messages.NetworkCreation; | ||
import com.spotify.docker.client.messages.ProgressMessage; | ||
import com.spotify.docker.client.messages.RegistryAuth; | ||
import com.spotify.docker.client.messages.RegistryAuthSupplier; | ||
import com.spotify.docker.client.messages.RegistryConfigs; | ||
import com.spotify.docker.client.messages.RemovedImage; | ||
import com.spotify.docker.client.messages.ServiceCreateResponse; | ||
|
@@ -118,7 +119,6 @@ | |
import com.spotify.docker.client.messages.swarm.SwarmSpec; | ||
import com.spotify.docker.client.messages.swarm.Task; | ||
import com.spotify.docker.client.messages.swarm.UnlockKey; | ||
|
||
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
import java.io.Closeable; | ||
import java.io.IOException; | ||
|
@@ -335,7 +335,7 @@ public ClientBuilder get() { | |
|
||
private final URI uri; | ||
private final String apiVersion; | ||
private final RegistryAuth registryAuth; | ||
private final RegistryAuthSupplier registryAuthSupplier; | ||
|
||
private final Map<String, Object> headers; | ||
|
||
|
@@ -414,7 +414,12 @@ protected DefaultDockerClient(final Builder builder) { | |
.property(ApacheClientProperties.CONNECTION_MANAGER, cm) | ||
.property(ApacheClientProperties.REQUEST_CONFIG, requestConfig); | ||
|
||
this.registryAuth = builder.registryAuth; | ||
|
||
if (builder.registryAuthSupplier == null) { | ||
this.registryAuthSupplier = new NoOpRegistryAuthSupplier(null); | ||
} else { | ||
this.registryAuthSupplier = builder.registryAuthSupplier; | ||
} | ||
|
||
this.client = clientBuilderSupplier.get().withConfig(config).build(); | ||
|
||
|
@@ -1179,17 +1184,16 @@ public InputStream saveMultiple(final String... images) | |
throws DockerException, IOException, InterruptedException { | ||
|
||
final WebTarget resource = resource().path("images").path("get"); | ||
if (images != null) { | ||
for (final String image : images) { | ||
resource.queryParam("names", urlEncode(image)); | ||
} | ||
for (final String image : images) { | ||
resource.queryParam("names", urlEncode(image)); | ||
} | ||
|
||
return request( | ||
GET, | ||
InputStream.class, | ||
resource, | ||
resource.request(APPLICATION_JSON_TYPE).header("X-Registry-Auth", authHeader(registryAuth)) | ||
resource.request(APPLICATION_JSON_TYPE).header("X-Registry-Auth", authHeader( | ||
registryAuthSupplier.authFor(images[0]))) | ||
); | ||
} | ||
|
||
|
@@ -1201,7 +1205,7 @@ public void pull(final String image) throws DockerException, InterruptedExceptio | |
@Override | ||
public void pull(final String image, final ProgressHandler handler) | ||
throws DockerException, InterruptedException { | ||
pull(image, registryAuth, handler); | ||
pull(image, registryAuthSupplier.authFor(image), handler); | ||
} | ||
|
||
@Override | ||
|
@@ -1255,7 +1259,7 @@ public void push(final String image, final RegistryAuth registryAuth) | |
@Override | ||
public void push(final String image, final ProgressHandler handler) | ||
throws DockerException, InterruptedException { | ||
push(image, handler, registryAuth); | ||
push(image, handler, registryAuthSupplier.authFor(image)); | ||
} | ||
|
||
@Override | ||
|
@@ -1363,6 +1367,7 @@ public String build(final Path directory, final String name, final String docker | |
|
||
WebTarget resource = noTimeoutResource().path("build"); | ||
|
||
final RegistryAuth registryAuth = registryAuthSupplier.authFor(name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this behavior is correct. When building an image, you are supposed to send all the RegistryAuths that the client is configured for: https://docs.docker.com/engine/api/v1.28/#operation/ImageBuild It might be that the image you are building There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we missing a test? Also, from my understanding, RegistryAuth only represents creds for one registry, not multiple, so I'm not sure how that would work on master right now |
||
for (final BuildParam param : params) { | ||
resource = resource.queryParam(param.name(), param.value()); | ||
} | ||
|
@@ -1798,8 +1803,7 @@ public void unlock(final UnlockKey unlockKey) throws DockerException, Interrupte | |
@Override | ||
public ServiceCreateResponse createService(ServiceSpec spec) | ||
throws DockerException, InterruptedException { | ||
|
||
return createService(spec, registryAuth); | ||
return createService(spec, registryAuthSupplier.authForSwarm()); | ||
} | ||
|
||
@Override | ||
|
@@ -2528,6 +2532,10 @@ public static Builder fromEnv() throws DockerCertificateException { | |
|
||
public static class Builder { | ||
|
||
public static final String ERROR_MESSAGE = | ||
"LOGIC ERROR: DefaultDockerClient does not support being built " | ||
+ "with both `registryAuth` and `registryAuthSupplier`. " | ||
+ "Please build with at most one of these options."; | ||
private URI uri; | ||
private String apiVersion; | ||
private long connectTimeoutMillis = DEFAULT_CONNECT_TIMEOUT_MILLIS; | ||
|
@@ -2536,6 +2544,7 @@ public static class Builder { | |
private DockerCertificatesStore dockerCertificatesStore; | ||
private boolean dockerAuth; | ||
private RegistryAuth registryAuth; | ||
private RegistryAuthSupplier registryAuthSupplier; | ||
private Map<String, Object> headers = new HashMap<>(); | ||
|
||
public URI uri() { | ||
|
@@ -2661,9 +2670,24 @@ public RegistryAuth registryAuth() { | |
* | ||
* @param registryAuth RegistryAuth object | ||
* @return Builder | ||
* | ||
* @deprecated in favor of registryAuthSupplier | ||
*/ | ||
@Deprecated | ||
public Builder registryAuth(final RegistryAuth registryAuth) { | ||
if (this.registryAuthSupplier != null) { | ||
throw new IllegalStateException(ERROR_MESSAGE); | ||
} | ||
this.registryAuth = registryAuth; | ||
this.registryAuthSupplier = new NoOpRegistryAuthSupplier(registryAuth); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line makes calling this method twice throw which might not be obvious to caller if it doesn't know about the implementation here. |
||
return this; | ||
} | ||
|
||
public Builder registryAuthSupplier(final RegistryAuthSupplier registryAuthSupplier) { | ||
if (this.registryAuthSupplier != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Delete these three lines to allow calling this method twice? |
||
throw new IllegalStateException(ERROR_MESSAGE); | ||
} | ||
this.registryAuthSupplier = registryAuthSupplier; | ||
return this; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
/*- | ||
* -\-\- | ||
* docker-client | ||
* -- | ||
* Copyright (C) 2016 Spotify AB | ||
* -- | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* -/-/- | ||
*/ | ||
|
||
package com.spotify.docker.client; | ||
|
||
import static com.google.common.base.Preconditions.checkNotNull; | ||
import static com.google.common.base.Strings.isNullOrEmpty; | ||
|
||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.spotify.docker.client.messages.RegistryAuth; | ||
import java.io.IOException; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import java.util.Iterator; | ||
import org.glassfish.jersey.internal.util.Base64; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class DockerConfigReader { | ||
private static final Logger LOG = LoggerFactory.getLogger(DockerConfigReader.class); | ||
|
||
private static final ObjectMapper MAPPER = ObjectMapperProvider.objectMapper(); | ||
|
||
|
||
public RegistryAuth fromComfig(Path configPath, String serverAddress) throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typo in |
||
return parseDockerConfig(configPath, serverAddress).build(); | ||
} | ||
|
||
public RegistryAuth.Builder parseDockerConfig(final Path configPath, String serverAddress) | ||
throws IOException { | ||
checkNotNull(configPath); | ||
final RegistryAuth.Builder authBuilder = RegistryAuth.builder(); | ||
final JsonNode authJson = this.extractAuthJson(configPath); | ||
|
||
if (isNullOrEmpty(serverAddress)) { | ||
final Iterator<String> servers = authJson.fieldNames(); | ||
if (servers.hasNext()) { | ||
serverAddress = servers.next(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will return the first credential in the file. We need the one, if any, that equals There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's test this method with a test fixture/file with multiple serverAddresses. |
||
} | ||
} else { | ||
if (!authJson.has(serverAddress)) { | ||
LOG.error("Could not find auth config for {}. Returning empty builder", serverAddress); | ||
return RegistryAuth.builder().serverAddress(serverAddress); | ||
} | ||
} | ||
|
||
final JsonNode serverAuth = authJson.get(serverAddress); | ||
if (serverAuth != null && serverAuth.has("auth")) { | ||
authBuilder.serverAddress(serverAddress); | ||
final String authString = serverAuth.get("auth").asText(); | ||
final String[] authParams = Base64.decodeAsString(authString).split(":"); | ||
|
||
if (authParams.length == 2) { | ||
authBuilder.username(authParams[0].trim()); | ||
authBuilder.password(authParams[1].trim()); | ||
} else if (serverAuth.has("identityToken")) { | ||
authBuilder.identityToken(serverAuth.get("identityToken").asText()); | ||
return authBuilder; | ||
} else { | ||
LOG.warn("Failed to parse auth string for {}", serverAddress); | ||
return authBuilder; | ||
} | ||
} else { | ||
LOG.warn("Could not find auth field for {}", serverAddress); | ||
return authBuilder; | ||
} | ||
|
||
if (serverAuth.has("email")) { | ||
authBuilder.email(serverAuth.get("email").asText()); | ||
} | ||
|
||
return authBuilder; | ||
} | ||
|
||
public Path defaultConfigPath() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be static. |
||
final String home = System.getProperty("user.home"); | ||
final Path dockerConfig = Paths.get(home, ".docker", "config.json"); | ||
final Path dockerCfg = Paths.get(home, ".dockercfg"); | ||
|
||
if (Files.exists(dockerConfig)) { | ||
LOG.debug("Using configfile: {}", dockerConfig); | ||
return dockerConfig; | ||
} else { | ||
LOG.debug("Using configfile: {} ", dockerCfg); | ||
return dockerCfg; | ||
} | ||
} | ||
|
||
public JsonNode extractAuthJson(final Path configPath) throws IOException { | ||
final JsonNode config = MAPPER.readTree(configPath.toFile()); | ||
|
||
if (config.has("auths")) { | ||
return config.get("auths"); | ||
} | ||
|
||
return config; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses auth for first image. Can you export/save images from different registries at the same time? I couldn't find any docs for this.