-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create helper for parsing an error code from error response #938
Changes from 3 commits
0438673
b0018d0
31c916e
2a77983
d3e285a
146c9a7
eaa3c7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
/* | ||
* Copyright 2018 Google Inc. | ||
* | ||
* 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.registry; | ||
|
||
import com.google.api.client.http.HttpResponseException; | ||
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 java.io.IOException; | ||
import java.util.List; | ||
|
||
/** Helper class for parsing {@link ErrorResponseTemplate} from JSON. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This javadoc looks out-of-place? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Originally I was going to have a few different methods, including to parse an ErrorResponseTemplate from a JSON body, etc. But it turned out this single method was enough. |
||
public class ErrorResponseUtil { | ||
|
||
/** | ||
* Extract an {@link ErrorCodes} response from an error object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
* | ||
* @param ex the response exception | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Javadoc error is causing the build failure. There is no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Argh! |
||
* @return the parsed {@link ErrorCodes} if found | ||
* @throws HttpResponseException 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 httpResponseException) | ||
throws HttpResponseException { | ||
// Obtain the error response code. | ||
String errorContent = httpResponseException.getContent(); | ||
if (errorContent == null) { | ||
throw httpResponseException; | ||
} | ||
|
||
try { | ||
ErrorResponseTemplate errorResponse = | ||
JsonTemplateMapper.readJson(errorContent, ErrorResponseTemplate.class); | ||
List<ErrorEntryTemplate> errors = errorResponse.getErrors(); | ||
// There may be multiple error objects | ||
if (errors.size() == 1) { | ||
String errorCodeString = errors.get(0).getCode(); | ||
// May not get an error code back. | ||
if (errorCodeString != null) { | ||
// throws IllegalArgumentException if unknown error code | ||
return ErrorCodes.valueOf(errorCodeString); | ||
} | ||
} | ||
|
||
} catch (IOException | IllegalArgumentException ex) { | ||
// Parse exception: either isn't an error object or unknown error code | ||
} | ||
|
||
// rethrow the original exception | ||
throw httpResponseException; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add empty private constructor to prevent instantiation |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
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.http.BlobHttpContent; | ||
import com.google.cloud.tools.jib.http.Response; | ||
import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate; | ||
|
@@ -25,6 +26,7 @@ | |
import java.net.URL; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import org.apache.http.HttpStatus; | ||
|
||
/** Pushes an image's manifest. */ | ||
class ManifestPusher implements RegistryEndpointProvider<Void> { | ||
|
@@ -53,6 +55,35 @@ public List<String> getAccept() { | |
return Collections.emptyList(); | ||
} | ||
|
||
@Override | ||
public Void handleHttpResponseException(HttpResponseException httpResponseException) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It includes — it should get tossed once #932 is merged. |
||
throws HttpResponseException, RegistryErrorException { | ||
// docker registry 2.0 and 2.1 returns: | ||
// 400 Bad Request | ||
// {"errors":[{"code":"TAG_INVALID","message":"manifest tag did not match URI"}]} | ||
// docker registry:2.2 returns: | ||
// 400 Bad Request | ||
// {"errors":[{"code":"MANIFEST_INVALID","message":"manifest invalid","detail":{}}]} | ||
// quay.io returns: | ||
// 415 UNSUPPORTED MEDIA TYPE | ||
// {"errors":[{"code":"MANIFEST_INVALID","detail": | ||
// {"message":"manifest schema version not supported"},"message":"manifest invalid"}]} | ||
|
||
if (httpResponseException.getStatusCode() != HttpStatus.SC_BAD_REQUEST | ||
&& httpResponseException.getStatusCode() != HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE) { | ||
throw httpResponseException; | ||
} | ||
|
||
ErrorCodes errorCode = ErrorResponseUtil.getErrorCode(httpResponseException); | ||
if (errorCode.equals(ErrorCodes.MANIFEST_INVALID) || errorCode.equals(ErrorCodes.TAG_INVALID)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto. |
||
throw new RegistryErrorExceptionBuilder(getActionDescription(), httpResponseException) | ||
.addReason("Registry may not support Image Manifest Version 2, Schema 2") | ||
.build(); | ||
} | ||
// rethrow: unhandled error response code. | ||
throw httpResponseException; | ||
} | ||
|
||
@Override | ||
public Void handleResponse(Response response) { | ||
return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* | ||
* Copyright 2018 Google Inc. | ||
* | ||
* 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.registry; | ||
|
||
import com.google.api.client.http.HttpHeaders; | ||
import com.google.api.client.http.HttpResponseException; | ||
import org.apache.http.HttpStatus; | ||
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
||
/** | ||
* Test for {@link ErrorReponseUtil}. | ||
*/ | ||
public class ErrorResponseUtilTest { | ||
|
||
@Test | ||
public void testGetErrorCode_knownErrorCode() throws HttpResponseException { | ||
HttpResponseException httpResponseException = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[{\"code\":\"MANIFEST_INVALID\",\"message\":\"manifest invalid\",\"detail\":{}}]}") | ||
.build(); | ||
|
||
Assert.assertEquals(ErrorCodes.MANIFEST_INVALID, ErrorResponseUtil.getErrorCode(httpResponseException)); | ||
} | ||
|
||
/** An unknown {@link ErrorCodes} should cause original exception to be rethrown. */ | ||
@Test | ||
public void testGetErrorCode_unknownErrorCode() { | ||
HttpResponseException httpResponseException = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[{\"code\":\"INVALID_ERROR_CODE\",\"message\":\"invalid code\",\"detail\":{}}]}") | ||
.build(); | ||
try { | ||
ErrorResponseUtil.getErrorCode(httpResponseException); | ||
Assert.fail(); | ||
} catch(HttpResponseException ex) { | ||
Assert.assertSame(httpResponseException, ex); | ||
} | ||
} | ||
|
||
/** Multiple error objects should cause original exception to be rethrown. */ | ||
@Test | ||
public void testGetErrorCode_multipleErrors() { | ||
HttpResponseException httpResponseException = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[" | ||
+ "{\"code\":\"MANIFEST_INVALID\",\"message\":\"message 1\",\"detail\":{}}," | ||
+ "{\"code\":\"TAG_INVALID\",\"message\":\"message 2\",\"detail\":{}}" | ||
+ "]}") | ||
.build(); | ||
try { | ||
ErrorResponseUtil.getErrorCode(httpResponseException); | ||
Assert.fail(); | ||
} catch (HttpResponseException ex) { | ||
Assert.assertSame(httpResponseException, ex); | ||
} | ||
} | ||
|
||
/** An non-error object should cause original exception to be rethrown. */ | ||
@Test | ||
public void testGetErrorCode_invalidErrorObject() { | ||
HttpResponseException httpResponseException = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent("{\"type\":\"other\",\"message\":\"some other object\"}") | ||
.build(); | ||
try { | ||
ErrorResponseUtil.getErrorCode(httpResponseException); | ||
Assert.fail(); | ||
} catch (HttpResponseException ex) { | ||
Assert.assertSame(httpResponseException, ex); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ | |
|
||
package com.google.cloud.tools.jib.registry; | ||
|
||
import com.google.api.client.http.HttpHeaders; | ||
import com.google.api.client.http.HttpResponseException; | ||
import com.google.cloud.tools.jib.http.BlobHttpContent; | ||
import com.google.cloud.tools.jib.http.Response; | ||
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate; | ||
|
@@ -30,6 +32,8 @@ | |
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
import org.apache.http.HttpStatus; | ||
import org.hamcrest.CoreMatchers; | ||
import org.junit.Assert; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
|
@@ -101,4 +105,96 @@ public void testGetActionDescription() { | |
public void testGetAccept() { | ||
Assert.assertEquals(0, testManifestPusher.getAccept().size()); | ||
} | ||
|
||
/** Docker Registry 2.0 and 2.1 return 400 / TAG_INVALID. */ | ||
@Test | ||
public void testHandleHttpResponseException_dockerRegistry_tagInvalid() { | ||
HttpResponseException exception = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[{\"code\":\"TAG_INVALID\",\"message\":\"manifest tag did not match URI\"}]}") | ||
.build(); | ||
try { | ||
testManifestPusher.handleHttpResponseException(exception); | ||
Assert.fail(); | ||
|
||
} catch (RegistryErrorException ex) { | ||
Assert.assertThat( | ||
ex.getMessage(), | ||
CoreMatchers.containsString( | ||
"Registry does not support Image Manifest Version 2, Schema 2")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Registry may not". |
||
|
||
} catch (HttpResponseException ex) { | ||
Assert.fail("should have been a RegistryErrorException"); | ||
} | ||
} | ||
|
||
/** Docker Registry 2.2 returns a 400 / MANIFEST_INVALID. */ | ||
@Test | ||
public void testHandleHttpResponseException_dockerRegistry_manifestInvalid() { | ||
HttpResponseException exception = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_BAD_REQUEST, "Bad Request", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[{\"code\":\"MANIFEST_INVALID\",\"message\":\"manifest invalid\",\"detail\":{}}]}") | ||
.build(); | ||
try { | ||
testManifestPusher.handleHttpResponseException(exception); | ||
Assert.fail(); | ||
|
||
} catch (RegistryErrorException ex) { | ||
Assert.assertThat( | ||
ex.getMessage(), | ||
CoreMatchers.containsString( | ||
"Registry may not support Image Manifest Version 2, Schema 2")); | ||
|
||
} catch (HttpResponseException ex) { | ||
Assert.fail("should have been a RegistryErrorException"); | ||
} | ||
} | ||
|
||
/** Quay.io returns an undocumented 415 / MANIFEST_INVALID. */ | ||
@Test | ||
public void testHandleHttpResponseException_quayIo() { | ||
HttpResponseException exception = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_UNSUPPORTED_MEDIA_TYPE, "UNSUPPORTED MEDIA TYPE", new HttpHeaders()) | ||
.setContent( | ||
"{\"errors\":[{\"code\":\"MANIFEST_INVALID\",\"detail\":" | ||
+ "{\"message\":\"manifest schema version not supported\"},\"message\":\"manifest invalid\"}]}") | ||
.build(); | ||
try { | ||
testManifestPusher.handleHttpResponseException(exception); | ||
Assert.fail(); | ||
|
||
} catch (RegistryErrorException ex) { | ||
Assert.assertThat( | ||
ex.getMessage(), | ||
CoreMatchers.containsString( | ||
"Registry may not support Image Manifest Version 2, Schema 2")); | ||
|
||
} catch (HttpResponseException ex) { | ||
Assert.fail("should have been a RegistryErrorException"); | ||
} | ||
} | ||
|
||
@Test | ||
public void testHandleHttpResponseException_otherError() { | ||
HttpResponseException exception = | ||
new HttpResponseException.Builder( | ||
HttpStatus.SC_UNAUTHORIZED, "Unauthorized", new HttpHeaders()) | ||
.setContent("{\"errors\":[{\"code\":\"UNAUTHORIZED\",\"message\":\"Unauthorized\"]}}") | ||
.build(); | ||
try { | ||
testManifestPusher.handleHttpResponseException(exception); | ||
Assert.fail(); | ||
|
||
} catch (RegistryErrorException ex) { | ||
Assert.fail("should have been a HttpResponseException"); | ||
|
||
} catch (HttpResponseException ex) { | ||
Assert.assertSame(exception, ex); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SInce it's enum,
errorCode == ErrorCodes.BLOB_UNKNOWN
will work, right?