Polish 'Correctly handle platform specific buildpack builds'

See gh-47292

Author: philwebb

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/build/Builder.java

@@ -103,23 +103,23 @@ public void build(BuildRequest request) throws DockerEngineException, IOExceptio
 		validateBindings(request.getBindings());
 		String domain = request.getBuilder().getDomain();
 		PullPolicy pullPolicy = request.getPullPolicy();
-		ImagePlatform requestedPlatform = request.getImagePlatform();
-		ImageFetcher imageFetcher = new ImageFetcher(domain, getBuilderAuthHeader(), pullPolicy, requestedPlatform);
-		Image builderImage = imageFetcher.fetchImage(ImageType.BUILDER, request.getBuilder());
+		ImagePlatform platform = request.getImagePlatform();
+		boolean specifiedPlatform = request.getImagePlatform() != null;
+		ImageFetcher imageFetcher = new ImageFetcher(domain, getBuilderAuthHeader(), pullPolicy);
+		Image builderImage = imageFetcher.fetchImage(ImageType.BUILDER, request.getBuilder(), platform);
 		BuilderMetadata builderMetadata = BuilderMetadata.fromImage(builderImage);
 		request = withRunImageIfNeeded(request, builderMetadata);
-		ImageReference imageReference = request.getRunImage();
-		Image runImage = imageFetcher.fetchImage(ImageType.RUNNER, imageReference);
-		String digest = this.docker.image().resolveManifestDigest(imageReference, requestedPlatform);
-		if (StringUtils.hasText(digest)) {
-			imageReference = imageReference.withDigest(digest);
-			runImage = imageFetcher.fetchImage(ImageType.RUNNER, imageReference);
+		platform = (platform != null) ? platform : ImagePlatform.from(builderImage);
+		Image runImage = imageFetcher.fetchImage(ImageType.RUNNER, request.getRunImage(), platform);
+		if (specifiedPlatform && runImage.getPrimaryDigest() != null) {
+			request = request.withRunImage(request.getRunImage().withDigest(runImage.getPrimaryDigest()));
+			runImage = imageFetcher.fetchImage(ImageType.RUNNER, request.getRunImage(), platform);
 		}
-		request = request.withRunImage(imageReference);
 		assertStackIdsMatch(runImage, builderImage);
 		BuildOwner buildOwner = BuildOwner.fromEnv(builderImage.getConfig().getEnv());
 		BuildpackLayersMetadata buildpackLayersMetadata = BuildpackLayersMetadata.fromImage(builderImage);
-		Buildpacks buildpacks = getBuildpacks(request, imageFetcher, builderMetadata, buildpackLayersMetadata);
+		Buildpacks buildpacks = getBuildpacks(request, imageFetcher, platform, builderMetadata,
+				buildpackLayersMetadata);
 		EphemeralBuilder ephemeralBuilder = new EphemeralBuilder(buildOwner, builderImage, request.getName(),
 				builderMetadata, request.getCreator(), request.getEnv(), buildpacks);
 		executeLifecycle(request, ephemeralBuilder);
@@ -163,9 +163,9 @@ private void assertStackIdsMatch(Image runImage, Image builderImage) {
 		}
 	}
 
-	private Buildpacks getBuildpacks(BuildRequest request, ImageFetcher imageFetcher, BuilderMetadata builderMetadata,
-			BuildpackLayersMetadata buildpackLayersMetadata) {
-		BuildpackResolverContext resolverContext = new BuilderResolverContext(imageFetcher, builderMetadata,
+	private Buildpacks getBuildpacks(BuildRequest request, ImageFetcher imageFetcher, ImagePlatform platform,
+			BuilderMetadata builderMetadata, BuildpackLayersMetadata buildpackLayersMetadata) {
+		BuildpackResolverContext resolverContext = new BuilderResolverContext(imageFetcher, platform, builderMetadata,
 				buildpackLayersMetadata);
 		return BuildpackResolvers.resolveAll(resolverContext, request.getBuildpacks());
 	}
@@ -234,51 +234,74 @@ private class ImageFetcher {
 
 		private final PullPolicy pullPolicy;
 
-		private ImagePlatform defaultPlatform;
-
-		ImageFetcher(String domain, String authHeader, PullPolicy pullPolicy, ImagePlatform platform) {
+		ImageFetcher(String domain, String authHeader, PullPolicy pullPolicy) {
 			this.domain = domain;
 			this.authHeader = authHeader;
 			this.pullPolicy = pullPolicy;
-			this.defaultPlatform = platform;
 		}
 
-		Image fetchImage(ImageType type, ImageReference reference) throws IOException {
+		Image fetchImage(ImageType type, ImageReference reference, ImagePlatform platform) throws IOException {
 			Assert.notNull(type, "Type must not be null");
 			Assert.notNull(reference, "Reference must not be null");
 			Assert.state(this.authHeader == null || reference.getDomain().equals(this.domain),
 					() -> String.format("%s '%s' must be pulled from the '%s' authenticated registry",
 							StringUtils.capitalize(type.getDescription()), reference, this.domain));
 			if (this.pullPolicy == PullPolicy.ALWAYS) {
-				return checkPlatformMismatch(pullImage(reference, type), reference);
+				return pullImageAndCheckForPlatformMismatch(type, reference, platform);
 			}
 			try {
-				return checkPlatformMismatch(Builder.this.docker.image().inspect(reference), reference);
+				Image image = Builder.this.docker.image().inspect(reference, platform);
+				return checkPlatformMismatch(image, reference, platform);
 			}
 			catch (DockerEngineException ex) {
 				if (this.pullPolicy == PullPolicy.IF_NOT_PRESENT && ex.getStatusCode() == 404) {
-					return checkPlatformMismatch(pullImage(reference, type), reference);
+					return pullImageAndCheckForPlatformMismatch(type, reference, platform);
 				}
 				throw ex;
 			}
 		}
 
-		private Image pullImage(ImageReference reference, ImageType imageType) throws IOException {
+		private Image pullImageAndCheckForPlatformMismatch(ImageType type, ImageReference reference,
+				ImagePlatform platform) throws IOException {
+			try {
+				Image image = pullImage(reference, type, platform);
+				return checkPlatformMismatch(image, reference, platform);
+			}
+			catch (DockerEngineException ex) {
+				// Try to throw our own exception for consistent log output. Matching
+				// on the message is a little brittle, but it doesn't matter too much
+				// if it fails as the original exception is still enough to stop the build
+				if (platform != null && ex.getMessage().contains("does not provide the specified platform")) {
+					throwAsPlatformMismatchException(type, reference, platform, ex);
+				}
+				throw ex;
+			}
+		}
+
+		private void throwAsPlatformMismatchException(ImageType type, ImageReference reference, ImagePlatform platform,
+				Throwable cause) throws IOException {
+			try {
+				Image image = pullImage(reference, type, null);
+				throw new PlatformMismatchException(reference, platform, ImagePlatform.from(image), cause);
+			}
+			catch (DockerEngineException ex) {
+			}
+		}
+
+		private Image pullImage(ImageReference reference, ImageType imageType, ImagePlatform platform)
+				throws IOException {
 			TotalProgressPullListener listener = new TotalProgressPullListener(
-					Builder.this.log.pullingImage(reference, this.defaultPlatform, imageType));
-			Image image = Builder.this.docker.image().pull(reference, this.defaultPlatform, listener, this.authHeader);
+					Builder.this.log.pullingImage(reference, platform, imageType));
+			Image image = Builder.this.docker.image().pull(reference, platform, listener, this.authHeader);
 			Builder.this.log.pulledImage(image, imageType);
-			if (this.defaultPlatform == null) {
-				this.defaultPlatform = ImagePlatform.from(image);
-			}
 			return image;
 		}
 
-		private Image checkPlatformMismatch(Image image, ImageReference imageReference) {
-			if (this.defaultPlatform != null) {
-				ImagePlatform imagePlatform = ImagePlatform.from(image);
-				if (!imagePlatform.equals(this.defaultPlatform)) {
-					throw new PlatformMismatchException(imageReference, this.defaultPlatform, imagePlatform);
+		private Image checkPlatformMismatch(Image image, ImageReference reference, ImagePlatform requestedPlatform) {
+			if (requestedPlatform != null) {
+				ImagePlatform actualPlatform = ImagePlatform.from(image);
+				if (!actualPlatform.equals(requestedPlatform)) {
+					throw new PlatformMismatchException(reference, requestedPlatform, actualPlatform, null);
 				}
 			}
 			return image;
@@ -289,9 +312,9 @@ private Image checkPlatformMismatch(Image image, ImageReference imageReference)
 	private static final class PlatformMismatchException extends RuntimeException {
 
 		private PlatformMismatchException(ImageReference imageReference, ImagePlatform requestedPlatform,
-				ImagePlatform actualPlatform) {
+				ImagePlatform actualPlatform, Throwable cause) {
 			super("Image platform mismatch detected. The configured platform '%s' is not supported by the image '%s'. Requested platform '%s' but got '%s'"
-				.formatted(requestedPlatform, imageReference, requestedPlatform, actualPlatform));
+				.formatted(requestedPlatform, imageReference, requestedPlatform, actualPlatform), cause);
 		}
 
 	}
@@ -303,13 +326,16 @@ private class BuilderResolverContext implements BuildpackResolverContext {
 
 		private final ImageFetcher imageFetcher;
 
+		private final ImagePlatform platform;
+
 		private final BuilderMetadata builderMetadata;
 
 		private final BuildpackLayersMetadata buildpackLayersMetadata;
 
-		BuilderResolverContext(ImageFetcher imageFetcher, BuilderMetadata builderMetadata,
+		BuilderResolverContext(ImageFetcher imageFetcher, ImagePlatform platform, BuilderMetadata builderMetadata,
 				BuildpackLayersMetadata buildpackLayersMetadata) {
 			this.imageFetcher = imageFetcher;
+			this.platform = platform;
 			this.builderMetadata = builderMetadata;
 			this.buildpackLayersMetadata = buildpackLayersMetadata;
 		}
@@ -326,21 +352,13 @@ public BuildpackLayersMetadata getBuildpackLayersMetadata() {
 
 		@Override
 		public Image fetchImage(ImageReference reference, ImageType imageType) throws IOException {
-			return this.imageFetcher.fetchImage(imageType, reference);
+			return this.imageFetcher.fetchImage(imageType, reference, this.platform);
 		}
 
 		@Override
 		public void exportImageLayers(ImageReference reference, IOBiConsumer<String, TarArchive> exports)
 				throws IOException {
-			String digest = Builder.this.docker.image()
-				.resolveManifestDigest(reference, this.imageFetcher.defaultPlatform);
-			if (StringUtils.hasText(digest)) {
-				ImageReference pinned = reference.withDigest(digest);
-				Builder.this.docker.image().exportLayers(pinned, null, exports);
-			}
-			else {
-				Builder.this.docker.image().exportLayers(reference, this.imageFetcher.defaultPlatform, exports);
-			}
+			Builder.this.docker.image().exportLayers(reference, exports);
 		}
 
 	}

spring-boot-project/spring-boot-tools/spring-boot-buildpack-platform/src/main/java/org/springframework/boot/buildpack/platform/docker/DockerApi.java

@@ -68,9 +68,7 @@ public class DockerApi {
 
 	static final ApiVersion PLATFORM_API_VERSION = ApiVersion.of(1, 41);
 
-	static final ApiVersion EXPORT_PLATFORM_API_VERSION = ApiVersion.of(1, 48);
-
-	static final ApiVersion INSPECT_PLATFORM_API_VERSION = ApiVersion.of(1, 49);
+	static final ApiVersion PLATFORM_INSPECT_API_VERSION = ApiVersion.of(1, 49);
 
 	static final ApiVersion UNKNOWN_API_VERSION = ApiVersion.of(0, 0);
 
@@ -239,10 +237,7 @@ public Image pull(ImageReference reference, ImagePlatform platform,
 						listener.onUpdate(event);
 					});
 				}
-				if (platform != null) {
-					return inspect(INSPECT_PLATFORM_API_VERSION, reference, platform);
-				}
-				return inspect(API_VERSION, reference);
+				return inspect(reference, platform);
 			}
 			finally {
 				listener.onFinish();
@@ -339,58 +334,16 @@ public void exportLayerFiles(ImageReference reference, IOBiConsumer<String, Path
 		 */
 		public void exportLayers(ImageReference reference, IOBiConsumer<String, TarArchive> exports)
 				throws IOException {
-			exportLayers(reference, null, exports);
-		}
-
-		/**
-		 * Export the layers of an image as {@link TarArchive TarArchives}.
-		 * @param reference the reference to export
-		 * @param platform the platform (os/architecture/variant) of the image to export
-		 * @param exports a consumer to receive the layers (contents can only be accessed
-		 * during the callback)
-		 * @throws IOException on IO error
-		 */
-		public void exportLayers(ImageReference reference, ImagePlatform platform,
-				IOBiConsumer<String, TarArchive> exports) throws IOException {
 			Assert.notNull(reference, "Reference must not be null");
 			Assert.notNull(exports, "Exports must not be null");
 			URI uri = buildUrl("/images/" + reference + "/get");
-			if (platform != null) {
-				uri = buildUrl(EXPORT_PLATFORM_API_VERSION, "/images/" + reference + "/get", "platform",
-						platform.toJson());
-			}
 			try (Response response = http().get(uri)) {
 				try (ExportedImageTar exportedImageTar = new ExportedImageTar(reference, response.getContent())) {
 					exportedImageTar.exportLayers(exports);
 				}
 			}
 		}
 
-		/**
-		 * Resolve an image manifest digest via Docker inspect. If {@code platform} is
-		 * provided, performs a platform-aware inspect. Preference order:
-		 * {@code Descriptor.digest} then first {@code RepoDigest}. Returns an empty
-		 * string if no digest can be determined.
-		 * @param reference image reference
-		 * @param platform desired platform
-		 * @return resolved digest or empty string
-		 * @throws IOException on IO error
-		 */
-		public String resolveManifestDigest(ImageReference reference, ImagePlatform platform) throws IOException {
-			Assert.notNull(reference, "'reference' must not be null");
-			Image image = inspect(API_VERSION, reference);
-			if (platform != null) {
-				image = inspect(INSPECT_PLATFORM_API_VERSION, reference, platform);
-			}
-			Image.Descriptor descriptor = image.getDescriptor();
-			if (descriptor != null && StringUtils.hasText(descriptor.getDigest())) {
-				return descriptor.getDigest();
-			}
-			List<String> repoDigests = image.getDigests();
-			String digest = repoDigests.isEmpty() ? "" : repoDigests.get(0);
-			return digest.substring(digest.indexOf('@') + 1);
-		}
-
 		/**
 		 * Remove a specific image.
 		 * @param reference the reference the remove
@@ -411,24 +364,36 @@ public void remove(ImageReference reference, boolean force) throws IOException {
 		 * @throws IOException on IO error
 		 */
 		public Image inspect(ImageReference reference) throws IOException {
-			return inspect(API_VERSION, reference);
-		}
-
-		private Image inspect(ApiVersion apiVersion, ImageReference reference) throws IOException {
-			return inspect(apiVersion, reference, null);
+			return inspect(reference, null);
 		}
 
-		private Image inspect(ApiVersion apiVersion, ImageReference reference, ImagePlatform platform)
-				throws IOException {
+		/**
+		 * Inspect an image.
+		 * @param reference the image reference
+		 * @param platform the platform (os/architecture/variant) of the image to inspect.
+		 * Ignored on older versions of Docker.
+		 * @return the image from the local repository
+		 * @throws IOException on IO error
+		 * @since 3.4.12
+		 */
+		public Image inspect(ImageReference reference, ImagePlatform platform) throws IOException {
+			// The Docker documentation is incomplete but platform parameters
+			// are supported since 1.49 (see https://github.com/moby/moby/pull/49586)
 			Assert.notNull(reference, "Reference must not be null");
-			URI imageUri = (platform != null)
-					? buildUrl(apiVersion, "/images/" + reference + "/json", "platform", platform.toJson())
-					: buildUrl(apiVersion, "/images/" + reference + "/json");
-			try (Response response = http().get(imageUri)) {
+			URI inspectUrl = inspectUrl(reference, platform);
+			try (Response response = http().get(inspectUrl)) {
 				return Image.of(response.getContent());
 			}
 		}
 
+		private URI inspectUrl(ImageReference reference, ImagePlatform platform) {
+			String path = "/images/" + reference + "/json";
+			if (platform != null && getApiVersion().supports(PLATFORM_INSPECT_API_VERSION)) {
+				return buildUrl(PLATFORM_INSPECT_API_VERSION, path, "platform", platform.toJson());
+			}
+			return buildUrl(path);
+		}
+
 		public void tag(ImageReference sourceReference, ImageReference targetReference) throws IOException {
 			Assert.notNull(sourceReference, "SourceReference must not be null");
 			Assert.notNull(targetReference, "TargetReference must not be null");