Skip to content

Commit 525482c

Browse files
gibfahngibfahn
committed
fix: handle caching when detecting a repo requires auth
Jib may in some circumstances incorrectly determine that a repository does not require authentication. Take for instance the case where caching is used for the manifests or other components of a base image. If the initial request succeeds then Jib assumes the repository does not require authentication. In the case of a cache miss where authentication is required, this may result in a 401 which is not handled and fails the build. Jib should use the /v2/ endpoint to initially determine if the repository requires authentication. Fixes: #4385
1 parent c43ab07 commit 525482c

File tree

6 files changed

+172
-60
lines changed

6 files changed

+172
-60
lines changed

‎jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java‎

Lines changed: 47 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
5151
import com.google.cloud.tools.jib.registry.ManifestAndDigest;
5252
import com.google.cloud.tools.jib.registry.RegistryClient;
53+
import com.google.cloud.tools.jib.registry.RegistryUnauthorizedExceptionHandler;
5354
import com.google.cloud.tools.jib.registry.credentials.CredentialRetrievalException;
5455
import com.google.common.annotations.VisibleForTesting;
5556
import com.google.common.base.Preconditions;
@@ -64,6 +65,7 @@
6465
import java.util.Optional;
6566
import java.util.Set;
6667
import java.util.concurrent.Callable;
68+
import java.util.function.Supplier;
6769
import javax.annotation.Nullable;
6870

6971
/** Pulls the base image manifests for the specified platforms. */
@@ -132,12 +134,8 @@ public ImagesAndRegistryClient call()
132134
} else if (imageReference.getDigest().isPresent()) {
133135
List<Image> images = getCachedBaseImages();
134136
if (!images.isEmpty()) {
135-
RegistryClient noAuthRegistryClient =
136-
buildContext.newBaseImageRegistryClientFactory().newRegistryClient();
137-
// TODO: passing noAuthRegistryClient may be problematic. It may return 401 unauthorized
138-
// if layers have to be downloaded.
139-
// https://github.com/GoogleContainerTools/jib/issues/2220
140-
return new ImagesAndRegistryClient(images, noAuthRegistryClient);
137+
RegistryClient onDemandAuthRegistryClient = createOnDemandAuthenticatingRegistryClient();
138+
return new ImagesAndRegistryClient(images, onDemandAuthRegistryClient);
141139
}
142140
}
143141

@@ -147,63 +145,62 @@ public ImagesAndRegistryClient call()
147145
return mirrorPull.get();
148146
}
149147

150-
try {
151-
// First, try with no credentials. This works with public GCR images (but not Docker Hub).
152-
// TODO: investigate if we should just pass credentials up front. However, this involves
153-
// some risk. https://github.com/GoogleContainerTools/jib/pull/2200#discussion_r359069026
154-
// contains some related discussions.
155-
RegistryClient noAuthRegistryClient =
156-
buildContext.newBaseImageRegistryClientFactory().newRegistryClient();
157-
return new ImagesAndRegistryClient(
158-
pullBaseImages(noAuthRegistryClient, progressDispatcher.newChildProducer()),
159-
noAuthRegistryClient);
160-
161-
} catch (RegistryUnauthorizedException ex) {
162-
eventHandlers.dispatch(
163-
LogEvent.lifecycle(
164-
"The base image requires auth. Trying again for " + imageReference + "..."));
165-
166-
Credential credential =
167-
RegistryCredentialRetriever.getBaseImageCredential(buildContext).orElse(null);
168-
RegistryClient registryClient =
169-
buildContext
170-
.newBaseImageRegistryClientFactory()
171-
.setCredential(credential)
172-
.newRegistryClient();
148+
RegistryClient onDemandAuthRegistryClient = createOnDemandAuthenticatingRegistryClient();
149+
return new ImagesAndRegistryClient(
150+
pullBaseImages(onDemandAuthRegistryClient, progressDispatcher.newChildProducer()),
151+
onDemandAuthRegistryClient);
152+
}
153+
}
154+
155+
private RegistryClient createOnDemandAuthenticatingRegistryClient()
156+
throws CredentialRetrievalException {
157+
Credential credential =
158+
RegistryCredentialRetriever.getBaseImageCredential(buildContext).orElse(null);
159+
return buildContext
160+
.newBaseImageRegistryClientFactory()
161+
.setCredential(credential)
162+
.setUnauthorizedExceptionHandlerSupplier(
163+
() -> PullBaseImageStep::handleRegistryUnauthorizedException)
164+
.newRegistryClient();
165+
}
173166

167+
/**
168+
* Handles an unauthorized exception by performing authentication on demand.
169+
*
170+
* @param registryClient the registry client to be reconfigured
171+
* @param ex the exception that was caught
172+
* @return a supplier of the next handler, used only if another exception is thrown
173+
* @throws RegistryException if a registry error occurs
174+
* @throws IOException if an I/O error occurs
175+
*/
176+
static Supplier<RegistryUnauthorizedExceptionHandler> handleRegistryUnauthorizedException(
177+
final RegistryClient registryClient, final RegistryUnauthorizedException ex)
178+
throws RegistryException, IOException {
179+
// Double indentation keeps code at same level as original code
180+
{
181+
{
182+
final EventHandlers eventHandlers = registryClient.getEventHandlers();
183+
final String imageReference = ex.getImageReference();
174184
String wwwAuthenticate = ex.getHttpResponseException().getHeaders().getAuthenticate();
175185
if (wwwAuthenticate != null) {
176186
eventHandlers.dispatch(
177187
LogEvent.debug("WWW-Authenticate for " + imageReference + ": " + wwwAuthenticate));
178188
registryClient.authPullByWwwAuthenticate(wwwAuthenticate);
179-
return new ImagesAndRegistryClient(
180-
pullBaseImages(registryClient, progressDispatcher.newChildProducer()),
181-
registryClient);
182-
183189
} else {
184190
// Not getting WWW-Authenticate is unexpected in practice, and we may just blame the
185191
// server and fail. However, to keep some old behavior, try a few things as a last resort.
186192
// TODO: consider removing this fallback branch.
187-
if (credential != null && !credential.isOAuth2RefreshToken()) {
188-
eventHandlers.dispatch(
189-
LogEvent.debug("Trying basic auth as fallback for " + imageReference + "..."));
190-
registryClient.configureBasicAuth();
191-
try {
192-
return new ImagesAndRegistryClient(
193-
pullBaseImages(registryClient, progressDispatcher.newChildProducer()),
194-
registryClient);
195-
} catch (RegistryUnauthorizedException ignored) {
196-
// Fall back to try bearer auth.
197-
}
198-
}
199-
200193
eventHandlers.dispatch(
201194
LogEvent.debug("Trying bearer auth as fallback for " + imageReference + "..."));
202-
registryClient.doPullBearerAuth();
203-
return new ImagesAndRegistryClient(
204-
pullBaseImages(registryClient, progressDispatcher.newChildProducer()),
205-
registryClient);
195+
if (!registryClient.doPullBearerAuth()) {
196+
eventHandlers.dispatch(
197+
LogEvent.error("Failed to bearer auth for pull of " + imageReference));
198+
throw ex;
199+
}
206200
}
201+
202+
// If another exception occurs, use the default behavior
203+
return RegistryClient::defaultRegistryUnauthorizedExceptionHandler;
207204
}
208205
}
209206
}

‎jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/RegistryCredentialRetriever.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,13 @@ static Optional<Credential> getBaseImageCredential(BuildContext buildContext)
3939
/** Retrieves credentials for the target image. */
4040
static Optional<Credential> getTargetImageCredential(BuildContext buildContext)
4141
throws CredentialRetrievalException {
42-
return retrieve(buildContext.getTargetImageConfiguration(), buildContext.getEventHandlers());
42+
final Optional<Credential> credential =
43+
retrieve(buildContext.getTargetImageConfiguration(), buildContext.getEventHandlers());
44+
if (!credential.isPresent()) {
45+
logNoCredentialsRetrieved(
46+
buildContext.getTargetImageConfiguration(), buildContext.getEventHandlers());
47+
}
48+
return credential;
4349
}
4450

4551
private static Optional<Credential> retrieve(
@@ -52,10 +58,14 @@ private static Optional<Credential> retrieve(
5258
}
5359
}
5460

61+
return Optional.empty();
62+
}
63+
64+
private static void logNoCredentialsRetrieved(
65+
ImageConfiguration imageConfiguration, EventHandlers eventHandlers) {
5566
String registry = imageConfiguration.getImageRegistry();
5667
String repository = imageConfiguration.getImageRepository();
5768
eventHandlers.dispatch(
5869
LogEvent.info("No credentials could be retrieved for " + registry + "/" + repository));
59-
return Optional.empty();
6070
}
6171
}

‎jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java‎

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import java.util.Optional;
4949
import java.util.concurrent.atomic.AtomicReference;
5050
import java.util.function.Consumer;
51+
import java.util.function.Supplier;
5152
import java.util.stream.Stream;
5253
import javax.annotation.Nullable;
5354
import javax.annotation.concurrent.ThreadSafe;
@@ -65,6 +66,8 @@ public static class Factory {
6566

6667
@Nullable private String userAgent;
6768
@Nullable private Credential credential;
69+
Supplier<RegistryUnauthorizedExceptionHandler> registryUnauthorizedExceptionHandlerSupplier =
70+
RegistryClient::defaultRegistryUnauthorizedExceptionHandler;
6871

6972
private Factory(
7073
EventHandlers eventHandlers,
@@ -86,6 +89,20 @@ public Factory setCredential(@Nullable Credential credential) {
8689
return this;
8790
}
8891

92+
/**
93+
* Sets the supplier for the registry unauthorized exception handler.
94+
*
95+
* @param registryUnauthorizedExceptionHandlerSupplier the supplier for the exception handler
96+
* @return this
97+
*/
98+
public Factory setUnauthorizedExceptionHandlerSupplier(
99+
Supplier<RegistryUnauthorizedExceptionHandler>
100+
registryUnauthorizedExceptionHandlerSupplier) {
101+
this.registryUnauthorizedExceptionHandlerSupplier =
102+
registryUnauthorizedExceptionHandlerSupplier;
103+
return this;
104+
}
105+
89106
/**
90107
* Sets the value of {@code User-Agent} in headers for registry requests.
91108
*
@@ -104,7 +121,12 @@ public Factory setUserAgent(@Nullable String userAgent) {
104121
*/
105122
public RegistryClient newRegistryClient() {
106123
return new RegistryClient(
107-
eventHandlers, credential, registryEndpointRequestProperties, userAgent, httpClient);
124+
eventHandlers,
125+
credential,
126+
registryUnauthorizedExceptionHandlerSupplier,
127+
registryEndpointRequestProperties,
128+
userAgent,
129+
httpClient);
108130
}
109131
}
110132

@@ -231,6 +253,8 @@ static Multimap<String, String> decodeTokenRepositoryGrants(String token) {
231253

232254
private final EventHandlers eventHandlers;
233255
@Nullable private final Credential credential;
256+
private final Supplier<RegistryUnauthorizedExceptionHandler>
257+
registryUnauthorizedExceptionHandlerSupplier;
234258
private final RegistryEndpointRequestProperties registryEndpointRequestProperties;
235259
@Nullable private final String userAgent;
236260
private final FailoverHttpClient httpClient;
@@ -254,11 +278,14 @@ static Multimap<String, String> decodeTokenRepositoryGrants(String token) {
254278
private RegistryClient(
255279
EventHandlers eventHandlers,
256280
@Nullable Credential credential,
281+
Supplier<RegistryUnauthorizedExceptionHandler> registryUnauthorizedExceptionHandlerSupplier,
257282
RegistryEndpointRequestProperties registryEndpointRequestProperties,
258283
@Nullable String userAgent,
259284
FailoverHttpClient httpClient) {
260285
this.eventHandlers = eventHandlers;
261286
this.credential = credential;
287+
this.registryUnauthorizedExceptionHandlerSupplier =
288+
registryUnauthorizedExceptionHandlerSupplier;
262289
this.registryEndpointRequestProperties = registryEndpointRequestProperties;
263290
this.userAgent = userAgent;
264291
this.httpClient = httpClient;
@@ -595,6 +622,16 @@ private static boolean isBearerAuth(@Nullable Authorization authorization) {
595622
return authorization != null && "bearer".equalsIgnoreCase(authorization.getScheme());
596623
}
597624

625+
/**
626+
* Obtains the event handlers. This is intended to be used by the {@link
627+
* RegistryUnauthorizedExceptionHandler}.
628+
*
629+
* @return the event
630+
*/
631+
public EventHandlers getEventHandlers() {
632+
return eventHandlers;
633+
}
634+
598635
@Nullable
599636
@VisibleForTesting
600637
String getUserAgent() {
@@ -610,7 +647,8 @@ String getUserAgent() {
610647
*/
611648
private <T> T callRegistryEndpoint(RegistryEndpointProvider<T> registryEndpointProvider)
612649
throws IOException, RegistryException {
613-
int bearerTokenRefreshes = 0;
650+
Supplier<RegistryUnauthorizedExceptionHandler> handlerSupplier =
651+
this.registryUnauthorizedExceptionHandlerSupplier;
614652
while (true) {
615653
try {
616654
return new RegistryEndpointCaller<>(
@@ -623,18 +661,37 @@ private <T> T callRegistryEndpoint(RegistryEndpointProvider<T> registryEndpointP
623661
.call();
624662

625663
} catch (RegistryUnauthorizedException ex) {
664+
handlerSupplier = handlerSupplier.get().handle(this, ex);
665+
}
666+
}
667+
}
668+
669+
static class DefaultRegistryUnauthorizedExceptionHandler
670+
implements RegistryUnauthorizedExceptionHandler {
671+
int bearerTokenRefreshes = 0;
672+
673+
public Supplier<RegistryUnauthorizedExceptionHandler> handle(
674+
final RegistryClient registryClient, final RegistryUnauthorizedException ex)
675+
throws RegistryException {
676+
{
626677
if (ex.getHttpResponseException().getStatusCode()
627678
!= HttpStatusCodes.STATUS_CODE_UNAUTHORIZED
628-
|| !isBearerAuth(authorization.get())
679+
|| !isBearerAuth(registryClient.authorization.get())
629680
|| ++bearerTokenRefreshes >= MAX_BEARER_TOKEN_REFRESH_TRIES) {
630681
throw ex;
631682
}
632683

633684
// Because we successfully did bearer authentication initially, getting 401 here probably
634685
// means the token was expired.
635686
String wwwAuthenticate = ex.getHttpResponseException().getHeaders().getAuthenticate();
636-
authorization.set(refreshBearerAuth(wwwAuthenticate));
687+
registryClient.authorization.set(registryClient.refreshBearerAuth(wwwAuthenticate));
688+
689+
return () -> this;
637690
}
638691
}
639692
}
693+
694+
public static RegistryUnauthorizedExceptionHandler defaultRegistryUnauthorizedExceptionHandler() {
695+
return new DefaultRegistryUnauthorizedExceptionHandler();
696+
}
640697
}

‎jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryUnauthorizedExceptionHandler.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package com.google.cloud.tools.jib.registry;
2+
3+
import com.google.cloud.tools.jib.api.RegistryException;
4+
import com.google.cloud.tools.jib.api.RegistryUnauthorizedException;
5+
import java.io.IOException;
6+
import java.util.function.Supplier;
7+
8+
public interface RegistryUnauthorizedExceptionHandler {
9+
10+
/**
11+
* Handle the exception caught by the registry client when it attempted to communicate with the
12+
* registry.
13+
*
14+
* <p>The most obvious action is to simply throw {@code ex}. Other possible actions are to
15+
* reauthenticate the client.
16+
*
17+
* @param registryClient the registry client which may be reconfigured
18+
* @param ex the exception being handled on behalf of the client
19+
* @return a supplier to use if another exception occurs on the next retry
20+
* @throws IOException if an I/O error occurs
21+
* @throws RegistryException if a registry error occurs
22+
*/
23+
Supplier<RegistryUnauthorizedExceptionHandler> handle(
24+
RegistryClient registryClient, RegistryUnauthorizedException ex)
25+
throws IOException, RegistryException;
26+
}

‎jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStepTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import com.google.cloud.tools.jib.registry.ManifestAndDigest;
5555
import com.google.cloud.tools.jib.registry.RegistryClient;
5656
import com.google.cloud.tools.jib.registry.credentials.CredentialRetrievalException;
57+
import com.google.common.collect.ImmutableList;
5758
import com.google.common.collect.ImmutableListMultimap;
5859
import com.google.common.collect.ImmutableSet;
5960
import java.io.IOException;
@@ -94,13 +95,18 @@ public void setUp() {
9495
Mockito.when(buildContext.getBaseImageLayersCache()).thenReturn(cache);
9596
Mockito.when(buildContext.newBaseImageRegistryClientFactory())
9697
.thenReturn(registryClientFactory);
98+
Mockito.when(registryClientFactory.setCredential(Mockito.any()))
99+
.thenReturn(registryClientFactory);
100+
Mockito.when(registryClientFactory.setUnauthorizedExceptionHandlerSupplier(Mockito.any()))
101+
.thenReturn(registryClientFactory);
97102
Mockito.when(registryClientFactory.newRegistryClient()).thenReturn(registryClient);
98103
Mockito.when(buildContext.getContainerConfiguration()).thenReturn(containerConfig);
99104
Mockito.when(containerConfig.getPlatforms())
100105
.thenReturn(ImmutableSet.of(new Platform("slim arch", "fat system")));
101106
Mockito.when(progressDispatcherFactory.create(Mockito.anyString(), Mockito.anyLong()))
102107
.thenReturn(progressDispatcher);
103108
Mockito.when(progressDispatcher.newChildProducer()).thenReturn(progressDispatcherFactory);
109+
Mockito.when(imageConfiguration.getCredentialRetrievers()).thenReturn(ImmutableList.of());
104110

105111
pullBaseImageStep = new PullBaseImageStep(buildContext, progressDispatcherFactory);
106112
}
@@ -678,6 +684,10 @@ private static RegistryClient.Factory setUpWorkingRegistryClientFactoryWithV22Ma
678684
manifest.setContainerConfiguration(1234, digest);
679685

680686
RegistryClient.Factory clientFactory = Mockito.mock(RegistryClient.Factory.class);
687+
Mockito.doCallRealMethod().when(clientFactory).setCredential(Mockito.any());
688+
Mockito.doCallRealMethod()
689+
.when(clientFactory)
690+
.setUnauthorizedExceptionHandlerSupplier(Mockito.any());
681691
RegistryClient client = Mockito.mock(RegistryClient.class);
682692
Mockito.when(clientFactory.newRegistryClient()).thenReturn(client);
683693
Mockito.when(client.pullManifest(Mockito.any()))
@@ -709,6 +719,10 @@ private static RegistryClient.Factory setUpWorkingRegistryClientFactoryWithV22Ma
709719
manifest.setContainerConfiguration(1234, digest);
710720

711721
RegistryClient.Factory clientFactory = Mockito.mock(RegistryClient.Factory.class);
722+
Mockito.doCallRealMethod().when(clientFactory).setCredential(Mockito.any());
723+
Mockito.doCallRealMethod()
724+
.when(clientFactory)
725+
.setUnauthorizedExceptionHandlerSupplier(Mockito.any());
712726
RegistryClient client = Mockito.mock(RegistryClient.class);
713727
Mockito.when(clientFactory.newRegistryClient()).thenReturn(client);
714728
Mockito.when(client.pullManifest(eq("sha256:aaaaaaa")))

0 commit comments

Comments
 (0)