diff --git a/src/main/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplier.java b/src/main/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplier.java index ef010bffc..1a8878812 100644 --- a/src/main/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplier.java +++ b/src/main/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplier.java @@ -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(); @@ -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 @@ -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 configs = new HashMap<>(GCR_REGISTRIES.size()); for (String serverName : GCR_REGISTRIES) { diff --git a/src/test/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplierTest.java b/src/test/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplierTest.java index d4513e707..c8efd0ab6 100644 --- a/src/test/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplierTest.java +++ b/src/test/java/com/spotify/docker/client/gcr/ContainerRegistryAuthSupplierTest.java @@ -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; @@ -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"; @@ -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()) @@ -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); @@ -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())); + } }