-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Support building multi-platform OCI index #4470
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -20,7 +20,6 @@ | |
| import com.google.cloud.tools.jib.blob.BlobDescriptor; | ||
| import com.google.cloud.tools.jib.hash.Digests; | ||
| import com.google.cloud.tools.jib.image.Image; | ||
| import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate.ManifestDescriptorTemplate; | ||
| import java.io.IOException; | ||
| import java.util.List; | ||
|
|
||
|
|
@@ -44,23 +43,27 @@ public ManifestListGenerator(List<Image> images) { | |
| */ | ||
| public <T extends BuildableManifestTemplate> ManifestTemplate getManifestListTemplate( | ||
| Class<T> manifestTemplateClass) throws IOException { | ||
| Preconditions.checkArgument( | ||
| manifestTemplateClass == V22ManifestTemplate.class, | ||
| "Build an OCI image index is not yet supported"); | ||
| Preconditions.checkState(!images.isEmpty(), "no images given"); | ||
|
|
||
| V22ManifestListTemplate manifestList = new V22ManifestListTemplate(); | ||
| for (Image image : images) { | ||
| ImageToJsonTranslator imageTranslator = new ImageToJsonTranslator(image); | ||
| if (manifestTemplateClass == V22ManifestTemplate.class) { | ||
| return getV22ManifestListTemplate(); | ||
|
|
||
| BlobDescriptor configDescriptor = | ||
| Digests.computeDigest(imageTranslator.getContainerConfiguration()); | ||
| } else if (manifestTemplateClass == OciManifestTemplate.class) { | ||
| return getOciIndexTemplate(); | ||
| } | ||
| throw new IllegalArgumentException( | ||
| "Unsupported manifestTemplateClass format " + manifestTemplateClass); | ||
| } | ||
rquinio marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| private V22ManifestListTemplate getV22ManifestListTemplate() throws IOException { | ||
| V22ManifestListTemplate manifestList = new V22ManifestListTemplate(); | ||
| for (Image image : images) { | ||
| BuildableManifestTemplate manifestTemplate = | ||
| imageTranslator.getManifestTemplate(manifestTemplateClass, configDescriptor); | ||
| getBuildableManifestTemplate(V22ManifestTemplate.class, image); | ||
| BlobDescriptor manifestDescriptor = Digests.computeDigest(manifestTemplate); | ||
|
|
||
| ManifestDescriptorTemplate manifest = new ManifestDescriptorTemplate(); | ||
| V22ManifestListTemplate.ManifestDescriptorTemplate manifest = | ||
| new V22ManifestListTemplate.ManifestDescriptorTemplate(); | ||
| manifest.setMediaType(manifestTemplate.getManifestMediaType()); | ||
| manifest.setSize(manifestDescriptor.getSize()); | ||
| manifest.setDigest(manifestDescriptor.getDigest().toString()); | ||
|
|
@@ -69,4 +72,31 @@ public <T extends BuildableManifestTemplate> ManifestTemplate getManifestListTem | |
| } | ||
| return manifestList; | ||
| } | ||
|
|
||
| private OciIndexTemplate getOciIndexTemplate() throws IOException { | ||
| OciIndexTemplate manifestList = new OciIndexTemplate(); | ||
| for (Image image : images) { | ||
| BuildableManifestTemplate manifestTemplate = | ||
| getBuildableManifestTemplate(OciManifestTemplate.class, image); | ||
| BlobDescriptor manifestDescriptor = Digests.computeDigest(manifestTemplate); | ||
|
|
||
| OciIndexTemplate.ManifestDescriptorTemplate manifest = | ||
| new OciIndexTemplate.ManifestDescriptorTemplate( | ||
| manifestTemplate.getManifestMediaType(), | ||
| manifestDescriptor.getSize(), | ||
| manifestDescriptor.getDigest()); | ||
| manifest.setPlatform(image.getArchitecture(), image.getOs()); | ||
| manifestList.addManifest(manifest); | ||
| } | ||
| return manifestList; | ||
| } | ||
|
Comment on lines
+76
to
+92
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. jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ManifestListGenerator.java:R76-92 V22ManifestListTemplate stores digest as String vs OciIndexTemplate uses DescriptorDigest The V22 manifest list path in https://app.devin.ai/review/GoogleContainerTools/jib/pull/4470 |
||
|
|
||
| private <T extends BuildableManifestTemplate> | ||
| BuildableManifestTemplate getBuildableManifestTemplate( | ||
| Class<T> manifestTemplateClass, Image image) throws IOException { | ||
| ImageToJsonTranslator imageTranslator = new ImageToJsonTranslator(image); | ||
| BlobDescriptor configDescriptor = | ||
| Digests.computeDigest(imageTranslator.getContainerConfiguration()); | ||
| return imageTranslator.getManifestTemplate(manifestTemplateClass, configDescriptor); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,8 @@ | |
| import com.google.cloud.tools.jib.image.Image; | ||
| import com.google.cloud.tools.jib.image.Layer; | ||
| import com.google.cloud.tools.jib.image.json.ManifestTemplate; | ||
| import com.google.cloud.tools.jib.image.json.OciIndexTemplate; | ||
| import com.google.cloud.tools.jib.image.json.OciManifestTemplate; | ||
| import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate; | ||
| import com.google.cloud.tools.jib.image.json.V22ManifestTemplate; | ||
| import java.io.IOException; | ||
|
|
@@ -36,7 +38,7 @@ | |
| import org.mockito.Mockito; | ||
| import org.mockito.junit.MockitoJUnitRunner; | ||
|
|
||
| /** Tests for {@link BuildManifestListOrSingleManifest}. */ | ||
| /** Tests for {@link BuildManifestListOrSingleManifestStep}. */ | ||
| @RunWith(MockitoJUnitRunner.class) | ||
| public class BuildManifestListOrSingleManifestStepTest { | ||
|
|
||
|
|
@@ -149,6 +151,52 @@ public void testCall_manifestList() throws IOException { | |
| manifestList.getDigestsForPlatform("arm64", "windows")); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCall_manifestOciIndex() throws IOException { | ||
|
|
||
| // Expected Manifest Index JSON | ||
| // { | ||
| // "schemaVersion":2, | ||
| // "mediaType":"application/vnd.oci.image.index.v1+json", | ||
| // "manifests":[ | ||
| // { | ||
| // "mediaType":"application/vnd.oci.image.manifest.v1+json", | ||
| // "digest":"sha256:9591d0e20a39c41abdf52d2f8f30c97d7aeccbc3835999152e73a85de434d781", | ||
| // "size":338, | ||
| // "platform":{ | ||
| // "architecture":"amd64", | ||
| // "os":"linux" | ||
| // } | ||
| // }, | ||
| // { | ||
| // "mediaType":"application/vnd.oci.image.manifest.v1+json", | ||
| // "digest":"sha256:8e0e6885ba5969d8fedf3f1b38ec68bb8fbf9f528c6e4c516328a81525ec479f", | ||
| // "size":338, | ||
| // "platform":{ | ||
| // "architecture":"arm64", | ||
| // "os":"windows" | ||
| // } | ||
| // } | ||
| // ] | ||
| // } | ||
|
|
||
| Mockito.doReturn(OciManifestTemplate.class).when(buildContext).getTargetFormat(); | ||
| ManifestTemplate manifestTemplate = | ||
| new BuildManifestListOrSingleManifestStep( | ||
| buildContext, progressDispatcherFactory, Arrays.asList(image1, image2)) | ||
| .call(); | ||
|
|
||
| Assert.assertTrue(manifestTemplate instanceof OciIndexTemplate); | ||
| OciIndexTemplate manifestList = (OciIndexTemplate) manifestTemplate; | ||
| Assert.assertEquals(2, manifestList.getSchemaVersion()); | ||
| Assert.assertEquals( | ||
| Arrays.asList("sha256:9591d0e20a39c41abdf52d2f8f30c97d7aeccbc3835999152e73a85de434d781"), | ||
| manifestList.getDigestsForPlatform("amd64", "linux")); | ||
| Assert.assertEquals( | ||
| Arrays.asList("sha256:8e0e6885ba5969d8fedf3f1b38ec68bb8fbf9f528c6e4c516328a81525ec479f"), | ||
| manifestList.getDigestsForPlatform("arm64", "windows")); | ||
|
Comment on lines
+183
to
+197
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. jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildManifestListOrSingleManifestStepTest.java:R183-197 Test images built with V22 format but tested against OCI output In https://app.devin.ai/review/GoogleContainerTools/jib/pull/4470 |
||
| } | ||
|
|
||
| @Test | ||
| public void testCall_emptyImagesList() throws IOException { | ||
| try { | ||
|
|
||
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.
The current assertions rely on the order of manifests in the manifest list, which could make the test fragile if the underlying implementation changes how platforms are ordered. To make the test more robust, I suggest verifying the platforms in an order-independent way. For example, you could collect the architectures into a
Setand assert its contents, and then verify the OS for all manifests.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.
Personally I prefer the current version, so that if the underlying implementation changes in Jib we see the impact somewhere.