Skip to content
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

Support manifest lists when referenced by sha256 #1811

Merged
merged 5 commits into from
Jun 28, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.cloud.tools.jib.Command;
import com.google.cloud.tools.jib.registry.LocalRegistry;
import com.google.cloud.tools.jib.registry.ManifestPullerIntegrationTest;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
Expand Down Expand Up @@ -188,4 +189,22 @@ public void testProvidedExecutorNotDisposed()

executorService.shutdown();
}

@Test
public void testManifestListReferenceByShaDoesNotFail()
throws InvalidImageReferenceException, IOException, InterruptedException, ExecutionException,
RegistryException, CacheDirectoryCreationException {
ImageReference sourceImageReferenceAsManifestList =
ImageReference.of(
"registry-1.docker.io",
"library/openjdk",
ManifestPullerIntegrationTest.KNOWN_MANIFEST_LIST_SHA);
Containerizer containerizer =
Containerizer.to(
TarImage.named("whatever")
.saveTo(cacheFolder.newFolder("goose").toPath().resolve("moose")));

Jib.from(sourceImageReferenceAsManifestList).containerize(containerizer);
// pass, no exceptions thrown
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

import com.google.cloud.tools.jib.api.RegistryException;
import com.google.cloud.tools.jib.event.EventHandlers;
import com.google.cloud.tools.jib.http.Authorization;
import com.google.cloud.tools.jib.image.json.ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
import java.io.IOException;
import org.hamcrest.CoreMatchers;
Expand All @@ -30,6 +32,10 @@
/** Integration tests for {@link ManifestPuller}. */
public class ManifestPullerIntegrationTest {

/** A known manifest list sha for openjdk:11-jre-slim */
public static final String KNOWN_MANIFEST_LIST_SHA =
"sha256:8ab7b3078b01ba66b937b7fbe0b9eccf60449cc101c42e99aeefaba0e1781155";

@ClassRule public static LocalRegistry localRegistry = new LocalRegistry(5000);

@Test
Expand Down Expand Up @@ -58,6 +64,40 @@ public void testPull_v22() throws IOException, RegistryException, InterruptedExc
Assert.assertTrue(v22ManifestTemplate.getLayers().size() > 0);
}

@Test
public void testPull_v22ManifestList() throws IOException, RegistryException {
RegistryClient.Factory factory =
RegistryClient.factory(EventHandlers.NONE, "registry-1.docker.io", "library/openjdk");
Authorization authorization =
factory.newRegistryClient().getRegistryAuthenticator().authenticatePull(null);
RegistryClient registryClient = factory.setAuthorization(authorization).newRegistryClient();

// Ensure 11-jre-slim is a manifest list
V22ManifestListTemplate manifestListTemplate =
registryClient.pullManifest("11-jre-slim", V22ManifestListTemplate.class);
Assert.assertEquals(2, manifestListTemplate.getSchemaVersion());
Assert.assertTrue(manifestListTemplate.getManifests().size() > 0);

// Generic call to 11-jre-slim should NOT pull a manifest list (delegate to registry default)
ManifestTemplate manifestTemplate = registryClient.pullManifest("11-jre-slim");
Assert.assertEquals(2, manifestTemplate.getSchemaVersion());
Assert.assertThat(manifestTemplate, CoreMatchers.instanceOf(V22ManifestTemplate.class));

// Make sure we can't cast a v22ManifestTemplate to v22ManifestListTemplate in ManifestPuller
try {
registryClient.pullManifest(KNOWN_MANIFEST_LIST_SHA, V22ManifestTemplate.class);
Assert.fail();
} catch (ClassCastException ex) {
// pass
}

// Referencing a manifest list by sha256, should return a manifest list
ManifestTemplate sha256ManifestList = registryClient.pullManifest(KNOWN_MANIFEST_LIST_SHA);
Assert.assertEquals(2, sha256ManifestList.getSchemaVersion());
Assert.assertThat(sha256ManifestList, CoreMatchers.instanceOf(V22ManifestListTemplate.class));
Assert.assertTrue(((V22ManifestListTemplate) sha256ManifestList).getManifests().size() > 0);
}

@Test
public void testPull_unknownManifest()
throws RegistryException, IOException, InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@
import com.google.cloud.tools.jib.image.json.ManifestTemplate;
import com.google.cloud.tools.jib.image.json.UnknownManifestFormatException;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.cloud.tools.jib.registry.RegistryAuthenticator;
import com.google.cloud.tools.jib.registry.RegistryClient;
import com.google.cloud.tools.jib.registry.credentials.CredentialRetrievalException;
import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.Callable;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -207,6 +209,19 @@ private Image pullBaseImage(
ManifestTemplate manifestTemplate =
registryClient.pullManifest(buildConfiguration.getBaseImageConfiguration().getImageTag());

// special handling if we happen upon a manifest list, redirect to a manifest and continue
// handling it normally
if (manifestTemplate instanceof V22ManifestListTemplate) {
buildConfiguration
.getEventHandlers()
.dispatch(
LogEvent.lifecycle(
"The base image reference is manifest list, searching for linux/amd64"));
manifestTemplate =
obtainPlatformSpecificImageManifest(
registryClient, (V22ManifestListTemplate) manifestTemplate);
}

// TODO: Make schema version be enum.
switch (manifestTemplate.getSchemaVersion()) {
case 1:
Expand Down Expand Up @@ -258,6 +273,25 @@ private Image pullBaseImage(
throw new IllegalStateException("Unknown manifest schema version");
}

/**
* Looks through a manifest list for any amd64/linux manifest and downloads and returns the first
* manifest it finds.
*/
private ManifestTemplate obtainPlatformSpecificImageManifest(
RegistryClient registryClient, V22ManifestListTemplate manifestListTemplate)
throws RegistryException, IOException {

List<String> digests = manifestListTemplate.getDigestsForPlatform("amd64", "linux");
chanseokoh marked this conversation as resolved.
Show resolved Hide resolved
if (digests.size() == 0) {
String errorMessage =
"Unable to find amd64/linux manifest in manifest list at: "
+ buildConfiguration.getBaseImageConfiguration().getImage();
buildConfiguration.getEventHandlers().dispatch(LogEvent.error(errorMessage));
throw new RegistryException(errorMessage);
}
return registryClient.pullManifest(digests.get(0));
}

/**
* Retrieves the cached base image.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public int getSchemaVersion() {
@Nullable private List<ManifestDescriptorTemplate> manifests;

@VisibleForTesting
List<ManifestDescriptorTemplate> getManifests() {
public List<ManifestDescriptorTemplate> getManifests() {
return Preconditions.checkNotNull(manifests);
}

Expand All @@ -93,10 +93,10 @@ public List<String> getDigestsForPlatform(String architecture, String os) {
}

/** Template for inner JSON object representing a single platform specific manifest. */
static class ManifestDescriptorTemplate implements JsonTemplate {
public static class ManifestDescriptorTemplate implements JsonTemplate {

@JsonIgnoreProperties(ignoreUnknown = true)
static class Platform implements JsonTemplate {
public static class Platform implements JsonTemplate {
@Nullable private String architecture;
@Nullable private String os;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.tools.jib.image.json.OCIManifestTemplate;
import com.google.cloud.tools.jib.image.json.UnknownManifestFormatException;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.common.io.CharStreams;
Expand Down Expand Up @@ -71,7 +72,13 @@ public List<String> getAccept() {
if (manifestTemplateClass.equals(OCIManifestTemplate.class)) {
return Collections.singletonList(OCIManifestTemplate.MANIFEST_MEDIA_TYPE);
}
if (manifestTemplateClass.equals(V22ManifestListTemplate.class)) {
return Collections.singletonList(V22ManifestListTemplate.MANIFEST_MEDIA_TYPE);
}

// V22ManifestListTemplate is not included by default, we don't explicitly accept
// it, we only handle it if referenced by sha256 (see getManifestTemplateFromJson) in which
// case registries ignore the "accept" directive and just return a manifest list anyway.
return Arrays.asList(
OCIManifestTemplate.MANIFEST_MEDIA_TYPE,
V22ManifestTemplate.MANIFEST_MEDIA_TYPE,
Expand Down Expand Up @@ -118,10 +125,6 @@ private T getManifestTemplateFromJson(String jsonString)
throw new UnknownManifestFormatException("Cannot find field 'schemaVersion' in manifest");
}

if (!manifestTemplateClass.equals(ManifestTemplate.class)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it makes sense to update this Javadoc too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, but a ClassCastException should only happen if something went horribly wrong.

return JsonTemplateMapper.readJson(jsonString, manifestTemplateClass);
}

int schemaVersion = node.get("schemaVersion").asInt(-1);
if (schemaVersion == -1) {
throw new UnknownManifestFormatException("`schemaVersion` field is not an integer");
Expand All @@ -142,6 +145,10 @@ private T getManifestTemplateFromJson(String jsonString)
return manifestTemplateClass.cast(
JsonTemplateMapper.readJson(jsonString, OCIManifestTemplate.class));
}
if (V22ManifestListTemplate.MANIFEST_MEDIA_TYPE.equals(mediaType)) {
return manifestTemplateClass.cast(
JsonTemplateMapper.readJson(jsonString, V22ManifestListTemplate.class));
}
throw new UnknownManifestFormatException("Unknown mediaType: " + mediaType);
}
throw new UnknownManifestFormatException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@
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.Response;
import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate;
import com.google.cloud.tools.jib.image.json.ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
import com.google.cloud.tools.jib.json.JsonTemplate;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -282,7 +281,8 @@ public RegistryAuthenticator getRegistryAuthenticator() throws IOException, Regi
* @param <T> child type of ManifestTemplate
* @param imageTag the tag to pull on
* @param manifestTemplateClass the specific version of manifest template to pull, or {@link
* ManifestTemplate} to pull either {@link V22ManifestTemplate} or {@link V21ManifestTemplate}
* ManifestTemplate} to pull predefined subclasses; see: {@link
* ManifestPuller#handleResponse(Response)}
* @return the manifest template
* @throws IOException if communicating with the endpoint fails
* @throws RegistryException if communicating with the endpoint fails
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.image.json.OCIManifestTemplate;
import com.google.cloud.tools.jib.image.json.UnknownManifestFormatException;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
import com.google.common.io.Resources;
import java.io.ByteArrayInputStream;
Expand Down Expand Up @@ -89,6 +90,42 @@ public void testHandleResponse_v22()
Assert.assertThat(manifestTemplate, CoreMatchers.instanceOf(V22ManifestTemplate.class));
}

@Test
public void testHandleResponse_v22ManifestListFailsWhenParsedAsV22Manifest()
throws URISyntaxException, IOException, UnknownManifestFormatException {
Path v22ManifestListFile =
Paths.get(Resources.getResource("core/json/v22manifest_list.json").toURI());
InputStream v22ManifestList = new ByteArrayInputStream(Files.readAllBytes(v22ManifestListFile));

Mockito.when(mockResponse.getBody()).thenReturn(v22ManifestList);
try {
new ManifestPuller<>(
fakeRegistryEndpointRequestProperties, "test-image-tag", V22ManifestTemplate.class)
.handleResponse(mockResponse);
Assert.fail();
} catch (ClassCastException ex) {
// pass
}
}

@Test
public void testHandleResponse_v22ManifestList()
throws URISyntaxException, IOException, UnknownManifestFormatException {
Path v22ManifestListFile =
Paths.get(Resources.getResource("core/json/v22manifest_list.json").toURI());
InputStream v22ManifestList = new ByteArrayInputStream(Files.readAllBytes(v22ManifestListFile));

Mockito.when(mockResponse.getBody()).thenReturn(v22ManifestList);
ManifestTemplate manifestTemplate =
new ManifestPuller<>(
fakeRegistryEndpointRequestProperties,
"test-image-tag",
V22ManifestListTemplate.class)
chanseokoh marked this conversation as resolved.
Show resolved Hide resolved
.handleResponse(mockResponse);

Assert.assertThat(manifestTemplate, CoreMatchers.instanceOf(V22ManifestListTemplate.class));
loosebazooka marked this conversation as resolved.
Show resolved Hide resolved
}

@Test
public void testHandleResponse_noSchemaVersion() throws IOException {
Mockito.when(mockResponse.getBody()).thenReturn(stringToInputStreamUtf8("{}"));
Expand Down Expand Up @@ -175,5 +212,12 @@ public void testGetAccept() {
new ManifestPuller<>(
fakeRegistryEndpointRequestProperties, "test-image-tag", V21ManifestTemplate.class)
.getAccept());
Assert.assertEquals(
Collections.singletonList(V22ManifestListTemplate.MANIFEST_MEDIA_TYPE),
new ManifestPuller<>(
fakeRegistryEndpointRequestProperties,
"test-image-tag",
V22ManifestListTemplate.class)
.getAccept());
}
}