diff --git a/jib-core/CHANGELOG.md b/jib-core/CHANGELOG.md index 203a9a7d52..e06e3c9785 100644 --- a/jib-core/CHANGELOG.md +++ b/jib-core/CHANGELOG.md @@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file. ### Fixed +- `Containerizer#setAllowInsecureRegistries(boolean)` and the `sendCredentialsOverHttp` system property are now effective for authentication service server connections. ([#2074](https://github.com/GoogleContainerTools/jib/pull/2074) + ## 0.12.0 ### Added diff --git a/jib-core/src/integration-test/java/com/google/cloud/tools/jib/registry/ManifestPusherIntegrationTest.java b/jib-core/src/integration-test/java/com/google/cloud/tools/jib/registry/ManifestPusherIntegrationTest.java index 88e8e7f982..d7c0410b2c 100644 --- a/jib-core/src/integration-test/java/com/google/cloud/tools/jib/registry/ManifestPusherIntegrationTest.java +++ b/jib-core/src/integration-test/java/com/google/cloud/tools/jib/registry/ManifestPusherIntegrationTest.java @@ -16,7 +16,6 @@ package com.google.cloud.tools.jib.registry; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.DescriptorDigest; import com.google.cloud.tools.jib.api.RegistryException; @@ -24,6 +23,7 @@ import com.google.cloud.tools.jib.blob.Blobs; import com.google.cloud.tools.jib.event.EventHandlers; import com.google.cloud.tools.jib.hash.Digests; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.image.json.ManifestTemplate; import com.google.cloud.tools.jib.image.json.V22ManifestTemplate; import java.io.IOException; @@ -58,9 +58,9 @@ public void testPush_missingBlobs() throws IOException, RegistryException { Assert.fail("Pushing manifest without its BLOBs should fail"); } catch (RegistryErrorException ex) { - HttpResponseException httpResponseException = (HttpResponseException) ex.getCause(); + ResponseException responseException = (ResponseException) ex.getCause(); Assert.assertEquals( - HttpStatusCodes.STATUS_CODE_BAD_REQUEST, httpResponseException.getStatusCode()); + HttpStatusCodes.STATUS_CODE_BAD_REQUEST, responseException.getStatusCode()); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/api/InsecureRegistryException.java b/jib-core/src/main/java/com/google/cloud/tools/jib/api/InsecureRegistryException.java index 82722ff557..588e38f3e9 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/api/InsecureRegistryException.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/api/InsecureRegistryException.java @@ -23,10 +23,11 @@ */ public class InsecureRegistryException extends RegistryException { - public InsecureRegistryException(URL insecureUrl) { + public InsecureRegistryException(URL insecureUrl, Throwable cause) { super( "Failed to verify the server at " + insecureUrl - + " because only secure connections are allowed."); + + " because only secure connections are allowed.", + cause); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/api/RegistryUnauthorizedException.java b/jib-core/src/main/java/com/google/cloud/tools/jib/api/RegistryUnauthorizedException.java index 4a0132143e..602ec3e9fd 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/api/RegistryUnauthorizedException.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/api/RegistryUnauthorizedException.java @@ -17,6 +17,7 @@ package com.google.cloud.tools.jib.api; import com.google.api.client.http.HttpResponseException; +import com.google.cloud.tools.jib.http.ResponseException; /** Thrown when a registry request was unauthorized and therefore authentication is needed. */ public class RegistryUnauthorizedException extends RegistryException { @@ -32,7 +33,7 @@ public class RegistryUnauthorizedException extends RegistryException { * @param cause the cause */ public RegistryUnauthorizedException( - String registry, String repository, HttpResponseException cause) { + String registry, String repository, ResponseException cause) { super("Unauthorized for " + registry + "/" + repository, cause); this.registry = registry; this.repository = repository; diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java b/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java index 6e678cbe21..f3671a6188 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java @@ -17,45 +17,70 @@ package com.google.cloud.tools.jib.http; import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpHeaders; import com.google.api.client.http.HttpMethods; import com.google.api.client.http.HttpRequest; -import com.google.api.client.http.HttpRequestFactory; -import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpTransport; import com.google.api.client.http.apache.ApacheHttpTransport; +import com.google.cloud.tools.jib.api.LogEvent; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import java.io.Closeable; import java.io.IOException; +import java.net.ConnectException; import java.net.URL; import java.security.GeneralSecurityException; -import java.util.function.Function; -import javax.annotation.Nullable; +import java.util.function.Consumer; +import java.util.function.Supplier; +import javax.net.ssl.SSLException; import org.apache.http.auth.AuthScope; import org.apache.http.auth.UsernamePasswordCredentials; import org.apache.http.impl.client.DefaultHttpClient; /** - * Sends an HTTP {@link Request} and stores the {@link Response}. Clients should not send more than - * one request. + * Thread-safe HTTP client that can automatically failover from secure HTTPS to insecure HTTPS or + * HTTP. Intended to be created once and shared to be called at multiple places. Callers should + * close the returned {@link Response}. * - *

Example usage: + *

The failover (if enabled) in the following way: * - * 

{@code
- * try (Connection connection = new Connection(url)) {
- *   Response response = connection.get(request);
- *   // ... process the response
- * }
- * }
+ * + * + * This failover behavior is similar to how the Docker client works: + * https://docs.docker.com/registry/insecure/#deploy-a-plain-http-registry */ -public class Connection implements Closeable { +public class Connection { // TODO: rename to TlsFailoverHttpClient - /** - * Returns a factory for {@link Connection}. - * - * @return {@link Connection} factory, a function that generates a {@link Connection} to a URL - */ - public static Function getConnectionFactory() { + private static boolean isHttpsProtocol(URL url) { + return "https".equals(url.getProtocol()); + } + + private static URL toHttp(URL url) { + GenericUrl httpUrl = new GenericUrl(url); + httpUrl.setScheme("http"); + return httpUrl.toURL(); + } + + private static HttpTransport getSecureHttpTransport() { // Do not use NetHttpTransport. It does not process response errors properly. // See https://github.com/google/google-http-java-client/issues/39 // @@ -63,22 +88,18 @@ public static Function getConnectionFactory() { // connection persistence causes the connection to throw NoHttpResponseException. ApacheHttpTransport transport = new ApacheHttpTransport(); addProxyCredentials(transport); - return url -> new Connection(url, transport); + return transport; } - /** - * Returns a factory for {@link Connection} that does not verify TLS peer verification. - * - * @throws GeneralSecurityException if unable to turn off TLS peer verification - * @return {@link Connection} factory, a function that generates a {@link Connection} to a URL - */ - public static Function getInsecureConnectionFactory() - throws GeneralSecurityException { - // Do not use NetHttpTransport. See comments in getConnectionFactory for details. - ApacheHttpTransport transport = - new ApacheHttpTransport.Builder().doNotValidateCertificate().build(); - addProxyCredentials(transport); - return url -> new Connection(url, transport); + private static HttpTransport getInsecureHttpTransport() { + try { + ApacheHttpTransport insecureTransport = + new ApacheHttpTransport.Builder().doNotValidateCertificate().build(); + addProxyCredentials(insecureTransport); + return insecureTransport; + } catch (GeneralSecurityException ex) { + throw new RuntimeException("platform does not support TLS protocol", ex); + } } /** @@ -114,87 +135,157 @@ private static void addProxyCredentials(ApacheHttpTransport transport, String pr new UsernamePasswordCredentials(proxyUser, proxyPassword)); } - private HttpRequestFactory requestFactory; - - @Nullable private HttpResponse httpResponse; - - /** The URL to send the request to. */ - private final GenericUrl url; + private final boolean enableHttpAndInsecureFailover; + private final boolean sendAuthorizationOverHttp; + private final Consumer logger; + private final Supplier secureHttpTransportFactory; + private final Supplier insecureHttpTransportFactory; - /** - * Make sure to wrap with a try-with-resource to ensure that the connection is closed after usage. - * - * @param url the url to send the request to - */ - @VisibleForTesting - Connection(URL url, HttpTransport transport) { - this.url = new GenericUrl(url); - requestFactory = transport.createRequestFactory(); + public Connection( + boolean enableHttpAndInsecureFailover, + boolean sendAuthorizationOverHttp, + Consumer logger) { + this( + enableHttpAndInsecureFailover, + sendAuthorizationOverHttp, + logger, + Connection::getSecureHttpTransport, + Connection::getInsecureHttpTransport); } - @Override - public void close() throws IOException { - if (httpResponse == null) { - return; - } - - httpResponse.disconnect(); + @VisibleForTesting + Connection( + boolean enableHttpAndInsecureFailover, + boolean sendAuthorizationOverHttp, + Consumer logger, + Supplier secureHttpTransportFactory, + Supplier insecureHttpTransportFactory) { + this.enableHttpAndInsecureFailover = enableHttpAndInsecureFailover; + this.sendAuthorizationOverHttp = sendAuthorizationOverHttp; + this.logger = logger; + this.secureHttpTransportFactory = secureHttpTransportFactory; + this.insecureHttpTransportFactory = insecureHttpTransportFactory; } /** * Sends the request with method GET. * + * @param url endpoint URL * @param request the request to send * @return the response to the sent request * @throws IOException if sending the request fails */ - public Response get(Request request) throws IOException { - return send(HttpMethods.GET, request); + public Response get(URL url, Request request) throws IOException { + return call(HttpMethods.GET, url, request); } /** * Sends the request with method POST. * + * @param url endpoint URL * @param request the request to send * @return the response to the sent request * @throws IOException if sending the request fails */ - public Response post(Request request) throws IOException { - return send(HttpMethods.POST, request); + public Response post(URL url, Request request) throws IOException { + return call(HttpMethods.POST, url, request); } /** * Sends the request with method PUT. * + * @param url endpoint URL * @param request the request to send * @return the response to the sent request * @throws IOException if sending the request fails */ - public Response put(Request request) throws IOException { - return send(HttpMethods.PUT, request); + public Response put(URL url, Request request) throws IOException { + return call(HttpMethods.PUT, url, request); } /** * Sends the request. * * @param httpMethod the HTTP request method + * @param url endpoint URL * @param request the request to send * @return the response to the sent request * @throws IOException if building the HTTP request fails. */ - public Response send(String httpMethod, Request request) throws IOException { - Preconditions.checkState(httpResponse == null, "Connection can send only one request"); + public Response call(String httpMethod, URL url, Request request) throws IOException { + if (!isHttpsProtocol(url) && !enableHttpAndInsecureFailover) { + throw new SSLException("insecure HTTP connection not allowed: " + url); + } + + try { + return call(httpMethod, url, request, secureHttpTransportFactory.get()); + + } catch (SSLException ex) { + if (!enableHttpAndInsecureFailover) { + throw ex; + } + + try { + logInsecureHttpsFailover(url); + return call(httpMethod, url, request, insecureHttpTransportFactory.get()); + + } catch (SSLException ignored) { // This is usually when the server is plain-HTTP. + logHttpFailover(url); + return call(httpMethod, toHttp(url), request, secureHttpTransportFactory.get()); + } + + } catch (ConnectException ex) { + // It is observed that Open/Oracle JDKs sometimes throw SocketTimeoutException but other times + // ConnectException for connection timeout. (Could be a JDK bug.) Note SocketTimeoutException + // does not extend ConnectException (or vice versa), and we want to be consistent to error out + // on timeouts: https://github.com/GoogleContainerTools/jib/issues/1895#issuecomment-527544094 + if (ex.getMessage() != null && ex.getMessage().contains("timed out")) { + throw ex; + } + + // Fall back to HTTP only if "url" had no port specified (i.e., we tried the default HTTPS + // port 443) and we could not connect to 443. It's worth trying port 80. + if (enableHttpAndInsecureFailover && isHttpsProtocol(url) && url.getPort() == -1) { + logHttpFailover(url); + return call(httpMethod, toHttp(url), request, secureHttpTransportFactory.get()); + } + throw ex; + } + } + + private Response call(String httpMethod, URL url, Request request, HttpTransport httpTransport) + throws IOException { + boolean clearAuthorization = !isHttpsProtocol(url) && !sendAuthorizationOverHttp; + + HttpHeaders requestHeaders = + clearAuthorization + ? request.getHeaders().clone().setAuthorization((String) null) // deep clone implemented + : request.getHeaders(); HttpRequest httpRequest = - requestFactory - .buildRequest(httpMethod, url, request.getHttpContent()) - .setHeaders(request.getHeaders()); + httpTransport + .createRequestFactory() + .buildRequest(httpMethod, new GenericUrl(url), request.getHttpContent()) + .setHeaders(requestHeaders); if (request.getHttpTimeout() != null) { httpRequest.setConnectTimeout(request.getHttpTimeout()); httpRequest.setReadTimeout(request.getHttpTimeout()); } - httpResponse = httpRequest.execute(); - return new Response(httpResponse); + try { + return new Response(httpRequest.execute()); + } catch (HttpResponseException ex) { + throw new ResponseException(ex, clearAuthorization); + } + } + + private void logHttpFailover(URL url) { + String log = "Failed to connect to " + url + " over HTTPS. Attempting again with HTTP."; + logger.accept(LogEvent.info(log)); + } + + private void logInsecureHttpsFailover(URL url) { + String log = "Cannot verify server at " + url + ". Attempting again with no TLS verification."; + logger.accept(LogEvent.info(log)); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Response.java b/jib-core/src/main/java/com/google/cloud/tools/jib/http/Response.java index 3c7f895586..e8bd74ee1d 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Response.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/http/Response.java @@ -19,12 +19,13 @@ import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpResponse; import com.google.common.net.HttpHeaders; +import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.util.List; /** Holds an HTTP response. */ -public class Response { +public class Response implements Closeable { private final HttpResponse httpResponse; @@ -75,4 +76,9 @@ public InputStream getBody() throws IOException { public GenericUrl getRequestUrl() { return httpResponse.getRequest().getUrl(); } + + @Override + public void close() throws IOException { + httpResponse.disconnect(); + } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/http/ResponseException.java b/jib-core/src/main/java/com/google/cloud/tools/jib/http/ResponseException.java new file mode 100644 index 0000000000..d798e01ce0 --- /dev/null +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/http/ResponseException.java @@ -0,0 +1,56 @@ +/* + * Copyright 2019 Google LLC. + * + * 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.google.cloud.tools.jib.http; + +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpResponseException; +import java.io.IOException; + +/** Holds an HTTP response exception. */ +public class ResponseException extends IOException { + + private final HttpResponseException httpResponseException; + private final boolean requestAuthorizationCleared; + + ResponseException( + HttpResponseException httpResponseException, boolean requestAuthorizationCleared) { + super(httpResponseException.getMessage(), httpResponseException); + this.httpResponseException = httpResponseException; + this.requestAuthorizationCleared = requestAuthorizationCleared; + } + + public int getStatusCode() { + return httpResponseException.getStatusCode(); + } + + public String getContent() { + return httpResponseException.getContent(); + } + + public HttpHeaders getHeaders() { + return httpResponseException.getHeaders(); + } + + /** + * Returns whether the {@code Authorization} HTTP header was cleared (and thus not sent). + * + * @return whether the {@code Authorization} HTTP header was cleared + */ + public boolean requestAuthorizationCleared() { + return requestAuthorizationCleared; + } +} diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java index eba5a28971..5176cec504 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java @@ -17,11 +17,12 @@ package com.google.cloud.tools.jib.registry; import com.google.api.client.http.HttpMethods; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException; import com.google.cloud.tools.jib.http.BlobHttpContent; +import com.google.cloud.tools.jib.http.Connection; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import java.net.MalformedURLException; import java.net.URL; import java.util.Collections; @@ -35,11 +36,15 @@ class AuthenticationMethodRetriever private final RegistryEndpointRequestProperties registryEndpointRequestProperties; private final String userAgent; + private final Connection httpClient; AuthenticationMethodRetriever( - RegistryEndpointRequestProperties registryEndpointRequestProperties, String userAgent) { + RegistryEndpointRequestProperties registryEndpointRequestProperties, + String userAgent, + Connection httpClient) { this.registryEndpointRequestProperties = registryEndpointRequestProperties; this.userAgent = userAgent; + this.httpClient = httpClient; } @Nullable @@ -81,8 +86,7 @@ public String getActionDescription() { @Override public Optional handleHttpResponseException( - HttpResponseException responseException) - throws HttpResponseException, RegistryErrorException { + ResponseException responseException) throws ResponseException, RegistryErrorException { // Only valid for status code of '401 Unauthorized'. if (responseException.getStatusCode() != HttpStatusCodes.STATUS_CODE_UNAUTHORIZED) { throw responseException; @@ -99,7 +103,7 @@ public Optional handleHttpResponseException( // Parses the header to retrieve the components. try { return RegistryAuthenticator.fromAuthenticationMethod( - authenticationMethod, registryEndpointRequestProperties, userAgent); + authenticationMethod, registryEndpointRequestProperties, userAgent, httpClient); } catch (RegistryAuthenticationFailedException ex) { throw new RegistryErrorExceptionBuilder(getActionDescription(), ex) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobChecker.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobChecker.java index eb3b3bb1ce..ff4e4c2a4d 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobChecker.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobChecker.java @@ -17,12 +17,12 @@ package com.google.cloud.tools.jib.registry; import com.google.api.client.http.HttpMethods; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.DescriptorDigest; import com.google.cloud.tools.jib.blob.BlobDescriptor; import com.google.cloud.tools.jib.http.BlobHttpContent; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import java.net.MalformedURLException; import java.net.URL; import java.util.Collections; @@ -60,8 +60,8 @@ public Optional handleResponse(Response response) throws Registr } @Override - public Optional handleHttpResponseException( - HttpResponseException responseException) throws HttpResponseException { + public Optional handleHttpResponseException(ResponseException responseException) + throws ResponseException { if (responseException.getStatusCode() != HttpStatusCodes.STATUS_CODE_NOT_FOUND) { throw responseException; } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ErrorResponseUtil.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ErrorResponseUtil.java index 6548317f7a..afe63988a0 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ErrorResponseUtil.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ErrorResponseUtil.java @@ -17,6 +17,7 @@ package com.google.cloud.tools.jib.registry; import com.google.api.client.http.HttpResponseException; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.json.JsonTemplateMapper; import com.google.cloud.tools.jib.registry.json.ErrorEntryTemplate; import com.google.cloud.tools.jib.registry.json.ErrorResponseTemplate; @@ -32,11 +33,11 @@ public class ErrorResponseUtil { * * @param responseException the response exception * @return the parsed {@link ErrorCodes} if found - * @throws HttpResponseException rethrows the original exception if an error object could not be + * @throws ResponseException rethrows the original exception if an error object could not be * parsed, if there were multiple error objects, or if the error code is unknown. */ - public static ErrorCodes getErrorCode(HttpResponseException responseException) - throws HttpResponseException { + public static ErrorCodes getErrorCode(ResponseException responseException) + throws ResponseException { // Obtain the error response code. String errorContent = responseException.getContent(); if (errorContent == null) { diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPusher.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPusher.java index e0797034b4..381fda1337 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPusher.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPusher.java @@ -17,7 +17,6 @@ package com.google.cloud.tools.jib.registry; import com.google.api.client.http.HttpMethods; -import com.google.api.client.http.HttpResponseException; import com.google.cloud.tools.jib.api.DescriptorDigest; import com.google.cloud.tools.jib.api.LogEvent; import com.google.cloud.tools.jib.blob.Blobs; @@ -25,6 +24,7 @@ import com.google.cloud.tools.jib.hash.Digests; import com.google.cloud.tools.jib.http.BlobHttpContent; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate; import java.io.IOException; import java.net.MalformedURLException; @@ -92,8 +92,8 @@ public List getAccept() { } @Override - public DescriptorDigest handleHttpResponseException(HttpResponseException responseException) - throws HttpResponseException, RegistryErrorException { + public DescriptorDigest handleHttpResponseException(ResponseException responseException) + throws ResponseException, RegistryErrorException { // docker registry 2.0 and 2.1 returns: // 400 Bad Request // {"errors":[{"code":"TAG_INVALID","message":"manifest tag did not match URI"}]} diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java index 05638595b7..8cd76441ab 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.google.api.client.http.HttpMethods; +import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.Credential; import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException; import com.google.cloud.tools.jib.blob.Blobs; @@ -27,6 +28,7 @@ import com.google.cloud.tools.jib.http.Connection; import com.google.cloud.tools.jib.http.Request; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.json.JsonTemplate; import com.google.cloud.tools.jib.json.JsonTemplateMapper; import com.google.common.annotations.VisibleForTesting; @@ -58,6 +60,7 @@ public class RegistryAuthenticator { * @param authenticationMethod the {@code WWW-Authenticate} header value * @param registryEndpointRequestProperties the registry request properties * @param userAgent the {@code User-Agent} header value to use in later authentication calls + * @param httpClient HTTP client * @return a new {@link RegistryAuthenticator} for authenticating with the registry service * @throws RegistryAuthenticationFailedException if authentication fails * @see fromAuthenticationMethod( String authenticationMethod, RegistryEndpointRequestProperties registryEndpointRequestProperties, - String userAgent) + String userAgent, + Connection httpClient) throws RegistryAuthenticationFailedException { // If the authentication method starts with 'basic ' (case insensitive), no registry // authentication is needed. @@ -96,7 +100,8 @@ static Optional fromAuthenticationMethod( String service = serviceMatcher.find() ? serviceMatcher.group(1) : registryUrl; return Optional.of( - new RegistryAuthenticator(realm, service, registryEndpointRequestProperties, userAgent)); + new RegistryAuthenticator( + realm, service, registryEndpointRequestProperties, userAgent, httpClient)); } private static RegistryAuthenticationFailedException newRegistryAuthenticationFailedException( @@ -138,16 +143,19 @@ private String getToken() { private final String realm; private final String service; private final String userAgent; + private final Connection httpClient; private RegistryAuthenticator( String realm, String service, RegistryEndpointRequestProperties registryEndpointRequestProperties, - String userAgent) { + String userAgent, + Connection httpClient) { this.realm = realm; this.service = service; this.registryEndpointRequestProperties = registryEndpointRequestProperties; this.userAgent = userAgent; + this.httpClient = httpClient; } /** @@ -156,9 +164,11 @@ private RegistryAuthenticator( * @param credential the credential used to authenticate * @return an {@code Authorization} authenticating the pull * @throws RegistryAuthenticationFailedException if authentication fails + * @throws RegistryCredentialsNotSentException if authentication is failed and credentials were + * not sent over plain HTTP */ public Authorization authenticatePull(@Nullable Credential credential) - throws RegistryAuthenticationFailedException { + throws RegistryAuthenticationFailedException, RegistryCredentialsNotSentException { return authenticate(credential, "pull"); } @@ -168,9 +178,11 @@ public Authorization authenticatePull(@Nullable Credential credential) * @param credential the credential used to authenticate * @return an {@code Authorization} authenticating the push * @throws RegistryAuthenticationFailedException if authentication fails + * @throws RegistryCredentialsNotSentException if authentication is failed and credentials were + * not sent over plain HTTP */ public Authorization authenticatePush(@Nullable Credential credential) - throws RegistryAuthenticationFailedException { + throws RegistryAuthenticationFailedException, RegistryCredentialsNotSentException { return authenticate(credential, "pull,push"); } @@ -220,11 +232,13 @@ boolean isOAuth2Auth(@Nullable Credential credential) { * @param scope the scope of permissions to authenticate for * @return the {@link Authorization} response * @throws RegistryAuthenticationFailedException if authentication fails + * @throws RegistryCredentialsNotSentException if authentication is failed and credentials were + * not sent over plain HTTP * @see https://docs.docker.com/registry/spec/auth/token/#how-to-authenticate */ private Authorization authenticate(@Nullable Credential credential, String scope) - throws RegistryAuthenticationFailedException { + throws RegistryAuthenticationFailedException, RegistryCredentialsNotSentException { // try authorizing against both the main repository and the source repository too // to enable cross-repository mounts on pushes String sourceImageName = registryEndpointRequestProperties.getSourceImageName(); @@ -241,12 +255,12 @@ private Authorization authenticate(@Nullable Credential credential, String scope private Authorization authenticate( @Nullable Credential credential, Map repositoryScopes) - throws RegistryAuthenticationFailedException { + throws RegistryAuthenticationFailedException, RegistryCredentialsNotSentException { String registryUrl = registryEndpointRequestProperties.getServerUrl(); String imageName = registryEndpointRequestProperties.getImageName(); - try (Connection connection = - Connection.getConnectionFactory() - .apply(getAuthenticationUrl(credential, repositoryScopes))) { + try { + URL url = getAuthenticationUrl(credential, repositoryScopes); + Request.Builder requestBuilder = Request.builder() .setHttpTimeout(JibSystemProperties.getHttpTimeout()) @@ -262,21 +276,29 @@ private Authorization authenticate( } String httpMethod = isOAuth2Auth(credential) ? HttpMethods.POST : HttpMethods.GET; - Response response = connection.send(httpMethod, requestBuilder.build()); + try (Response response = httpClient.call(httpMethod, url, requestBuilder.build())) { + + AuthenticationResponseTemplate responseJson = + JsonTemplateMapper.readJson(response.getBody(), AuthenticationResponseTemplate.class); - AuthenticationResponseTemplate responseJson = - JsonTemplateMapper.readJson(response.getBody(), AuthenticationResponseTemplate.class); + if (responseJson.getToken() == null) { + throw new RegistryAuthenticationFailedException( + registryUrl, + imageName, + "Did not get token in authentication response from " + + getAuthenticationUrl(credential, repositoryScopes) + + "; parameters: " + + getAuthRequestParameters(credential, repositoryScopes)); + } + return Authorization.fromBearerToken(responseJson.getToken()); + } - if (responseJson.getToken() == null) { - throw new RegistryAuthenticationFailedException( - registryUrl, - imageName, - "Did not get token in authentication response from " - + getAuthenticationUrl(credential, repositoryScopes) - + "; parameters: " - + getAuthRequestParameters(credential, repositoryScopes)); + } catch (ResponseException ex) { + if (ex.getStatusCode() == HttpStatusCodes.STATUS_CODE_UNAUTHORIZED + && ex.requestAuthorizationCleared()) { + throw new RegistryCredentialsNotSentException(registryUrl, imageName); } - return Authorization.fromBearerToken(responseJson.getToken()); + throw new RegistryAuthenticationFailedException(registryUrl, imageName, ex); } catch (IOException ex) { throw new RegistryAuthenticationFailedException(registryUrl, imageName, ex); diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java index 834d040c2a..1214f6aa3b 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java @@ -28,6 +28,7 @@ import com.google.cloud.tools.jib.event.EventHandlers; import com.google.cloud.tools.jib.global.JibSystemProperties; import com.google.cloud.tools.jib.http.Authorization; +import com.google.cloud.tools.jib.http.Connection; import com.google.cloud.tools.jib.http.Response; import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate; import com.google.cloud.tools.jib.image.json.ManifestTemplate; @@ -242,8 +243,8 @@ static Multimap decodeTokenRepositoryGrants(String token) { private final EventHandlers eventHandlers; @Nullable private final Authorization authorization; private final RegistryEndpointRequestProperties registryEndpointRequestProperties; - private final boolean allowInsecureRegistries; private final String userAgent; + private final Connection httpClient; /** * Instantiate with {@link #factory}. @@ -252,6 +253,7 @@ static Multimap decodeTokenRepositoryGrants(String token) { * @param authorization the {@link Authorization} to access the registry/repository * @param registryEndpointRequestProperties properties of registry endpoint requests * @param allowInsecureRegistries if {@code true}, insecure connections will be allowed + * @param userAgent {@code User-Agent} header to send with the request */ private RegistryClient( EventHandlers eventHandlers, @@ -262,8 +264,12 @@ private RegistryClient( this.eventHandlers = eventHandlers; this.authorization = authorization; this.registryEndpointRequestProperties = registryEndpointRequestProperties; - this.allowInsecureRegistries = allowInsecureRegistries; this.userAgent = userAgent; + this.httpClient = + new Connection( + allowInsecureRegistries, + JibSystemProperties.sendCredentialsOverHttp(), + eventHandlers::dispatch); } /** @@ -277,7 +283,8 @@ public Optional getRegistryAuthenticator() // Gets the WWW-Authenticate header (eg. 'WWW-Authenticate: Bearer // realm="https://gcr.io/v2/token",service="gcr.io"') return callRegistryEndpoint( - new AuthenticationMethodRetriever(registryEndpointRequestProperties, getUserAgent())); + new AuthenticationMethodRetriever( + registryEndpointRequestProperties, getUserAgent(), httpClient)); } /** @@ -465,7 +472,7 @@ private T callRegistryEndpoint(RegistryEndpointProvider registryEndpointP registryEndpointProvider, authorization, registryEndpointRequestProperties, - allowInsecureRegistries) + httpClient) .call(); } } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java index c0a40be4ae..49cc444716 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java @@ -16,8 +16,6 @@ package com.google.cloud.tools.jib.registry; -import com.google.api.client.http.GenericUrl; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.InsecureRegistryException; import com.google.cloud.tools.jib.api.LogEvent; @@ -29,16 +27,14 @@ import com.google.cloud.tools.jib.http.Connection; import com.google.cloud.tools.jib.http.Request; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.json.JsonTemplateMapper; import com.google.cloud.tools.jib.registry.json.ErrorEntryTemplate; import com.google.cloud.tools.jib.registry.json.ErrorResponseTemplate; import com.google.common.annotations.VisibleForTesting; import java.io.IOException; -import java.net.ConnectException; import java.net.URL; -import java.security.GeneralSecurityException; import java.util.Locale; -import java.util.function.Function; import javax.annotation.Nullable; import javax.net.ssl.SSLException; @@ -55,10 +51,6 @@ class RegistryEndpointCaller { */ @VisibleForTesting static final int STATUS_CODE_PERMANENT_REDIRECT = 308; - private static boolean isHttpsProtocol(URL url) { - return "https".equals(url.getProtocol()); - } - // https://github.com/GoogleContainerTools/jib/issues/1316 @VisibleForTesting static boolean isBrokenPipe(IOException original) { @@ -82,13 +74,7 @@ static boolean isBrokenPipe(IOException original) { private final RegistryEndpointProvider registryEndpointProvider; @Nullable private final Authorization authorization; private final RegistryEndpointRequestProperties registryEndpointRequestProperties; - private final boolean allowInsecureRegistries; - - /** Makes a {@link Connection} to the specified {@link URL}. */ - private final Function connectionFactory; - - /** Makes an insecure {@link Connection} to the specified {@link URL}. */ - @Nullable private Function insecureConnectionFactory; + private final Connection httpClient; /** * Constructs with parameters for making the request. @@ -98,26 +84,8 @@ static boolean isBrokenPipe(IOException original) { * @param registryEndpointProvider the {@link RegistryEndpointProvider} to the endpoint * @param authorization optional authentication credentials to use * @param registryEndpointRequestProperties properties of the registry endpoint request - * @param allowInsecureRegistries if {@code true}, insecure connections will be allowed + * @param httpClient HTTP client */ - RegistryEndpointCaller( - EventHandlers eventHandlers, - String userAgent, - RegistryEndpointProvider registryEndpointProvider, - @Nullable Authorization authorization, - RegistryEndpointRequestProperties registryEndpointRequestProperties, - boolean allowInsecureRegistries) { - this( - eventHandlers, - userAgent, - registryEndpointProvider, - authorization, - registryEndpointRequestProperties, - allowInsecureRegistries, - Connection.getConnectionFactory(), - null /* might never be used, so create lazily to delay throwing potential GeneralSecurityException */); - } - @VisibleForTesting RegistryEndpointCaller( EventHandlers eventHandlers, @@ -125,17 +93,13 @@ static boolean isBrokenPipe(IOException original) { RegistryEndpointProvider registryEndpointProvider, @Nullable Authorization authorization, RegistryEndpointRequestProperties registryEndpointRequestProperties, - boolean allowInsecureRegistries, - Function connectionFactory, - @Nullable Function insecureConnectionFactory) { + Connection httpClient) { this.eventHandlers = eventHandlers; this.userAgent = userAgent; this.registryEndpointProvider = registryEndpointProvider; this.authorization = authorization; this.registryEndpointRequestProperties = registryEndpointRequestProperties; - this.allowInsecureRegistries = allowInsecureRegistries; - this.connectionFactory = connectionFactory; - this.insecureConnectionFactory = insecureConnectionFactory; + this.httpClient = httpClient; } /** @@ -148,73 +112,7 @@ static boolean isBrokenPipe(IOException original) { T call() throws IOException, RegistryException { String apiRouteBase = "https://" + registryEndpointRequestProperties.getServerUrl() + "/v2/"; URL initialRequestUrl = registryEndpointProvider.getApiRoute(apiRouteBase); - return callWithAllowInsecureRegistryHandling(initialRequestUrl); - } - - private T callWithAllowInsecureRegistryHandling(URL url) throws IOException, RegistryException { - if (!isHttpsProtocol(url) && !allowInsecureRegistries) { - throw new InsecureRegistryException(url); - } - - try { - return call(url, connectionFactory); - - } catch (SSLException ex) { - return handleUnverifiableServerException(url); - - } catch (ConnectException ex) { - // It is observed that Open/Oracle JDKs sometimes throw SocketTimeoutException but other times - // ConnectException for connection timeout. (Could be a JDK bug.) Note SocketTimeoutException - // does not extend ConnectException (or vice versa), and we want to be consistent to error out - // on timeouts: https://github.com/GoogleContainerTools/jib/issues/1895#issuecomment-527544094 - if (ex.getMessage() != null && ex.getMessage().contains("timed out")) { - throw ex; - } - - if (allowInsecureRegistries && isHttpsProtocol(url) && url.getPort() == -1) { - // Fall back to HTTP only if "url" had no port specified (i.e., we tried the default HTTPS - // port 443) and we could not connect to 443. It's worth trying port 80. - return fallBackToHttp(url); - } - throw ex; - } - } - - private T handleUnverifiableServerException(URL url) throws IOException, RegistryException { - if (!allowInsecureRegistries) { - throw new InsecureRegistryException(url); - } - - try { - eventHandlers.dispatch( - LogEvent.info( - "Cannot verify server at " + url + ". Attempting again with no TLS verification.")); - return call(url, getInsecureConnectionFactory()); - - } catch (SSLException ex) { - return fallBackToHttp(url); - } - } - - private T fallBackToHttp(URL url) throws IOException, RegistryException { - GenericUrl httpUrl = new GenericUrl(url); - httpUrl.setScheme("http"); - eventHandlers.dispatch( - LogEvent.info( - "Failed to connect to " + url + " over HTTPS. Attempting again with HTTP: " + httpUrl)); - return call(httpUrl.toURL(), connectionFactory); - } - - private Function getInsecureConnectionFactory() throws RegistryException { - try { - if (insecureConnectionFactory == null) { - insecureConnectionFactory = Connection.getInsecureConnectionFactory(); - } - return insecureConnectionFactory; - - } catch (GeneralSecurityException ex) { - throw new RegistryException("cannot turn off TLS peer verification", ex); - } + return call(initialRequestUrl); } /** @@ -225,34 +123,29 @@ private Function getInsecureConnectionFactory() throws Registry * @throws IOException for most I/O exceptions when making the request * @throws RegistryException for known exceptions when interacting with the registry */ - private T call(URL url, Function connectionFactory) - throws IOException, RegistryException { - // Only sends authorization if using HTTPS or explicitly forcing over HTTP. - boolean sendCredentials = isHttpsProtocol(url) || JibSystemProperties.sendCredentialsOverHttp(); + private T call(URL url) throws IOException, RegistryException { String serverUrl = registryEndpointRequestProperties.getServerUrl(); String imageName = registryEndpointRequestProperties.getImageName(); - try (Connection connection = connectionFactory.apply(url)) { - Request.Builder requestBuilder = - Request.builder() - .setUserAgent(userAgent) - .setHttpTimeout(JibSystemProperties.getHttpTimeout()) - .setAccept(registryEndpointProvider.getAccept()) - .setBody(registryEndpointProvider.getContent()); - if (sendCredentials) { - requestBuilder.setAuthorization(authorization); - } - Response response = - connection.send(registryEndpointProvider.getHttpMethod(), requestBuilder.build()); + Request.Builder requestBuilder = + Request.builder() + .setUserAgent(userAgent) + .setHttpTimeout(JibSystemProperties.getHttpTimeout()) + .setAccept(registryEndpointProvider.getAccept()) + .setBody(registryEndpointProvider.getContent()) + .setAuthorization(authorization); + + try (Response response = + httpClient.call(registryEndpointProvider.getHttpMethod(), url, requestBuilder.build())) { return registryEndpointProvider.handleResponse(response); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { // First, see if the endpoint provider handles an exception as an expected response. try { return registryEndpointProvider.handleHttpResponseException(ex); - } catch (HttpResponseException responseException) { + } catch (ResponseException responseException) { if (responseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_BAD_REQUEST || responseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_NOT_FOUND || responseException.getStatusCode() @@ -264,11 +157,11 @@ private T call(URL url, Function connectionFactory) throw new RegistryUnauthorizedException(serverUrl, imageName, responseException); } else if (responseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_UNAUTHORIZED) { - if (sendCredentials) { + if (responseException.requestAuthorizationCleared()) { + throw new RegistryCredentialsNotSentException(serverUrl, imageName); + } else { // Credentials are either missing or wrong. throw new RegistryUnauthorizedException(serverUrl, imageName, responseException); - } else { - throw new RegistryCredentialsNotSentException(serverUrl, imageName); } } else if (responseException.getStatusCode() @@ -277,7 +170,7 @@ private T call(URL url, Function connectionFactory) || responseException.getStatusCode() == STATUS_CODE_PERMANENT_REDIRECT) { // 'Location' header can be relative or absolute. URL redirectLocation = new URL(url, responseException.getHeaders().getLocation()); - return callWithAllowInsecureRegistryHandling(redirectLocation); + return call(redirectLocation); } else { // Unknown @@ -285,20 +178,20 @@ private T call(URL url, Function connectionFactory) } } - } catch (SSLException ex) { - logErrorIfBrokenPipe(ex); - throw ex; - } catch (IOException ex) { logError("I/O error for image [" + serverUrl + "/" + imageName + "]:"); logError(" " + ex.getMessage()); logErrorIfBrokenPipe(ex); + + if (ex instanceof SSLException) { + throw new InsecureRegistryException(url, ex); + } throw ex; } } @VisibleForTesting - RegistryErrorException newRegistryErrorException(HttpResponseException responseException) { + RegistryErrorException newRegistryErrorException(ResponseException responseException) { RegistryErrorExceptionBuilder registryErrorExceptionBuilder = new RegistryErrorExceptionBuilder( registryEndpointProvider.getActionDescription(), responseException); diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java index 0381352dc3..d6a2e7c502 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java @@ -20,6 +20,7 @@ import com.google.cloud.tools.jib.api.RegistryException; import com.google.cloud.tools.jib.http.BlobHttpContent; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; @@ -53,15 +54,15 @@ interface RegistryEndpointProvider { T handleResponse(Response response) throws IOException, RegistryException; /** - * Handles an {@link HttpResponseException} that occurs. + * Handles an {@link ResponseException} that occurs. * - * @param responseException the {@link HttpResponseException} to handle + * @param responseException the {@link ResponseException} to handle * @throws HttpResponseException {@code responseException} if {@code responseException} could not * be handled * @throws RegistryErrorException if there is an error with a remote registry */ - default T handleHttpResponseException(HttpResponseException responseException) - throws HttpResponseException, RegistryErrorException { + default T handleHttpResponseException(ResponseException responseException) + throws ResponseException, RegistryErrorException { throw responseException; } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java index e68be69f92..6049f28bf8 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java @@ -22,73 +22,91 @@ import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpResponse; +import com.google.api.client.http.HttpTransport; +import com.google.cloud.tools.jib.api.LogEvent; import com.google.cloud.tools.jib.blob.Blobs; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.net.ConnectException; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.concurrent.atomic.LongAdder; +import java.util.function.Consumer; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLPeerUnverifiedException; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.InjectMocks; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; /** Tests for {@link Connection}. */ @RunWith(MockitoJUnitRunner.class) -public class ConnectionTest { +public class ConnectionTest { // TODO: rename to TlsFailoverHttpClient @FunctionalInterface - private interface SendFunction { + private interface CallFunction { - Response send(Connection connection, Request request) throws IOException; + Response call(Connection httpClient, URL url, Request request) throws IOException; } + @Mock private HttpTransport mockHttpTransport; + @Mock private HttpTransport mockInsecureHttpTransport; @Mock private HttpRequestFactory mockHttpRequestFactory; + @Mock private HttpRequestFactory mockInsecureHttpRequestFactory; @Mock private HttpRequest mockHttpRequest; + @Mock private HttpRequest mockInsecureHttpRequest; + @Mock private HttpResponse mockHttpResponse; + @Mock private Consumer logger; - private final ArgumentCaptor httpHeadersArgumentCaptor = - ArgumentCaptor.forClass(HttpHeaders.class); - private final ArgumentCaptor blobHttpContentArgumentCaptor = - ArgumentCaptor.forClass(BlobHttpContent.class); + @Captor private ArgumentCaptor httpHeadersCaptor; + @Captor private ArgumentCaptor blobHttpContentCaptor; + @Captor private ArgumentCaptor urlCaptor; - private final GenericUrl fakeUrl = new GenericUrl("http://crepecake/fake/url"); + private final GenericUrl fakeUrl = new GenericUrl("https://crepecake/fake/url"); private final LongAdder totalByteCount = new LongAdder(); - private Request fakeRequest; - private HttpResponse mockHttpResponse; + @Before + public void setUp() throws IOException { + ByteArrayInputStream inStream = new ByteArrayInputStream(new byte[] {'b', 'o', 'd', 'y'}); + Mockito.when(mockHttpResponse.getContent()).thenReturn(inStream); + } - @InjectMocks - private final Connection testConnection = - Connection.getConnectionFactory().apply(fakeUrl.toURL()); + private Connection newHttpClient(boolean insecure, boolean authOverHttp) throws IOException { + setUpMocks(mockHttpTransport, mockHttpRequestFactory, mockHttpRequest); + if (insecure) { + setUpMocks( + mockInsecureHttpTransport, mockInsecureHttpRequestFactory, mockInsecureHttpRequest); + } + return new Connection( + insecure, authOverHttp, logger, () -> mockHttpTransport, () -> mockInsecureHttpTransport); + } @Test public void testGet() throws IOException { - setUpMocksAndFakes(null); - testSend(HttpMethods.GET, Connection::get); + verifyCall(HttpMethods.GET, Connection::get); } @Test public void testPost() throws IOException { - setUpMocksAndFakes(null); - testSend(HttpMethods.POST, Connection::post); + verifyCall(HttpMethods.POST, Connection::post); } @Test public void testPut() throws IOException { - setUpMocksAndFakes(null); - testSend(HttpMethods.PUT, Connection::put); + verifyCall(HttpMethods.PUT, Connection::put); } @Test public void testHttpTimeout_doNotSetByDefault() throws IOException { - setUpMocksAndFakes(null); - try (Connection connection = testConnection) { - connection.send(HttpMethods.GET, fakeRequest); - } + try (Response ignored = newHttpClient(false, false).get(fakeUrl.toURL(), fakeRequest(null))) {} Mockito.verify(mockHttpRequest, Mockito.never()).setConnectTimeout(Mockito.anyInt()); Mockito.verify(mockHttpRequest, Mockito.never()).setReadTimeout(Mockito.anyInt()); @@ -96,66 +114,283 @@ public void testHttpTimeout_doNotSetByDefault() throws IOException { @Test public void testHttpTimeout() throws IOException { - setUpMocksAndFakes(5982); - try (Connection connection = testConnection) { - connection.send(HttpMethods.GET, fakeRequest); - } + Connection httpClient = newHttpClient(false, false); + try (Response ignored = httpClient.get(fakeUrl.toURL(), fakeRequest(5982))) {} Mockito.verify(mockHttpRequest).setConnectTimeout(5982); Mockito.verify(mockHttpRequest).setReadTimeout(5982); } - private void setUpMocksAndFakes(Integer httpTimeout) throws IOException { - fakeRequest = - Request.builder() - .setAccept(Arrays.asList("fake.accept", "another.fake.accept")) - .setUserAgent("fake user agent") - .setBody( - new BlobHttpContent( - Blobs.from("crepecake"), "fake.content.type", totalByteCount::add)) - .setAuthorization(Authorization.fromBasicCredentials("fake-username", "fake-secret")) - .setHttpTimeout(httpTimeout) - .build(); + private Request fakeRequest(Integer httpTimeout) { + return Request.builder() + .setAccept(Arrays.asList("fake.accept", "another.fake.accept")) + .setUserAgent("fake user agent") + .setBody( + new BlobHttpContent(Blobs.from("crepecake"), "fake.content.type", totalByteCount::add)) + .setAuthorization(Authorization.fromBasicCredentials("fake-username", "fake-secret")) + .setHttpTimeout(httpTimeout) + .build(); + } + private void setUpMocks( + HttpTransport mockHttpTransport, + HttpRequestFactory mockHttpRequestFactory, + HttpRequest mockHttpRequest) + throws IOException { + Mockito.when(mockHttpTransport.createRequestFactory()).thenReturn(mockHttpRequestFactory); Mockito.when( - mockHttpRequestFactory.buildRequest( - Mockito.anyString(), Mockito.eq(fakeUrl), Mockito.any(BlobHttpContent.class))) + mockHttpRequestFactory.buildRequest(Mockito.any(), urlCaptor.capture(), Mockito.any())) .thenReturn(mockHttpRequest); - Mockito.when(mockHttpRequest.setHeaders(Mockito.any(HttpHeaders.class))) + Mockito.when(mockHttpRequest.setHeaders(httpHeadersCaptor.capture())) .thenReturn(mockHttpRequest); Mockito.when(mockHttpRequest.setConnectTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest); Mockito.when(mockHttpRequest.setReadTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest); - mockHttpResponse = Mockito.mock(HttpResponse.class); Mockito.when(mockHttpRequest.execute()).thenReturn(mockHttpResponse); } - private void testSend(String httpMethod, SendFunction sendFunction) throws IOException { - try (Connection connection = testConnection) { - sendFunction.send(connection, fakeRequest); - } - - Mockito.verify(mockHttpRequest).setHeaders(httpHeadersArgumentCaptor.capture()); - Mockito.verify(mockHttpResponse).disconnect(); + private void verifyCall(String httpMethod, CallFunction callFunction) throws IOException { + Connection httpClient = newHttpClient(false, false); + try (Response ignored = callFunction.call(httpClient, fakeUrl.toURL(), fakeRequest(null))) {} Assert.assertEquals( - "fake.accept,another.fake.accept", httpHeadersArgumentCaptor.getValue().getAccept()); - Assert.assertEquals("fake user agent", httpHeadersArgumentCaptor.getValue().getUserAgent()); + "fake.accept,another.fake.accept", httpHeadersCaptor.getValue().getAccept()); + Assert.assertEquals("fake user agent", httpHeadersCaptor.getValue().getUserAgent()); // Base64 representation of "fake-username:fake-secret" Assert.assertEquals( "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==", - httpHeadersArgumentCaptor.getValue().getAuthorization()); + httpHeadersCaptor.getValue().getAuthorization()); Mockito.verify(mockHttpRequestFactory) - .buildRequest( - Mockito.eq(httpMethod), Mockito.eq(fakeUrl), blobHttpContentArgumentCaptor.capture()); - Assert.assertEquals("fake.content.type", blobHttpContentArgumentCaptor.getValue().getType()); + .buildRequest(Mockito.eq(httpMethod), Mockito.eq(fakeUrl), blobHttpContentCaptor.capture()); + Assert.assertEquals("fake.content.type", blobHttpContentCaptor.getValue().getType()); ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); - blobHttpContentArgumentCaptor.getValue().writeTo(byteArrayOutputStream); + blobHttpContentCaptor.getValue().writeTo(byteArrayOutputStream); - Assert.assertEquals( - "crepecake", new String(byteArrayOutputStream.toByteArray(), StandardCharsets.UTF_8)); + Assert.assertEquals("crepecake", byteArrayOutputStream.toString(StandardCharsets.UTF_8.name())); Assert.assertEquals("crepecake".length(), totalByteCount.longValue()); } + + @Test + public void testGet_nonHttpsServer_insecureConnectionAndFailoverDisabled() + throws MalformedURLException, IOException { + Connection httpClient = newHttpClient(false, false); + try (Response response = httpClient.get(new URL("http://plain.http"), fakeRequest(null))) { + Assert.fail("Should disallow non-HTTP attempt"); + } catch (SSLException ex) { + Assert.assertEquals( + "insecure HTTP connection not allowed: http://plain.http", ex.getMessage()); + } + } + + @Test + public void testCall_secureClientOnUnverifiableServer() throws IOException { + Connection httpClient = newHttpClient(false, false); + + Mockito.when(mockHttpRequest.execute()).thenThrow(new SSLPeerUnverifiedException("unverified")); + + try (Response response = httpClient.get(new URL("https://insecure"), fakeRequest(null))) { + Assert.fail("Secure caller should fail if cannot verify server"); + } catch (SSLException ex) { + Assert.assertEquals("unverified", ex.getMessage()); + Mockito.verifyNoInteractions(logger); + } + } + + @Test + public void testGet_insecureClientOnUnverifiableServer() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Mockito.when(mockHttpRequest.execute()).thenThrow(new SSLPeerUnverifiedException("")); + + try (Response response = + insecureHttpClient.get(new URL("https://insecure"), fakeRequest(null))) { + byte[] bytes = new byte[4]; + Assert.assertEquals(4, response.getBody().read(bytes)); + Assert.assertEquals("body", new String(bytes, StandardCharsets.UTF_8)); + } + + Assert.assertEquals(2, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(0)); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(1)); + + String log = + "Cannot verify server at https://insecure. Attempting again with no TLS verification."; + Mockito.verify(logger).accept(LogEvent.info(log)); + Mockito.verifyNoMoreInteractions(logger); + } + + @Test + public void testGet_insecureClientOnHttpServer() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Mockito.when(mockHttpRequest.execute()) + .thenThrow(new SSLException("")) // server is not HTTPS + .thenReturn(mockHttpResponse); + Mockito.when(mockInsecureHttpRequest.execute()) + .thenThrow(new SSLException("")); // server is not HTTPS + + try (Response response = + insecureHttpClient.get(new URL("https://insecure"), fakeRequest(null))) { + byte[] bytes = new byte[4]; + Assert.assertEquals(4, response.getBody().read(bytes)); + Assert.assertEquals("body", new String(bytes, StandardCharsets.UTF_8)); + } + + Mockito.verify(mockHttpRequest, Mockito.times(2)).execute(); + Mockito.verify(mockInsecureHttpRequest, Mockito.times(1)).execute(); + + Assert.assertEquals(3, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(0)); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(1)); + Assert.assertEquals(new GenericUrl("http://insecure"), urlCaptor.getAllValues().get(2)); + + String log1 = + "Cannot verify server at https://insecure. Attempting again with no TLS verification."; + String log2 = "Failed to connect to https://insecure over HTTPS. Attempting again with HTTP."; + Mockito.verify(logger).accept(LogEvent.info(log1)); + Mockito.verify(logger).accept(LogEvent.info(log2)); + Mockito.verifyNoMoreInteractions(logger); + } + + @Test + public void testGet_insecureClientOnHttpServerAndNoPortSpecified() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Mockito.when(mockHttpRequest.execute()) + .thenThrow(new ConnectException()) // server is not listening on 443 + .thenReturn(mockHttpResponse); // respond when connected through 80 + + try (Response response = + insecureHttpClient.get(new URL("https://insecure"), fakeRequest(null))) { + byte[] bytes = new byte[4]; + Assert.assertEquals(4, response.getBody().read(bytes)); + Assert.assertEquals("body", new String(bytes, StandardCharsets.UTF_8)); + } + + Mockito.verify(mockHttpRequest, Mockito.times(2)).execute(); + Mockito.verifyNoInteractions(mockInsecureHttpRequest); + + Assert.assertEquals(2, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(0)); + Assert.assertEquals(new GenericUrl("http://insecure"), urlCaptor.getAllValues().get(1)); + + String log = "Failed to connect to https://insecure over HTTPS. Attempting again with HTTP."; + Mockito.verify(logger).accept(LogEvent.info(log)); + Mockito.verifyNoMoreInteractions(logger); + } + + @Test + public void testGet_secureClientOnNonListeningServerAndNoPortSpecified() throws IOException { + Connection httpClient = newHttpClient(false, false); + + Mockito.when(mockHttpRequest.execute()) + .thenThrow(new ConnectException("my exception")); // server not listening on 443 + + try (Response response = httpClient.get(new URL("https://insecure"), fakeRequest(null))) { + Assert.fail("Should not fall back to HTTP if port was explicitly given and cannot connect"); + } catch (ConnectException ex) { + Assert.assertEquals("my exception", ex.getMessage()); + + Assert.assertEquals(1, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getValue()); + + Mockito.verify(mockHttpRequest, Mockito.times(1)).execute(); + Mockito.verifyNoInteractions(mockInsecureHttpRequest, logger); + } + } + + @Test + public void testGet_insecureClientOnNonListeningServerAndPortSpecified() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Mockito.when(mockHttpRequest.execute()) + .thenThrow(new ConnectException("my exception")); // server is not listening on 5000 + + try (Response response = + insecureHttpClient.get(new URL("https://insecure:5000"), fakeRequest(null))) { + Assert.fail("Should not fall back to HTTP if port was explicitly given and cannot connect"); + } catch (ConnectException ex) { + Assert.assertEquals("my exception", ex.getMessage()); + + Assert.assertEquals(1, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure:5000"), urlCaptor.getValue()); + + Mockito.verify(mockHttpRequest, Mockito.times(1)).execute(); + Mockito.verifyNoInteractions(mockInsecureHttpRequest, logger); + } + } + + @Test + public void testGet_timeoutFromConnectException() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Mockito.when(mockHttpRequest.execute()).thenThrow(new ConnectException("Connection timed out")); + + try (Response response = + insecureHttpClient.get(new URL("https://insecure"), fakeRequest(null))) { + Assert.fail("Should not fall back to HTTP if timed out even for ConnectionException"); + } catch (ConnectException ex) { + Assert.assertEquals("Connection timed out", ex.getMessage()); + + Assert.assertEquals(1, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getValue()); + + Mockito.verify(mockHttpRequest, Mockito.times(1)).execute(); + Mockito.verifyNoInteractions(mockInsecureHttpRequest, logger); + } + } + + @Test + public void testGet_doNotSendCredentialsOverHttp() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + // make it fall back to HTTP + Mockito.when(mockHttpRequest.execute()) + .thenThrow(new ConnectException()) // server is not listening on 443 + .thenReturn(mockHttpResponse); // respond when connected through 80 + + try (Response response = + insecureHttpClient.get(new URL("https://insecure"), fakeRequest(null))) {} + + Assert.assertEquals(2, urlCaptor.getAllValues().size()); + Assert.assertEquals(new GenericUrl("https://insecure"), urlCaptor.getAllValues().get(0)); + Assert.assertEquals(new GenericUrl("http://insecure"), urlCaptor.getAllValues().get(1)); + + Assert.assertEquals(2, httpHeadersCaptor.getAllValues().size()); + Assert.assertEquals( + "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==", + httpHeadersCaptor.getAllValues().get(0).getAuthorization()); + Assert.assertNull(httpHeadersCaptor.getAllValues().get(1).getAuthorization()); + } + + @Test + public void testGet_sendCredentialsOverHttp() throws IOException { + Connection insecureHttpClient = newHttpClient(true, true); // sendCredentialsOverHttp + + try (Response response = + insecureHttpClient.get(new URL("http://plain.http"), fakeRequest(null))) {} + + Assert.assertEquals(1, urlCaptor.getAllValues().size()); + + Assert.assertEquals( + "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==", + httpHeadersCaptor.getValue().getAuthorization()); + } + + @Test + public void testGet_originalRequestHeaderUntouchedWhenClearingHeader() throws IOException { + Connection insecureHttpClient = newHttpClient(true, false); + + Request request = fakeRequest(null); + try (Response response = insecureHttpClient.get(new URL("http://plain.http"), request)) {} + + Assert.assertEquals(1, urlCaptor.getAllValues().size()); + Assert.assertEquals(1, httpHeadersCaptor.getAllValues().size()); + + Assert.assertNull(httpHeadersCaptor.getValue().getAuthorization()); + Assert.assertEquals( + "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==", request.getHeaders().getAuthorization()); + } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java index 5821f97911..385a05b21b 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java @@ -28,6 +28,7 @@ import org.junit.contrib.java.lang.system.RestoreSystemProperties; /** Tests for {@link Connection} with setting proxy credentials. */ +// TODO: rename to TlsFailoverHttpClientProxyCredentialsTest public class ConnectionWithProxyCredentialsTest { private static final ImmutableList proxyProperties = diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ResponseTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ResponseTest.java index 0cba195cd0..dd27e0bd7e 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ResponseTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ResponseTest.java @@ -41,8 +41,8 @@ public void testGetContent() throws IOException { Mockito.when(httpResponseMock.getContent()).thenReturn(responseInputStream); - Response response = new Response(httpResponseMock); - - Assert.assertArrayEquals(expectedResponse, ByteStreams.toByteArray(response.getBody())); + try (Response response = new Response(httpResponseMock)) { + Assert.assertArrayEquals(expectedResponse, ByteStreams.toByteArray(response.getBody())); + } } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java index 6d843b2c7c..ed2cd1063d 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java @@ -16,6 +16,7 @@ package com.google.cloud.tools.jib.http; +import com.google.cloud.tools.jib.api.LogEvent; import com.google.common.io.ByteStreams; import java.io.IOException; import java.net.URISyntaxException; @@ -23,76 +24,95 @@ import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.util.Arrays; +import java.util.function.Consumer; import javax.net.ssl.SSLException; import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.contrib.java.lang.system.RestoreSystemProperties; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; /** Tests for {@link Connection} using an actual local server. */ -public class WithServerConnectionTest { +@RunWith(MockitoJUnitRunner.class) +public class WithServerConnectionTest { // TODO: rename to WithServerTlsFailoverHttpClientTest @Rule public final RestoreSystemProperties systemPropertyRestorer = new RestoreSystemProperties(); + @Mock private Consumer logger; + + private final Request request = new Request.Builder().build(); + @Test public void testGet() throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { + Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger); try (TestWebServer server = new TestWebServer(false); - Connection connection = - Connection.getConnectionFactory().apply(new URL(server.getEndpoint()))) { - Response response = connection.send("GET", new Request.Builder().build()); + Response response = insecureHttpClient.get(new URL(server.getEndpoint()), request)) { Assert.assertEquals(200, response.getStatusCode()); Assert.assertArrayEquals( "Hello World!".getBytes(StandardCharsets.UTF_8), ByteStreams.toByteArray(response.getBody())); - } - } - - @Test - public void testErrorOnSecondSend() - throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { - try (TestWebServer server = new TestWebServer(false); - Connection connection = - Connection.getConnectionFactory().apply(new URL(server.getEndpoint()))) { - connection.send("GET", new Request.Builder().build()); - try { - connection.send("GET", new Request.Builder().build()); - Assert.fail("Should fail on the second send"); - } catch (IllegalStateException ex) { - Assert.assertEquals("Connection can send only one request", ex.getMessage()); - } + Mockito.verifyNoInteractions(logger); } } @Test public void testSecureConnectionOnInsecureHttpsServer() throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { + Connection secureHttpClient = new Connection(false /*secure*/, false, logger); try (TestWebServer server = new TestWebServer(true); - Connection connection = - Connection.getConnectionFactory().apply(new URL(server.getEndpoint()))) { - try { - connection.send("GET", new Request.Builder().build()); - Assert.fail("Should fail if cannot verify peer"); - } catch (SSLException ex) { - Assert.assertNotNull(ex.getMessage()); - } + Response ignored = secureHttpClient.get(new URL(server.getEndpoint()), request)) { + Assert.fail("Should fail if cannot verify peer"); + + } catch (SSLException ex) { + Assert.assertNotNull(ex.getMessage()); } } @Test - public void testInsecureConnection() + public void testInsecureConnection_insecureHttpsFailover() throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { - try (TestWebServer server = new TestWebServer(true); - Connection connection = - Connection.getInsecureConnectionFactory().apply(new URL(server.getEndpoint()))) { - Response response = connection.send("GET", new Request.Builder().build()); + Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger); + try (TestWebServer server = new TestWebServer(true, 2); + Response response = insecureHttpClient.get(new URL(server.getEndpoint()), request)) { Assert.assertEquals(200, response.getStatusCode()); Assert.assertArrayEquals( "Hello World!".getBytes(StandardCharsets.UTF_8), ByteStreams.toByteArray(response.getBody())); + + String endpoint = server.getEndpoint(); + String expectedLog = + "Cannot verify server at " + endpoint + ". Attempting again with no TLS verification."; + Mockito.verify(logger).accept(LogEvent.info(expectedLog)); + } + } + + @Test + public void testInsecureConnection_plainHttpFailover() + throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { + Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger); + try (TestWebServer server = new TestWebServer(false, 3)) { + String httpsUrl = server.getEndpoint().replace("http://", "https://"); + try (Response response = insecureHttpClient.get(new URL(httpsUrl), request)) { + + Assert.assertEquals(200, response.getStatusCode()); + Assert.assertArrayEquals( + "Hello World!".getBytes(StandardCharsets.UTF_8), + ByteStreams.toByteArray(response.getBody())); + + String expectedLog1 = + "Cannot verify server at " + httpsUrl + ". Attempting again with no TLS verification."; + String expectedLog2 = + "Failed to connect to " + httpsUrl + " over HTTPS. Attempting again with HTTP."; + Mockito.verify(logger).accept(LogEvent.info(expectedLog1)); + Mockito.verify(logger).accept(LogEvent.info(expectedLog2)); + } } } @@ -107,6 +127,7 @@ public void testProxyCredentialProperties() + "Content-Length: 0\n\n"; String targetServerResponse = "HTTP/1.1 200 OK\nContent-Length:12\n\nHello World!"; + Connection httpClient = new Connection(true /*insecure*/, false, logger); try (TestWebServer server = new TestWebServer(false, Arrays.asList(proxyResponse, targetServerResponse), 1)) { System.setProperty("http.proxyHost", "localhost"); @@ -114,9 +135,7 @@ public void testProxyCredentialProperties() System.setProperty("http.proxyUser", "user_sys_prop"); System.setProperty("http.proxyPassword", "pass_sys_prop"); - try (Connection connection = - Connection.getConnectionFactory().apply(new URL("http://does.not.matter"))) { - Response response = connection.send("GET", new Request.Builder().build()); + try (Response response = httpClient.get(new URL("http://does.not.matter"), request)) { Assert.assertThat( server.getInputRead(), CoreMatchers.containsString( diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java index 0a5bde73fb..9e0ccfc0d6 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java @@ -18,9 +18,10 @@ import com.google.api.client.http.HttpHeaders; import com.google.api.client.http.HttpMethods; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; +import com.google.cloud.tools.jib.http.Connection; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import java.net.MalformedURLException; import java.net.URL; import java.util.Collections; @@ -36,13 +37,15 @@ @RunWith(MockitoJUnitRunner.class) public class AuthenticationMethodRetrieverTest { - @Mock private HttpResponseException mockResponseException; + @Mock private ResponseException mockResponseException; @Mock private HttpHeaders mockHeaders; + @Mock private Connection httpClient; private final RegistryEndpointRequestProperties fakeRegistryEndpointRequestProperties = new RegistryEndpointRequestProperties("someServerUrl", "someImageName"); private final AuthenticationMethodRetriever testAuthenticationMethodRetriever = - new AuthenticationMethodRetriever(fakeRegistryEndpointRequestProperties, "user-agent"); + new AuthenticationMethodRetriever( + fakeRegistryEndpointRequestProperties, "user-agent", httpClient); @Test public void testGetContent() { @@ -88,13 +91,13 @@ public void testHandleHttpResponseException_invalidStatusCode() throws RegistryE Assert.fail( "Authentication method retriever should only handle HTTP 401 Unauthorized errors"); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertEquals(mockResponseException, ex); } } @Test - public void tsetHandleHttpResponseException_noHeader() throws HttpResponseException { + public void tsetHandleHttpResponseException_noHeader() throws ResponseException { Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED); Mockito.when(mockResponseException.getHeaders()).thenReturn(mockHeaders); @@ -112,8 +115,7 @@ public void tsetHandleHttpResponseException_noHeader() throws HttpResponseExcept } @Test - public void testHandleHttpResponseException_badAuthenticationMethod() - throws HttpResponseException { + public void testHandleHttpResponseException_badAuthenticationMethod() throws ResponseException { String authenticationMethod = "bad authentication method"; Mockito.when(mockResponseException.getStatusCode()) @@ -136,7 +138,7 @@ public void testHandleHttpResponseException_badAuthenticationMethod() @Test public void testHandleHttpResponseException_pass() - throws RegistryErrorException, HttpResponseException, MalformedURLException { + throws RegistryErrorException, ResponseException, MalformedURLException { String authenticationMethod = "Bearer realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\""; diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/BlobCheckerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/BlobCheckerTest.java index 0e364e79e7..b25f8a5078 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/BlobCheckerTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/BlobCheckerTest.java @@ -16,11 +16,11 @@ package com.google.cloud.tools.jib.registry; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.DescriptorDigest; import com.google.cloud.tools.jib.blob.BlobDescriptor; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.json.JsonTemplateMapper; import com.google.cloud.tools.jib.registry.json.ErrorEntryTemplate; import com.google.cloud.tools.jib.registry.json.ErrorResponseTemplate; @@ -83,7 +83,7 @@ public void testHandleResponse_noContentLength() { @Test public void testHandleHttpResponseException() throws IOException { - HttpResponseException mockResponseException = Mockito.mock(HttpResponseException.class); + ResponseException mockResponseException = Mockito.mock(ResponseException.class); Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_NOT_FOUND); @@ -99,7 +99,7 @@ public void testHandleHttpResponseException() throws IOException { @Test public void testHandleHttpResponseException_hasOtherErrors() throws IOException { - HttpResponseException mockResponseException = Mockito.mock(HttpResponseException.class); + ResponseException mockResponseException = Mockito.mock(ResponseException.class); Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_NOT_FOUND); @@ -114,14 +114,14 @@ public void testHandleHttpResponseException_hasOtherErrors() throws IOException testBlobChecker.handleHttpResponseException(mockResponseException); Assert.fail("Non-BLOB_UNKNOWN errors should not be handled"); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertEquals(mockResponseException, ex); } } @Test public void testHandleHttpResponseException_notBlobUnknown() throws IOException { - HttpResponseException mockResponseException = Mockito.mock(HttpResponseException.class); + ResponseException mockResponseException = Mockito.mock(ResponseException.class); Mockito.when(mockResponseException.getStatusCode()) .thenReturn(HttpStatusCodes.STATUS_CODE_NOT_FOUND); @@ -133,21 +133,21 @@ public void testHandleHttpResponseException_notBlobUnknown() throws IOException testBlobChecker.handleHttpResponseException(mockResponseException); Assert.fail("Non-BLOB_UNKNOWN errors should not be handled"); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertEquals(mockResponseException, ex); } } @Test public void testHandleHttpResponseException_invalidStatusCode() { - HttpResponseException mockResponseException = Mockito.mock(HttpResponseException.class); + ResponseException mockResponseException = Mockito.mock(ResponseException.class); Mockito.when(mockResponseException.getStatusCode()).thenReturn(-1); try { testBlobChecker.handleHttpResponseException(mockResponseException); Assert.fail("Non-404 status codes should not be handled"); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertEquals(mockResponseException, ex); } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ErrorResponseUtilTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ErrorResponseUtilTest.java index 5c8981338a..f440e61a83 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ErrorResponseUtilTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ErrorResponseUtilTest.java @@ -16,7 +16,7 @@ package com.google.cloud.tools.jib.registry; -import com.google.api.client.http.HttpResponseException; +import com.google.cloud.tools.jib.http.ResponseException; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,10 +28,10 @@ @RunWith(MockitoJUnitRunner.class) public class ErrorResponseUtilTest { - @Mock HttpResponseException responseException; + @Mock private ResponseException responseException; @Test - public void testGetErrorCode_knownErrorCode() throws HttpResponseException { + public void testGetErrorCode_knownErrorCode() throws ResponseException { Mockito.when(responseException.getContent()) .thenReturn( "{\"errors\":[{\"code\":\"MANIFEST_INVALID\",\"message\":\"manifest invalid\",\"detail\":{}}]}"); @@ -49,7 +49,7 @@ public void testGetErrorCode_unknownErrorCode() { try { ErrorResponseUtil.getErrorCode(responseException); Assert.fail(); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertSame(responseException, ex); } } @@ -66,7 +66,7 @@ public void testGetErrorCode_multipleErrors() { try { ErrorResponseUtil.getErrorCode(responseException); Assert.fail(); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertSame(responseException, ex); } } @@ -79,7 +79,7 @@ public void testGetErrorCode_invalidErrorObject() { try { ErrorResponseUtil.getErrorCode(responseException); Assert.fail(); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertSame(responseException, ex); } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPusherTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPusherTest.java index 87ca1cdb2c..e9ff217e7c 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPusherTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPusherTest.java @@ -16,13 +16,13 @@ package com.google.cloud.tools.jib.registry; -import com.google.api.client.http.HttpResponseException; import com.google.cloud.tools.jib.api.DescriptorDigest; import com.google.cloud.tools.jib.api.LogEvent; import com.google.cloud.tools.jib.event.EventHandlers; import com.google.cloud.tools.jib.hash.Digests; import com.google.cloud.tools.jib.http.BlobHttpContent; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.image.json.V22ManifestTemplate; import com.google.cloud.tools.jib.json.JsonTemplateMapper; import com.google.common.io.Resources; @@ -156,9 +156,8 @@ public void testGetAccept() { /** Docker Registry 2.0 and 2.1 return 400 / TAG_INVALID. */ @Test - public void testHandleHttpResponseException_dockerRegistry_tagInvalid() - throws HttpResponseException { - HttpResponseException exception = Mockito.mock(HttpResponseException.class); + public void testHandleHttpResponseException_dockerRegistry_tagInvalid() throws ResponseException { + ResponseException exception = Mockito.mock(ResponseException.class); Mockito.when(exception.getStatusCode()).thenReturn(HttpStatus.SC_BAD_REQUEST); Mockito.when(exception.getContent()) .thenReturn( @@ -180,8 +179,8 @@ public void testHandleHttpResponseException_dockerRegistry_tagInvalid() /** Docker Registry 2.2 returns a 400 / MANIFEST_INVALID. */ @Test public void testHandleHttpResponseException_dockerRegistry_manifestInvalid() - throws HttpResponseException { - HttpResponseException exception = Mockito.mock(HttpResponseException.class); + throws ResponseException { + ResponseException exception = Mockito.mock(ResponseException.class); Mockito.when(exception.getStatusCode()).thenReturn(HttpStatus.SC_BAD_REQUEST); Mockito.when(exception.getContent()) .thenReturn( @@ -202,8 +201,8 @@ public void testHandleHttpResponseException_dockerRegistry_manifestInvalid() /** Quay.io returns an undocumented 415 / MANIFEST_INVALID. */ @Test - public void testHandleHttpResponseException_quayIo() throws HttpResponseException { - HttpResponseException exception = Mockito.mock(HttpResponseException.class); + public void testHandleHttpResponseException_quayIo() throws ResponseException { + ResponseException exception = Mockito.mock(ResponseException.class); Mockito.when(exception.getStatusCode()).thenReturn(HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE); Mockito.when(exception.getContent()) .thenReturn( @@ -225,13 +224,13 @@ public void testHandleHttpResponseException_quayIo() throws HttpResponseExceptio @Test public void testHandleHttpResponseException_otherError() throws RegistryErrorException { - HttpResponseException exception = Mockito.mock(HttpResponseException.class); + ResponseException exception = Mockito.mock(ResponseException.class); Mockito.when(exception.getStatusCode()).thenReturn(HttpStatus.SC_UNAUTHORIZED); try { testManifestPusher.handleHttpResponseException(exception); Assert.fail(); - } catch (HttpResponseException ex) { + } catch (ResponseException ex) { Assert.assertSame(exception, ex); } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java index 74fab1eb8c..268dda3475 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java @@ -18,33 +18,57 @@ import com.google.cloud.tools.jib.api.Credential; import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException; +import com.google.cloud.tools.jib.http.Connection; +import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.http.TestWebServer; +import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.MalformedURLException; import java.net.URISyntaxException; import java.net.URL; +import java.nio.charset.StandardCharsets; import java.security.GeneralSecurityException; import java.util.Collections; import org.hamcrest.CoreMatchers; import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; /** Tests for {@link RegistryAuthenticator}. */ +@RunWith(MockitoJUnitRunner.class) public class RegistryAuthenticatorTest { private final RegistryEndpointRequestProperties registryEndpointRequestProperties = new RegistryEndpointRequestProperties("someserver", "someimage"); + @Mock private Connection httpClient; + @Mock private Response response; + + @Captor private ArgumentCaptor urlCaptor; + private RegistryAuthenticator registryAuthenticator; @Before - public void setUp() throws RegistryAuthenticationFailedException { + public void setUp() throws RegistryAuthenticationFailedException, IOException { registryAuthenticator = RegistryAuthenticator.fromAuthenticationMethod( "Bearer realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .get(); + + ByteArrayInputStream tokenJson = + new ByteArrayInputStream("{\"token\":\"my_token\"}".getBytes(StandardCharsets.UTF_8)); + Mockito.when(response.getBody()).thenReturn(tokenJson); + Mockito.when(httpClient.call(Mockito.any(), urlCaptor.capture(), Mockito.any())) + .thenReturn(response); } @Test @@ -54,7 +78,8 @@ public void testFromAuthenticationMethod_bearer() RegistryAuthenticator.fromAuthenticationMethod( "Bearer realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .get(); Assert.assertEquals( new URL("https://somerealm?service=someservice&scope=repository:someimage:scope"), @@ -65,7 +90,8 @@ public void testFromAuthenticationMethod_bearer() RegistryAuthenticator.fromAuthenticationMethod( "bEaReR realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .get(); Assert.assertEquals( new URL("https://somerealm?service=someservice&scope=repository:someimage:scope"), @@ -131,21 +157,24 @@ public void testFromAuthenticationMethod_basic() throws RegistryAuthenticationFa RegistryAuthenticator.fromAuthenticationMethod( "Basic realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .isPresent()); Assert.assertFalse( RegistryAuthenticator.fromAuthenticationMethod( "BASIC realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .isPresent()); Assert.assertFalse( RegistryAuthenticator.fromAuthenticationMethod( "bASIC realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .isPresent()); } @@ -155,7 +184,8 @@ public void testFromAuthenticationMethod_noBearer() { RegistryAuthenticator.fromAuthenticationMethod( "realm=\"https://somerealm\",service=\"someservice\",scope=\"somescope\"", registryEndpointRequestProperties, - "user-agent"); + "user-agent", + httpClient); Assert.fail("Authentication method without 'Bearer ' or 'Basic ' should fail"); } catch (RegistryAuthenticationFailedException ex) { @@ -169,7 +199,10 @@ public void testFromAuthenticationMethod_noBearer() { public void testFromAuthenticationMethod_noRealm() { try { RegistryAuthenticator.fromAuthenticationMethod( - "Bearer scope=\"somescope\"", registryEndpointRequestProperties, "user-agent"); + "Bearer scope=\"somescope\"", + registryEndpointRequestProperties, + "user-agent", + httpClient); Assert.fail("Authentication method without 'realm' should fail"); } catch (RegistryAuthenticationFailedException ex) { @@ -186,7 +219,8 @@ public void testFromAuthenticationMethod_noService() RegistryAuthenticator.fromAuthenticationMethod( "Bearer realm=\"https://somerealm\"", registryEndpointRequestProperties, - "user-agent") + "user-agent", + httpClient) .get(); Assert.assertEquals( @@ -197,14 +231,16 @@ public void testFromAuthenticationMethod_noService() @Test public void testUserAgent() - throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { + throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException, + RegistryCredentialsNotSentException { try (TestWebServer server = new TestWebServer(false)) { try { RegistryAuthenticator authenticator = RegistryAuthenticator.fromAuthenticationMethod( "Bearer realm=\"" + server.getEndpoint() + "\"", registryEndpointRequestProperties, - "Competent-Agent") + "Competent-Agent", + new Connection(true, false, ignored -> {})) .get(); authenticator.authenticatePush(null); } catch (RegistryAuthenticationFailedException ex) { @@ -217,48 +253,53 @@ public void testUserAgent() @Test public void testSourceImage_differentSourceRepository() - throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { - try (TestWebServer server = new TestWebServer(false, 2)) { - try { - RegistryEndpointRequestProperties registryEndpointRequestProperties = - new RegistryEndpointRequestProperties("someserver", "someimage", "anotherimage"); - RegistryAuthenticator authenticator = - RegistryAuthenticator.fromAuthenticationMethod( - "Bearer realm=\"" + server.getEndpoint() + "\"", - registryEndpointRequestProperties, - "Competent-Agent") - .get(); - authenticator.authenticatePush(null); - } catch (RegistryAuthenticationFailedException ex) { - // Doesn't matter if auth fails. We only examine what we sent. - } - Assert.assertThat( - server.getInputRead(), - CoreMatchers.containsString( - "scope=repository:someimage:pull,push&scope=repository:anotherimage:pull ")); - } + throws RegistryCredentialsNotSentException, RegistryAuthenticationFailedException { + RegistryAuthenticator authenticator = + RegistryAuthenticator.fromAuthenticationMethod( + "Bearer realm=\"https://1.2.3.4:5\"", + new RegistryEndpointRequestProperties("someserver", "someimage", "anotherimage"), + "Competent-Agent", + httpClient) + .get(); + authenticator.authenticatePush(null); + Assert.assertThat( + urlCaptor.getValue().toString(), + CoreMatchers.endsWith( + "scope=repository:someimage:pull,push&scope=repository:anotherimage:pull")); } @Test public void testSourceImage_sameSourceRepository() - throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException { - try (TestWebServer server = new TestWebServer(false)) { - try { - RegistryEndpointRequestProperties registryEndpointRequestProperties = - new RegistryEndpointRequestProperties("someserver", "someimage", "someimage"); - RegistryAuthenticator authenticator = - RegistryAuthenticator.fromAuthenticationMethod( - "Bearer realm=\"" + server.getEndpoint() + "\"", - registryEndpointRequestProperties, - "Competent-Agent") - .get(); - authenticator.authenticatePush(null); - } catch (RegistryAuthenticationFailedException ex) { - // Doesn't matter if auth fails. We only examine what we sent. - } - Assert.assertThat( - server.getInputRead(), - CoreMatchers.containsString("service=someserver&scope=repository:someimage:pull,push ")); + throws RegistryCredentialsNotSentException, RegistryAuthenticationFailedException { + RegistryAuthenticator authenticator = + RegistryAuthenticator.fromAuthenticationMethod( + "Bearer realm=\"https://1.2.3.4:5\"", + new RegistryEndpointRequestProperties("someserver", "someimage", "someimage"), + "Competent-Agent", + httpClient) + .get(); + authenticator.authenticatePush(null); + Assert.assertThat( + urlCaptor.getValue().toString(), + CoreMatchers.endsWith("service=someserver&scope=repository:someimage:pull,push")); + } + + @Test + public void testAuthorizationCleared() throws RegistryAuthenticationFailedException, IOException { + ResponseException responseException = Mockito.mock(ResponseException.class); + Mockito.when(responseException.getStatusCode()).thenReturn(401); + Mockito.when(responseException.requestAuthorizationCleared()).thenReturn(true); + Mockito.when(httpClient.call(Mockito.any(), Mockito.any(), Mockito.any())) + .thenThrow(responseException); + + try { + registryAuthenticator.authenticatePush(null); + Assert.fail(); + } catch (RegistryCredentialsNotSentException ex) { + Assert.assertEquals( + "Required credentials for someserver/someimage were not sent because the connection was " + + "over HTTP", + ex.getMessage()); } } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java index 20e5f542e9..bfebf7455b 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java @@ -17,8 +17,6 @@ package com.google.cloud.tools.jib.registry; import com.google.api.client.http.HttpHeaders; -import com.google.api.client.http.HttpResponse; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.InsecureRegistryException; import com.google.cloud.tools.jib.api.LogEvent; @@ -32,18 +30,17 @@ import com.google.cloud.tools.jib.http.Request; import com.google.cloud.tools.jib.http.RequestWrapper; import com.google.cloud.tools.jib.http.Response; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.common.io.CharStreams; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; -import java.net.ConnectException; import java.net.MalformedURLException; import java.net.SocketException; import java.net.URL; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; -import java.util.function.Function; import javax.annotation.Nullable; import javax.net.ssl.SSLException; import javax.net.ssl.SSLPeerUnverifiedException; @@ -101,216 +98,64 @@ public String getActionDescription() { } } - private static HttpResponse mockHttpResponse(int statusCode, @Nullable HttpHeaders headers) - throws IOException { - HttpResponse mock = Mockito.mock(HttpResponse.class); + private static ResponseException mockResponseException( + int statusCode, @Nullable HttpHeaders headers) { + ResponseException mock = Mockito.mock(ResponseException.class); Mockito.when(mock.getStatusCode()).thenReturn(statusCode); - Mockito.when(mock.parseAsString()).thenReturn(""); Mockito.when(mock.getHeaders()).thenReturn(headers != null ? headers : new HttpHeaders()); return mock; } - private static HttpResponse mockRedirectHttpResponse(String redirectLocation) throws IOException { - int code307 = HttpStatusCodes.STATUS_CODE_TEMPORARY_REDIRECT; - return mockHttpResponse(code307, new HttpHeaders().setLocation(redirectLocation)); - } - @Rule public final RestoreSystemProperties systemPropertyRestorer = new RestoreSystemProperties(); @Mock private EventHandlers mockEventHandlers; - @Mock private Connection mockConnection; - @Mock private Connection mockInsecureConnection; + @Mock private Connection mockHttpClient; @Mock private Response mockResponse; - @Mock private Function mockConnectionFactory; - @Mock private Function mockInsecureConnectionFactory; - private RegistryEndpointCaller secureEndpointCaller; + private RegistryEndpointCaller endpointCaller; @Before public void setUp() throws IOException { - secureEndpointCaller = createRegistryEndpointCaller(false, -1); + endpointCaller = + new RegistryEndpointCaller<>( + mockEventHandlers, + "userAgent", + new TestRegistryEndpointProvider(), + Authorization.fromBasicToken("token"), + new RegistryEndpointRequestProperties("serverUrl", "imageName"), + mockHttpClient); - Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection); - Mockito.when(mockInsecureConnectionFactory.apply(Mockito.any())) - .thenReturn(mockInsecureConnection); Mockito.when(mockResponse.getBody()) .thenReturn(new ByteArrayInputStream("body".getBytes(StandardCharsets.UTF_8))); } @Test public void testCall_secureCallerOnUnverifiableServer() throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // unverifiable HTTPS server + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), Mockito.any())) + .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); try { - secureEndpointCaller.call(); - Assert.fail("Secure caller should fail if cannot verify server"); + endpointCaller.call(); + Assert.fail("Should throw InsecureRegistryException when getting SSLException"); } catch (InsecureRegistryException ex) { Assert.assertEquals( - "Failed to verify the server at https://serverUrl/v2/api because only secure connections are allowed.", + "Failed to verify the server at https://serverUrl/v2/api because only secure connections " + + "are allowed.", ex.getMessage()); } } - @Test - public void testCall_insecureCallerOnUnverifiableServer() throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // unverifiable HTTPS server - Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenReturn(mockResponse); // OK with non-verifying connection - - RegistryEndpointCaller insecureCaller = createRegistryEndpointCaller(true, -1); - Assert.assertEquals("body", insecureCaller.call()); - - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(0)); - - Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(1)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Cannot verify server at https://serverUrl/v2/api. Attempting again with no TLS verification.")); - } - - @Test - public void testCall_insecureCallerOnHttpServer() throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)) // server is not HTTPS - .thenReturn(mockResponse); - Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS - - RegistryEndpointCaller insecureEndpointCaller = createRegistryEndpointCaller(true, -1); - Assert.assertEquals("body", insecureEndpointCaller.call()); - - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory, Mockito.times(2)).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(0)); - Assert.assertEquals(new URL("http://serverUrl/v2/api"), urlCaptor.getAllValues().get(1)); - - Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(2)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Cannot verify server at https://serverUrl/v2/api. Attempting again with no TLS verification.")); - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Failed to connect to https://serverUrl/v2/api over HTTPS. Attempting again with HTTP: http://serverUrl/v2/api")); - } - - @Test - public void testCall_insecureCallerOnHttpServerAndNoPortSpecified() - throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(ConnectException.class)) // server is not listening on 443 - .thenReturn(mockResponse); // respond when connected through 80 - - RegistryEndpointCaller insecureEndpointCaller = createRegistryEndpointCaller(true, -1); - Assert.assertEquals("body", insecureEndpointCaller.call()); - - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory, Mockito.times(2)).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(0)); - Assert.assertEquals(new URL("http://serverUrl/v2/api"), urlCaptor.getAllValues().get(1)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Failed to connect to https://serverUrl/v2/api over HTTPS. Attempting again with HTTP: http://serverUrl/v2/api")); - } - - @Test - public void testCall_secureCallerOnNonListeningServerAndNoPortSpecified() - throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(ConnectException.class)); // server is not listening on 443 - - try { - secureEndpointCaller.call(); - Assert.fail("Should not fall back to HTTP if not allowInsecureRegistries"); - } catch (ConnectException ex) { - Assert.assertNull(ex.getMessage()); - } - - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(0)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - } - - @Test - public void testCall_insecureCallerOnNonListeningServerAndPortSpecified() - throws IOException, RegistryException { - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(ConnectException.class)); // server is not listening on 5000 - - RegistryEndpointCaller insecureEndpointCaller = - createRegistryEndpointCaller(true, 5000); - try { - insecureEndpointCaller.call(); - Assert.fail("Should not fall back to HTTP if port was explicitly given and cannot connect"); - } catch (ConnectException ex) { - Assert.assertNull(ex.getMessage()); - } - - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl:5000/v2/api"), urlCaptor.getAllValues().get(0)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - } - - @Test - public void testCall_timeoutFromConnectException() throws IOException, RegistryException { - ConnectException mockConnectException = Mockito.mock(ConnectException.class); - Mockito.when(mockConnectException.getMessage()).thenReturn("Connection timed out"); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(mockConnectException) // server times out on 443 - .thenReturn(mockResponse); // respond when connected through 80 - - try { - RegistryEndpointCaller insecureEndpointCaller = - createRegistryEndpointCaller(true, -1); - insecureEndpointCaller.call(); - Assert.fail("Should not fall back to HTTP if timed out even for ConnectionException"); - - } catch (ConnectException ex) { - Assert.assertSame(mockConnectException, ex); - Mockito.verify(mockConnection).send(Mockito.anyString(), Mockito.any()); - } - } - @Test public void testCall_noHttpResponse() throws IOException, RegistryException { - NoHttpResponseException mockNoHttpResponseException = - Mockito.mock(NoHttpResponseException.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(mockNoHttpResponseException); + NoHttpResponseException mockNoResponseException = Mockito.mock(NoHttpResponseException.class); + setUpRegistryResponse(mockNoResponseException); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail("Call should have failed"); } catch (NoHttpResponseException ex) { - Assert.assertSame(mockNoHttpResponseException, ex); + Assert.assertSame(mockNoResponseException, ex); } } @@ -321,20 +166,13 @@ public void testCall_unauthorized() throws IOException, RegistryException { @Test public void testCall_credentialsNotSentOverHttp() throws IOException, RegistryException { - HttpResponse redirectResponse = mockRedirectHttpResponse("http://newlocation"); - HttpResponse unauthroizedResponse = - mockHttpResponse(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED, null); - - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)) // server is not HTTPS - .thenThrow(new HttpResponseException(redirectResponse)) // redirect to HTTP - .thenThrow(new HttpResponseException(unauthroizedResponse)); // final response - Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS - - RegistryEndpointCaller insecureEndpointCaller = createRegistryEndpointCaller(true, -1); + ResponseException unauthorizedException = + mockResponseException(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED, null); + Mockito.when(unauthorizedException.requestAuthorizationCleared()).thenReturn(true); + setUpRegistryResponse(unauthorizedException); + try { - insecureEndpointCaller.call(); + endpointCaller.call(); Assert.fail("Call should have failed"); } catch (RegistryCredentialsNotSentException ex) { @@ -346,39 +184,20 @@ public void testCall_credentialsNotSentOverHttp() throws IOException, RegistryEx @Test public void testCall_credentialsForcedOverHttp() throws IOException, RegistryException { - HttpResponse redirectResponse = mockRedirectHttpResponse("http://newlocation"); - - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)) // server is not HTTPS - .thenThrow(new HttpResponseException(redirectResponse)) // redirect to HTTP - .thenReturn(mockResponse); // final response - Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS - + ResponseException unauthorizedException = + mockResponseException(HttpStatusCodes.STATUS_CODE_UNAUTHORIZED, null); + setUpRegistryResponse(unauthorizedException); System.setProperty(JibSystemProperties.SEND_CREDENTIALS_OVER_HTTP, "true"); - RegistryEndpointCaller insecureEndpointCaller = createRegistryEndpointCaller(true, -1); - Assert.assertEquals("body", insecureEndpointCaller.call()); - ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory, Mockito.times(3)).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(0)); - Assert.assertEquals(new URL("http://serverUrl/v2/api"), urlCaptor.getAllValues().get(1)); - Assert.assertEquals(new URL("http://newlocation"), urlCaptor.getAllValues().get(2)); - - Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture()); - Assert.assertEquals(new URL("https://serverUrl/v2/api"), urlCaptor.getAllValues().get(3)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); + try { + endpointCaller.call(); + Assert.fail("Call should have failed"); - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Cannot verify server at https://serverUrl/v2/api. Attempting again with no TLS verification.")); - Mockito.verify(mockEventHandlers) - .dispatch( - LogEvent.info( - "Failed to connect to https://serverUrl/v2/api over HTTPS. Attempting again with HTTP: http://serverUrl/v2/api")); + } catch (RegistryCredentialsNotSentException ex) { + throw new AssertionError("should have sent credentials", ex); + } catch (RegistryUnauthorizedException ex) { + Assert.assertEquals("Unauthorized for serverUrl/imageName", ex.getMessage()); + } } @Test @@ -403,19 +222,16 @@ public void testCall_methodNotAllowed() throws IOException, RegistryException { @Test public void testCall_unknown() throws IOException, RegistryException { - HttpResponse mockHttpResponse = - mockHttpResponse(HttpStatusCodes.STATUS_CODE_SERVER_ERROR, null); - HttpResponseException httpResponseException = new HttpResponseException(mockHttpResponse); - - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(httpResponseException); + ResponseException responseException = + mockResponseException(HttpStatusCodes.STATUS_CODE_SERVER_ERROR, null); + setUpRegistryResponse(responseException); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail("Call should have failed"); - } catch (HttpResponseException ex) { - Assert.assertSame(httpResponseException, ex); + } catch (ResponseException ex) { + Assert.assertSame(responseException, ex); } } @@ -434,32 +250,13 @@ public void testCall_permanentRedirect() throws IOException, RegistryException { verifyRetriesWithNewLocation(RegistryEndpointCaller.STATUS_CODE_PERMANENT_REDIRECT); } - @Test - public void testCall_disallowInsecure() throws IOException, RegistryException { - // Mocks a response for temporary redirect to a new location. - HttpResponse redirectResponse = mockRedirectHttpResponse("http://newlocation"); - - HttpResponseException redirectException = new HttpResponseException(redirectResponse); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(redirectException); - - try { - secureEndpointCaller.call(); - Assert.fail("Call should have failed"); - - } catch (InsecureRegistryException ex) { - // pass - } - } - @Test public void testCall_logErrorOnIoExceptions() throws IOException, RegistryException { IOException ioException = new IOException("detailed exception message"); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(ioException); + setUpRegistryResponse(ioException); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail(); } catch (IOException ex) { @@ -476,11 +273,10 @@ public void testCall_logErrorOnIoExceptions() throws IOException, RegistryExcept @Test public void testCall_logErrorOnBrokenPipe() throws IOException, RegistryException { IOException ioException = new IOException("this is due to broken pipe"); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(ioException); + setUpRegistryResponse(ioException); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail(); } catch (IOException ex) { @@ -503,11 +299,11 @@ public void testCall_logErrorOnBrokenPipe() throws IOException, RegistryExceptio @Test public void testHttpTimeout_propertyNotSet() throws IOException, RegistryException { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), requestCaptor.capture())) + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), requestCaptor.capture())) .thenReturn(mockResponse); System.clearProperty(JibSystemProperties.HTTP_TIMEOUT); - secureEndpointCaller.call(); + endpointCaller.call(); // We fall back to the default timeout: // https://github.com/GoogleContainerTools/jib/pull/656#discussion_r203562639 @@ -517,11 +313,11 @@ public void testHttpTimeout_propertyNotSet() throws IOException, RegistryExcepti @Test public void testHttpTimeout_stringValue() throws IOException, RegistryException { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), requestCaptor.capture())) + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), requestCaptor.capture())) .thenReturn(mockResponse); System.setProperty(JibSystemProperties.HTTP_TIMEOUT, "random string"); - secureEndpointCaller.call(); + endpointCaller.call(); Assert.assertEquals(20000, new RequestWrapper(requestCaptor.getValue()).getHttpTimeout()); } @@ -529,11 +325,11 @@ public void testHttpTimeout_stringValue() throws IOException, RegistryException @Test public void testHttpTimeout_negativeValue() throws IOException, RegistryException { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), requestCaptor.capture())) + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), requestCaptor.capture())) .thenReturn(mockResponse); System.setProperty(JibSystemProperties.HTTP_TIMEOUT, "-1"); - secureEndpointCaller.call(); + endpointCaller.call(); // We let the negative value pass through: // https://github.com/GoogleContainerTools/jib/pull/656#discussion_r203562639 @@ -543,11 +339,11 @@ public void testHttpTimeout_negativeValue() throws IOException, RegistryExceptio @Test public void testHttpTimeout_0accepted() throws IOException, RegistryException { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), requestCaptor.capture())) + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), requestCaptor.capture())) .thenReturn(mockResponse); System.setProperty(JibSystemProperties.HTTP_TIMEOUT, "0"); - secureEndpointCaller.call(); + endpointCaller.call(); Assert.assertEquals(0, new RequestWrapper(requestCaptor.getValue()).getHttpTimeout()); } @@ -555,11 +351,11 @@ public void testHttpTimeout_0accepted() throws IOException, RegistryException { @Test public void testHttpTimeout() throws IOException, RegistryException { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(Request.class); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), requestCaptor.capture())) + Mockito.when(mockHttpClient.call(Mockito.any(), Mockito.any(), requestCaptor.capture())) .thenReturn(mockResponse); System.setProperty(JibSystemProperties.HTTP_TIMEOUT, "7593"); - secureEndpointCaller.call(); + endpointCaller.call(); Assert.assertEquals(7593, new RequestWrapper(requestCaptor.getValue()).getHttpTimeout()); } @@ -594,13 +390,13 @@ public void testIsBrokenPipe_terminatesWhenCauseIsOriginal() { @Test public void testNewRegistryErrorException_jsonErrorOutput() { - HttpResponseException httpException = Mockito.mock(HttpResponseException.class); + ResponseException httpException = Mockito.mock(ResponseException.class); Mockito.when(httpException.getContent()) .thenReturn( "{\"errors\": [{\"code\": \"MANIFEST_UNKNOWN\", \"message\": \"manifest unknown\"}]}"); RegistryErrorException registryException = - secureEndpointCaller.newRegistryErrorException(httpException); + endpointCaller.newRegistryErrorException(httpException); Assert.assertSame(httpException, registryException.getCause()); Assert.assertEquals( "Tried to actionDescription but failed because: manifest unknown | If this is a bug, " @@ -610,13 +406,13 @@ public void testNewRegistryErrorException_jsonErrorOutput() { @Test public void testNewRegistryErrorException_nonJsonErrorOutput() { - HttpResponseException httpException = Mockito.mock(HttpResponseException.class); + ResponseException httpException = Mockito.mock(ResponseException.class); // Registry returning non-structured error output Mockito.when(httpException.getContent()).thenReturn(">>>>> (404) page not found <<<<<"); Mockito.when(httpException.getStatusCode()).thenReturn(404); RegistryErrorException registryException = - secureEndpointCaller.newRegistryErrorException(httpException); + endpointCaller.newRegistryErrorException(httpException); Assert.assertSame(httpException, registryException.getCause()); Assert.assertEquals( "Tried to actionDescription but failed because: registry returned error code 404; " @@ -633,19 +429,16 @@ public void testNewRegistryErrorException_nonJsonErrorOutput() { */ private void verifyThrowsRegistryUnauthorizedException(int httpStatusCode) throws IOException, RegistryException { - HttpResponse mockHttpResponse = mockHttpResponse(httpStatusCode, null); - HttpResponseException httpResponseException = new HttpResponseException(mockHttpResponse); - - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(httpResponseException); + ResponseException responseException = mockResponseException(httpStatusCode, null); + setUpRegistryResponse(responseException); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail("Call should have failed"); } catch (RegistryUnauthorizedException ex) { Assert.assertEquals("serverUrl/imageName", ex.getImageReference()); - Assert.assertSame(httpResponseException, ex.getHttpResponseException()); + Assert.assertSame(responseException, ex.getCause()); } } @@ -655,16 +448,13 @@ private void verifyThrowsRegistryUnauthorizedException(int httpStatusCode) */ private void verifyThrowsRegistryErrorException(int httpStatusCode) throws IOException, RegistryException { - HttpResponse errorResponse = mockHttpResponse(httpStatusCode, null); - Mockito.when(errorResponse.parseAsString()) + ResponseException errorResponse = mockResponseException(httpStatusCode, null); + Mockito.when(errorResponse.getContent()) .thenReturn("{\"errors\":[{\"code\":\"code\",\"message\":\"message\"}]}"); - HttpResponseException httpResponseException = new HttpResponseException(errorResponse); - - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(httpResponseException); + setUpRegistryResponse(errorResponse); try { - secureEndpointCaller.call(); + endpointCaller.call(); Assert.fail("Call should have failed"); } catch (RegistryErrorException ex) { @@ -682,39 +472,28 @@ private void verifyThrowsRegistryErrorException(int httpStatusCode) private void verifyRetriesWithNewLocation(int httpStatusCode) throws IOException, RegistryException { // Mocks a response for temporary redirect to a new location. - HttpResponse redirectResponse = - mockHttpResponse(httpStatusCode, new HttpHeaders().setLocation("https://newlocation")); - - // Has mockConnection.send throw first, then succeed. - HttpResponseException redirectException = new HttpResponseException(redirectResponse); - Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any())) - .thenThrow(redirectException) + ResponseException redirectException = + mockResponseException(httpStatusCode, new HttpHeaders().setLocation("https://newlocation")); + + // Make httpClient.call() throw first, then succeed. + setUpRegistryResponse(redirectException); + Mockito.when( + mockHttpClient.call( + Mockito.eq("httpMethod"), + Mockito.eq(new URL("https://newlocation")), + Mockito.any())) .thenReturn(mockResponse); - Assert.assertEquals("body", secureEndpointCaller.call()); + Assert.assertEquals("body", endpointCaller.call()); + } - // Checks that the URL was changed to the new location. - ArgumentCaptor urlArgumentCaptor = ArgumentCaptor.forClass(URL.class); - Mockito.verify(mockConnectionFactory, Mockito.times(2)).apply(urlArgumentCaptor.capture()); - Assert.assertEquals( - new URL("https://serverUrl/v2/api"), urlArgumentCaptor.getAllValues().get(0)); - Assert.assertEquals(new URL("https://newlocation"), urlArgumentCaptor.getAllValues().get(1)); - - Mockito.verifyNoMoreInteractions(mockConnectionFactory); - Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory); - } - - private RegistryEndpointCaller createRegistryEndpointCaller( - boolean allowInsecure, int port) { - return new RegistryEndpointCaller<>( - mockEventHandlers, - "userAgent", - new TestRegistryEndpointProvider(), - Authorization.fromBasicToken("token"), - new RegistryEndpointRequestProperties( - (port == -1 ? "serverUrl" : "serverUrl:" + port), "imageName"), - allowInsecure, - mockConnectionFactory, - mockInsecureConnectionFactory); + private void setUpRegistryResponse(Exception exceptionToThrow) + throws MalformedURLException, IOException { + Mockito.when( + mockHttpClient.call( + Mockito.eq("httpMethod"), + Mockito.eq(new URL("https://serverUrl/v2/api")), + Mockito.any())) + .thenThrow(exceptionToThrow); } } diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index e40f378907..4b346e1db8 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. - Fixed reporting parent build file when `skaffold init` is run on multi-module projects. ([#2091](https://github.com/GoogleContainerTools/jib/pull/2091)) - Now correctly uses the `war` task if it is enabled and the `bootWar` task is disabled for Spring WAR projects. ([#2096](https://github.com/GoogleContainerTools/jib/issues/2096)) +- `allowInsecureRegistries` and the `sendCredentialsOverHttp` system property are now effective for authentication service server connections. ([#2074](https://github.com/GoogleContainerTools/jib/pull/2074) ## 1.7.0 diff --git a/jib-maven-plugin/CHANGELOG.md b/jib-maven-plugin/CHANGELOG.md index 4e4522e35a..0b1bb415a3 100644 --- a/jib-maven-plugin/CHANGELOG.md +++ b/jib-maven-plugin/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file. ### Fixed - Fixed reporting wrong module name when `skaffold init` is run on multi-module projects ([#2088](https://github.com/GoogleContainerTools/jib/issues/2088)). +- `` and the `sendCredentialsOverHttp` system property are now effective for authentication service server connections. ([#2074](https://github.com/GoogleContainerTools/jib/pull/2074) ## 1.7.0 diff --git a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/JibBuildRunner.java b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/JibBuildRunner.java index a913c1e264..0ca73689ad 100644 --- a/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/JibBuildRunner.java +++ b/jib-plugins-common/src/main/java/com/google/cloud/tools/jib/plugins/common/JibBuildRunner.java @@ -16,7 +16,6 @@ package com.google.cloud.tools.jib.plugins.common; -import com.google.api.client.http.HttpResponseException; import com.google.api.client.http.HttpStatusCodes; import com.google.cloud.tools.jib.api.CacheDirectoryCreationException; import com.google.cloud.tools.jib.api.Containerizer; @@ -28,6 +27,7 @@ import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException; import com.google.cloud.tools.jib.api.RegistryException; import com.google.cloud.tools.jib.api.RegistryUnauthorizedException; +import com.google.cloud.tools.jib.http.ResponseException; import com.google.cloud.tools.jib.registry.RegistryCredentialsNotSentException; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Verify; @@ -251,10 +251,10 @@ public JibContainer runBuild() throw new BuildStepsExecutionException(helpfulSuggestions.forCredentialsNotSent(), ex); } catch (RegistryAuthenticationFailedException ex) { - if (ex.getCause() instanceof HttpResponseException) { + if (ex.getCause() instanceof ResponseException) { handleRegistryUnauthorizedException( new RegistryUnauthorizedException( - ex.getServerUrl(), ex.getImageName(), (HttpResponseException) ex.getCause()), + ex.getServerUrl(), ex.getImageName(), (ResponseException) ex.getCause()), helpfulSuggestions); } else { // Unknown cause