From 263b7c20e4ad4bdc9bbd5f0e68bcd1d706d8a29c Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 16 Mar 2021 17:22:48 -0500 Subject: [PATCH] Fail fast when finalName is misconfigured When the `finalName` parameter is incorrectly set in the Spring Boot Maven plugin configuration instead of in the `build` configuration, the repackaged and original archive files are not named as expected. Prior to this commit, the image building goal would detect this error condition and throw an exception late in the process of creating the build container, leaving the container in an unstable state. This commit changes the image building goal to detect this condition early, before attempting to create the container. Fixes gh-25590 --- .../boot/loader/tools/ImagePackager.java | 19 +++++++---- .../boot/loader/tools/Packager.java | 30 ++++++----------- .../boot/loader/tools/Repackager.java | 13 ++++++-- .../boot/maven/BuildImageTests.java | 9 ++++- .../projects/build-image-final-name/pom.xml | 33 +++++++++++++++++++ .../main/java/org/test/SampleApplication.java | 28 ++++++++++++++++ .../boot/maven/BuildImageMojo.java | 9 ++--- 7 files changed, 107 insertions(+), 34 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/pom.xml create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/src/main/java/org/test/SampleApplication.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/ImagePackager.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/ImagePackager.java index 9553cfc23f..95f4dbd1f3 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/ImagePackager.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/ImagePackager.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import org.springframework.util.Assert; * Utility class that can be used to export a fully packaged archive to an OCI image. * * @author Phillip Webb + * @author Scott Frederick * @since 2.3.0 */ public class ImagePackager extends Packager { @@ -37,11 +38,18 @@ public class ImagePackager extends Packager { * @param source the source file to package */ public ImagePackager(File source) { - super(source, null); + Assert.notNull(source, "Source file must not be null"); + this.source = source.getAbsoluteFile(); + if (isAlreadyPackaged()) { + this.source = getBackupFile(); + } + Assert.isTrue(this.source.exists() && this.source.isFile(), + "Source '" + this.source + "' must refer to an existing file"); + Assert.state(!isAlreadyPackaged(), () -> "Repackaged archive file " + getSource() + " cannot be exported"); } /** - * Create an packaged image. + * Create a packaged image. * @param libraries the contained libraries * @param exporter the exporter used to write the image * @throws IOException on IO error @@ -51,10 +59,7 @@ public class ImagePackager extends Packager { } private void packageImage(Libraries libraries, AbstractJarWriter writer) throws IOException { - File source = isAlreadyPackaged() ? getBackupFile() : getSource(); - Assert.state(source.exists() && source.isFile(), () -> "Unable to read jar file " + source); - Assert.state(!isAlreadyPackaged(source), () -> "Repackaged jar file " + source + " cannot be exported"); - try (JarFile sourceJar = new JarFile(source)) { + try (JarFile sourceJar = new JarFile(getSource())) { write(sourceJar, libraries, writer); } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java index ee76df63f9..e3a8f5c5c9 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java @@ -45,6 +45,7 @@ import org.springframework.util.StringUtils; * @author Andy Wilkinson * @author Stephane Nicoll * @author Madhura Bhave + * @author Scott Frederick * @since 2.3.0 */ public abstract class Packager { @@ -69,15 +70,15 @@ public abstract class Packager { private static final String SPRING_BOOT_APPLICATION_CLASS_NAME = "org.springframework.boot.autoconfigure.SpringBootApplication"; - private List mainClassTimeoutListeners = new ArrayList<>(); + private final List mainClassTimeoutListeners = new ArrayList<>(); private String mainClass; - private final File source; + protected File source; private Layout layout; - private LayoutFactory layoutFactory; + protected LayoutFactory layoutFactory; private Layers layers; @@ -85,19 +86,6 @@ public abstract class Packager { private boolean includeRelevantJarModeJars = true; - /** - * Create a new {@link Packager} instance. - * @param source the source JAR file to package - * @param layoutFactory the layout factory to use or {@code null} - */ - protected Packager(File source, LayoutFactory layoutFactory) { - Assert.notNull(source, "Source file must not be null"); - Assert.isTrue(source.exists() && source.isFile(), - "Source must refer to an existing file, got " + source.getAbsolutePath()); - this.source = source.getAbsoluteFile(); - this.layoutFactory = layoutFactory; - } - /** * Add a listener that will be triggered to display a warning if searching for the * main class takes too long. @@ -153,15 +141,18 @@ public abstract class Packager { this.includeRelevantJarModeJars = includeRelevantJarModeJars; } - protected final boolean isAlreadyPackaged() throws IOException { + protected final boolean isAlreadyPackaged() { return isAlreadyPackaged(this.source); } - protected final boolean isAlreadyPackaged(File file) throws IOException { + protected final boolean isAlreadyPackaged(File file) { try (JarFile jarFile = new JarFile(file)) { Manifest manifest = jarFile.getManifest(); return (manifest != null && manifest.getMainAttributes().getValue(BOOT_VERSION_ATTRIBUTE) != null); } + catch (IOException ex) { + throw new IllegalStateException("Error reading archive file", ex); + } } protected final void write(JarFile sourceJar, Libraries libraries, AbstractJarWriter writer) throws IOException { @@ -285,8 +276,7 @@ public abstract class Packager { * @return the file to use to backup the original source */ public final File getBackupFile() { - File source = getSource(); - return new File(source.getParentFile(), source.getName() + ".original"); + return new File(this.source.getParentFile(), this.source.getName() + ".original"); } protected final File getSource() { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java index f4879363be..e0f5f772f8 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ import org.springframework.util.Assert; * @author Andy Wilkinson * @author Stephane Nicoll * @author Madhura Bhave + * @author Scott Frederick * @since 1.0.0 */ public class Repackager extends Packager { @@ -42,8 +43,16 @@ public class Repackager extends Packager { this(source, null); } + /** + * Create a new {@link Repackager} instance. + * @param source the source archive file to package + * @param layoutFactory the layout factory to use or {@code null} + */ public Repackager(File source, LayoutFactory layoutFactory) { - super(source, layoutFactory); + Assert.notNull(source, "Source file must not be null"); + Assert.isTrue(source.exists() && source.isFile(), "Source '" + source + "' must refer to an existing file"); + this.source = source.getAbsoluteFile(); + this.layoutFactory = layoutFactory; } /** diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java index 28769b1408..54b77c7ffb 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -158,6 +158,13 @@ public class BuildImageTests extends AbstractArchiveIntegrationTests { (project) -> assertThat(buildLog(project)).contains("Executable jar file required for building image")); } + @TestTemplate + void failsWhenFinalNameIsMisconfigured(MavenBuild mavenBuild) { + mavenBuild.project("build-image-final-name").goals("package") + .executeAndFail((project) -> assertThat(buildLog(project)).contains("final-name.jar.original") + .contains("must refer to an existing file")); + } + private void writeLongNameResource(File project) { StringBuilder name = new StringBuilder(); new Random().ints('a', 'z' + 1).limit(128).forEach((i) -> name.append((char) i)); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/pom.xml b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/pom.xml new file mode 100644 index 0000000000..599371978a --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/pom.xml @@ -0,0 +1,33 @@ + + + 4.0.0 + org.springframework.boot.maven.it + build-image-final-name + 0.0.1.BUILD-SNAPSHOT + + UTF-8 + @java.version@ + @java.version@ + + + + + @project.groupId@ + @project.artifactId@ + @project.version@ + + + + repackage + build-image + + + final-name + + + + + + + diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/src/main/java/org/test/SampleApplication.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/src/main/java/org/test/SampleApplication.java new file mode 100644 index 0000000000..27259ff01a --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/src/main/java/org/test/SampleApplication.java @@ -0,0 +1,28 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * 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 + * + * https://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 org.test; + +public class SampleApplication { + + public static void main(String[] args) throws Exception { + System.out.println("Launched"); + synchronized(args) { + args.wait(); // Prevent exit" + } + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java index e977d1e2bb..b69fe05823 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -153,7 +153,8 @@ public class BuildImageMojo extends AbstractPackagerMojo { } private BuildRequest getBuildRequest(Libraries libraries) { - Function content = (owner) -> getApplicationContent(owner, libraries); + ImagePackager imagePackager = new ImagePackager(getJarFile()); + Function content = (owner) -> getApplicationContent(owner, libraries, imagePackager); Image image = (this.image != null) ? this.image : new Image(); if (image.name == null && this.imageName != null) { image.setName(this.imageName); @@ -167,8 +168,8 @@ public class BuildImageMojo extends AbstractPackagerMojo { return customize(image.getBuildRequest(this.project.getArtifact(), content)); } - private TarArchive getApplicationContent(Owner owner, Libraries libraries) { - ImagePackager packager = getConfiguredPackager(() -> new ImagePackager(getJarFile())); + private TarArchive getApplicationContent(Owner owner, Libraries libraries, ImagePackager imagePackager) { + ImagePackager packager = getConfiguredPackager(() -> imagePackager); return new PackagedTarArchive(owner, libraries, packager); }