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

Add plugin configuration for environment variables #930

Merged
merged 7 commits into from
Sep 5, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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 @@ -104,18 +104,7 @@ public void testSteps_forBuildToDockerRegistry()

String imageReference = "localhost:5000/testimage:testtag";
localRegistry.pull(imageReference);
Assert.assertThat(
new Command("docker", "inspect", imageReference).run(),
CoreMatchers.containsString(
" \"ExposedPorts\": {\n"
+ " \"1000/tcp\": {},\n"
+ " \"2000/tcp\": {},\n"
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
+ " \"3000/udp\": {}"));
String history = new Command("docker", "history", imageReference).run();
Assert.assertThat(history, CoreMatchers.containsString("jib-integration-test"));
Assert.assertThat(history, CoreMatchers.containsString("bazel build ..."));
assertDockerInspect(imageReference);
Assert.assertEquals(
"Hello, world. An argument.\n", new Command("docker", "run", imageReference).run());
}
Expand Down Expand Up @@ -152,26 +141,7 @@ public void testSteps_forBuildToDockerDaemon()
new Caches.Initializer(cacheDirectory, false, cacheDirectory, false))
.run();

String dockerContainerConfig = new Command("docker", "inspect", imageReference).run();
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
" \"ExposedPorts\": {\n"
+ " \"1000/tcp\": {},\n"
+ " \"2000/tcp\": {},\n"
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
+ " \"3000/udp\": {}"));
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
" \"Labels\": {\n"
+ " \"key1\": \"value1\",\n"
+ " \"key2\": \"value2\"\n"
+ " }"));
String history = new Command("docker", "history", imageReference).run();
Assert.assertThat(history, CoreMatchers.containsString("jib-integration-test"));
Assert.assertThat(history, CoreMatchers.containsString("bazel build ..."));
assertDockerInspect(imageReference);
Assert.assertEquals(
"Hello, world. An argument.\n", new Command("docker", "run", imageReference).run());
}
Expand Down Expand Up @@ -213,6 +183,7 @@ private BuildConfiguration getBuildConfiguration(
JavaEntrypointConstructor.makeDefaultEntrypoint(
Collections.emptyList(), "HelloWorld"))
.setProgramArguments(Collections.singletonList("An argument."))
.setEnvironment(ImmutableMap.of("var1", "value1", "var2", "value2"))
.setExposedPorts(
ExposedPortsParser.parse(Arrays.asList("1000", "2000-2002/tcp", "3000/udp")))
.setLabels(ImmutableMap.of("key1", "value1", "key2", "value2"))
Expand All @@ -226,4 +197,31 @@ private BuildConfiguration getBuildConfiguration(
.setCreatedBy("jib-integration-test")
.build();
}

private void assertDockerInspect(String imageReference) throws IOException, InterruptedException {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it can be static?

String dockerContainerConfig = new Command("docker", "inspect", imageReference).run();
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
" \"ExposedPorts\": {\n"
+ " \"1000/tcp\": {},\n"
+ " \"2000/tcp\": {},\n"
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
+ " \"3000/udp\": {}"));
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
" \"Labels\": {\n"
+ " \"key1\": \"value1\",\n"
+ " \"key2\": \"value2\"\n"
+ " }"));
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
" \"var1=value1\",\n \"var2=value2\"\n"));
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to match the environment part ("Env": [) too, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that there are other environment variables already in there, like PATH and SSL_CERT_FILE. I guess I could use regex to skip those or just match those variables in the string as well (I'm not sure if they change though)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I can just use docker inspect -f {{.Config.Env}} <image>.

String history = new Command("docker", "history", imageReference).run();
Assert.assertThat(history, CoreMatchers.containsString("jib-integration-test"));
Assert.assertThat(history, CoreMatchers.containsString("bazel build ..."));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ private static void addIfNotEmpty(
@Nullable private String baseImage;
private List<String> entrypoint = Collections.emptyList();
private List<String> javaArguments = Collections.emptyList();
private Map<String, String> environment = Collections.emptyMap();
private List<String> exposedPorts = Collections.emptyList();
private Map<String, String> labels = Collections.emptyMap();

Expand Down Expand Up @@ -170,6 +171,17 @@ public JavaDockerContextGenerator setJavaArguments(List<String> javaArguments) {
return this;
}

/**
* Sets the environment variables
*
* @param environment the environment variables
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: map from environment variable name to value?

* @return this
*/
public JavaDockerContextGenerator setEnvironment(Map<String, String> environment) {
this.environment = environment;
return this;
}

/**
* Sets the exposed ports.
*
Expand Down Expand Up @@ -240,6 +252,9 @@ public void generate(Path targetDirectory) throws IOException {
*
* EXPOSE [port]
* [More EXPOSE instructions, if necessary]
* ENV [key1]="[value1]" \
* [key2]="[value2]" \
* [...]
* LABEL [key1]="[value1]" \
* [key2]="[value2]" \
* [...]
Expand Down Expand Up @@ -267,14 +282,24 @@ String makeDockerfile() throws JsonProcessingException {
dockerfile.append("\nEXPOSE ").append(port);
}

boolean firstLabel = true;
boolean firstElement = true;
for (Entry<String, String> env : environment.entrySet()) {
Copy link
Member

Choose a reason for hiding this comment

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

Reusing booleans is a error prone. Maybe this code should use java.util.StringJoiner instead — it supports a prefix.

dockerfile
.append(firstElement ? "\nENV " : " \\\n ")
.append(env.getKey())
.append("=")
.append(objectMapper.writeValueAsString(env.getValue()));
firstElement = false;
}

firstElement = true;
for (Entry<String, String> label : labels.entrySet()) {
dockerfile
.append(firstLabel ? "\nLABEL " : " \\\n ")
.append(firstElement ? "\nLABEL " : " \\\n ")
.append(label.getKey())
.append("=")
.append(objectMapper.writeValueAsString(label.getValue()));
firstLabel = false;
firstElement = false;
}

dockerfile
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public void testMakeDockerfile() throws IOException {
List<String> expectedJvmFlags = Arrays.asList("-flag", "another\"Flag");
String expectedMainClass = "SomeMainClass";
List<String> expectedJavaArguments = Arrays.asList("arg1", "arg2");
Map<String, String> expectedEnv = ImmutableMap.of("key1", "value1", "key2", "value2");
List<String> exposedPorts = Arrays.asList("1000/tcp", "2000-2010/udp");
Map<String, String> expectedLabels =
ImmutableMap.of(
Expand Down Expand Up @@ -155,6 +156,7 @@ public void testMakeDockerfile() throws IOException {
JavaEntrypointConstructor.makeDefaultEntrypoint(
expectedJvmFlags, expectedMainClass))
.setJavaArguments(expectedJavaArguments)
.setEnvironment(expectedEnv)
.setExposedPorts(exposedPorts)
.setLabels(expectedLabels)
.makeDockerfile();
Expand Down
2 changes: 2 additions & 0 deletions jib-core/src/test/resources/sampleDockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ COPY root /

EXPOSE 1000/tcp
EXPOSE 2000-2010/udp
ENV key1="value1" \
key2="value2"
LABEL key1="value" \
key2="value with\\backslashes\"and\\\\\"\"quotes\"\\" \
key3="value3"
Expand Down
2 changes: 2 additions & 0 deletions jib-gradle-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file.

### Added

- `container.environment` configuration parameter to configure environment variables ([#890](https://github.com/GoogleContainerTools/jib/issues/890))

### Changed

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public void testBuild_complex() throws IOException, InterruptedException {
String targetImage = "localhost:6000/compleximage:gradle" + System.nanoTime();
Instant beforeBuild = Instant.now();
Assert.assertEquals(
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\n",
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\nvalue1\nvalue2\n",
buildAndRunComplex(targetImage, "testuser2", "testpassword2", localRegistry2));
assertSimpleCreationTimeIsAfter(beforeBuild, targetImage);
}
Expand All @@ -248,7 +248,7 @@ public void testBuild_complex_sameFromAndToRegistry() throws IOException, Interr
String targetImage = "localhost:5000/compleximage:gradle" + System.nanoTime();
Instant beforeBuild = Instant.now();
Assert.assertEquals(
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\n",
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\nvalue1\nvalue2\n",
buildAndRunComplex(targetImage, "testuser", "testpassword", localRegistry1));
assertSimpleCreationTimeIsAfter(beforeBuild, targetImage);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jib {
args = ['An argument.']
mainClass = 'com.test.HelloWorld'
jvmFlags = ['-Xms512m', '-Xdebug']
environment = [var1:'value1', var2:'value2']
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it might be clearer if the values were like env1/env2?

ports = ['1000/tcp', '2000-2003/udp']
labels = [key1:'value1', key2:'value2']
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,12 @@ public static void main(String[] args) throws IOException, URISyntaxException {
for (String jvmFlag : ManagementFactory.getRuntimeMXBean().getInputArguments()) {
System.out.println(jvmFlag);
}

if (System.getenv("var1") != null) {
System.out.println(System.getenv("var1"));
}
if (System.getenv("var2") != null) {
System.out.println(System.getenv("var2"));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class ContainerParameters {

private boolean useCurrentTimestamp = false;
private List<String> jvmFlags = Collections.emptyList();
private Map<String, String> environment = Collections.emptyMap();
private List<String> entrypoint = Collections.emptyList();
@Nullable private String mainClass;
private List<String> args = Collections.emptyList();
Expand Down Expand Up @@ -71,6 +72,16 @@ public void setJvmFlags(List<String> jvmFlags) {
this.jvmFlags = jvmFlags;
}

@Input
@Optional
public Map<String, String> getEnvironment() {
return environment;
}

public void setEnvironment(Map<String, String> environment) {
this.environment = environment;
}

@Input
@Nullable
@Optional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ List<String> getJvmFlags() {
return container.getJvmFlags();
}

@Internal
@Optional
Map<String, String> getEnvironment() {
return container.getEnvironment();
}

// TODO: Make @Internal (deprecated)
@Input
@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ static PluginConfigurationProcessor processCommonConfiguration(
ContainerConfiguration.Builder containerConfigurationBuilder =
ContainerConfiguration.builder()
.setEntrypoint(entrypoint)
.setEnvironment(jibExtension.getEnvironment())
.setProgramArguments(jibExtension.getArgs())
.setExposedPorts(ExposedPortsParser.parse(jibExtension.getExposedPorts()))
.setLabels(jibExtension.getLabels());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,17 @@ public void testTo() {
@Test
public void testContainer() {
Assert.assertEquals(Collections.emptyList(), testJibExtension.getContainer().getJvmFlags());
Assert.assertEquals(Collections.emptyMap(), testJibExtension.getContainer().getEnvironment());
Assert.assertNull(testJibExtension.getContainer().getMainClass());
Assert.assertEquals(Collections.emptyList(), testJibExtension.getContainer().getArgs());
Assert.assertEquals(V22ManifestTemplate.class, testJibExtension.getContainer().getFormat());
Assert.assertEquals(Collections.emptyList(), testJibExtension.getContainer().getPorts());
Assert.assertEquals(Collections.emptyMap(), testJibExtension.getContainer().getLabels());

testJibExtension.container(
container -> {
container.setJvmFlags(Arrays.asList("jvmFlag1", "jvmFlag2"));
container.setEnvironment(ImmutableMap.of("var1", "value1", "var2", "value2"));
container.setEntrypoint(Arrays.asList("foo", "bar", "baz"));
container.setMainClass("mainClass");
container.setArgs(Arrays.asList("arg1", "arg2", "arg3"));
Expand All @@ -114,6 +117,9 @@ public void testContainer() {
Arrays.asList("foo", "bar", "baz"), testJibExtension.getContainer().getEntrypoint());
Assert.assertEquals(
Arrays.asList("jvmFlag1", "jvmFlag2"), testJibExtension.getContainer().getJvmFlags());
Assert.assertEquals(
ImmutableMap.of("var1", "value1", "var2", "value2"),
testJibExtension.getContainer().getEnvironment());
Assert.assertEquals("mainClass", testJibExtension.getContainer().getMainClass());
Assert.assertEquals(
Arrays.asList("arg1", "arg2", "arg3"), testJibExtension.getContainer().getArgs());
Expand Down
3 changes: 3 additions & 0 deletions jib-maven-plugin/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file.

### Added

- `<skip>` configuration parameter to skip Jib execution in multi-module projects ([#865](https://github.com/GoogleContainerTools/jib/issues/865))
Copy link
Member

Choose a reason for hiding this comment

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

and add also settable via property jib.skip

- `<container><environment>` configuration parameter to configure environment variables ([#890](https://github.com/GoogleContainerTools/jib/issues/890))

### Changed

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

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

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

@Nullable @Parameter private String mainClass;

@Parameter private List<String> args = Collections.emptyList();
Expand Down Expand Up @@ -152,8 +154,6 @@ public static class ContainerParameters {

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

@Nullable @Parameter private Map<String, String> environment;

@Deprecated @Nullable @Parameter private String mainClass;

@Deprecated @Parameter private List<String> args = Collections.emptyList();
Expand Down Expand Up @@ -268,7 +268,7 @@ List<String> getJvmFlags() {

@Nullable
Map<String, String> getEnvironment() {
return environment;
return container.environment;
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public void testExecute_complex()
throws IOException, InterruptedException, VerificationException {
String targetImage = "localhost:6000/compleximage:maven" + System.nanoTime();
Assert.assertEquals(
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\n",
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\nvalue1\nvalue2\n",
buildAndRunComplex(targetImage, "testuser2", "testpassword2", localRegistry2));
}

Expand All @@ -270,7 +270,7 @@ public void testExecute_complex_sameFromAndToRegistry()
throws IOException, InterruptedException, VerificationException {
String targetImage = "localhost:5000/compleximage:maven" + System.nanoTime();
Assert.assertEquals(
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\n",
"Hello, world. An argument.\nfoo\ncat\n-Xms512m\n-Xdebug\nvalue1\nvalue2\n",
buildAndRunComplex(targetImage, "testuser", "testpassword", localRegistry1));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@
<jvmFlag>-Xms512m</jvmFlag>
<jvmFlag>-Xdebug</jvmFlag>
</jvmFlags>
<environment>
<var1>value1</var1>
<var2>value2</var2>
</environment>
<ports>
<port>1000/tcp</port>
<port>2000-2003/udp</port>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,12 @@ public static void main(String[] args) throws IOException, URISyntaxException {
for (String jvmFlag : ManagementFactory.getRuntimeMXBean().getInputArguments()) {
System.out.println(jvmFlag);
}

if (System.getenv("var1") != null) {
System.out.println(System.getenv("var1"));
}
if (System.getenv("var2") != null) {
System.out.println(System.getenv("var2"));
}
}
}