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

Allow configuring working directory #1266

Merged
merged 10 commits into from
Nov 27, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,17 @@ public JibContainerBuilder setUser(@Nullable String user) {
return this;
}

/**
* Sets the working directory in the container.
*
* @param workingDirectory the working directory
* @return this
*/
public JibContainerBuilder setWorkingDirectory(@Nullable AbsoluteUnixPath workingDirectory) {
containerConfigurationBuilder.setWorkingDirectory(workingDirectory);
return this;
}

/**
* Builds the container.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ private Image<Layer> afterCachedLayerSteps()
.setExposedPorts(containerConfiguration.getExposedPorts())
.setVolumes(containerConfiguration.getVolumes())
.addLabels(containerConfiguration.getLabels());
if (containerConfiguration.getWorkingDirectory() != null) {
imageBuilder.setWorkingDirectory(containerConfiguration.getWorkingDirectory().toString());
}
}

// Gets the container configuration content descriptor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public static class Builder {
@Nullable private List<AbsoluteUnixPath> volumes;
@Nullable private Map<String, String> labels;
@Nullable private String user;
@Nullable private AbsoluteUnixPath workingDirectory;

/**
* Sets the image creation time.
Expand Down Expand Up @@ -198,6 +199,17 @@ public Builder setUser(@Nullable String user) {
return this;
}

/**
* Sets the working directory in the container.
*
* @param workingDirectory the working directory
* @return this
*/
public Builder setWorkingDirectory(@Nullable AbsoluteUnixPath workingDirectory) {
this.workingDirectory = workingDirectory;
return this;
}

/**
* Builds the {@link ContainerConfiguration}.
*
Expand All @@ -212,7 +224,8 @@ public ContainerConfiguration build() {
exposedPorts == null ? null : ImmutableList.copyOf(exposedPorts),
volumes == null ? null : ImmutableList.copyOf(volumes),
labels == null ? null : ImmutableMap.copyOf(labels),
user);
user,
workingDirectory);
}

private Builder() {}
Expand All @@ -235,6 +248,7 @@ public static Builder builder() {
@Nullable private final ImmutableList<AbsoluteUnixPath> volumes;
@Nullable private final ImmutableMap<String, String> labels;
@Nullable private final String user;
@Nullable private final AbsoluteUnixPath workingDirectory;

private ContainerConfiguration(
Instant creationTime,
Expand All @@ -244,7 +258,8 @@ private ContainerConfiguration(
@Nullable ImmutableList<Port> exposedPorts,
@Nullable ImmutableList<AbsoluteUnixPath> volumes,
@Nullable ImmutableMap<String, String> labels,
@Nullable String user) {
@Nullable String user,
@Nullable AbsoluteUnixPath workingDirectory) {
this.creationTime = creationTime;
this.entrypoint = entrypoint;
this.programArguments = programArguments;
Expand All @@ -253,6 +268,7 @@ private ContainerConfiguration(
this.volumes = volumes;
this.labels = labels;
this.user = user;
this.workingDirectory = workingDirectory;
}

public Instant getCreationTime() {
Expand Down Expand Up @@ -294,6 +310,11 @@ public ImmutableMap<String, String> getLabels() {
return labels;
}

@Nullable
public AbsoluteUnixPath getWorkingDirectory() {
return workingDirectory;
}

@Override
@VisibleForTesting
public boolean equals(Object other) {
Expand All @@ -310,7 +331,8 @@ public boolean equals(Object other) {
&& Objects.equals(environmentMap, otherContainerConfiguration.environmentMap)
&& Objects.equals(exposedPorts, otherContainerConfiguration.exposedPorts)
&& Objects.equals(labels, otherContainerConfiguration.labels)
&& Objects.equals(user, otherContainerConfiguration.user);
&& Objects.equals(user, otherContainerConfiguration.user)
&& Objects.equals(workingDirectory, otherContainerConfiguration.workingDirectory);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.tools.jib.configuration.credentials.CredentialRetriever;
import com.google.cloud.tools.jib.event.EventHandlers;
import com.google.cloud.tools.jib.event.JibEvent;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.image.ImageFormat;
import com.google.cloud.tools.jib.image.ImageReference;
import com.google.cloud.tools.jib.image.InvalidImageReferenceException;
Expand Down Expand Up @@ -69,7 +70,8 @@ public void testToBuildConfiguration_containerConfigurationSet()
.setLabels(ImmutableMap.of("key", "value"))
.setProgramArguments(Arrays.asList("program", "arguments"))
.setCreationTime(Instant.ofEpochMilli(1000))
.setUser("user");
.setUser("user")
.setWorkingDirectory(AbsoluteUnixPath.get("/working/directory"));

BuildConfiguration buildConfiguration =
jibContainerBuilder.toBuildConfiguration(Containerizer.to(baseImage));
Expand All @@ -84,6 +86,8 @@ public void testToBuildConfiguration_containerConfigurationSet()
Arrays.asList("program", "arguments"), containerConfiguration.getProgramArguments());
Assert.assertEquals(Instant.ofEpochMilli(1000), containerConfiguration.getCreationTime());
Assert.assertEquals("user", containerConfiguration.getUser());
Assert.assertEquals(
AbsoluteUnixPath.get("/working/directory"), containerConfiguration.getWorkingDirectory());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.configuration.ContainerConfiguration;
import com.google.cloud.tools.jib.event.EventDispatcher;
import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.cloud.tools.jib.image.DescriptorDigest;
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.image.Layer;
Expand Down Expand Up @@ -224,6 +225,26 @@ public void test_propagateBaseImageConfiguration()
Assert.assertEquals(ImmutableList.of(), image.getProgramArguments());
}

@Test
public void testOverrideWorkingDirectory() throws InterruptedException, ExecutionException {
Mockito.when(mockContainerConfiguration.getWorkingDirectory())
.thenReturn(AbsoluteUnixPath.get("/my/directory"));

BuildImageStep buildImageStep =
new BuildImageStep(
MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()),
mockBuildConfiguration,
mockPullBaseImageStep,
mockPullAndCacheBaseImageLayersStep,
ImmutableList.of(
mockBuildAndCacheApplicationLayerStepDependencies,
mockBuildAndCacheApplicationLayerStepResources,
mockBuildAndCacheApplicationLayerStepClasses));
Image<Layer> image = buildImageStep.getFuture().get().getFuture().get();

Assert.assertEquals("/my/directory", image.getWorkingDirectory());
}

@Test
public void test_inheritedEntrypoint() throws ExecutionException, InterruptedException {
Mockito.when(mockContainerConfiguration.getEntrypoint()).thenReturn(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

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

import com.google.cloud.tools.jib.filesystem.AbsoluteUnixPath;
import com.google.common.collect.ImmutableMap;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -104,4 +105,11 @@ public void testBuilder_user() {
ContainerConfiguration configuration = ContainerConfiguration.builder().setUser("john").build();
Assert.assertEquals("john", configuration.getUser());
}

@Test
public void testBuilder_workingDirectory() {
ContainerConfiguration configuration =
ContainerConfiguration.builder().setWorkingDirectory(AbsoluteUnixPath.get("/path")).build();
Assert.assertEquals(AbsoluteUnixPath.get("/path"), configuration.getWorkingDirectory());
}
}
1 change: 1 addition & 0 deletions jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
### Added

- Image ID is now written to `build/jib-image.id` ([#1204](https://github.com/GoogleContainerTools/jib/issues/1204))
- `container.workingDirectory` configuration parameter to set the working directory ([#1225](https://github.com/GoogleContainerTools/jib/issues/1225))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class ContainerParameters {
private Map<String, String> labels = Collections.emptyMap();
private String appRoot = "";
@Nullable private String user;
@Nullable private String workingDirectory;

@Input
@Optional
Expand Down Expand Up @@ -197,4 +198,18 @@ public String getUser() {
public void setUser(String user) {
this.user = user;
}

@Input
@Nullable
TadCordle marked this conversation as resolved.
Show resolved Hide resolved
@Optional
public String getWorkingDirectory() {
if (System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY) != null) {
return System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY);
}
return workingDirectory;
}

public void setWorkingDirectory(String workingDirectory) {
this.workingDirectory = workingDirectory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ public Optional<String> getUser() {
return Optional.ofNullable(jibExtension.getContainer().getUser());
}

@Override
public Optional<String> getWorkingDirectory() {
return Optional.ofNullable(jibExtension.getContainer().getWorkingDirectory());
}

@Override
public boolean getUseCurrentTimestamp() {
return jibExtension.getContainer().getUseCurrentTimestamp();
Expand Down
3 changes: 2 additions & 1 deletion jib-maven-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.
### Added

- Image ID is now written to `target/jib-image.id` ([#1204](https://github.com/GoogleContainerTools/jib/issues/1204))
- `<container><workingDirectory>` configuration parameter to set the working directory ([#1225](https://github.com/GoogleContainerTools/jib/issues/1225))

### Changed

Expand All @@ -19,7 +20,7 @@ All notable changes to this project will be documented in this file.

- Properties for each configuration parameter, allowing any parameter to be set via commandline ([#728](https://github.com/GoogleContainerTools/jib/issues/728))
- `<to><credHelper>` and `<from><credHelper>` can be used to specify a credential helper suffix or a full path to a credential helper executable ([#925](https://github.com/GoogleContainerTools/jib/issues/925))
- `container.user` configuration parameter to configure the user and group to run the container as ([#1029](https://github.com/GoogleContainerTools/jib/issues/1029))
- `<container><user>` configuration parameter to configure the user and group to run the container as ([#1029](https://github.com/GoogleContainerTools/jib/issues/1029))
- Preliminary support for building images for WAR projects ([#431](https://github.com/GoogleContainerTools/jib/issues/431))
- `<extraDirectory>` object with a `<path>` and `<permissions>` field ([#794](https://github.com/GoogleContainerTools/jib/issues/794))
- `<extraDirectory><path>` configures the extra layer directory (still also configurable via `<extraDirectory>...</extraDirectory>`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ public static class ContainerParameters {
@Parameter private String appRoot = "";

@Nullable @Parameter private String user;

@Nullable @Parameter private String workingDirectory;
}

/** Configuration for the {@code extraDirectory} parameter. */
Expand Down Expand Up @@ -375,6 +377,19 @@ String getUser() {
return container.user;
}

/**
* Gets the working directory in the container.
*
* @return the working directory
*/
@Nullable
String getWorkingDirectory() {
if (System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY) != null) {
return System.getProperty(PropertyNames.CONTAINER_WORKING_DIRECTORY);
}
return container.workingDirectory;
}

/**
* Gets the configured main arguments.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ public Optional<String> getUser() {
return Optional.ofNullable(jibPluginConfiguration.getUser());
}

@Override
public Optional<String> getWorkingDirectory() {
return Optional.ofNullable(jibPluginConfiguration.getWorkingDirectory());
}

@Override
public boolean getUseCurrentTimestamp() {
return jibPluginConfiguration.getUseCurrentTimestamp();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ private static void clearProperties() {
System.clearProperty("jib.container.ports");
System.clearProperty("jib.container.useCurrentTimestamp");
System.clearProperty("jib.container.user");
System.clearProperty("jib.container.workingDirectory");
System.clearProperty("jib.extraDirectory.path");
System.clearProperty("jib.extraDirectory.permissions");
}
Expand All @@ -72,6 +73,7 @@ public void teardown() {
@Test
public void testDefaults() {
Assert.assertEquals("", testPluginConfiguration.getAppRoot());
Assert.assertNull(testPluginConfiguration.getWorkingDirectory());
}

@Test
Expand Down Expand Up @@ -121,6 +123,8 @@ public void testSystemProperties() {
Assert.assertTrue(testPluginConfiguration.getUseCurrentTimestamp());
System.setProperty("jib.container.user", "myUser");
Assert.assertEquals("myUser", testPluginConfiguration.getUser());
System.setProperty("jib.container.workingDirectory", "working directory");
Assert.assertEquals("working directory", testPluginConfiguration.getWorkingDirectory());

System.setProperty("jib.extraDirectory.path", "custom-jib");
Assert.assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ static PluginConfigurationProcessor processCommonConfiguration(
.setExposedPorts(ExposedPortsParser.parse(rawConfiguration.getPorts()))
.setLabels(rawConfiguration.getLabels())
.setUser(rawConfiguration.getUser().orElse(null));
rawConfiguration
.getWorkingDirectory()
.ifPresent(
workingDirectory ->
jibContainerBuilder.setWorkingDirectory(AbsoluteUnixPath.get(workingDirectory)));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably be consistent as to the validation behavior of all the configurations that are AbsoluteUnixPath (workingDirectory, appRoot, volumes (#1278)) - currently for appRoot, an invalid absolute Unix-style path is thrown as AppRootInvalidException, but here it would be thrown as the IllegalArgumentException.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. Added WorkingDirectoryInvalidException, which follows the pattern of AppRootInvalidException. (However, I see InvalidContainerVolumeException in #1278 is a bit different.)

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 the naming should start with Invalid... since in most exception names, the adjective comes first? (ie. IllegalArgumentException, NullPointerException, UnsupportedOperationException)

if (rawConfiguration.getUseCurrentTimestamp()) {
eventDispatcher.dispatch(
LogEvent.warn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class PropertyNames {
public static final String CONTAINER_LABELS = "jib.container.labels";
public static final String CONTAINER_MAIN_CLASS = "jib.container.mainClass";
public static final String CONTAINER_USER = "jib.container.user";
public static final String CONTAINER_WORKING_DIRECTORY = "jib.container.workingDirectory";
public static final String CONTAINER_PORTS = "jib.container.ports";
public static final String CONTAINER_USE_CURRENT_TIMESTAMP = "jib.container.useCurrentTimestamp";
public static final String USE_ONLY_PROJECT_CACHE = "jib.useOnlyProjectCache";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public interface RawConfiguration {

Optional<String> getUser();

Optional<String> getWorkingDirectory();

boolean getUseCurrentTimestamp();

boolean getAllowInsecureRegistries();
Expand Down