Support manifest lists when referenced by sha256 #1811
Conversation
...integration-test/java/com/google/cloud/tools/jib/registry/ManifestPullerIntegrationTest.java
Outdated
Show resolved
Hide resolved
jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java
Show resolved
Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPullerTest.java
Show resolved
Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPullerTest.java
Show resolved
Hide resolved
| V22ManifestListTemplate manifestListTemplate = | ||
| registryClient.pullManifest("11-jre-slim", V22ManifestListTemplate.class); | ||
| Assert.assertEquals(2, manifestListTemplate.getSchemaVersion()); | ||
| Assert.assertTrue(manifestListTemplate.getDigestsForPlatform("amd64", "linux").size() > 0); |
There was a problem hiding this comment.
Maybe check for another platform too?
jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPuller.java
Outdated
Show resolved
Hide resolved
| @@ -118,10 +125,6 @@ private T getManifestTemplateFromJson(String jsonString) | |||
| throw new UnknownManifestFormatException("Cannot find field 'schemaVersion' in manifest"); | |||
| } | |||
|
|
|||
| if (!manifestTemplateClass.equals(ManifestTemplate.class)) { | |||
There was a problem hiding this comment.
This code would unsafely create template objects that didn't first validate schema and mediatype. So now we always validate the value and then create the objects. Any mismatch will trigger a ClassCastException (indicating programming error or registry error).
There was a problem hiding this comment.
I remember something. This is about whether the registry client API should support the usage of pullManifest(imageTag, templateClass): https://github.com/GoogleContainerTools/jib/blob/v1.3.0-maven/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java#L196-L197
Our plugins don't make use of it, but as an API, it may make sense to keep it.
If we remove this, we can actually clean up a lot more. From what I recall, ManfiestPuller won't need to accept the manifestTemplateClass argument.
There was a problem hiding this comment.
And it makes sense to update this Javadoc too.
There was a problem hiding this comment.
You can still verify that you're actually pulling something you wanted... in which case ManifestPuller should probably still accept a template class. Either way, I think we should do it in a new PR.
There was a problem hiding this comment.
Agreed, we can do it later. (It is basically the question of whether we maintain the RegistryClient API surface in a sane manner. A library throwing a ClassCastException won't look nice. But we aren't even sure if we will ever release this as a library.)
There was a problem hiding this comment.
Ah yeah, but a ClassCastException should only happen if something went horribly wrong.
jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPullerTest.java
Show resolved
Hide resolved
jib-core/src/test/java/com/google/cloud/tools/jib/registry/ManifestPullerTest.java
Show resolved
Hide resolved
This PR adds support for manifest lists when referenced by sha256 as described in #1360
This PR: