From ceb6e290cd954634ee3ba7292c3ce6247df2de58 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 30 Sep 2020 15:27:01 -0400 Subject: [PATCH 1/6] Check configured platforms for single image building --- .../builder/steps/LocalBaseImageSteps.java | 23 +++-- .../jib/builder/steps/PlatformChecker.java | 71 ++++++++++++++ .../jib/builder/steps/PullBaseImageStep.java | 1 + .../builder/steps/PlatformCheckerTest.java | 98 +++++++++++++++++++ 4 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java create mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java index 2070790de1..4f7a76545b 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java @@ -125,11 +125,14 @@ static Callable retrieveDockerDaemonLayersStep( dockerClient.save(imageReference, tarPath, throttledProgressReporter); } - return cacheDockerImageTar( - buildContext, - tarPath, - progressEventDispatcher.newChildProducer(), - tempDirectoryProvider); + LocalImage localImage = + cacheDockerImageTar( + buildContext, + tarPath, + progressEventDispatcher.newChildProducer(), + tempDirectoryProvider); + PlatformChecker.checkManifestPlatform(buildContext, localImage.configurationTemplate); + return localImage; } }; } @@ -139,9 +142,13 @@ static Callable retrieveTarLayersStep( ProgressEventDispatcher.Factory progressEventDispatcherFactory, Path tarPath, TempDirectoryProvider tempDirectoryProvider) { - return () -> - cacheDockerImageTar( - buildContext, tarPath, progressEventDispatcherFactory, tempDirectoryProvider); + return () -> { + LocalImage localImage = + cacheDockerImageTar( + buildContext, tarPath, progressEventDispatcherFactory, tempDirectoryProvider); + PlatformChecker.checkManifestPlatform(buildContext, localImage.configurationTemplate); + return localImage; + }; } static Callable returnImageAndRegistryClientStep( diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java new file mode 100644 index 0000000000..79c08c9201 --- /dev/null +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java @@ -0,0 +1,71 @@ +/* + * Copyright 20120 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.builder.steps; + +import com.google.cloud.tools.jib.api.ImageReference; +import com.google.cloud.tools.jib.api.LogEvent; +import com.google.cloud.tools.jib.api.buildplan.Platform; +import com.google.cloud.tools.jib.configuration.BuildContext; +import com.google.cloud.tools.jib.event.EventHandlers; +import com.google.cloud.tools.jib.image.json.ContainerConfigurationTemplate; +import com.google.common.base.Verify; +import java.util.Set; + +/** Provides helper methods to check platforms. */ +public class PlatformChecker { + + /** + * Assuming the base image is not a manifest list, checks and warns misconfigured platforms. + * + * @param buildContext the {@link BuildContext} + * @param containerConfig container configuration JSON of the base image + */ + static void checkManifestPlatform( + BuildContext buildContext, ContainerConfigurationTemplate containerConfig) { + EventHandlers eventHandlers = buildContext.getEventHandlers(); + ImageReference baseImage = buildContext.getBaseImageConfiguration().getImage(); + Set platforms = buildContext.getContainerConfiguration().getPlatforms(); + Verify.verify(!platforms.isEmpty()); + + if (platforms.size() != 1) { + eventHandlers.dispatch( + LogEvent.warn("platforms configured, but '" + baseImage + "' is not a manifest list")); + } else { + Platform platform = platforms.iterator().next(); + if (!platform.getArchitecture().equals(containerConfig.getArchitecture()) + || !platform.getOs().equals(containerConfig.getOs())) { + + // Unfortunately, "platforms" has amd64/linux by default even if the user didn't explicitly + // configure it. Skip reporting to suppress false alarm. + if (!(platform.getArchitecture().equals("amd64") && platform.getOs().equals("linux"))) { + String warning = + "the configured platform (%s/%s) doesn't match the platform (%s/%s) of the base " + + "image (%s)"; + eventHandlers.dispatch( + LogEvent.warn( + String.format( + warning, + platform.getArchitecture(), + platform.getOs(), + containerConfig.getArchitecture(), + containerConfig.getOs(), + baseImage))); + } + } + } + } +} diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java index 8aed86f8ed..1bec8aa731 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java @@ -233,6 +233,7 @@ private List pullBaseImages( BuildableManifestTemplate imageManifest = (BuildableManifestTemplate) manifestTemplate; ContainerConfigurationTemplate containerConfig = pullContainerConfigJson(manifestAndDigest, registryClient, progressEventDispatcher); + PlatformChecker.checkManifestPlatform(buildContext, containerConfig); cache.writeMetadata(baseImageConfig.getImage(), imageManifest, containerConfig); return Collections.singletonList( JsonToImageTranslator.toImage(imageManifest, containerConfig)); diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java new file mode 100644 index 0000000000..7823682fa3 --- /dev/null +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java @@ -0,0 +1,98 @@ +/* + * Copyright 2020 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.builder.steps; + +import com.google.cloud.tools.jib.api.ImageReference; +import com.google.cloud.tools.jib.api.LogEvent; +import com.google.cloud.tools.jib.api.buildplan.Platform; +import com.google.cloud.tools.jib.builder.ProgressEventDispatcher; +import com.google.cloud.tools.jib.configuration.BuildContext; +import com.google.cloud.tools.jib.configuration.ContainerConfiguration; +import com.google.cloud.tools.jib.configuration.ImageConfiguration; +import com.google.cloud.tools.jib.event.EventHandlers; +import com.google.cloud.tools.jib.image.json.ContainerConfigurationTemplate; +import com.google.cloud.tools.jib.registry.RegistryClient; +import com.google.common.collect.ImmutableSet; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +/** Tests for {@link PlatformChecker}. */ +@RunWith(MockitoJUnitRunner.class) +public class PlatformCheckerTest { + + @Mock private ProgressEventDispatcher.Factory progressDispatcherFactory; + @Mock private BuildContext buildContext; + @Mock private RegistryClient registryClient; + @Mock private ImageConfiguration imageConfiguration; + @Mock private ContainerConfiguration containerConfig; + @Mock private EventHandlers eventHandlers; + + @Before + public void setUp() { + Mockito.when(buildContext.getBaseImageConfiguration()) + .thenReturn(ImageConfiguration.builder(ImageReference.scratch()).build()); + Mockito.when(buildContext.getEventHandlers()).thenReturn(eventHandlers); + Mockito.when(buildContext.getContainerConfiguration()).thenReturn(containerConfig); + } + + @Test + public void testCheckManifestPlatform_mismatch() { + Mockito.when(containerConfig.getPlatforms()) + .thenReturn(ImmutableSet.of(new Platform("configured arch", "configured OS"))); + + ContainerConfigurationTemplate containerConfigJson = new ContainerConfigurationTemplate(); + containerConfigJson.setArchitecture("actual arch"); + containerConfigJson.setOs("actual OS"); + + PlatformChecker.checkManifestPlatform(buildContext, containerConfigJson); + + Mockito.verify(eventHandlers) + .dispatch( + LogEvent.warn( + "the configured platform (configured arch/configured OS) doesn't match the " + + "platform (actual arch/actual OS) of the base image (scratch)")); + } + + @Test + public void testCheckManifestPlatform_noWarningIfDefaultAmd64Linux() { + Mockito.when(containerConfig.getPlatforms()) + .thenReturn(ImmutableSet.of(new Platform("amd64", "linux"))); + + ContainerConfigurationTemplate containerConfigJson = new ContainerConfigurationTemplate(); + containerConfigJson.setArchitecture("actual arch"); + containerConfigJson.setOs("actual OS"); + + PlatformChecker.checkManifestPlatform(buildContext, containerConfigJson); + + Mockito.verifyNoInteractions(eventHandlers); + } + + @Test + public void testCheckManifestPlatform_multiplePlatformsConfigured() { + Mockito.when(containerConfig.getPlatforms()) + .thenReturn(ImmutableSet.of(new Platform("amd64", "linux"), new Platform("arch", "os"))); + + PlatformChecker.checkManifestPlatform(buildContext, new ContainerConfigurationTemplate()); + + Mockito.verify(eventHandlers) + .dispatch(LogEvent.warn("platforms configured, but 'scratch' is not a manifest list")); + } +} From d64ba0fecb4881363efd45237327664933702167 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Wed, 30 Sep 2020 15:41:46 -0400 Subject: [PATCH 2/6] Fix copyright header --- .../google/cloud/tools/jib/builder/steps/PlatformChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java index 79c08c9201..08cf4e92b8 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java @@ -1,5 +1,5 @@ /* - * Copyright 20120 Google LLC. + * Copyright 2020 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 From 29196821dbc9b8ccc4ed4065195e9b9d2d99f190 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 1 Oct 2020 10:16:27 -0400 Subject: [PATCH 3/6] Check cached manifest too --- .../cloud/tools/jib/builder/steps/PullBaseImageStep.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java index 1bec8aa731..52ab6daa67 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java @@ -380,10 +380,14 @@ List getCachedBaseImages() return Collections.singletonList( JsonToImageTranslator.toImage((V21ManifestTemplate) manifest)); } + + ContainerConfigurationTemplate containerConfig = + Verify.verifyNotNull(manifestsAndConfigs.get(0).getConfig()); + PlatformChecker.checkManifestPlatform(buildContext, containerConfig); + return Collections.singletonList( JsonToImageTranslator.toImage( - (BuildableManifestTemplate) Verify.verifyNotNull(manifest), - Verify.verifyNotNull(manifestsAndConfigs.get(0).getConfig()))); + (BuildableManifestTemplate) Verify.verifyNotNull(manifest), containerConfig)); } // Manifest list cached. Identify matching platforms and check if all of them are cached. From e89e511b917f4d0fd80c2b90c83e02ffc60cc056 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 1 Oct 2020 12:09:04 -0400 Subject: [PATCH 4/6] Log when using cached local image too --- .../jib/builder/steps/LocalBaseImageSteps.java | 2 ++ .../tools/jib/builder/steps/PlatformChecker.java | 11 +++++++---- .../jib/builder/steps/PlatformCheckerTest.java | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java index 4f7a76545b..4cd9f09bf8 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/LocalBaseImageSteps.java @@ -111,6 +111,8 @@ static Callable retrieveDockerDaemonLayersStep( Optional cachedImage = getCachedDockerImage(buildContext.getBaseImageLayersCache(), dockerImageDetails); if (cachedImage.isPresent()) { + PlatformChecker.checkManifestPlatform( + buildContext, cachedImage.get().configurationTemplate); return cachedImage.get(); } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java index 08cf4e92b8..a76e8e81c3 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java @@ -16,7 +16,6 @@ package com.google.cloud.tools.jib.builder.steps; -import com.google.cloud.tools.jib.api.ImageReference; import com.google.cloud.tools.jib.api.LogEvent; import com.google.cloud.tools.jib.api.buildplan.Platform; import com.google.cloud.tools.jib.configuration.BuildContext; @@ -37,13 +36,17 @@ public class PlatformChecker { static void checkManifestPlatform( BuildContext buildContext, ContainerConfigurationTemplate containerConfig) { EventHandlers eventHandlers = buildContext.getEventHandlers(); - ImageReference baseImage = buildContext.getBaseImageConfiguration().getImage(); + String baseImageName = + buildContext.getBaseImageConfiguration().getTarPath().isPresent() + ? buildContext.getBaseImageConfiguration().getTarPath().get().toString() + : buildContext.getBaseImageConfiguration().getImage().toString(); + Set platforms = buildContext.getContainerConfiguration().getPlatforms(); Verify.verify(!platforms.isEmpty()); if (platforms.size() != 1) { eventHandlers.dispatch( - LogEvent.warn("platforms configured, but '" + baseImage + "' is not a manifest list")); + LogEvent.warn("platforms configured, but '" + baseImageName + "' is not a manifest list")); } else { Platform platform = platforms.iterator().next(); if (!platform.getArchitecture().equals(containerConfig.getArchitecture()) @@ -63,7 +66,7 @@ static void checkManifestPlatform( platform.getOs(), containerConfig.getArchitecture(), containerConfig.getOs(), - baseImage))); + baseImageName))); } } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java index 7823682fa3..769c34c4d5 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java @@ -27,6 +27,8 @@ import com.google.cloud.tools.jib.image.json.ContainerConfigurationTemplate; import com.google.cloud.tools.jib.registry.RegistryClient; import com.google.common.collect.ImmutableSet; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -95,4 +97,18 @@ public void testCheckManifestPlatform_multiplePlatformsConfigured() { Mockito.verify(eventHandlers) .dispatch(LogEvent.warn("platforms configured, but 'scratch' is not a manifest list")); } + + @Test + public void testCheckManifestPlatform_tarBaseImage() { + Path tar = Paths.get("/foo/bar.tar"); + Mockito.when(buildContext.getBaseImageConfiguration()) + .thenReturn(ImageConfiguration.builder(ImageReference.scratch()).setTarPath(tar).build()); + Mockito.when(containerConfig.getPlatforms()) + .thenReturn(ImmutableSet.of(new Platform("amd64", "linux"), new Platform("arch", "os"))); + + PlatformChecker.checkManifestPlatform(buildContext, new ContainerConfigurationTemplate()); + + Mockito.verify(eventHandlers) + .dispatch(LogEvent.warn("platforms configured, but '/foo/bar.tar' is not a manifest list")); + } } From 70a3897d3af47db4fae5e918522c74201a9f4b64 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 1 Oct 2020 12:18:17 -0400 Subject: [PATCH 5/6] Fix style violation --- .../google/cloud/tools/jib/builder/steps/PlatformChecker.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java index a76e8e81c3..8348ef72e2 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java @@ -46,7 +46,8 @@ static void checkManifestPlatform( if (platforms.size() != 1) { eventHandlers.dispatch( - LogEvent.warn("platforms configured, but '" + baseImageName + "' is not a manifest list")); + LogEvent.warn( + "platforms configured, but '" + baseImageName + "' is not a manifest list")); } else { Platform platform = platforms.iterator().next(); if (!platform.getArchitecture().equals(containerConfig.getArchitecture()) From 5ed7661cd1ced00f7792b0826e8590b01c01fe47 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 1 Oct 2020 12:26:08 -0400 Subject: [PATCH 6/6] Fix path separater issue in tests on Windows --- .../cloud/tools/jib/builder/steps/PlatformCheckerTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java index 769c34c4d5..2dd77d731a 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java @@ -109,6 +109,8 @@ public void testCheckManifestPlatform_tarBaseImage() { PlatformChecker.checkManifestPlatform(buildContext, new ContainerConfigurationTemplate()); Mockito.verify(eventHandlers) - .dispatch(LogEvent.warn("platforms configured, but '/foo/bar.tar' is not a manifest list")); + .dispatch( + LogEvent.warn( + "platforms configured, but '" + tar.toString() + "' is not a manifest list")); } }