Propagate base image's environment variable config to built image

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

@@ -29,6 +29,7 @@
 import com.google.common.util.concurrent.ListeningExecutorService;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 
@@ -39,6 +40,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 +50,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 +101,16 @@ private Image<CachedLayer> afterCachedLayersSteps()
           buildAndCacheApplicationLayerSteps) {
         imageBuilder.addLayer(NonBlockingSteps.get(buildAndCacheApplicationLayerStep));
       }
+
+      // Use environment from base image
+      imageBuilder.setEnvironment(
+          NonBlockingSteps.get(pullBaseImageStep).getBaseImage().getEnvironment());
+      if (buildConfiguration.getEnvironment() != null) {
+        for (Map.Entry<String, String> entry : buildConfiguration.getEnvironment().entrySet()) {
+          imageBuilder.setEnvironmentVariable(entry.getKey(), entry.getValue());
+        }
+      }
       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;
@@ -56,11 +57,9 @@ public Builder<T> setCreated(Instant created) {
      */
     public Builder<T> setEnvironment(@Nullable Map<String, String> environment) {
       if (environment == null) {
-        this.environmentBuilder = ImmutableList.builder();
+        this.environmentBuilder = ImmutableMap.builder();
       } else {
-        for (Map.Entry<String, String> environmentVariable : environment.entrySet()) {
-          setEnvironmentVariable(environmentVariable.getKey(), environmentVariable.getValue());
-        }
+        this.environmentBuilder.putAll(ImmutableMap.copyOf(environment));
       }
       return this;
     }
@@ -73,18 +72,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 +155,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 +169,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 +187,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,7 @@
 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.collect.ImmutableList;
 import com.google.common.collect.ImmutableSortedMap;
 import java.lang.reflect.InvocationTargetException;
 import java.util.Collections;
@@ -67,6 +68,24 @@ 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;
+    }
+    ImmutableList.Builder<String> builder = ImmutableList.builder();
+    for (Map.Entry<String, String> environmentVariable : environment.entrySet()) {
+      builder.add(environmentVariable.getKey() + "=" + environmentVariable.getValue());
+    }
+    return builder.build();
+  }
+
   private final Image<CachedLayer> image;
 
   /**
@@ -96,7 +115,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

@@ -49,6 +49,15 @@ public class JsonToImageTranslator {
    */
   private static final Pattern portPattern = Pattern.compile("(\\d+)(?:/(tcp|udp))?");
 
+  /**
+   * Pattern used for parsing environment variables in the format {@code NAME=VALUE}. {@code NAME}
+   * should start with a letter and contain only letters (uppercase and lower) and numbers. {@code
+   * VALUE} can contain anything.
+   *
+   * <p>Example matches: NAME=VALUE, A12345=$$$$$
+   */
+  @VisibleForTesting static final Pattern environmentPattern = Pattern.compile("([^=].*?)=(.*)");
+
   /**
    * Translates {@link V21ManifestTemplate} to {@link Image}.
    *
@@ -139,7 +148,14 @@ public static Image<Layer> toImage(
 
     if (containerConfigurationTemplate.getContainerEnvironment() != null) {
       for (String environmentVariable : containerConfigurationTemplate.getContainerEnvironment()) {
-        imageBuilder.addEnvironmentVariableDefinition(environmentVariable);
+        Matcher matcher = environmentPattern.matcher(environmentVariable);
+        if (!matcher.matches()) {
+          throw new BadContainerConfigurationFormatException(
+              "Invalid environment variable definition: " + environmentVariable);
+        }
+        String name = matcher.group(1);
+        String value = matcher.group(2);
+        imageBuilder.setEnvironmentVariable(name, value);
       }
     }
 

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().setEnvironment(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)

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

@@ -37,6 +37,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -110,6 +111,31 @@ public void testPortMapToList() throws BadContainerConfigurationFormatException
     }
   }
 
+  @Test
+  public void testJsonToImageTranslatorRegex() {
+    assertGoodEnvironmentPattern("NAME=VALUE", "NAME", "VALUE");
+    assertGoodEnvironmentPattern("A1203921=www=ww", "A1203921", "www=ww");
+    assertGoodEnvironmentPattern("&*%(&#$(*@(%&@$*$(=", "&*%(&#$(*@(%&@$*$(", "");
+    assertGoodEnvironmentPattern("m_a_8943=100", "m_a_8943", "100");
+    assertGoodEnvironmentPattern("A_B_C_D=*****", "A_B_C_D", "*****");
+
+    assertBadEnvironmentPattern("=================");
+    assertBadEnvironmentPattern("A_B_C");
+  }
+
+  private void assertGoodEnvironmentPattern(
+      String input, String expectedName, String expectedValue) {
+    Matcher matcher = JsonToImageTranslator.environmentPattern.matcher(input);
+    Assert.assertTrue(matcher.matches());
+    Assert.assertEquals(expectedName, matcher.group(1));
+    Assert.assertEquals(expectedValue, matcher.group(2));
+  }
+
+  private void assertBadEnvironmentPattern(String input) {
+    Matcher matcher = JsonToImageTranslator.environmentPattern.matcher(input);
+    Assert.assertFalse(matcher.matches());
+  }
+
   private <T extends BuildableManifestTemplate> void testToImage_buildable(
       String jsonFilename, Class<T> manifestTemplateClass)
       throws IOException, LayerPropertyNotFoundException, LayerCountMismatchException,
@@ -144,7 +170,7 @@ private <T extends BuildableManifestTemplate> void testToImage_buildable(
         layers.get(0).getDiffId());
     Assert.assertEquals(Instant.ofEpochSecond(20), image.getCreated());
     Assert.assertEquals(Arrays.asList("some", "entrypoint", "command"), image.getEntrypoint());
-    Assert.assertEquals(Arrays.asList("VAR1=VAL1", "VAR2=VAL2"), image.getEnvironment());
+    Assert.assertEquals(ImmutableMap.of("VAR1", "VAL1", "VAR2", "VAL2"), image.getEnvironment());
     Assert.assertEquals(
         ImmutableList.of(
             new Port(1000, Protocol.TCP),