Fixes review comments

jib-core/src/main/java/com/google/cloud/tools/jib/builder/BuildSteps.java

@@ -158,7 +158,7 @@ public DescriptorDigest run() throws InterruptedException, ExecutionException {
 
  if (buildConfiguration.getContainerConfiguration() != null) {
  buildConfiguration.getEventDispatcher().dispatch(LogEvent.lifecycle(""));
-
+ // TODO refactor below
  if (buildConfiguration.getContainerConfiguration().getEntrypoint() != null) {
  buildConfiguration
  .getEventDispatcher()

jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildImageStep.java

@@ -190,45 +190,58 @@ private Image<Layer> afterCacheEntrySteps()
  }
  }
 
+ /**
+ * Computes the image entrypoint. If {@link ContainerConfiguration#getEntrypoint()} is null, the
+ * entrypoint is inherited from the base image. Otherwise {@link
+ * ContainerConfiguration#getEntrypoint()} is returned.
+ *
+ * @param baseImage the base image
+ * @param containerConfiguration the container configuration
+ * @return the container entrypoint
+ */
  @Nullable
  private ImmutableList<String> computeEntrypoint(
  Image<Layer> baseImage, ContainerConfiguration containerConfiguration) {
- boolean shouldInheritEntrypoint = containerConfiguration.getEntrypoint() == null;
-
- if (shouldInheritEntrypoint && baseImage.getEntrypoint() != null) {
- buildConfiguration
- .getEventDispatcher()
- .dispatch(
- LogEvent.lifecycle(
- "Container entrypoint set to "
- + baseImage.getEntrypoint()
- + " (inherited from base image)"));
+ if (baseImage.getEntrypoint() == null || containerConfiguration.getEntrypoint() != null) {
+ return containerConfiguration.getEntrypoint();
  }
 
- return shouldInheritEntrypoint
- ? baseImage.getEntrypoint()
- : containerConfiguration.getEntrypoint();
+ buildConfiguration
+ .getEventDispatcher()
+ .dispatch(
+ LogEvent.lifecycle(
+ "Container entrypoint set to "
+ + baseImage.getEntrypoint()
+ + " (inherited from base image)"));
+ return baseImage.getEntrypoint();
  }
 
+ /**
+ * Computes the image program arguments. If {@link ContainerConfiguration#getEntrypoint()} and
+ * {@link ContainerConfiguration#getProgramArguments()} are null, the program arguments are
+ * inherited from the base image. Otherwise {@link ContainerConfiguration#getProgramArguments()}
+ * is returned.
+ *
+ * @param baseImage the base image
+ * @param containerConfiguration the container configuration
+ * @return the container program arguments
+ */
  @Nullable
  private ImmutableList<String> computeProgramArguments(
  Image<Layer> baseImage, ContainerConfiguration containerConfiguration) {
- boolean shouldInheritProgramArguments =
- containerConfiguration.getEntrypoint() == null
- && containerConfiguration.getProgramArguments() == null;
-
- if (shouldInheritProgramArguments && baseImage.getProgramArguments() != null) {
- buildConfiguration
- .getEventDispatcher()
- .dispatch(
- LogEvent.lifecycle(
- "Container program arguments set to "
- + baseImage.getProgramArguments()
- + " (inherited from base image)"));
+ if (baseImage.getProgramArguments() == null
+ || containerConfiguration.getEntrypoint() != null
+ || containerConfiguration.getProgramArguments() != null) {
+ return containerConfiguration.getProgramArguments();
  }
 
- return shouldInheritProgramArguments
- ? baseImage.getProgramArguments()
- : containerConfiguration.getProgramArguments();
+ buildConfiguration
+ .getEventDispatcher()
+ .dispatch(
+ LogEvent.lifecycle(
+ "Container program arguments set to "
+ + baseImage.getProgramArguments()
+ + " (inherited from base image)"));
+ return baseImage.getProgramArguments();
  }
 }

jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java

@@ -137,18 +137,18 @@ public Builder setEntrypoint(@Nullable List<String> entrypoint) {
  return this;
  }
 
-  /**
-  * Sets the user and group to run the container as. {@code user} can be a username or UID along
-  * with an optional groupname or GID. The following are all valid: {@code user}, {@code uid},
-  * {@code user:group}, {@code uid:gid}, {@code uid:group}, {@code user:gid}.
-  *
-  * @param user the username/UID and optionally the groupname/GID
-  * @return this
-  */
-  public Builder setUser(@Nullable String user) {
-  this.user = user;
-  return this;
-  }
+ /**
+ * Sets the user and group to run the container as. {@code user} can be a username or UID along
+ * with an optional groupname or GID. The following are all valid: {@code user}, {@code uid},
+ * {@code user:group}, {@code uid:gid}, {@code uid:group}, {@code user:gid}.
+ *
+ * @param user the username/UID and optionally the groupname/GID
+ * @return this
+ */
+ public Builder setUser(@Nullable String user) {
+ this.user = user;
+ return this;
+ }
 
  /**
  * Builds the {@link ContainerConfiguration}.

jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGenerator.java

@@ -343,6 +343,9 @@ String makeDockerfile() throws JsonProcessingException {
  }
  if (programArguments != null) {
  dockerfile.append("\nCMD ").append(objectMapper.writeValueAsString(programArguments));
+ } else if (entrypoint != null) {
+ // Do not inherit CMD from base image if ENTRYPOINT is not inherited
+ dockerfile.append("\nCMD []");
  }
  if (user != null) {
  dockerfile.append("\nUSER ").append(user);

jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildImageStepTest.java

@@ -234,6 +234,28 @@ public void test_inheritedEntrypointAndProgramArguments()
  Assert.assertEquals(ImmutableList.of("catalina.sh", "run"), image.getProgramArguments());
  }
 
+ @Test
+ public void test_notInheritedProgramArguments() throws ExecutionException, InterruptedException {
+ Mockito.when(mockContainerConfiguration.getEntrypoint())
+ .thenReturn(ImmutableList.of("myEntrypoint"));
+ Mockito.when(mockContainerConfiguration.getProgramArguments()).thenReturn(null);
+
+ BuildImageStep buildImageStep =
+ new BuildImageStep(
+ MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()),
+ mockBuildConfiguration,
+ mockPullBaseImageStep,
+ mockPullAndCacheBaseImageLayersStep,
+ ImmutableList.of(
+ mockBuildAndCacheApplicationLayerStep,
+ mockBuildAndCacheApplicationLayerStep,
+ mockBuildAndCacheApplicationLayerStep));
+ Image<Layer> image = buildImageStep.getFuture().get().getFuture().get();
+
+ Assert.assertEquals(ImmutableList.of("myEntrypoint"), image.getEntrypoint());
+ Assert.assertNull(image.getProgramArguments());
+ }
+
  @Test
  public void test_generateHistoryObjects() throws ExecutionException, InterruptedException {
  BuildImageStep buildImageStep =

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/ContainerParameters.java

@@ -39,7 +39,7 @@ public class ContainerParameters {
  private Map<String, String> environment = Collections.emptyMap();
  private List<String> entrypoint = Collections.emptyList();
  @Nullable private String mainClass;
- @Nullable private List<String> args = null;
+ @Nullable private List<String> args;
  private ImageFormat format = ImageFormat.Docker;
  private List<String> ports = Collections.emptyList();
  private Map<String, String> labels = Collections.emptyMap();

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/DockerContextTask.java

@@ -125,8 +125,7 @@ public void generateDockerContext() {
  new JavaDockerContextGenerator(gradleProjectProperties.getJavaLayerConfigurations())
  .setBaseImage(jibExtension.getFrom().getImage())
  .setEntrypoint(entrypoint)
- .setProgramArguments(
- PluginConfigurationProcessor.computeProgramArguments(entrypoint, jibExtension))
+ .setProgramArguments(jibExtension.getContainer().getArgs())
  .setExposedPorts(jibExtension.getContainer().getPorts())
  .setLabels(jibExtension.getContainer().getLabels())
  .setUser(jibExtension.getContainer().getUser())

jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/PluginConfigurationProcessor.java

@@ -34,7 +34,6 @@
 import com.google.cloud.tools.jib.plugins.common.PropertyNames;
 import com.google.common.base.Preconditions;
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -129,12 +128,11 @@ static PluginConfigurationProcessor processCommonConfiguration(
  .setCredentialRetrievers(defaultCredentialRetrievers.asList());
 
  List<String> entrypoint = computeEntrypoint(logger, jibExtension, projectProperties);
- List<String> programArguments = computeProgramArguments(entrypoint, jibExtension);
  ContainerConfiguration.Builder containerConfigurationBuilder =
  ContainerConfiguration.builder()
  .setEntrypoint(entrypoint)
  .setEnvironment(jibExtension.getContainer().getEnvironment())
- .setProgramArguments(programArguments)
+ .setProgramArguments(jibExtension.getContainer().getArgs())
  .setExposedPorts(ExposedPortsParser.parse(jibExtension.getContainer().getPorts()))
  .setLabels(jibExtension.getContainer().getLabels())
  .setUser(jibExtension.getContainer().getUser());
@@ -201,24 +199,6 @@ static List<String> computeEntrypoint(
  AbsoluteUnixPath.get(parameters.getAppRoot()), parameters.getJvmFlags(), mainClass);
  }
 
- /**
- * Compute the container program arguments. If the entrypoint is not inherited, program arguments
- * must not be inherited .
- *
- * @param entrypoint the container entrypoint
- * @param jibExtension the {@link JibExtension} providing the configuration data
- * @return the program arguments
- */
- @Nullable
- static List<String> computeProgramArguments(
- @Nullable List<String> entrypoint, JibExtension jibExtension) {
- if (entrypoint != null && jibExtension.getContainer().getArgs() == null) {
- return Collections.emptyList();
- } else {
- return jibExtension.getContainer().getArgs();
- }
- }
-
  private final BuildConfiguration.Builder buildConfigurationBuilder;
  private final ImageConfiguration.Builder baseImageConfigurationBuilder;
  private final ContainerConfiguration.Builder containerConfigurationBuilder;

jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/DockerContextTaskTest.java

@@ -59,6 +59,7 @@ public void setUp() throws IOException {
  Mockito.when(jibExtension.getFrom()).thenReturn(baseImageParameters);
  Mockito.when(baseImageParameters.getImage()).thenReturn("base image");
  Mockito.when(containerParameters.getAppRoot()).thenReturn("/app");
+ Mockito.when(containerParameters.getArgs()).thenCallRealMethod();
 
  project = ProjectBuilder.builder().withProjectDir(projectRoot.getRoot()).build();
  project.getPluginManager().apply("java");
@@ -98,14 +99,14 @@ public void testEntrypoint_inheritedEntrypoint() throws IOException {
  try {
  getEntrypoint();
  Assert.fail();
- } catch (NoSuchElementException e) {
- Assert.assertEquals("No value present", e.getMessage());
+ } catch (NoSuchElementException ex) {
+ // pass
  }
  try {
  getCmd();
  Assert.fail();
- } catch (NoSuchElementException e) {
- Assert.assertEquals("No value present", e.getMessage());
+ } catch (NoSuchElementException ex) {
+ // pass
  }
  }
 
@@ -169,21 +170,21 @@ public void testGenerateDockerContext_errorOnWindowsAppRootWithDriveLetter() {
  }
  }
 
+ private String getUser() throws IOException {
+ return getDockerfileLine("USER");
+ }
+
  private String getEntrypoint() throws IOException {
- Path dockerfile = projectRoot.getRoot().toPath().resolve("build/jib-docker-context/Dockerfile");
- List<String> lines = Files.readAllLines(dockerfile);
- return lines.stream().filter(line -> line.startsWith("ENTRYPOINT")).findFirst().get();
+ return getDockerfileLine("ENTRYPOINT");
  }
 
  private String getCmd() throws IOException {
- Path dockerfile = projectRoot.getRoot().toPath().resolve("build/jib-docker-context/Dockerfile");
- List<String> lines = Files.readAllLines(dockerfile);
- return lines.stream().filter(line -> line.startsWith("CMD")).findFirst().get();
+ return getDockerfileLine("CMD");
  }
 
- private String getUser() throws IOException {
+ private String getDockerfileLine(String command) throws IOException {
  Path dockerfile = projectRoot.getRoot().toPath().resolve("build/jib-docker-context/Dockerfile");
  List<String> lines = Files.readAllLines(dockerfile);
- return lines.stream().filter(line -> line.startsWith("USER")).findFirst().get();
+ return lines.stream().filter(line -> line.startsWith(command)).findFirst().get();
  }
 }

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/DockerContextMojo.java

@@ -86,8 +86,7 @@ public void execute() throws MojoExecutionException {
  new JavaDockerContextGenerator(mavenProjectProperties.getJavaLayerConfigurations())
  .setBaseImage(PluginConfigurationProcessor.getBaseImage(this))
  .setEntrypoint(entrypoint)
- .setProgramArguments(
- PluginConfigurationProcessor.computeProgramArguments(entrypoint, getArgs()))
+ .setProgramArguments(getArgs())
  .setExposedPorts(getExposedPorts())
  .setLabels(getLabels())
  .setUser(getUser())

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/JibPluginConfiguration.java

@@ -128,7 +128,7 @@ public static class ContainerParameters {
 
  @Nullable @Parameter private String mainClass;
 
- @Nullable @Parameter private List<String> args = null;
+ @Nullable @Parameter private List<String> args;
 
  @Nullable
  @Parameter(required = true)

jib-maven-plugin/src/main/java/com/google/cloud/tools/jib/maven/PluginConfigurationProcessor.java

@@ -34,7 +34,6 @@
 import com.google.cloud.tools.jib.plugins.common.PropertyNames;
 import com.google.common.base.Preconditions;
 import java.time.Instant;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -170,12 +169,10 @@ static PluginConfigurationProcessor processCommonConfiguration(
  .setCredentialRetrievers(defaultCredentialRetrievers.asList());
 
  List<String> entrypoint = computeEntrypoint(logger, jibPluginConfiguration, projectProperties);
- List<String> programArguments =
- computeProgramArguments(entrypoint, jibPluginConfiguration.getArgs());
  ContainerConfiguration.Builder containerConfigurationBuilder =
  ContainerConfiguration.builder()
  .setEntrypoint(entrypoint)
- .setProgramArguments(programArguments)
+ .setProgramArguments(jibPluginConfiguration.getArgs())
  .setEnvironment(jibPluginConfiguration.getEnvironment())
  .setExposedPorts(ExposedPortsParser.parse(jibPluginConfiguration.getExposedPorts()))
  .setLabels(jibPluginConfiguration.getLabels())
@@ -263,24 +260,6 @@ static List<String> computeEntrypoint(
  getAppRootChecked(jibPluginConfiguration), jibPluginConfiguration.getJvmFlags(), mainClass);
  }
 
- /**
- * Compute the container program arguments. If the entrypoint is not inherited, program arguments
- * must not be inherited .
- *
- * @param entrypoint the container entrypoint
- * @param containerArgs the container arguments
- * @return the program arguments
- */
- @Nullable
- static List<String> computeProgramArguments(
- @Nullable List<String> entrypoint, List<String> containerArgs) {
- if (entrypoint != null && containerArgs == null) {
- return Collections.EMPTY_LIST;
- } else {
- return containerArgs;
- }
- }
-
  private final BuildConfiguration.Builder buildConfigurationBuilder;
  private final ImageConfiguration.Builder baseImageConfigurationBuilder;
  private final ContainerConfiguration.Builder containerConfigurationBuilder;

jib-maven-plugin/src/test/java/com/google/cloud/tools/jib/maven/DockerContextMojoTest.java

@@ -211,14 +211,14 @@ public void testEntrypoint_defaultWarPackaging() throws MojoExecutionException,
  try {
  getEntrypoint();
  Assert.fail();
- } catch (NoSuchElementException e) {
- Assert.assertEquals("No value present", e.getMessage());
+ } catch (NoSuchElementException ex) {
+ // passs
  }
  try {
  getCmd();
  Assert.fail();
- } catch (NoSuchElementException e) {
- Assert.assertEquals("No value present", e.getMessage());
+ } catch (NoSuchElementException ex) {
+ // pass
  }
  }
 
@@ -326,13 +326,12 @@ String getBaseImage() {
  getUser();
  Assert.fail();
  } catch (NoSuchElementException ex) {
+ // pass
  }
  }
 
  private String getUser() throws IOException {
- Path dockerfile = projectRoot.getRoot().toPath().resolve("target/Dockerfile");
- List<String> lines = Files.readAllLines(dockerfile);
- return lines.stream().filter(line -> line.startsWith("USER")).findFirst().get();
+ return getDockerfileLine("USER");
  }
 
  private String getEntrypoint() throws IOException {