Skip to content

Commit 263b7c2

Browse files
scottfrederickscottfrederick
committed
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
1 parent f823bbb commit 263b7c2

File tree

7 files changed

+107
-34
lines changed

7 files changed

+107
-34
lines changed

‎spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/ImagePackager.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -28,6 +28,7 @@
2828
* Utility class that can be used to export a fully packaged archive to an OCI image.
2929
*
3030
* @author Phillip Webb
31+
* @author Scott Frederick
3132
* @since 2.3.0
3233
*/
3334
public class ImagePackager extends Packager {
@@ -37,11 +38,18 @@ public class ImagePackager extends Packager {
3738
* @param source the source file to package
3839
*/
3940
public ImagePackager(File source) {
40-
super(source, null);
41+
Assert.notNull(source, "Source file must not be null");
42+
this.source = source.getAbsoluteFile();
43+
if (isAlreadyPackaged()) {
44+
this.source = getBackupFile();
45+
}
46+
Assert.isTrue(this.source.exists() && this.source.isFile(),
47+
"Source '" + this.source + "' must refer to an existing file");
48+
Assert.state(!isAlreadyPackaged(), () -> "Repackaged archive file " + getSource() + " cannot be exported");
4149
}
4250

4351
/**
44-
* Create an packaged image.
52+
* Create a packaged image.
4553
* @param libraries the contained libraries
4654
* @param exporter the exporter used to write the image
4755
* @throws IOException on IO error
@@ -51,10 +59,7 @@ public void packageImage(Libraries libraries, BiConsumer<ZipEntry, EntryWriter>
5159
}
5260

5361
private void packageImage(Libraries libraries, AbstractJarWriter writer) throws IOException {
54-
File source = isAlreadyPackaged() ? getBackupFile() : getSource();
55-
Assert.state(source.exists() && source.isFile(), () -> "Unable to read jar file " + source);
56-
Assert.state(!isAlreadyPackaged(source), () -> "Repackaged jar file " + source + " cannot be exported");
57-
try (JarFile sourceJar = new JarFile(source)) {
62+
try (JarFile sourceJar = new JarFile(getSource())) {
5863
write(sourceJar, libraries, writer);
5964
}
6065
}

‎spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* @author Andy Wilkinson
4646
* @author Stephane Nicoll
4747
* @author Madhura Bhave
48+
* @author Scott Frederick
4849
* @since 2.3.0
4950
*/
5051
public abstract class Packager {
@@ -69,35 +70,22 @@ public abstract class Packager {
6970

7071
private static final String SPRING_BOOT_APPLICATION_CLASS_NAME = "org.springframework.boot.autoconfigure.SpringBootApplication";
7172

72-
private List<MainClassTimeoutWarningListener> mainClassTimeoutListeners = new ArrayList<>();
73+
private final List<MainClassTimeoutWarningListener> mainClassTimeoutListeners = new ArrayList<>();
7374

7475
private String mainClass;
7576

76-
private final File source;
77+
protected File source;
7778

7879
private Layout layout;
7980

80-
private LayoutFactory layoutFactory;
81+
protected LayoutFactory layoutFactory;
8182

8283
private Layers layers;
8384

8485
private LayersIndex layersIndex;
8586

8687
private boolean includeRelevantJarModeJars = true;
8788

88-
/**
89-
* Create a new {@link Packager} instance.
90-
* @param source the source JAR file to package
91-
* @param layoutFactory the layout factory to use or {@code null}
92-
*/
93-
protected Packager(File source, LayoutFactory layoutFactory) {
94-
Assert.notNull(source, "Source file must not be null");
95-
Assert.isTrue(source.exists() && source.isFile(),
96-
"Source must refer to an existing file, got " + source.getAbsolutePath());
97-
this.source = source.getAbsoluteFile();
98-
this.layoutFactory = layoutFactory;
99-
}
100-
10189
/**
10290
* Add a listener that will be triggered to display a warning if searching for the
10391
* main class takes too long.
@@ -153,15 +141,18 @@ public void setIncludeRelevantJarModeJars(boolean includeRelevantJarModeJars) {
153141
this.includeRelevantJarModeJars = includeRelevantJarModeJars;
154142
}
155143

156-
protected final boolean isAlreadyPackaged() throws IOException {
144+
protected final boolean isAlreadyPackaged() {
157145
return isAlreadyPackaged(this.source);
158146
}
159147

160-
protected final boolean isAlreadyPackaged(File file) throws IOException {
148+
protected final boolean isAlreadyPackaged(File file) {
161149
try (JarFile jarFile = new JarFile(file)) {
162150
Manifest manifest = jarFile.getManifest();
163151
return (manifest != null && manifest.getMainAttributes().getValue(BOOT_VERSION_ATTRIBUTE) != null);
164152
}
153+
catch (IOException ex) {
154+
throw new IllegalStateException("Error reading archive file", ex);
155+
}
165156
}
166157

167158
protected final void write(JarFile sourceJar, Libraries libraries, AbstractJarWriter writer) throws IOException {
@@ -285,8 +276,7 @@ protected String findMainMethod(JarFile source) throws IOException {
285276
* @return the file to use to backup the original source
286277
*/
287278
public final File getBackupFile() {
288-
File source = getSource();
289-
return new File(source.getParentFile(), source.getName() + ".original");
279+
return new File(this.source.getParentFile(), this.source.getName() + ".original");
290280
}
291281

292282
protected final File getSource() {

‎spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -32,6 +32,7 @@
3232
* @author Andy Wilkinson
3333
* @author Stephane Nicoll
3434
* @author Madhura Bhave
35+
* @author Scott Frederick
3536
* @since 1.0.0
3637
*/
3738
public class Repackager extends Packager {
@@ -42,8 +43,16 @@ public Repackager(File source) {
4243
this(source, null);
4344
}
4445

46+
/**
47+
* Create a new {@link Repackager} instance.
48+
* @param source the source archive file to package
49+
* @param layoutFactory the layout factory to use or {@code null}
50+
*/
4551
public Repackager(File source, LayoutFactory layoutFactory) {
46-
super(source, layoutFactory);
52+
Assert.notNull(source, "Source file must not be null");
53+
Assert.isTrue(source.exists() && source.isFile(), "Source '" + source + "' must refer to an existing file");
54+
this.source = source.getAbsoluteFile();
55+
this.layoutFactory = layoutFactory;
4756
}
4857

4958
/**

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/java/org/springframework/boot/maven/BuildImageTests.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -158,6 +158,13 @@ void failsWithWarPackaging(MavenBuild mavenBuild) {
158158
(project) -> assertThat(buildLog(project)).contains("Executable jar file required for building image"));
159159
}
160160

161+
@TestTemplate
162+
void failsWhenFinalNameIsMisconfigured(MavenBuild mavenBuild) {
163+
mavenBuild.project("build-image-final-name").goals("package")
164+
.executeAndFail((project) -> assertThat(buildLog(project)).contains("final-name.jar.original")
165+
.contains("must refer to an existing file"));
166+
}
167+
161168
private void writeLongNameResource(File project) {
162169
StringBuilder name = new StringBuilder();
163170
new Random().ints('a', 'z' + 1).limit(128).forEach((i) -> name.append((char) i));

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/pom.xml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
4+
<modelVersion>4.0.0</modelVersion>
5+
<groupId>org.springframework.boot.maven.it</groupId>
6+
<artifactId>build-image-final-name</artifactId>
7+
<version>0.0.1.BUILD-SNAPSHOT</version>
8+
<properties>
9+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
10+
<maven.compiler.source>@java.version@</maven.compiler.source>
11+
<maven.compiler.target>@java.version@</maven.compiler.target>
12+
</properties>
13+
<build>
14+
<plugins>
15+
<plugin>
16+
<groupId>@project.groupId@</groupId>
17+
<artifactId>@project.artifactId@</artifactId>
18+
<version>@project.version@</version>
19+
<executions>
20+
<execution>
21+
<goals>
22+
<goal>repackage</goal>
23+
<goal>build-image</goal>
24+
</goals>
25+
<configuration>
26+
<finalName>final-name</finalName>
27+
</configuration>
28+
</execution>
29+
</executions>
30+
</plugin>
31+
</plugins>
32+
</build>
33+
</project>

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/intTest/projects/build-image-final-name/src/main/java/org/test/SampleApplication.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright 2012-2020 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.test;
18+
19+
public class SampleApplication {
20+
21+
public static void main(String[] args) throws Exception {
22+
System.out.println("Launched");
23+
synchronized(args) {
24+
args.wait(); // Prevent exit"
25+
}
26+
}
27+
28+
}

‎spring-boot-project/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/BuildImageMojo.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2020 the original author or authors.
2+
* Copyright 2012-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -153,7 +153,8 @@ private void buildImage() throws MojoExecutionException {
153153
}
154154

155155
private BuildRequest getBuildRequest(Libraries libraries) {
156-
Function<Owner, TarArchive> content = (owner) -> getApplicationContent(owner, libraries);
156+
ImagePackager imagePackager = new ImagePackager(getJarFile());
157+
Function<Owner, TarArchive> content = (owner) -> getApplicationContent(owner, libraries, imagePackager);
157158
Image image = (this.image != null) ? this.image : new Image();
158159
if (image.name == null && this.imageName != null) {
159160
image.setName(this.imageName);
@@ -167,8 +168,8 @@ private BuildRequest getBuildRequest(Libraries libraries) {
167168
return customize(image.getBuildRequest(this.project.getArtifact(), content));
168169
}
169170

170-
private TarArchive getApplicationContent(Owner owner, Libraries libraries) {
171-
ImagePackager packager = getConfiguredPackager(() -> new ImagePackager(getJarFile()));
171+
private TarArchive getApplicationContent(Owner owner, Libraries libraries, ImagePackager imagePackager) {
172+
ImagePackager packager = getConfiguredPackager(() -> imagePackager);
172173
return new PackagedTarArchive(owner, libraries, packager);
173174
}
174175

0 commit comments

Comments
 (0)