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.

Sign up for GitHub

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

Jump to bottom

fix: error when base image doesn't support target platforms #3707

Merged
merged 2 commits into from Jul 18, 2022
Merged

fix: error when base image doesn't support target platforms #3707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9d6f56
fix: error when base image doesn't support target platforms
gsquared94 Jul 7, 2022
60b33c4
minor test naming modification
mpeddada1 Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 17 additions & 19 deletions jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PlatformChecker.java
View file
Open in desktop
Expand Up @@ -16,11 +16,10 @@

package com.google.cloud.tools.jib.builder.steps;

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.cloud.tools.jib.image.json.PlatformNotFoundInBaseImageException;
import com.google.common.base.Verify;
import java.nio.file.Path;
import java.util.Optional;
Expand All @@ -38,8 +37,8 @@ private PlatformChecker() {}
* @param containerConfig container configuration JSON of the base image
*/
static void checkManifestPlatform(
BuildContext buildContext, ContainerConfigurationTemplate containerConfig) {
EventHandlers eventHandlers = buildContext.getEventHandlers();
BuildContext buildContext, ContainerConfigurationTemplate containerConfig)
throws PlatformNotFoundInBaseImageException {
Optional<Path> path = buildContext.getBaseImageConfiguration().getTarPath();
String baseImageName =
path.map(Path::toString)
Expand All @@ -49,9 +48,11 @@ static void checkManifestPlatform(
Verify.verify(!platforms.isEmpty());

if (platforms.size() != 1) {
eventHandlers.dispatch(
LogEvent.warn(
"platforms configured, but '" + baseImageName + "' is not a manifest list"));
String msg =
String.format(
"cannot build for multiple platforms since the base image '%s' is not a manifest list.",
baseImageName);
throw new PlatformNotFoundInBaseImageException(msg);
} else {
Platform platform = platforms.iterator().next();
if (!platform.getArchitecture().equals(containerConfig.getArchitecture())
Expand All @@ -60,18 +61,15 @@ static void checkManifestPlatform(
// 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(),
baseImageName)));
String msg =
String.format(
"the configured platform (%s/%s) doesn't match the platform (%s/%s) of the base image (%s)",
platform.getArchitecture(),
platform.getOs(),
containerConfig.getArchitecture(),
containerConfig.getOs(),
baseImageName);
throw new PlatformNotFoundInBaseImageException(msg);
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@
import com.google.cloud.tools.jib.image.json.JsonToImageTranslator;
import com.google.cloud.tools.jib.image.json.ManifestAndConfigTemplate;
import com.google.cloud.tools.jib.image.json.ManifestTemplate;
import com.google.cloud.tools.jib.image.json.PlatformNotFoundInBaseImageException;
import com.google.cloud.tools.jib.image.json.UnknownManifestFormatException;
import com.google.cloud.tools.jib.image.json.UnlistedPlatformInManifestListException;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
Expand Down Expand Up @@ -442,7 +443,8 @@ private ContainerConfigurationTemplate pullContainerConfigJson(
@VisibleForTesting
List<Image> getCachedBaseImages()
throws IOException, CacheCorruptedException, BadContainerConfigurationFormatException,
LayerCountMismatchException, UnlistedPlatformInManifestListException {
LayerCountMismatchException, UnlistedPlatformInManifestListException,
PlatformNotFoundInBaseImageException {
ImageReference baseImage = buildContext.getBaseImageConfiguration().getImage();
Optional<ImageMetadataTemplate> metadata =
buildContext.getBaseImageLayersCache().retrieveMetadata(baseImage);
Expand Down
27 changes: 27 additions & 0 deletions ...main/java/com/google/cloud/tools/jib/image/json/PlatformNotFoundInBaseImageException.java
View file
Open in desktop
@@ -0,0 +1,27 @@
/*
* Copyright 2022 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.image.json;

import com.google.cloud.tools.jib.api.RegistryException;

/** Exception thrown when build target platforms are not found in the base image. */
public class PlatformNotFoundInBaseImageException extends RegistryException {

public PlatformNotFoundInBaseImageException(String message) {
super(message);
}
}
64 changes: 37 additions & 27 deletions jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PlatformCheckerTest.java
View file
Open in desktop
Expand Up @@ -16,14 +16,16 @@

package com.google.cloud.tools.jib.builder.steps;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

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.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.image.json.PlatformNotFoundInBaseImageException;
import com.google.common.collect.ImmutableSet;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -40,13 +42,11 @@ public class PlatformCheckerTest {

@Mock private BuildContext buildContext;
@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);
}

Expand All @@ -58,39 +58,43 @@ public void testCheckManifestPlatform_mismatch() {
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)"));
Exception ex =
assertThrows(
PlatformNotFoundInBaseImageException.class,
() -> PlatformChecker.checkManifestPlatform(buildContext, containerConfigJson));
assertThat(ex)
.hasMessageThat()
.isEqualTo(
"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() {
public void testCheckManifestPlatform_noWarningIfDefaultAmd64Linux()
mpeddada1 marked this conversation as resolved.
Show resolved Hide resolved
throws PlatformNotFoundInBaseImageException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops sorry I missed this earlier. This test no longer performs an assertion so just invoking the method in the test may not give us many insights into what is expected from the method's behavior. How about we replace this test with the following?: testCheckManifestPlatform_arm64Linux where the architecture doesn't match but the os matches. This will help us test the && in the following condition: !(platform.getArchitecture().equals("amd64") && platform.getOs().equals("linux"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just invoking the method in the test may not give us many insights into what is expected from the method's behavior

On the contrary I think that the passing test implies the expectation that it should not throw any exception. Also this yaqs answer recommends that.

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"));
Exception ex =
assertThrows(
PlatformNotFoundInBaseImageException.class,
() ->
PlatformChecker.checkManifestPlatform(
buildContext, new ContainerConfigurationTemplate()));
assertThat(ex)
.hasMessageThat()
.isEqualTo(
"cannot build for multiple platforms since the base image 'scratch' is not a manifest list.");
}

@Test
Expand All @@ -101,11 +105,17 @@ public void testCheckManifestPlatform_tarBaseImage() {
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 '" + tar.toString() + "' is not a manifest list"));
Exception ex =
assertThrows(
PlatformNotFoundInBaseImageException.class,
() ->
PlatformChecker.checkManifestPlatform(
buildContext, new ContainerConfigurationTemplate()));
assertThat(ex)
.hasMessageThat()
.isEqualTo(
"cannot build for multiple platforms since the base image '"
+ tar
+ "' is not a manifest list.");
}
}
25 changes: 15 additions & 10 deletions jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStepTest.java
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
import com.google.cloud.tools.jib.image.json.ContainerConfigurationTemplate;
import com.google.cloud.tools.jib.image.json.ImageMetadataTemplate;
import com.google.cloud.tools.jib.image.json.ManifestAndConfigTemplate;
import com.google.cloud.tools.jib.image.json.PlatformNotFoundInBaseImageException;
import com.google.cloud.tools.jib.image.json.UnlistedPlatformInManifestListException;
import com.google.cloud.tools.jib.image.json.V21ManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestListTemplate;
Expand Down Expand Up @@ -243,8 +244,8 @@ public void testLookUpPlatformSpecificImageManifest()
@Test
public void testGetCachedBaseImages_emptyCache()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException {
UnlistedPlatformInManifestListException, PlatformNotFoundInBaseImageException,
BadContainerConfigurationFormatException, LayerCountMismatchException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand All @@ -257,7 +258,7 @@ public void testGetCachedBaseImages_emptyCache()
public void testGetCachedBaseImages_v21ManifestCached()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException, DigestException {
LayerCountMismatchException, DigestException, PlatformNotFoundInBaseImageException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand Down Expand Up @@ -286,7 +287,7 @@ public void testGetCachedBaseImages_v21ManifestCached()
public void testGetCachedBaseImages_v22ManifestCached()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException {
LayerCountMismatchException, PlatformNotFoundInBaseImageException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand All @@ -312,7 +313,7 @@ public void testGetCachedBaseImages_v22ManifestCached()
public void testGetCachedBaseImages_v22ManifestListCached()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException {
LayerCountMismatchException, PlatformNotFoundInBaseImageException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand Down Expand Up @@ -352,7 +353,7 @@ public void testGetCachedBaseImages_v22ManifestListCached()
public void testGetCachedBaseImages_v22ManifestListCached_partialMatches()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException {
LayerCountMismatchException, PlatformNotFoundInBaseImageException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand Down Expand Up @@ -382,8 +383,8 @@ public void testGetCachedBaseImages_v22ManifestListCached_partialMatches()
@Test
public void testGetCachedBaseImages_v22ManifestListCached_onlyPlatforms()
throws InvalidImageReferenceException, IOException, CacheCorruptedException,
UnlistedPlatformInManifestListException, BadContainerConfigurationFormatException,
LayerCountMismatchException {
UnlistedPlatformInManifestListException, PlatformNotFoundInBaseImageException,
BadContainerConfigurationFormatException, LayerCountMismatchException {
ImageReference imageReference = ImageReference.parse("cat");
Mockito.when(buildContext.getBaseImageConfiguration())
.thenReturn(ImageConfiguration.builder(imageReference).build());
Expand Down Expand Up @@ -422,7 +423,8 @@ public void testGetCachedBaseImages_v22ManifestListCached_onlyPlatforms()

@Test
public void testTryMirrors_noMatchingMirrors()
throws LayerCountMismatchException, BadContainerConfigurationFormatException {
throws LayerCountMismatchException, BadContainerConfigurationFormatException,
PlatformNotFoundInBaseImageException {
Mockito.when(imageConfiguration.getImageRegistry()).thenReturn("registry");
Mockito.when(buildContext.getRegistryMirrors())
.thenReturn(ImmutableListMultimap.of("unmatched1", "mirror1", "unmatched2", "mirror2"));
Expand Down Expand Up @@ -475,6 +477,8 @@ public void testTryMirrors_multipleMirrors()
.thenReturn(registryClientFactory);
Mockito.when(registryClient.pullManifest(Mockito.any()))
.thenThrow(new RegistryException("not found"));
Mockito.when(containerConfig.getPlatforms())
.thenReturn(ImmutableSet.of(new Platform("amd64", "linux")));

RegistryClient.Factory gcrRegistryClientFactory = setUpWorkingRegistryClientFactory();
Mockito.when(buildContext.newBaseImageRegistryClientFactory("gcr.io"))
Expand Down Expand Up @@ -513,7 +517,8 @@ public void testCall_allMirrorsFail()
.thenReturn(registryClientFactory);
Mockito.when(registryClient.pullManifest(Mockito.any()))
.thenThrow(new RegistryException("not found"));

Mockito.when(containerConfig.getPlatforms())
.thenReturn(ImmutableSet.of(new Platform("amd64", "linux")));
RegistryClient.Factory dockerHubRegistryClientFactory = setUpWorkingRegistryClientFactory();
Mockito.when(buildContext.newBaseImageRegistryClientFactory())
.thenReturn(dockerHubRegistryClientFactory);
Expand Down