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.

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

Use AbsoluteUnixPath for container.appRoot propagation #1012

Merged
merged 40 commits into from
Sep 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
520222a
Add container.appRoot config parameter
chanseokoh Sep 13, 2018
971b95d
Merge branch 'master' into i964-container.appRoot
chanseokoh Sep 13, 2018
5b16ded
Change appRoot type from String to Path
chanseokoh Sep 14, 2018
5a7f486
List digests as Set
chanseokoh Sep 14, 2018
5024da9
Merge branch 'i985-list-cache-test-failure' into i964-container.appRoot
chanseokoh Sep 14, 2018
ee6fe2c
Add tests
chanseokoh Sep 14, 2018
d568f6e
listDigests --> fetchDigests
chanseokoh Sep 14, 2018
886d71d
Merge remote-tracking branch 'origin/i985-list-cache-test-failure' in…
chanseokoh Sep 14, 2018
d86d749
Fix typo
chanseokoh Sep 14, 2018
85c6132
Merge branch 'i985-list-cache-test-failure' into i964-container.appRoot
chanseokoh Sep 14, 2018
4ef4148
Add tests
chanseokoh Sep 14, 2018
a1ba423
Add tests
chanseokoh Sep 14, 2018
12face5
Add tests
chanseokoh Sep 14, 2018
93b42a7
Update Javadocs and copyright
chanseokoh Sep 14, 2018
35a64f6
Merge remote-tracking branch 'origin/master' into i964-container.appRoot
chanseokoh Sep 14, 2018
c16531d
Update CHANGLOG and README
chanseokoh Sep 14, 2018
a978e4c
Fix test
chanseokoh Sep 14, 2018
6330547
Set appRoot for JavaLayerConfigurations
chanseokoh Sep 14, 2018
0e8c597
Check appRoot early
chanseokoh Sep 14, 2018
6489596
Various fixes
chanseokoh Sep 14, 2018
713c673
Add tests
chanseokoh Sep 14, 2018
6dd49a9
Do not update docs yet
chanseokoh Sep 14, 2018
984f51c
Merge branch 'master' into i964-container.appRoot
chanseokoh Sep 17, 2018
c70f5a3
feedback
chanseokoh Sep 17, 2018
3b7c510
Accept default extraction path along with files
chanseokoh Sep 17, 2018
21bec72
Clean up
chanseokoh Sep 17, 2018
4d5154d
format
chanseokoh Sep 17, 2018
becad9c
Merge branch 'master' into i964-container.appRoot
chanseokoh Sep 17, 2018
e699be8
Move field / rename methods
chanseokoh Sep 18, 2018
99c7d1d
Rename argument
chanseokoh Sep 18, 2018
2d74e9d
Update Javadoc
chanseokoh Sep 18, 2018
28ac010
Fix typo
chanseokoh Sep 19, 2018
566561f
Fix typo
chanseokoh Sep 19, 2018
f46ba73
Merge branch 'master' into i964-container.appRoot
chanseokoh Sep 19, 2018
9cf86fc
Migrate to use AbsoluteUnixPath
chanseokoh Sep 19, 2018
83731c5
Improve readability
chanseokoh Sep 19, 2018
10c4493
Merge branch 'master' into AbsoluteUnixPath-for-appRoot
chanseokoh Sep 19, 2018
60223ca
Merge branch 'master' into AbsoluteUnixPath-for-appRoot
chanseokoh Sep 20, 2018
d90b3ed
Add trailing slash for Dockerfile
chanseokoh Sep 20, 2018
54e2a26
Restore original code
chanseokoh Sep 20, 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
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.tools.jib.configuration.ContainerConfiguration;
import com.google.cloud.tools.jib.configuration.ImageConfiguration;
import com.google.cloud.tools.jib.configuration.LayerConfiguration;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.frontend.ExposedPortsParser;
import com.google.cloud.tools.jib.frontend.JavaEntrypointConstructor;
import com.google.cloud.tools.jib.image.ImageReference;
Expand Down Expand Up @@ -286,7 +287,7 @@ private BuildConfiguration.Builder getBuildConfigurationBuilder(
ContainerConfiguration.builder()
.setEntrypoint(
JavaEntrypointConstructor.makeDefaultEntrypoint(
"/app", Collections.emptyList(), "HelloWorld"))
AbsoluteUnixPath.get("/app"), Collections.emptyList(), "HelloWorld"))
.setProgramArguments(Collections.singletonList("An argument."))
.setEnvironment(ImmutableMap.of("env1", "envvalue1", "env2", "envvalue2"))
.setExposedPorts(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.image.LayerEntry;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -74,10 +75,12 @@ private static class CopyDirective {
private final String directoryInContext;

/** The extraction path in the image. */
private final String extractionPath;
private final AbsoluteUnixPath extractionPath;

private CopyDirective(
ImmutableList<LayerEntry> layerEntries, String directoryInContext, String extractionPath) {
ImmutableList<LayerEntry> layerEntries,
String directoryInContext,
AbsoluteUnixPath extractionPath) {
this.layerEntries = layerEntries;
this.directoryInContext = directoryInContext;
this.extractionPath = extractionPath;
Expand All @@ -96,7 +99,7 @@ private static void addIfNotEmpty(
ImmutableList.Builder<CopyDirective> listBuilder,
ImmutableList<LayerEntry> layerEntries,
String directoryInContext,
String extractionPath) {
AbsoluteUnixPath extractionPath) {
if (layerEntries.isEmpty()) {
return;
}
Expand Down Expand Up @@ -282,9 +285,9 @@ public void generate(Path targetDirectory) throws IOException {
// 'baseExtractionPath' of '/app/classes', and an 'actualExtractionPath' of
// '/app/classes/com/test/HelloWorld.class', the resolved destination would be
// 'target/jib-docker-context/classes/com/test/HelloWorld.class'.
Path destination =
directoryInContext.resolve(
Paths.get(copyDirective.extractionPath).relativize(layerEntry.getExtractionPath()));
Path baseExtractionPath = Paths.get(copyDirective.extractionPath.toString());
Path relativeEntryPath = baseExtractionPath.relativize(layerEntry.getExtractionPath());
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe we can use copyDirective.extractionPath directly and add a relativize method to AbsoluteUnixPath after we change LayerEntry#getExtractionPath to return an AbsoluteUnixPath.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we can think about this after #1014 (or in #1014).

Path destination = directoryInContext.resolve(relativeEntryPath);

if (Files.isDirectory(layerEntry.getSourceFile())) {
Files.createDirectories(destination);
Expand All @@ -305,11 +308,11 @@ public void generate(Path targetDirectory) throws IOException {
* <pre>{@code
* FROM [base image]
*
* COPY libs [path/to/dependencies]
* COPY snapshot-libs [path/to/dependencies]
* COPY resources [path/to/resources]
* COPY classes [path/to/classes]
* COPY root [path/to/classes]
* COPY libs [path/to/dependencies/]
* COPY snapshot-libs [path/to/dependencies/]
* COPY resources [path/to/resources/]
* COPY classes [path/to/classes/]
* COPY root [path/to/root/]
*
* EXPOSE [port]
* [More EXPOSE instructions, if necessary]
Expand All @@ -330,11 +333,13 @@ String makeDockerfile() throws JsonProcessingException {
StringBuilder dockerfile = new StringBuilder();
dockerfile.append("FROM ").append(Preconditions.checkNotNull(baseImage)).append("\n");
for (CopyDirective copyDirective : copyDirectives) {
boolean hasTrailingSlash = copyDirective.extractionPath.toString().endsWith("/");
dockerfile
.append("\nCOPY ")
.append(copyDirective.directoryInContext)
.append(" ")
.append(copyDirective.extractionPath);
.append(copyDirective.extractionPath)
.append(hasTrailingSlash ? "" : "/");
}

dockerfile.append("\n");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,32 @@

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

import com.google.common.base.Preconditions;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.filesystem.RelativeUnixPath;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

/** Constructs an image entrypoint for the Java application. */
public class JavaEntrypointConstructor {

public static final String DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE = "resources/";
public static final String DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE = "classes/";
public static final String DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE = "libs/";
public static final RelativeUnixPath DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE =
RelativeUnixPath.get("resources");
public static final RelativeUnixPath DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE =
RelativeUnixPath.get("classes");
public static final RelativeUnixPath DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE =
RelativeUnixPath.get("libs");

public static List<String> makeDefaultEntrypoint(
String appRoot, List<String> jvmFlags, String mainClass) {
Preconditions.checkArgument(
appRoot.startsWith("/"), "appRoot should be an absolute path in Unix-style: " + appRoot);
appRoot = appRoot.endsWith("/") ? appRoot : appRoot + '/';

AbsoluteUnixPath appRoot, List<String> jvmFlags, String mainClass) {
return makeEntrypoint(
Arrays.asList(
appRoot + DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE,
appRoot + DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE,
appRoot + DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE + "*"),
appRoot.resolve(DEFAULT_RELATIVE_RESOURCES_PATH_ON_IMAGE).toString(),
appRoot.resolve(DEFAULT_RELATIVE_CLASSES_PATH_ON_IMAGE).toString(),
appRoot
.resolve(DEFAULT_RELATIVE_DEPENDENCIES_PATH_ON_IMAGE)
.resolve(RelativeUnixPath.get("*"))
.toString()),
jvmFlags,
mainClass);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.tools.jib.frontend;

import com.google.cloud.tools.jib.configuration.LayerConfiguration;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.image.LayerEntry;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -66,49 +67,51 @@ String getName() {
public static class Builder {

private final Map<LayerType, List<Path>> layerFilesMap = new EnumMap<>(LayerType.class);
private final Map<LayerType, String> extractionPathMap = new EnumMap<>(LayerType.class);
private final Map<LayerType, AbsoluteUnixPath> extractionPathMap =
new EnumMap<>(LayerType.class);

private Builder() {
for (LayerType layerType : LayerType.values()) {
layerFilesMap.put(layerType, new ArrayList<>());
extractionPathMap.put(layerType, "/");
extractionPathMap.put(layerType, AbsoluteUnixPath.get("/"));
}
}

public Builder setDependencyFiles(List<Path> dependencyFiles, String extractionPath) {
public Builder setDependencyFiles(List<Path> dependencyFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.DEPENDENCIES, dependencyFiles);
extractionPathMap.put(LayerType.DEPENDENCIES, extractionPath);
return this;
}

public Builder setSnapshotDependencyFiles(
List<Path> snapshotDependencyFiles, String extractionPath) {
List<Path> snapshotDependencyFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.SNAPSHOT_DEPENDENCIES, snapshotDependencyFiles);
extractionPathMap.put(LayerType.SNAPSHOT_DEPENDENCIES, extractionPath);
return this;
}

public Builder setResourceFiles(List<Path> resourceFiles, String extractionPath) {
public Builder setResourceFiles(List<Path> resourceFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.RESOURCES, resourceFiles);
extractionPathMap.put(LayerType.RESOURCES, extractionPath);
return this;
}

public Builder setClassFiles(List<Path> classFiles, String extractionPath) {
public Builder setClassFiles(List<Path> classFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.CLASSES, classFiles);
extractionPathMap.put(LayerType.CLASSES, extractionPath);
return this;
}

public Builder setExtraFiles(List<Path> extraFiles, String extractionPath) {
public Builder setExtraFiles(List<Path> extraFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.EXTRA_FILES, extraFiles);
extractionPathMap.put(LayerType.EXTRA_FILES, extractionPath);
return this;
}

// TODO: remove this and put files in WAR into the relevant layers (i.e., dependencies, snapshot
// dependencies, resources, and classes layers).
public Builder setExplodedWarFiles(List<Path> explodedWarFiles, String extractionPath) {
public Builder setExplodedWarFiles(
List<Path> explodedWarFiles, AbsoluteUnixPath extractionPath) {
layerFilesMap.put(LayerType.EXPLODED_WAR, explodedWarFiles);
extractionPathMap.put(LayerType.EXPLODED_WAR, extractionPath);
return this;
Expand All @@ -118,19 +121,17 @@ public JavaLayerConfigurations build() throws IOException {
ImmutableMap.Builder<LayerType, LayerConfiguration> layerConfigurationsMap =
ImmutableMap.builderWithExpectedSize(LayerType.values().length);
for (LayerType layerType : LayerType.values()) {
String extractionPath = Preconditions.checkNotNull(extractionPathMap.get(layerType));
// Windows filenames cannot have "/", so this also blocks Windows-style path.
Preconditions.checkState(
extractionPath.startsWith("/"),
"extractionPath should be an absolute path in Unix-style: " + extractionPath);
AbsoluteUnixPath extractionPath =
Preconditions.checkNotNull(extractionPathMap.get(layerType));

LayerConfiguration.Builder layerConfigurationBuilder =
LayerConfiguration.builder().setName(layerType.getName());

// Adds all the layer files recursively.
List<Path> layerFiles = Preconditions.checkNotNull(layerFilesMap.get(layerType));
for (Path layerFile : layerFiles) {
Path pathInContainer = Paths.get(extractionPath).resolve(layerFile.getFileName());
Path pathInContainer =
Paths.get(extractionPath.toString()).resolve(layerFile.getFileName());
layerConfigurationBuilder.addEntryRecursive(layerFile, pathInContainer);
}

Expand All @@ -152,11 +153,11 @@ public static Builder builder() {
public static final String DEFAULT_APP_ROOT = "/app";

private final ImmutableMap<LayerType, LayerConfiguration> layerConfigurationMap;
private final ImmutableMap<LayerType, String> extractionPathMap;
private final ImmutableMap<LayerType, AbsoluteUnixPath> extractionPathMap;

private JavaLayerConfigurations(
ImmutableMap<LayerType, LayerConfiguration> layerConfigurationMap,
ImmutableMap<LayerType, String> extractionPathMap) {
ImmutableMap<LayerType, AbsoluteUnixPath> extractionPathMap) {
this.layerConfigurationMap = layerConfigurationMap;
this.extractionPathMap = extractionPathMap;
}
Expand Down Expand Up @@ -191,35 +192,35 @@ public ImmutableList<LayerEntry> getExplodedWarEntries() {
return getLayerEntries(LayerType.EXPLODED_WAR);
}

public String getDependencyExtractionPath() {
public AbsoluteUnixPath getDependencyExtractionPath() {
return getExtractionPath(LayerType.DEPENDENCIES);
}

public String getSnapshotDependencyExtractionPath() {
public AbsoluteUnixPath getSnapshotDependencyExtractionPath() {
return getExtractionPath(LayerType.SNAPSHOT_DEPENDENCIES);
}

public String getResourceExtractionPath() {
public AbsoluteUnixPath getResourceExtractionPath() {
return getExtractionPath(LayerType.RESOURCES);
}

public String getClassExtractionPath() {
public AbsoluteUnixPath getClassExtractionPath() {
return getExtractionPath(LayerType.CLASSES);
}

public String getExtraFilesExtractionPath() {
public AbsoluteUnixPath getExtraFilesExtractionPath() {
return getExtractionPath(LayerType.EXTRA_FILES);
}

public String getExplodedWarExtractionPath() {
public AbsoluteUnixPath getExplodedWarExtractionPath() {
return getExtractionPath(LayerType.EXPLODED_WAR);
}

private ImmutableList<LayerEntry> getLayerEntries(LayerType layerType) {
return Preconditions.checkNotNull(layerConfigurationMap.get(layerType)).getLayerEntries();
}

private String getExtractionPath(LayerType layerType) {
private AbsoluteUnixPath getExtractionPath(LayerType layerType) {
return Preconditions.checkNotNull(extractionPathMap.get(layerType));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

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

import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.filesystem.DirectoryWalker;
import com.google.cloud.tools.jib.image.LayerEntry;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -116,16 +117,17 @@ public void testGenerate() throws IOException, URISyntaxException {
.thenReturn(filesToLayerEntries(testDependencies, Paths.get(EXPECTED_DEPENDENCIES_PATH)));

Mockito.when(mockJavaLayerConfigurations.getDependencyExtractionPath())
.thenReturn(EXPECTED_DEPENDENCIES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_DEPENDENCIES_PATH));
Mockito.when(mockJavaLayerConfigurations.getSnapshotDependencyExtractionPath())
.thenReturn(EXPECTED_DEPENDENCIES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_DEPENDENCIES_PATH));
Mockito.when(mockJavaLayerConfigurations.getResourceExtractionPath())
.thenReturn(EXPECTED_RESOURCES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_RESOURCES_PATH));
Mockito.when(mockJavaLayerConfigurations.getClassExtractionPath())
.thenReturn(EXPECTED_CLASSES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_CLASSES_PATH));
Mockito.when(mockJavaLayerConfigurations.getExplodedWarExtractionPath())
.thenReturn(EXPECTED_EXPLODED_WAR_PATH);
Mockito.when(mockJavaLayerConfigurations.getExtraFilesExtractionPath()).thenReturn("/");
.thenReturn(AbsoluteUnixPath.get(EXPECTED_EXPLODED_WAR_PATH));
Mockito.when(mockJavaLayerConfigurations.getExtraFilesExtractionPath())
.thenReturn(AbsoluteUnixPath.get("/"));

new JavaDockerContextGenerator(mockJavaLayerConfigurations)
.setBaseImage("somebaseimage")
Expand Down Expand Up @@ -175,23 +177,24 @@ public void testMakeDockerfile() throws IOException {
.thenReturn(ImmutableList.of(new LayerEntry(ignored, Paths.get("/"))));

Mockito.when(mockJavaLayerConfigurations.getDependencyExtractionPath())
.thenReturn(EXPECTED_DEPENDENCIES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_DEPENDENCIES_PATH));
Mockito.when(mockJavaLayerConfigurations.getSnapshotDependencyExtractionPath())
.thenReturn(EXPECTED_DEPENDENCIES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_DEPENDENCIES_PATH));
Mockito.when(mockJavaLayerConfigurations.getResourceExtractionPath())
.thenReturn(EXPECTED_RESOURCES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_RESOURCES_PATH));
Mockito.when(mockJavaLayerConfigurations.getClassExtractionPath())
.thenReturn(EXPECTED_CLASSES_PATH);
.thenReturn(AbsoluteUnixPath.get(EXPECTED_CLASSES_PATH));
Mockito.when(mockJavaLayerConfigurations.getExplodedWarExtractionPath())
.thenReturn(EXPECTED_EXPLODED_WAR_PATH);
Mockito.when(mockJavaLayerConfigurations.getExtraFilesExtractionPath()).thenReturn("/");
.thenReturn(AbsoluteUnixPath.get(EXPECTED_EXPLODED_WAR_PATH));
Mockito.when(mockJavaLayerConfigurations.getExtraFilesExtractionPath())
.thenReturn(AbsoluteUnixPath.get("/"));

String dockerfile =
new JavaDockerContextGenerator(mockJavaLayerConfigurations)
.setBaseImage(expectedBaseImage)
.setEntrypoint(
JavaEntrypointConstructor.makeDefaultEntrypoint(
"/app", expectedJvmFlags, expectedMainClass))
AbsoluteUnixPath.get("/app"), expectedJvmFlags, expectedMainClass))
.setJavaArguments(expectedJavaArguments)
.setEnvironment(expectedEnv)
.setExposedPorts(exposedPorts)
Expand Down
Loading