Propagate base image's environment variable config to built image

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

@@ -39,6 +39,7 @@ class BuildImageStep
   private static final String DESCRIPTION = "Building container configuration";
 
   private final BuildConfiguration buildConfiguration;
+  private final PullBaseImageStep pullBaseImageStep;
   private final PullAndCacheBaseImageLayersStep pullAndCacheBaseImageLayersStep;
   private final ImmutableList<BuildAndCacheApplicationLayerStep> buildAndCacheApplicationLayerSteps;
 
@@ -48,15 +49,18 @@ class BuildImageStep
   BuildImageStep(
       ListeningExecutorService listeningExecutorService,
       BuildConfiguration buildConfiguration,
+      PullBaseImageStep pullBaseImageStep,
       PullAndCacheBaseImageLayersStep pullAndCacheBaseImageLayersStep,
       ImmutableList<BuildAndCacheApplicationLayerStep> buildAndCacheApplicationLayerSteps) {
     this.listeningExecutorService = listeningExecutorService;
     this.buildConfiguration = buildConfiguration;
+    this.pullBaseImageStep = pullBaseImageStep;
     this.pullAndCacheBaseImageLayersStep = pullAndCacheBaseImageLayersStep;
     this.buildAndCacheApplicationLayerSteps = buildAndCacheApplicationLayerSteps;
 
     listenableFuture =
-        Futures.whenAllSucceed(pullAndCacheBaseImageLayersStep.getFuture())
+        Futures.whenAllSucceed(
+                pullBaseImageStep.getFuture(), pullAndCacheBaseImageLayersStep.getFuture())
             .call(this, listeningExecutorService);
   }
 
@@ -96,8 +100,13 @@ private Image<CachedLayer> afterCachedLayersSteps()
           buildAndCacheApplicationLayerSteps) {
         imageBuilder.addLayer(NonBlockingSteps.get(buildAndCacheApplicationLayerStep));
       }
+
+      // Start with environment from base image and overlay build configuration
+      imageBuilder.addEnvironment(
+          NonBlockingSteps.get(pullBaseImageStep).getBaseImage().getEnvironment());
+      imageBuilder.addEnvironment(buildConfiguration.getEnvironment());
+
       imageBuilder.setCreated(buildConfiguration.getCreationTime());
-      imageBuilder.setEnvironment(buildConfiguration.getEnvironment());
       imageBuilder.setEntrypoint(buildConfiguration.getEntrypoint());
       imageBuilder.setJavaArguments(buildConfiguration.getJavaArguments());
       imageBuilder.setExposedPorts(buildConfiguration.getExposedPorts());

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

@@ -40,6 +40,7 @@
 import com.google.cloud.tools.jib.registry.RegistryClient;
 import com.google.cloud.tools.jib.registry.RegistryException;
 import com.google.cloud.tools.jib.registry.RegistryUnauthorizedException;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.common.util.concurrent.MoreExecutors;
@@ -62,7 +63,8 @@ static class BaseImageWithAuthorization {
     private final Image<Layer> baseImage;
     private final @Nullable Authorization baseImageAuthorization;
 
-    private BaseImageWithAuthorization(
+    @VisibleForTesting
+    BaseImageWithAuthorization(
         Image<Layer> baseImage, @Nullable Authorization baseImageAuthorization) {
       this.baseImage = baseImage;
       this.baseImageAuthorization = baseImageAuthorization;

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

@@ -131,6 +131,7 @@ public StepsRunner runBuildImageStep() {
         new BuildImageStep(
             listeningExecutorService,
             buildConfiguration,
+            Preconditions.checkNotNull(pullBaseImageStep),
             Preconditions.checkNotNull(pullAndCacheBaseImageLayersStep),
             Preconditions.checkNotNull(buildAndCacheApplicationLayerSteps));
     return this;

jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java

@@ -18,6 +18,7 @@
 
 import com.google.cloud.tools.jib.configuration.Port;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
@@ -30,7 +31,7 @@ public class Image<T extends Layer> {
   public static class Builder<T extends Layer> {
 
     private final ImageLayers.Builder<T> imageLayersBuilder = ImageLayers.builder();
-    private ImmutableList.Builder<String> environmentBuilder = ImmutableList.builder();
+    private ImmutableMap.Builder<String, String> environmentBuilder = ImmutableMap.builder();
 
     @Nullable private Instant created;
     @Nullable private ImmutableList<String> entrypoint;
@@ -49,18 +50,14 @@ public Builder<T> setCreated(Instant created) {
     }
 
     /**
-     * Sets the environment with a map from environment variable names to values.
+     * Adds a map of environment variables to the current map.
      *
      * @param environment the map of environment variables
      * @return this
      */
-    public Builder<T> setEnvironment(@Nullable Map<String, String> environment) {
-      if (environment == null) {
-        this.environmentBuilder = ImmutableList.builder();
-      } else {
-        for (Map.Entry<String, String> environmentVariable : environment.entrySet()) {
-          setEnvironmentVariable(environmentVariable.getKey(), environmentVariable.getValue());
-        }
+    public Builder<T> addEnvironment(@Nullable Map<String, String> environment) {
+      if (environment != null) {
+        this.environmentBuilder.putAll(environment);
       }
       return this;
     }
@@ -73,18 +70,7 @@ public Builder<T> setEnvironment(@Nullable Map<String, String> environment) {
      * @return this
      */
     public Builder<T> setEnvironmentVariable(String name, String value) {
-      environmentBuilder.add(name + "=" + value);
-      return this;
-    }
-
-    /**
-     * Adds an environment variable definition in the format {@code NAME=VALUE}.
-     *
-     * @param environmentVariableDefinition the definition to add
-     * @return this
-     */
-    public Builder<T> addEnvironmentVariableDefinition(String environmentVariableDefinition) {
-      environmentBuilder.add(environmentVariableDefinition);
+      environmentBuilder.put(name, value);
       return this;
     }
 
@@ -167,7 +153,7 @@ public static <T extends Layer> Builder<T> builder() {
   private final ImageLayers<T> layers;
 
   /** Environment variable definitions for running the image, in the format {@code NAME=VALUE}. */
-  @Nullable private final ImmutableList<String> environment;
+  @Nullable private final ImmutableMap<String, String> environment;
 
   /** Initial command to run when running the image. */
   @Nullable private final ImmutableList<String> entrypoint;
@@ -181,7 +167,7 @@ public static <T extends Layer> Builder<T> builder() {
   private Image(
       @Nullable Instant created,
       ImageLayers<T> layers,
-      @Nullable ImmutableList<String> environment,
+      @Nullable ImmutableMap<String, String> environment,
       @Nullable ImmutableList<String> entrypoint,
       @Nullable ImmutableList<String> javaArguments,
       @Nullable ImmutableList<Port> exposedPorts) {
@@ -199,7 +185,7 @@ public Instant getCreated() {
   }
 
   @Nullable
-  public ImmutableList<String> getEnvironment() {
+  public ImmutableMap<String, String> getEnvironment() {
     return environment;
   }
 

jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslator.java

@@ -24,6 +24,8 @@
 import com.google.cloud.tools.jib.image.Image;
 import com.google.cloud.tools.jib.json.JsonTemplateMapper;
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSortedMap;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Collections;
@@ -67,6 +69,27 @@ public class ImageToJsonTranslator {
     return result.build();
   }
 
+  /**
+   * Converts the map of environment variables to a list with items in the format "NAME=VALUE".
+   *
+   * @return the list
+   */
+  @VisibleForTesting
+  @Nullable
+  static ImmutableList<String> environmentMapToList(@Nullable Map<String, String> environment) {
+    if (environment == null) {
+      return null;
+    }
+    Preconditions.checkArgument(
+        environment.keySet().stream().noneMatch(key -> key.contains("=")),
+        "Illegal environment variable: name cannot contain '='");
+    return environment
+        .entrySet()
+        .stream()
+        .map(entry -> entry.getKey() + "=" + entry.getValue())
+        .collect(ImmutableList.toImmutableList());
+  }
+
   private final Image<CachedLayer> image;
 
   /**
@@ -96,7 +119,7 @@ public Blob getContainerConfigurationBlob() {
     template.setCreated(image.getCreated() == null ? null : image.getCreated().toString());
 
     // Adds the environment variables.
-    template.setContainerEnvironment(image.getEnvironment());
+    template.setContainerEnvironment(environmentMapToList(image.getEnvironment()));
 
     // Sets the entrypoint.
     template.setContainerEntrypoint(image.getEntrypoint());

jib-core/src/main/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslator.java

@@ -47,7 +47,17 @@ public class JsonToImageTranslator {
    *
    * <p>Example matches: 100, 1000/tcp, 2000/udp
    */
-  private static final Pattern portPattern = Pattern.compile("(\\d+)(?:/(tcp|udp))?");
+  private static final Pattern PORT_PATTERN =
+      Pattern.compile("(?<portNum>\\d+)(?:/(?<protocol>tcp|udp))?");
+
+  /**
+   * Pattern used for parsing environment variables in the format {@code NAME=VALUE}. {@code NAME}
+   * should not contain an '='.
+   *
+   * <p>Example matches: NAME=VALUE, A12345=$$$$$
+   */
+  @VisibleForTesting
+  static final Pattern ENVIRONMENT_PATTERN = Pattern.compile("(?<name>[^=]+)=(?<value>.*)");
 
   /**
    * Translates {@link V21ManifestTemplate} to {@link Image}.
@@ -139,7 +149,12 @@ public static Image<Layer> toImage(
 
     if (containerConfigurationTemplate.getContainerEnvironment() != null) {
       for (String environmentVariable : containerConfigurationTemplate.getContainerEnvironment()) {
-        imageBuilder.addEnvironmentVariableDefinition(environmentVariable);
+        Matcher matcher = ENVIRONMENT_PATTERN.matcher(environmentVariable);
+        if (!matcher.matches()) {
+          throw new BadContainerConfigurationFormatException(
+              "Invalid environment variable definition: " + environmentVariable);
+        }
+        imageBuilder.setEnvironmentVariable(matcher.group("name"), matcher.group("value"));
       }
     }
 
@@ -162,14 +177,14 @@ static ImmutableList<Port> portMapToList(@Nullable Map<String, Map<?, ?>> portMa
     ImmutableList.Builder<Port> ports = new ImmutableList.Builder<>();
     for (Map.Entry<String, Map<?, ?>> entry : portMap.entrySet()) {
       String port = entry.getKey();
-      Matcher matcher = portPattern.matcher(port);
+      Matcher matcher = PORT_PATTERN.matcher(port);
       if (!matcher.matches()) {
         throw new BadContainerConfigurationFormatException(
             "Invalid port configuration: '" + port + "'.");
       }
 
-      int portNumber = Integer.parseInt(matcher.group(1));
-      String protocol = matcher.group(2);
+      int portNumber = Integer.parseInt(matcher.group("portNum"));
+      String protocol = matcher.group("protocol");
       ports.add(new Port(portNumber, Protocol.getFromString(protocol)));
     }
     return ports.build();

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

@@ -23,6 +23,7 @@
 import com.google.cloud.tools.jib.configuration.BuildConfiguration;
 import com.google.cloud.tools.jib.image.DescriptorDigest;
 import com.google.cloud.tools.jib.image.Image;
+import com.google.cloud.tools.jib.image.Layer;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.Futures;
@@ -46,6 +47,7 @@ public class BuildImageStepTest {
 
   @Mock private BuildConfiguration mockBuildConfiguration;
   @Mock private BuildLogger mockBuildLogger;
+  @Mock private PullBaseImageStep mockPullBaseImageStep;
   @Mock private PullAndCacheBaseImageLayersStep mockPullAndCacheBaseImageLayersStep;
   @Mock private PullAndCacheBaseImageLayerStep mockPullAndCacheBaseImageLayerStep;
   @Mock private BuildAndCacheApplicationLayerStep mockBuildAndCacheApplicationLayerStep;
@@ -70,15 +72,21 @@ public void setUp() throws DigestException {
     Mockito.when(mockBuildConfiguration.getExposedPorts()).thenReturn(ImmutableList.of());
     Mockito.when(mockBuildConfiguration.getEntrypoint()).thenReturn(ImmutableList.of());
 
+    Image<Layer> baseImage =
+        Image.builder().addEnvironment(ImmutableMap.of("NAME", "VALUE")).build();
+    Mockito.when(mockPullAndCacheBaseImageLayerStep.getFuture())
+        .thenReturn(Futures.immediateFuture(testCachedLayer));
     Mockito.when(mockPullAndCacheBaseImageLayersStep.getFuture())
         .thenReturn(
             Futures.immediateFuture(
                 ImmutableList.of(
                     mockPullAndCacheBaseImageLayerStep,
                     mockPullAndCacheBaseImageLayerStep,
                     mockPullAndCacheBaseImageLayerStep)));
-    Mockito.when(mockPullAndCacheBaseImageLayerStep.getFuture())
-        .thenReturn(Futures.immediateFuture(testCachedLayer));
+    Mockito.when(mockPullBaseImageStep.getFuture())
+        .thenReturn(
+            Futures.immediateFuture(
+                new PullBaseImageStep.BaseImageWithAuthorization(baseImage, null)));
     Mockito.when(mockBuildAndCacheApplicationLayerStep.getFuture())
         .thenReturn(Futures.immediateFuture(testCachedLayer));
   }
@@ -89,6 +97,7 @@ public void test_validateAsyncDependencies() throws ExecutionException, Interrup
         new BuildImageStep(
             MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()),
             mockBuildConfiguration,
+            mockPullBaseImageStep,
             mockPullAndCacheBaseImageLayersStep,
             ImmutableList.of(
                 mockBuildAndCacheApplicationLayerStep,
@@ -98,4 +107,23 @@ public void test_validateAsyncDependencies() throws ExecutionException, Interrup
     Assert.assertEquals(
         testDescriptorDigest, image.getLayers().asList().get(0).getBlobDescriptor().getDigest());
   }
+
+  @Test
+  public void test_propagateBaseImageConfiguration()
+      throws ExecutionException, InterruptedException {
+    Mockito.when(mockBuildConfiguration.getEnvironment())
+        .thenReturn(ImmutableMap.of("BASE", "IMAGE"));
+    BuildImageStep buildImageStep =
+        new BuildImageStep(
+            MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor()),
+            mockBuildConfiguration,
+            mockPullBaseImageStep,
+            mockPullAndCacheBaseImageLayersStep,
+            ImmutableList.of(
+                mockBuildAndCacheApplicationLayerStep,
+                mockBuildAndCacheApplicationLayerStep,
+                mockBuildAndCacheApplicationLayerStep));
+    Image<CachedLayer> image = buildImageStep.getFuture().get().getFuture().get();
+    Assert.assertEquals(ImmutableMap.of("NAME", "VALUE", "BASE", "IMAGE"), image.getEnvironment());
+  }
 }

jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageTest.java

@@ -20,6 +20,7 @@
 import com.google.cloud.tools.jib.configuration.Port;
 import com.google.cloud.tools.jib.configuration.Port.Protocol;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import java.time.Instant;
 import java.util.Arrays;
 import org.junit.Assert;
@@ -45,9 +46,6 @@ public void setUp() throws LayerPropertyNotFoundException {
 
   @Test
   public void test_smokeTest() throws LayerPropertyNotFoundException {
-    ImmutableList<String> expectedEnvironment =
-        ImmutableList.of("crepecake=is great", "VARIABLE=VALUE");
-
     Image<Layer> image =
         Image.builder()
             .setCreated(Instant.ofEpochSecond(10000))
@@ -63,7 +61,8 @@ public void test_smokeTest() throws LayerPropertyNotFoundException {
     Assert.assertEquals(
         mockDescriptorDigest, image.getLayers().get(0).getBlobDescriptor().getDigest());
     Assert.assertEquals(Instant.ofEpochSecond(10000), image.getCreated());
-    Assert.assertEquals(expectedEnvironment, image.getEnvironment());
+    Assert.assertEquals(
+        ImmutableMap.of("crepecake", "is great", "VARIABLE", "VALUE"), image.getEnvironment());
     Assert.assertEquals(Arrays.asList("some", "command"), image.getEntrypoint());
     Assert.assertEquals(Arrays.asList("arg1", "arg2"), image.getJavaArguments());
     Assert.assertEquals(

jib-core/src/test/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslatorTest.java

@@ -111,6 +111,13 @@ public void testPortListToMap() {
     Assert.assertEquals(expected, ImageToJsonTranslator.portListToMap(input));
   }
 
+  @Test
+  public void testEnvironmentMapToList() {
+    ImmutableMap<String, String> input = ImmutableMap.of("NAME1", "VALUE1", "NAME2", "VALUE2");
+    ImmutableList<String> expected = ImmutableList.of("NAME1=VALUE1", "NAME2=VALUE2");
+    Assert.assertEquals(expected, ImageToJsonTranslator.environmentMapToList(input));
+  }
+
   /** Tests translation of image to {@link BuildableManifestTemplate}. */
   private <T extends BuildableManifestTemplate> void testGetManifest(
       Class<T> manifestTemplateClass, String translatedJsonFilename)