Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Commit

Permalink
GCR: ignore exceptions when building images
Browse files Browse the repository at this point in the history
Ignore exceptions refreshing the accessToken in
ContainerRegistryAuthSupplier when building the image or getting the
RegistryAuth to be used for swarm.

Since `authForBuild()` has no arguments, we currently try to return an
AccessToken to use _in case_ the image being built needs to pull from
gcr.io, but we should ignore failures in the case that the image being
built does not need to pull anything from gcr.io.

This way someone using ContainerRegistryAuthSupplier with credentials
that do not actually have access to GCR do not get exceptions when
building non-GCR images.

If the accessToken can't be fetched in `authForBuild` for a gcr.io
image, then the build will still fail but at a later point when the
docker daemon tries to pull the FROM image and throws an error about how
the image is "missing or access is denied".
  • Loading branch information
mattnworb committed Jun 1, 2017
1 parent 3e616fd commit 3a9f04d
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,11 @@ public void refresh(final GoogleCredentials credentials) throws IOException {
* Get an accessToken to use, possibly refreshing the token if it expires within the
* minimumExpiryMillis.
*/
private AccessToken getAccessToken() throws DockerException {
private AccessToken getAccessToken() throws IOException {
// synchronize attempts to refresh the accessToken
synchronized (credentials) {
if (needsRefresh(credentials.getAccessToken())) {
try {
credentialRefresher.refresh(credentials);
} catch (IOException e) {
// rethrow as DockerException to match the signature of the RegistryAuthSupplier methods
throw new DockerException("Could not refresh access token", e);
}
credentialRefresher.refresh(credentials);
}
}
return credentials.getAccessToken();
Expand All @@ -254,7 +249,13 @@ public RegistryAuth authFor(final String imageName) throws DockerException {
return null;
}

return authForAccessToken(getAccessToken());
final AccessToken accessToken;
try {
accessToken = getAccessToken();
} catch (IOException e) {
throw new DockerException(e);
}
return authForAccessToken(accessToken);
}

// see https://cloud.google.com/container-registry/docs/advanced-authentication
Expand All @@ -267,12 +268,32 @@ private RegistryAuth authForAccessToken(final AccessToken accessToken) {

@Override
public RegistryAuth authForSwarm() throws DockerException {
return authForAccessToken(getAccessToken());
final AccessToken accessToken;
try {
accessToken = getAccessToken();
} catch (IOException e) {
// ignore the exception, as the user may not care if swarm is authenticated to use GCR
log.warn("unable to get access token for Google Container Registry due to exception, "
+ "configuration for Swarm will not contain RegistryAuth for GCR",
e);
return null;
}
return authForAccessToken(accessToken);
}

@Override
public RegistryConfigs authForBuild() throws DockerException {
final AccessToken accessToken = getAccessToken();
final AccessToken accessToken;
try {
accessToken = getAccessToken();
} catch (IOException e) {
// do not fail as the GCR access token may not be necessary for building the image currently
// being built
log.warn("unable to get access token for Google Container Registry, "
+ "configuration for building image will not contain RegistryAuth for GCR",
e);
return RegistryConfigs.empty();
}

final Map<String, RegistryAuth> configs = new HashMap<>(GCR_REGISTRIES.size());
for (String serverName : GCR_REGISTRIES) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
package com.spotify.docker.client.gcr;

import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand All @@ -33,16 +36,23 @@
import com.google.api.client.util.Clock;
import com.google.auth.oauth2.AccessToken;
import com.google.auth.oauth2.GoogleCredentials;
import com.spotify.docker.client.exceptions.DockerException;
import com.spotify.docker.client.messages.RegistryAuth;
import com.spotify.docker.client.messages.RegistryConfigs;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import org.hamcrest.FeatureMatcher;
import org.hamcrest.Matcher;
import org.joda.time.DateTime;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

public class ContainerRegistryAuthSupplierTest {

@Rule
public final ExpectedException exception = ExpectedException.none();

private final DateTime expiration = new DateTime(2017, 5, 23, 16, 25);
private final String tokenValue = "abc123.foobar";

Expand Down Expand Up @@ -130,6 +140,21 @@ public void testAuthForImage_NonGcrImage() throws Exception {
assertThat(supplier.authFor("foobar"), is(nullValue()));
}

@Test
public void testAuthForImage_ExceptionOnRefresh() throws Exception {
when(clock.currentTimeMillis())
.thenReturn(expiration.minusSeconds(minimumExpirationSecs - 1).getMillis());

final IOException ex = new IOException("failure!!");
doThrow(ex).when(refresher).refresh(credentials);

// the exception should propagate up
exception.expect(DockerException.class);
exception.expectCause(is(ex));

supplier.authFor("gcr.io/example/foobar:1.2.3");
}

@Test
public void testAuthForSwarm_NoRefresh() throws Exception {
when(clock.currentTimeMillis())
Expand All @@ -150,12 +175,23 @@ public void testAuthForSwarm_RefreshNeeded() throws Exception {
verify(refresher).refresh(credentials);
}

@Test
public void testAuthForSwarm_ExceptionOnRefresh() throws Exception {
when(clock.currentTimeMillis())
.thenReturn(expiration.minusSeconds(minimumExpirationSecs - 1).getMillis());

doThrow(new IOException("failure!!")).when(refresher).refresh(credentials);

assertThat(supplier.authForSwarm(), is(nullValue()));
}

@Test
public void testAuthForBuild_NoRefresh() throws Exception {
when(clock.currentTimeMillis())
.thenReturn(expiration.minusSeconds(minimumExpirationSecs + 1).getMillis());

final RegistryConfigs configs = supplier.authForBuild();
assertThat(configs.configs().values(), is(not(empty())));
assertThat(configs.configs().values(), everyItem(matchesAccessToken(accessToken)));

verify(refresher, never()).refresh(credentials);
Expand All @@ -167,8 +203,20 @@ public void testAuthForBuild_RefreshNeeded() throws Exception {
.thenReturn(expiration.minusSeconds(minimumExpirationSecs - 1).getMillis());

final RegistryConfigs configs = supplier.authForBuild();
assertThat(configs.configs().values(), is(not(empty())));
assertThat(configs.configs().values(), everyItem(matchesAccessToken(accessToken)));

verify(refresher).refresh(credentials);
}

@Test
public void testAuthForBuild_ExceptionOnRefresh() throws Exception {
when(clock.currentTimeMillis())
.thenReturn(expiration.minusSeconds(minimumExpirationSecs - 1).getMillis());

doThrow(new IOException("failure!!")).when(refresher).refresh(credentials);

final RegistryConfigs configs = supplier.authForBuild();
assertThat(configs.configs().values(), is(empty()));
}
}

0 comments on commit 3a9f04d

Please sign in to comment.