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

Maven War support #1068

Merged
merged 4 commits into from
Oct 4, 2018
Merged

Maven War support #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91c8912
Maven War Support
CyrilleH Sep 29, 2018
4717c65
Fix review comments
CyrilleH Oct 1, 2018
2a82e4d
Fix review comments
CyrilleH Oct 3, 2018
25da9e4
Fix conflict resolution error
CyrilleH Oct 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaLayerConfigurations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ public static Builder builder() {
*/
public static final String DEFAULT_APP_ROOT = "/app";

/**
* The default webapp root in the image. For example, if this is set to {@code
* "/jetty/webapps/ROOT"}, dependency JARs will be in {@code "/jetty/webapps/ROOT/WEB-INF/lib"}.
*/
public static final String DEFAULT_WEB_APP_ROOT = "/jetty/webapps/ROOT";

/** The filename suffix for a maven/gradle snapshot dependency */
public static final String SNAPSHOT_FILENAME_SUFFIX = "SNAPSHOT";

private final ImmutableMap<LayerType, LayerConfiguration> layerConfigurationMap;

private JavaLayerConfigurations(
Expand Down
7 changes: 2 additions & 5 deletions ...dle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/GradleLayerConfigurations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ class GradleLayerConfigurations {
/** Name of the `main` {@link SourceSet} to use as source files. */
private static final String MAIN_SOURCE_SET_NAME = "main";

/** The filename suffix for snapshot dependency JARs. */
private static final String SNAPSHOT = "SNAPSHOT";

/**
* Resolves the {@link JavaLayerConfigurations} for a Gradle {@link Project}.
*
Expand Down Expand Up @@ -116,7 +113,7 @@ private static JavaLayerConfigurations getForNonWarProject(
if (resourcesOutputDirectory.equals(dependencyFile.toPath())) {
continue;
}
if (dependencyFile.getName().contains(SNAPSHOT)) {
if (dependencyFile.getName().contains(JavaLayerConfigurations.SNAPSHOT_FILENAME_SUFFIX)) {
snapshotDependenciesFiles.add(dependencyFile.toPath());
} else {
dependenciesFiles.add(dependencyFile.toPath());
Expand Down Expand Up @@ -183,7 +180,7 @@ private static JavaLayerConfigurations getForWarProject(
try (Stream<Path> dependencyFileStream = Files.list(libOutputDirectory)) {
dependencyFileStream.forEach(
path -> {
if (path.toString().contains(SNAPSHOT)) {
if (path.toString().contains(JavaLayerConfigurations.SNAPSHOT_FILENAME_SUFFIX)) {
snapshotDependenciesFiles.add(path);
} else {
dependenciesFiles.add(path);
Expand Down
10 changes: 3 additions & 7 deletions jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/JibPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ public class JibPlugin implements Plugin<Project> {
@VisibleForTesting static final String DEFAULT_FROM_IMAGE = "gcr.io/distroless/java";
@VisibleForTesting static final String DEFAULT_WAR_FROM_IMAGE = "gcr.io/distroless/java/jetty";

/**
* The default app root in the image for WAR. For example, if this is set to {@code
* /jetty/webapps/ROOT}, dependency JARs will be in {@code /jetty/webapps/ROOT/WEB-INF/lib}.
*/
@VisibleForTesting static final String DEFAULT_WEB_APP_ROOT = "/jetty/webapps/ROOT";

/**
* Collects all project dependencies of the style "compile project(':mylib')" for any kind of
* configuration [compile, runtime, etc]. It potentially will collect common test libraries in
Expand Down Expand Up @@ -129,7 +123,9 @@ public void apply(Project project) {
jibExtension.getFrom().setImage(DEFAULT_WAR_FROM_IMAGE);
}
if (jibExtension.getContainer().getAppRoot().isEmpty()) {
jibExtension.getContainer().setAppRoot(DEFAULT_WEB_APP_ROOT);
jibExtension
.getContainer()
.setAppRoot(JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT);
}
ExplodedWarTask explodedWarTask =
(ExplodedWarTask)
Expand Down
8 changes: 4 additions & 4 deletions ...plugin/src/test/java/com/google/cloud/tools/jib/gradle/GradleLayerConfigurationsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ public void testWebApp_defaultWebAppRoot() throws URISyntaxException, IOExceptio
mockWebAppProject,
mockLogger,
extraFilesDirectory,
AbsoluteUnixPath.get(JibPlugin.DEFAULT_WEB_APP_ROOT));
AbsoluteUnixPath.get(JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT));

assertExtractionPathsUnordered(
Arrays.asList("/jetty/webapps/ROOT/WEB-INF/lib/dependency-1.0.0.jar"),
Expand Down Expand Up @@ -400,7 +400,7 @@ public void testGetForWarProject_noErrorIfWebInfClassesDoesNotExist() throws IOE
mockWebAppProject,
mockLogger,
extraFilesDirectory,
AbsoluteUnixPath.get(JibPlugin.DEFAULT_WEB_APP_ROOT)); // should pass
AbsoluteUnixPath.get(JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT)); // should pass
}

@Test
Expand All @@ -412,7 +412,7 @@ public void testGetForWarProject_noErrorIfWebInfLibDoesNotExist() throws IOExcep
mockWebAppProject,
mockLogger,
extraFilesDirectory,
AbsoluteUnixPath.get(JibPlugin.DEFAULT_WEB_APP_ROOT)); // should pass
AbsoluteUnixPath.get(JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT)); // should pass
}

@Test
Expand All @@ -424,7 +424,7 @@ public void testGetForWarProject_noErrorIfWebInfDoesNotExist() throws IOExceptio
mockWebAppProject,
mockLogger,
extraFilesDirectory,
AbsoluteUnixPath.get(JibPlugin.DEFAULT_WEB_APP_ROOT)); // should pass
AbsoluteUnixPath.get(JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT)); // should pass
}

private void setUpWarProject(Path webAppDirectory) {
Expand Down
2 changes: 1 addition & 1 deletion jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/JibPluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public void testWebAppProject() {
.getFrom()
.getImage());
Assert.assertEquals(
JibPlugin.DEFAULT_WEB_APP_ROOT,
JavaLayerConfigurations.DEFAULT_WEB_APP_ROOT,
((BuildImageTask) rootProject.getTasks().getByPath(JibPlugin.BUILD_IMAGE_TASK_NAME))
.getJib()
.getContainer()
Expand Down
13 changes: 3 additions & 10 deletions jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.frontend.ExposedPortsParser;
import com.google.cloud.tools.jib.frontend.JavaDockerContextGenerator;
import com.google.cloud.tools.jib.frontend.JavaEntrypointConstructor;
import com.google.cloud.tools.jib.global.JibSystemProperties;
import com.google.cloud.tools.jib.plugins.common.HelpfulSuggestions;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -76,22 +75,16 @@ public void execute() throws MojoExecutionException {
MavenProjectProperties mavenProjectProperties =
MavenProjectProperties.getForProject(getProject(), getLog(), getExtraDirectory(), appRoot);

List<String> entrypoint = getEntrypoint();
if (entrypoint.isEmpty()) {
String mainClass = mavenProjectProperties.getMainClass(this);
entrypoint =
JavaEntrypointConstructor.makeDefaultEntrypoint(appRoot, getJvmFlags(), mainClass);
} else if (getMainClass() != null || !getJvmFlags().isEmpty()) {
getLog().warn("<mainClass> and <jvmFlags> are ignored when <entrypoint> is specified");
}
List<String> entrypoint =
PluginConfigurationProcessor.computeEntrypoint(getLog(), this, mavenProjectProperties);

try {
// Validate port input, but don't save the output because we don't want the ranges expanded
// here.
ExposedPortsParser.parse(getExposedPorts());

new JavaDockerContextGenerator(mavenProjectProperties.getJavaLayerConfigurations())
.setBaseImage(getBaseImage())
.setBaseImage(PluginConfigurationProcessor.getBaseImage(this))
.setEntrypoint(entrypoint)
.setJavaArguments(getArgs())
.setExposedPorts(getExposedPorts())
Expand Down
13 changes: 5 additions & 8 deletions jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/JibPluginConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.tools.jib.maven;

import com.google.cloud.tools.jib.frontend.JavaLayerConfigurations;
import com.google.cloud.tools.jib.plugins.common.AuthProperty;
import com.google.cloud.tools.jib.plugins.common.ConfigurationPropertyValidator;
import com.google.cloud.tools.jib.plugins.common.PropertyNames;
Expand Down Expand Up @@ -88,15 +87,12 @@ private void setPropertyDescriptors(String descriptorPrefix) {
}
}

/**
* Configuration for {@code from} parameter, where image by default is {@code
* gcr.io/distroless/java}.
*/
/** Configuration for {@code from} parameter, */
public static class FromConfiguration {

@Nullable
@Parameter(required = true)
private String image = "gcr.io/distroless/java";
private String image;

@Nullable @Parameter private String credHelper;

Expand Down Expand Up @@ -142,7 +138,7 @@ public static class ContainerParameters {

@Parameter private Map<String, String> labels = Collections.emptyMap();

@Parameter private String appRoot = JavaLayerConfigurations.DEFAULT_APP_ROOT;
@Parameter private String appRoot = "";
}

@Nullable
Expand Down Expand Up @@ -203,11 +199,12 @@ MavenProject getProject() {
*
* @return the configured base image reference
*/
@Nullable
chanseokoh marked this conversation as resolved.
Show resolved Hide resolved
String getBaseImage() {
if (System.getProperty(PropertyNames.FROM_IMAGE) != null) {
return System.getProperty(PropertyNames.FROM_IMAGE);
}
return Preconditions.checkNotNull(Preconditions.checkNotNull(from).image);
return Preconditions.checkNotNull(from).image;
}

/**
Expand Down
89 changes: 87 additions & 2 deletions ...maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/MavenLayerConfigurations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@

/** Builds {@link JavaLayerConfigurations} based on inputs from a {@link MavenProject}. */
class MavenLayerConfigurations {

/**
* Resolves the source files configuration for a {@link MavenProject}.
* Resolves the {@link JavaLayerConfigurations} for a {@link MavenProject}.
*
* @param project the {@link MavenProject}
* @param extraDirectory path to the directory for the extra files layer
Expand All @@ -46,6 +45,24 @@ class MavenLayerConfigurations {
*/
static JavaLayerConfigurations getForProject(
MavenProject project, Path extraDirectory, AbsoluteUnixPath appRoot) throws IOException {
if ("war".equals(project.getPackaging())) {
return getForWarProject(project, extraDirectory, appRoot);
} else {
return getForNonWarProject(project, extraDirectory, appRoot);
}
}

/**
* Resolves the source files configuration for a non-war {@link MavenProject}.
*
* @param project the {@link MavenProject}
* @param extraDirectory path to the directory for the extra files layer
* @param appRoot root directory in the image where the app will be placed
* @return a {@link JavaLayerConfigurations} for the project
* @throws IOException if collecting the project files fails
*/
private static JavaLayerConfigurations getForNonWarProject(
MavenProject project, Path extraDirectory, AbsoluteUnixPath appRoot) throws IOException {

AbsoluteUnixPath dependenciesExtractionPath =
appRoot.resolve(JavaEntrypointConstructor.DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE);
Expand Down Expand Up @@ -91,6 +108,74 @@ static JavaLayerConfigurations getForProject(
return layerBuilder.build();
}

/**
* Resolves the source files configuration for a War {@link MavenProject}.
*
* @param project the {@link MavenProject}
* @param extraDirectory path to the directory for the extra files layer
* @param appRoot root directory in the image where the app will be placed
* @return a {@link JavaLayerConfigurations} for the project
* @throws IOException if collecting the project files fails
*/
private static JavaLayerConfigurations getForWarProject(
MavenProject project, Path extraDirectory, AbsoluteUnixPath appRoot) throws IOException {

chanseokoh marked this conversation as resolved.
Show resolved Hide resolved
// TODO explode the WAR file rather than using this directory. The contents of the final WAR may
// be different from this directory (it's possible to include or exclude files when packaging a
// WAR). Also the exploded WAR directory is configurable with <webappDirectory> and may not be
// at build.getFinalName().
Path explodedWarPath =
CyrilleH marked this conversation as resolved.
Show resolved Hide resolved
Paths.get(project.getBuild().getDirectory()).resolve(project.getBuild().getFinalName());
Copy link
Member

@chanseokoh chanseokoh Oct 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Also, I think getFinalName() may not work (admittedly in rare cases), because the exploded WAR directory is configurable with <webappDirectory> in the Maven WAR plugin, although the default is ${project.build.directory}/${project.build.finalName}. For example,

      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-war-plugin</artifactId>
        <version>3.2.2</version>
        <configuration>
          <webappDirectory>${project.build.directory}/my-webapp</webappDirectory>
        </configuration>
      </plugin>
$ ls target
classes/  generated-sources/  helloworld-1.war  maven-archiver/  maven-status/  my-webapp/

But since we will eventually unzip the WAR file, I think it's OK for now to use finalName().

AbsoluteUnixPath dependenciesExtractionPath = appRoot.resolve("WEB-INF/lib/");
AbsoluteUnixPath classesExtractionPath = appRoot.resolve("WEB-INF/classes/");

Builder layerBuilder = JavaLayerConfigurations.builder();

// Gets all the dependencies.
Predicate<Path> isSnapshotDependency =
path -> path.toString().contains(JavaLayerConfigurations.SNAPSHOT_FILENAME_SUFFIX);
if (Files.exists(explodedWarPath.resolve("WEB-INF/lib"))) {
addFilesToLayer(
explodedWarPath.resolve("WEB-INF/lib/"),
isSnapshotDependency,
dependenciesExtractionPath,
layerBuilder::addSnapshotDependencyFile);
addFilesToLayer(
explodedWarPath.resolve("WEB-INF/lib/"),
isSnapshotDependency.negate(),
dependenciesExtractionPath,
layerBuilder::addDependencyFile);
}

// Gets the classes files in the 'WEB-INF/classes' output directory.
Predicate<Path> isClassFile = path -> path.getFileName().toString().endsWith(".class");
if (Files.exists(explodedWarPath.resolve("WEB-INF/classes"))) {
addFilesToLayer(
explodedWarPath.resolve("WEB-INF/classes/"),
isClassFile,
classesExtractionPath,
layerBuilder::addClassFile);
}

// Gets the resources
Predicate<Path> isResources =
path -> {
boolean inWebInfClasses = path.startsWith(explodedWarPath.resolve("WEB-INF/classes/"));
boolean inWebInfLib = path.startsWith(explodedWarPath.resolve("WEB-INF/lib/"));

return (!inWebInfClasses && !inWebInfLib) || (inWebInfClasses && !isClassFile.test(path));
};
addFilesToLayer(explodedWarPath, isResources, appRoot, layerBuilder::addResourceFile);

// Adds all the extra files.
if (Files.exists(extraDirectory)) {
AbsoluteUnixPath extractionBase = AbsoluteUnixPath.get("/");
addFilesToLayer(extraDirectory, path -> true, extractionBase, layerBuilder::addExtraFile);
}

return layerBuilder.build();
}

@FunctionalInterface
@VisibleForTesting
static interface FileToLayerAdder {
Expand Down
2 changes: 1 addition & 1 deletion jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/MavenProjectProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public String getJarPluginName() {

@Override
public boolean isWarProject() {
return false; // TODO: to be implemented. For now, assume false.
return "war".equals(project.getPackaging());
}

/**
Expand Down
Loading