From ad15f70e6d482db51017b50bb365c9f360571238 Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Mon, 21 Nov 2016 09:01:22 +0000 Subject: [PATCH 1/2] LocalBuilder: Allow additionalEnvironment to override systemEnvironment The docker reserved variables (DOCKER_TLS_VERIFY, DOCKER_HOST, DOCKER_CERT_PATH) are still not allowed in additionalEnvironment. But the user can now override their own environment variables programatically. (See AtlasDB PR 1237). Fixes #131. --- .../docker/compose/connection/DockerMachine.java | 7 +++++-- .../compose/connection/LocalBuilderShould.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java b/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java index c3ec0478c..e377f43bd 100644 --- a/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java +++ b/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java @@ -22,6 +22,7 @@ import static com.palantir.docker.compose.configuration.EnvironmentVariables.DOCKER_TLS_VERIFY; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.palantir.docker.compose.configuration.AdditionalEnvironmentValidator; import com.palantir.docker.compose.configuration.DockerType; import com.palantir.docker.compose.configuration.RemoteHostIpResolver; @@ -100,9 +101,11 @@ public LocalBuilder withEnvironment(Map newEnvironment) { public DockerMachine build() { dockerType.validateEnvironmentVariables(systemEnvironment); AdditionalEnvironmentValidator.validate(additionalEnvironment); + Map combinedEnvironment = Maps.newHashMap(); + combinedEnvironment.putAll(systemEnvironment); + combinedEnvironment.putAll(additionalEnvironment); Map environment = ImmutableMap.builder() - .putAll(systemEnvironment) - .putAll(additionalEnvironment) + .putAll(combinedEnvironment) .build(); String dockerHost = systemEnvironment.getOrDefault(DOCKER_HOST, ""); diff --git a/src/test/java/com/palantir/docker/compose/connection/LocalBuilderShould.java b/src/test/java/com/palantir/docker/compose/connection/LocalBuilderShould.java index cbed032e2..df13660a7 100644 --- a/src/test/java/com/palantir/docker/compose/connection/LocalBuilderShould.java +++ b/src/test/java/com/palantir/docker/compose/connection/LocalBuilderShould.java @@ -111,6 +111,22 @@ public void get_variable_overriden_with_additional_environment() throws Exceptio assertThat(localMachine, containsEnvironment(expected)); } + @Test + public void override_system_environment_with_additional_environment() throws Exception { + Map systemEnv = ImmutableMap.builder() + .put("ENV_1", "VAL_1") + .build(); + Map overrideEnv = ImmutableMap.builder() + .put("ENV_1", "DIFFERENT_VALUE") + .build(); + DockerMachine localMachine = new LocalBuilder(DAEMON, systemEnv) + .withEnvironment(overrideEnv) + .build(); + + assertThat(localMachine, not(containsEnvironment(systemEnv))); + assertThat(localMachine, containsEnvironment(overrideEnv)); + } + @Test public void have_invalid_variables_daemon() throws Exception { Map invalidDockerVariables = ImmutableMap.builder() From 0356ee72cb77f6eb5262db2fe92aa176b5ef5d84 Mon Sep 17 00:00:00 2001 From: Glenn Sheasby Date: Tue, 22 Nov 2016 10:17:07 +0000 Subject: [PATCH 2/2] Use ImmutableMap.copyOf rather than .builder --- .../palantir/docker/compose/connection/DockerMachine.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java b/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java index e377f43bd..0140d8b05 100644 --- a/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java +++ b/src/main/java/com/palantir/docker/compose/connection/DockerMachine.java @@ -22,7 +22,6 @@ import static com.palantir.docker.compose.configuration.EnvironmentVariables.DOCKER_TLS_VERIFY; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Maps; import com.palantir.docker.compose.configuration.AdditionalEnvironmentValidator; import com.palantir.docker.compose.configuration.DockerType; import com.palantir.docker.compose.configuration.RemoteHostIpResolver; @@ -101,15 +100,12 @@ public LocalBuilder withEnvironment(Map newEnvironment) { public DockerMachine build() { dockerType.validateEnvironmentVariables(systemEnvironment); AdditionalEnvironmentValidator.validate(additionalEnvironment); - Map combinedEnvironment = Maps.newHashMap(); + Map combinedEnvironment = newHashMap(); combinedEnvironment.putAll(systemEnvironment); combinedEnvironment.putAll(additionalEnvironment); - Map environment = ImmutableMap.builder() - .putAll(combinedEnvironment) - .build(); String dockerHost = systemEnvironment.getOrDefault(DOCKER_HOST, ""); - return new DockerMachine(dockerType.resolveIp(dockerHost), environment); + return new DockerMachine(dockerType.resolveIp(dockerHost), ImmutableMap.copyOf(combinedEnvironment)); } }