Skip to content

Commit

Permalink
Add option to expand wildcard in entrypoint runtime classpath (#2866)
Browse files Browse the repository at this point in the history
* Expand wildcard in entrypoint runtime classpath
* Update CHANGELOG
* Update and migrate system property
* Rename config to "expandClasspathDependencies" and flip default
* Document potential failure in CHANGELOG
  • Loading branch information
chanseokoh authored Nov 3, 2020
1 parent a720aa1 commit f0123a8
Show file tree
Hide file tree
Showing 21 changed files with 233 additions and 32 deletions.
2 changes: 1 addition & 1 deletion jib-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ All notable changes to this project will be documented in this file.

### Fixed

- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/pull/2819))
- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/issues/2819))
- Fixed build failure when using a Docker daemon base image (`docker://...`) that has duplicate layers. ([#2829](https://github.com/GoogleContainerTools/jib/issues/2829))

## 0.16.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.image.LayerPropertyNotFoundException;
import com.google.cloud.tools.jib.image.json.HistoryEntry;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.UnmodifiableIterator;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import javax.annotation.Nullable;
Expand All @@ -35,6 +38,26 @@ class BuildImageStep implements Callable<Image> {

private static final String DESCRIPTION = "Building container configuration";

@VisibleForTesting
static String truncateLongClasspath(ImmutableList<String> imageEntrypoint) {
List<String> truncated = new ArrayList<>();
UnmodifiableIterator<String> iterator = imageEntrypoint.iterator();
while (iterator.hasNext()) {
String element = iterator.next();
truncated.add(element);

if (element.equals("-cp") || element.equals("-classpath")) {
String classpath = iterator.next();
if (classpath.length() > 200) {
truncated.add(classpath.substring(0, 200) + "<... classpath truncated ...>");
} else {
truncated.add(classpath);
}
}
}
return truncated.toString();
}

private final BuildContext buildContext;
private final ProgressEventDispatcher.Factory progressEventDispatcherFactory;
private final Image baseImage;
Expand Down Expand Up @@ -148,10 +171,15 @@ private ImmutableList<String> computeEntrypoint(
shouldInherit ? baseImage.getEntrypoint() : containerConfiguration.getEntrypoint();

if (entrypointToUse != null) {
String logSuffix = shouldInherit ? " (inherited from base image)" : "";
String message = "Container entrypoint set to " + entrypointToUse + logSuffix;
buildContext.getEventHandlers().dispatch(LogEvent.lifecycle(""));
buildContext.getEventHandlers().dispatch(LogEvent.lifecycle(message));
if (shouldInherit) {
String message =
"Container entrypoint set to " + entrypointToUse + " (inherited from base image)";
buildContext.getEventHandlers().dispatch(LogEvent.lifecycle(message));
} else {
String message = "Container entrypoint set to " + truncateLongClasspath(entrypointToUse);
buildContext.getEventHandlers().dispatch(LogEvent.lifecycle(message));
}
}

return entrypointToUse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
public class JibSystemProperties {

public static final String UPSTREAM_CLIENT = "_JIB_UPSTREAM_CLIENT";
private static final String DISABLE_USER_AGENT = "_JIB_DISABLE_USER_AGENT";

@VisibleForTesting public static final String HTTP_TIMEOUT = "jib.httpTimeout";

Expand All @@ -32,8 +33,6 @@ public class JibSystemProperties {
public static final String SEND_CREDENTIALS_OVER_HTTP = "sendCredentialsOverHttp";
public static final String SERIALIZE = "jib.serialize";

private static final String DISABLE_USER_AGENT = "_JIB_DISABLE_USER_AGENT";

@VisibleForTesting public static final String SKIP_EXISTING_IMAGES = "jib.skipExistingImages";

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,4 +370,33 @@ public void test_generateHistoryObjects() {
// Should be exactly 9 total
Assert.assertEquals(9, image.getHistory().size());
}

@Test
public void testTruncateLongClasspath_shortClasspath() {
ImmutableList<String> entrypoint =
ImmutableList.of(
"java", "-Dmy-property=value", "-cp", "/app/classes:/app/libs/*", "com.example.Main");

Assert.assertEquals(
"[java, -Dmy-property=value, -cp, /app/classes:/app/libs/*, com.example.Main]",
BuildImageStep.truncateLongClasspath(entrypoint));
}

@Test
public void testTruncateLongClasspath_longClasspath() {
String classpath =
"/app/resources:/app/classes:/app/libs/spring-boot-starter-web-2.0.3.RELEASE.jar:/app/libs/"
+ "shared-library-0.1.0.jar:/app/libs/spring-boot-starter-json-2.0.3.RELEASE.jar:/app/"
+ "libs/spring-boot-starter-2.0.3.RELEASE.jar:/app/libs/spring-boot-starter-tomcat-2.0."
+ "3.RELEASE.jar";
ImmutableList<String> entrypoint =
ImmutableList.of("java", "-Dmy-property=value", "-cp", classpath, "com.example.Main");

Assert.assertEquals(
"[java, -Dmy-property=value, -cp, /app/resources:/app/classes:/app/libs/spring-boot-starter"
+ "-web-2.0.3.RELEASE.jar:/app/libs/shared-library-0.1.0.jar:/app/libs/spring-boot-"
+ "starter-json-2.0.3.RELEASE.jar:/app/libs/spring-boot-starter-2.<... classpath "
+ "truncated ...>, com.example.Main]",
BuildImageStep.truncateLongClasspath(entrypoint));
}
}
6 changes: 5 additions & 1 deletion jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ All notable changes to this project will be documented in this file.

### Added

- Added an option `jib.container.expandClasspathDependencies` to preserve the order of loading dependencies as configured in a project. The option enumerates dependency JARs instead of using a wildcard (`/app/libs/*`) in the Java runtime classpath for an image entrypoint. ([#1871](https://github.com/GoogleContainerTools/jib/issues/1871), [#1907](https://github.com/GoogleContainerTools/jib/issues/1907), [#2228](https://github.com/GoogleContainerTools/jib/issues/2228), [#2733](https://github.com/GoogleContainerTools/jib/issues/2733))
- The option is also useful for AppCDS. ([#2471](https://github.com/GoogleContainerTools/jib/issues/2471))
- Turning on the option may result in a very long classpath string, and the OS may not support passing such a long string to JVM.

### Changed

### Fixed

- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/pull/2819))
- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/issues/2819))
- Fixed build failure when using a Docker daemon base image (`docker://...`) that has duplicate layers. ([#2829](https://github.com/GoogleContainerTools/jib/issues/2829))

## 2.6.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class ContainerParameters {
private Map<String, String> environment = Collections.emptyMap();
@Nullable private List<String> entrypoint;
private List<String> extraClasspath = Collections.emptyList();
private boolean expandClasspathDependencies;
@Nullable private String mainClass;
@Nullable private List<String> args;
private ImageFormat format = ImageFormat.Docker;
Expand Down Expand Up @@ -103,12 +104,24 @@ public List<String> getExtraClasspath() {
return ConfigurationPropertyValidator.parseListProperty(
System.getProperty(PropertyNames.CONTAINER_EXTRA_CLASSPATH));
}

return extraClasspath;
}

public void setExtraClasspath(List<String> classpath) {
this.extraClasspath = classpath;
extraClasspath = classpath;
}

@Input
@Optional
public boolean getExpandClasspathDependencies() {
if (System.getProperty(PropertyNames.EXPAND_CLASSPATH_DEPENDENCIES) != null) {
return Boolean.valueOf(System.getProperty(PropertyNames.EXPAND_CLASSPATH_DEPENDENCIES));
}
return expandClasspathDependencies;
}

public void setExpandClasspathDependencies(boolean expand) {
expandClasspathDependencies = expand;
}

@Input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.function.Predicate;
Expand Down Expand Up @@ -212,11 +213,7 @@ public JibContainerBuilder createJibContainerBuilder(
projectDependencies.getFiles().stream().map(File::getName).collect(Collectors.toSet()));
}

JavaPluginConvention javaPluginConvention =
project.getConvention().getPlugin(JavaPluginConvention.class);
SourceSet mainSourceSet =
javaPluginConvention.getSourceSets().getByName(MAIN_SOURCE_SET_NAME);

SourceSet mainSourceSet = getMainSourceSet();
FileCollection classesOutputDirectories =
mainSourceSet.getOutput().getClassesDirs().filter(File::exists);
Path resourcesOutputDirectory = mainSourceSet.getOutput().getResourcesDir().toPath();
Expand Down Expand Up @@ -287,18 +284,31 @@ public JibContainerBuilder createJibContainerBuilder(
@Override
public List<Path> getClassFiles() throws IOException {
// TODO: Consolidate with createJibContainerBuilder
JavaPluginConvention javaPluginConvention =
project.getConvention().getPlugin(JavaPluginConvention.class);
SourceSet mainSourceSet = javaPluginConvention.getSourceSets().getByName(MAIN_SOURCE_SET_NAME);
FileCollection classesOutputDirectories =
mainSourceSet.getOutput().getClassesDirs().filter(File::exists);
getMainSourceSet().getOutput().getClassesDirs().filter(File::exists);
List<Path> classFiles = new ArrayList<>();
for (File classesOutputDirectory : classesOutputDirectories) {
classFiles.addAll(new DirectoryWalker(classesOutputDirectory.toPath()).walk().asList());
}
return classFiles;
}

@Override
public List<Path> getDependencies() {
List<Path> dependencies = new ArrayList<>();
FileCollection runtimeClasspath = getMainSourceSet().getRuntimeClasspath();
// To be on the safe side with the order, calling "forEach" first (no filtering operations).
runtimeClasspath.forEach(
file -> {
if (file.exists()
&& file.isFile()
&& file.getName().toLowerCase(Locale.US).endsWith(".jar")) {
dependencies.add(file.toPath());
}
});
return dependencies;
}

@Override
public void waitForLoggingThread() {
singleThreadedExecutor.shutDownAndAwaitTermination(LOGGING_THREAD_SHUTDOWN_TIMEOUT);
Expand Down Expand Up @@ -506,4 +516,10 @@ private JibGradlePluginExtension<?> findConfiguredExtension(
}
return found.get();
}

private SourceSet getMainSourceSet() {
JavaPluginConvention javaPluginConvention =
project.getConvention().getPlugin(JavaPluginConvention.class);
return javaPluginConvention.getSourceSets().getByName(MAIN_SOURCE_SET_NAME);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ public List<String> getExtraClasspath() {
return jibExtension.getContainer().getExtraClasspath();
}

@Override
public boolean getExpandClasspathDependencies() {
return jibExtension.getContainer().getExpandClasspathDependencies();
}

@Override
public Optional<String> getMainClass() {
return Optional.ofNullable(jibExtension.getContainer().getMainClass());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.StringJoiner;
Expand Down Expand Up @@ -100,10 +100,12 @@ public class GradleProjectPropertiesTest {
/** Implementation of {@link FileCollection} that just holds a set of {@link File}s. */
private static class TestFileCollection extends AbstractFileCollection {

private final Set<File> files;
private final Set<File> files = new LinkedHashSet<>();

private TestFileCollection(Set<Path> files) {
this.files = files.stream().map(Path::toFile).collect(Collectors.toSet());
for (Path file : files) {
this.files.add(file.toFile());
}
}

@Override
Expand Down Expand Up @@ -237,7 +239,7 @@ public void setUp() throws URISyntaxException, IOException {
FileCollection classesFileCollection = new TestFileCollection(classesFiles);
Path resourcesOutputDir = getResource("gradle/application/resources");

Set<Path> allFiles = new HashSet<>(classesFiles);
Set<Path> allFiles = new LinkedHashSet<>(classesFiles);
allFiles.add(resourcesOutputDir);
allFiles.add(getResource("gradle/application/dependencies/library.jarC.jar"));
allFiles.add(getResource("gradle/application/dependencies/libraryB.jar"));
Expand Down Expand Up @@ -598,6 +600,20 @@ public void testGetWarFilePath_bootWarDisabled() {
Assert.assertEquals("war.war", gradleProjectProperties.getWarFilePath());
}

@Test
public void testGetDependencies() throws URISyntaxException {
Assert.assertEquals(
Arrays.asList(
getResource("gradle/application/dependencies/library.jarC.jar"),
getResource("gradle/application/dependencies/libraryB.jar"),
getResource("gradle/application/dependencies/libraryA.jar"),
getResource("gradle/application/dependencies/dependency-1.0.0.jar"),
getResource("gradle/application/dependencies/more/dependency-1.0.0.jar"),
getResource("gradle/application/dependencies/another/one/dependency-1.0.0.jar"),
getResource("gradle/application/dependencies/dependencyX-1.0.0-SNAPSHOT.jar")),
gradleProjectProperties.getDependencies());
}

private BuildContext setupBuildContext(String appRoot)
throws InvalidImageReferenceException, CacheDirectoryCreationException {
JavaContainerBuilder javaContainerBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public void testContainer() {
Assert.assertEquals(Collections.emptyMap(), testJibExtension.getContainer().getEnvironment());
Assert.assertEquals(
Collections.emptyList(), testJibExtension.getContainer().getExtraClasspath());
Assert.assertFalse(testJibExtension.getContainer().getExpandClasspathDependencies());
Assert.assertNull(testJibExtension.getContainer().getMainClass());
Assert.assertNull(testJibExtension.getContainer().getArgs());
Assert.assertSame(ImageFormat.Docker, testJibExtension.getContainer().getFormat());
Expand All @@ -156,6 +157,7 @@ public void testContainer() {
container.setEnvironment(ImmutableMap.of("var1", "value1", "var2", "value2"));
container.setEntrypoint(Arrays.asList("foo", "bar", "baz"));
container.setExtraClasspath(Arrays.asList("/d1", "/d2", "/d3"));
container.setExpandClasspathDependencies(true);
container.setMainClass("mainClass");
container.setArgs(Arrays.asList("arg1", "arg2", "arg3"));
container.setPorts(Arrays.asList("1000", "2000-2010", "3000"));
Expand All @@ -170,6 +172,7 @@ public void testContainer() {
Assert.assertEquals(
ImmutableMap.of("var1", "value1", "var2", "value2"), container.getEnvironment());
Assert.assertEquals(ImmutableList.of("/d1", "/d2", "/d3"), container.getExtraClasspath());
Assert.assertTrue(testJibExtension.getContainer().getExpandClasspathDependencies());
Assert.assertEquals("mainClass", testJibExtension.getContainer().getMainClass());
Assert.assertEquals(Arrays.asList("arg1", "arg2", "arg3"), container.getArgs());
Assert.assertEquals(Arrays.asList("1000", "2000-2010", "3000"), container.getPorts());
Expand Down Expand Up @@ -381,6 +384,8 @@ public void testProperties() {
System.setProperty("jib.container.extraClasspath", "/d1,/d2,/d3");
Assert.assertEquals(
ImmutableList.of("/d1", "/d2", "/d3"), testJibExtension.getContainer().getExtraClasspath());
System.setProperty("jib.container.expandClasspathDependencies", "true");
Assert.assertTrue(testJibExtension.getContainer().getExpandClasspathDependencies());
System.setProperty("jib.container.format", "OCI");
Assert.assertSame(ImageFormat.OCI, testJibExtension.getContainer().getFormat());
System.setProperty("jib.container.jvmFlags", "flag1,flag2,flag3");
Expand Down
6 changes: 5 additions & 1 deletion jib-maven-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ All notable changes to this project will be documented in this file.

### Changed

- Added an option `<container><expandClasspathDependencies>` to preserve the order of loading dependencies as configured in a project. The option enumerates dependency JARs instead of using a wildcard (`/app/libs/*`) in the Java runtime classpath for an image entrypoint. ([#1871](https://github.com/GoogleContainerTools/jib/issues/1871), [#1907](https://github.com/GoogleContainerTools/jib/issues/1907), [#2228](https://github.com/GoogleContainerTools/jib/issues/2228), [#2733](https://github.com/GoogleContainerTools/jib/issues/2733))
- The option is also useful for AppCDS. ([#2471](https://github.com/GoogleContainerTools/jib/issues/2471))
- Turning on the option may result in a very long classpath string, and the OS may not support passing such a long string to JVM.

### Fixed

- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/pull/2819))
- Fixed `NullPointerException` when pulling an OCI base image whose manifest does not have `mediaType` information. ([#2819](https://github.com/GoogleContainerTools/jib/issues/2819))
- Fixed build failure when using a Docker daemon base image (`docker://...`) that has duplicate layers. ([#2829](https://github.com/GoogleContainerTools/jib/issues/2829))

## 2.6.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ public static class ContainerParameters {

@Parameter private List<String> extraClasspath = Collections.emptyList();

private boolean expandClasspathDependencies;

@Nullable @Parameter private String mainClass;

@Nullable @Parameter private List<String> args;
Expand Down Expand Up @@ -505,6 +507,19 @@ List<String> getExtraClasspath() {
return container.extraClasspath;
}

/**
* Returns whether to expand classpath dependencies.
*
* @return {@code true} to expand classpath dependencies. {@code false} otherwise.
*/
public boolean getExpandClasspathDependencies() {
String property = getProperty(PropertyNames.EXPAND_CLASSPATH_DEPENDENCIES);
if (property != null) {
return Boolean.valueOf(property);
}
return container.expandClasspathDependencies;
}

/**
* Gets the name of the main class.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,15 @@ public List<Path> getClassFiles() throws IOException {
return new DirectoryWalker(Paths.get(project.getBuild().getOutputDirectory())).walk().asList();
}

@Override
public List<Path> getDependencies() {
return project
.getArtifacts()
.stream()
.map(artifact -> artifact.getFile().toPath())
.collect(Collectors.toList());
}

@Override
public void waitForLoggingThread() {
singleThreadedExecutor.shutDownAndAwaitTermination(LOGGING_THREAD_SHUTDOWN_TIMEOUT);
Expand Down
Loading

0 comments on commit f0123a8

Please sign in to comment.