diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildImageStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildImageStep.java index b2030bd982..9ebd5a4589 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/BuildImageStep.java +++ b/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 buildAndCacheApplicationLayerSteps; @@ -48,15 +49,18 @@ class BuildImageStep BuildImageStep( ListeningExecutorService listeningExecutorService, BuildConfiguration buildConfiguration, + PullBaseImageStep pullBaseImageStep, PullAndCacheBaseImageLayersStep pullAndCacheBaseImageLayersStep, ImmutableList 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 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()); diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java index b528f6d85a..3d758a82f4 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/PullBaseImageStep.java +++ b/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 baseImage; private final @Nullable Authorization baseImageAuthorization; - private BaseImageWithAuthorization( + @VisibleForTesting + BaseImageWithAuthorization( Image baseImage, @Nullable Authorization baseImageAuthorization) { this.baseImage = baseImage; this.baseImageAuthorization = baseImageAuthorization; diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java b/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java index ff959b9b31..936ed8619f 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/builder/steps/StepsRunner.java +++ b/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; diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java b/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java index 58704e2b7a..60d3c2d7f3 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java +++ b/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 { public static class Builder { private final ImageLayers.Builder imageLayersBuilder = ImageLayers.builder(); - private ImmutableList.Builder environmentBuilder = ImmutableList.builder(); + private ImmutableMap.Builder environmentBuilder = ImmutableMap.builder(); @Nullable private Instant created; @Nullable private ImmutableList entrypoint; @@ -49,18 +50,14 @@ public Builder 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 setEnvironment(@Nullable Map environment) { - if (environment == null) { - this.environmentBuilder = ImmutableList.builder(); - } else { - for (Map.Entry environmentVariable : environment.entrySet()) { - setEnvironmentVariable(environmentVariable.getKey(), environmentVariable.getValue()); - } + public Builder addEnvironment(@Nullable Map environment) { + if (environment != null) { + this.environmentBuilder.putAll(environment); } return this; } @@ -73,18 +70,7 @@ public Builder setEnvironment(@Nullable Map environment) { * @return this */ public Builder 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 addEnvironmentVariableDefinition(String environmentVariableDefinition) { - environmentBuilder.add(environmentVariableDefinition); + environmentBuilder.put(name, value); return this; } @@ -167,7 +153,7 @@ public static Builder builder() { private final ImageLayers layers; /** Environment variable definitions for running the image, in the format {@code NAME=VALUE}. */ - @Nullable private final ImmutableList environment; + @Nullable private final ImmutableMap environment; /** Initial command to run when running the image. */ @Nullable private final ImmutableList entrypoint; @@ -181,7 +167,7 @@ public static Builder builder() { private Image( @Nullable Instant created, ImageLayers layers, - @Nullable ImmutableList environment, + @Nullable ImmutableMap environment, @Nullable ImmutableList entrypoint, @Nullable ImmutableList javaArguments, @Nullable ImmutableList exposedPorts) { @@ -199,7 +185,7 @@ public Instant getCreated() { } @Nullable - public ImmutableList getEnvironment() { + public ImmutableMap getEnvironment() { return environment; } diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslator.java b/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslator.java index 5731005163..8e07d870a4 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslator.java +++ b/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 environmentMapToList(@Nullable Map 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 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()); diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslator.java b/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslator.java index 890ce1d0c3..a6ac326c1f 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslator.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslator.java @@ -47,7 +47,17 @@ public class JsonToImageTranslator { * *

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("(?\\d+)(?:/(?tcp|udp))?"); + + /** + * Pattern used for parsing environment variables in the format {@code NAME=VALUE}. {@code NAME} + * should not contain an '='. + * + *

Example matches: NAME=VALUE, A12345=$$$$$ + */ + @VisibleForTesting + static final Pattern ENVIRONMENT_PATTERN = Pattern.compile("(?[^=]+)=(?.*)"); /** * Translates {@link V21ManifestTemplate} to {@link Image}. @@ -139,7 +149,12 @@ public static Image 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 portMapToList(@Nullable Map> portMa ImmutableList.Builder ports = new ImmutableList.Builder<>(); for (Map.Entry> 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(); diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildImageStepTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildImageStepTest.java index 1ee4181982..188989484b 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/builder/steps/BuildImageStepTest.java +++ b/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,6 +72,10 @@ public void setUp() throws DigestException { Mockito.when(mockBuildConfiguration.getExposedPorts()).thenReturn(ImmutableList.of()); Mockito.when(mockBuildConfiguration.getEntrypoint()).thenReturn(ImmutableList.of()); + Image baseImage = + Image.builder().addEnvironment(ImmutableMap.of("NAME", "VALUE")).build(); + Mockito.when(mockPullAndCacheBaseImageLayerStep.getFuture()) + .thenReturn(Futures.immediateFuture(testCachedLayer)); Mockito.when(mockPullAndCacheBaseImageLayersStep.getFuture()) .thenReturn( Futures.immediateFuture( @@ -77,8 +83,10 @@ public void setUp() throws DigestException { 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 image = buildImageStep.getFuture().get().getFuture().get(); + Assert.assertEquals(ImmutableMap.of("NAME", "VALUE", "BASE", "IMAGE"), image.getEnvironment()); + } } diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageTest.java index c78f47febe..f21fe3396b 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/image/ImageTest.java +++ b/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 expectedEnvironment = - ImmutableList.of("crepecake=is great", "VARIABLE=VALUE"); - Image 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( diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslatorTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslatorTest.java index 77125884c7..8e6b3e2451 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslatorTest.java +++ b/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 input = ImmutableMap.of("NAME1", "VALUE1", "NAME2", "VALUE2"); + ImmutableList expected = ImmutableList.of("NAME1=VALUE1", "NAME2=VALUE2"); + Assert.assertEquals(expected, ImageToJsonTranslator.environmentMapToList(input)); + } + /** Tests translation of image to {@link BuildableManifestTemplate}. */ private void testGetManifest( Class manifestTemplateClass, String translatedJsonFilename) diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslatorTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslatorTest.java index 5953ad0018..267d3915f7 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/image/json/JsonToImageTranslatorTest.java +++ b/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.ENVIRONMENT_PATTERN.matcher(input); + Assert.assertTrue(matcher.matches()); + Assert.assertEquals(expectedName, matcher.group("name")); + Assert.assertEquals(expectedValue, matcher.group("value")); + } + + private void assertBadEnvironmentPattern(String input) { + Matcher matcher = JsonToImageTranslator.ENVIRONMENT_PATTERN.matcher(input); + Assert.assertFalse(matcher.matches()); + } + private void testToImage_buildable( String jsonFilename, Class manifestTemplateClass) throws IOException, LayerPropertyNotFoundException, LayerCountMismatchException, @@ -144,7 +170,7 @@ private 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), diff --git a/jib-gradle-plugin/CHANGELOG.md b/jib-gradle-plugin/CHANGELOG.md index 009e3327ab..3fd325eba8 100644 --- a/jib-gradle-plugin/CHANGELOG.md +++ b/jib-gradle-plugin/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. - Only builds non-empty layers ([#516](https://github.com/GoogleContainerTools/jib/pull/516/files)) - Fixed slow image reference parsing ([#680](https://github.com/GoogleContainerTools/jib/pull/680)) +- Propagates environment variables from the base image ([#716](https://github.com/GoogleContainerTools/jib/pull/716)) ## 0.9.7 diff --git a/jib-maven-plugin/CHANGELOG.md b/jib-maven-plugin/CHANGELOG.md index bfe5e9db41..3095735226 100644 --- a/jib-maven-plugin/CHANGELOG.md +++ b/jib-maven-plugin/CHANGELOG.md @@ -7,6 +7,8 @@ All notable changes to this project will be documented in this file. ### Changed +- Propagates environment variables from the base image ([#716](https://github.com/GoogleContainerTools/jib/pull/716)) + ### Fixed - Fixed slow image reference parsing ([#680](https://github.com/GoogleContainerTools/jib/pull/680))