From 110a8f88eb70a258cdd60be6ff856e181572dac4 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Fri, 23 Feb 2024 12:24:09 +0530 Subject: [PATCH] fix (BuildService) : Image Build Config BuildArgs should be passed while pulling images (#1756) Fixes regression introduced by #1731 In #1731, we added support for specifying docker build args from maven/system properties. However, I missed it in review that build args specified in plugin image build configuration are no longer passed to autoPullBaseImage. Ensure we merge image build configuration build args with args provided via maven/system properties before passing them to autoPullBaseImage Signed-off-by: Rohan Kumar --- it/dockerfile-base-as-arg-buildconfig/pom.xml | 46 +++++++++++++++++++ .../src/main/docker/Dockerfile | 2 + it/pom.xml | 1 + .../maven/docker/service/BuildService.java | 2 +- .../docker/service/BuildServiceTest.java | 33 +++++++++++++ .../docker/util/Dockerfile_from_build_arg | 2 + 6 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 it/dockerfile-base-as-arg-buildconfig/pom.xml create mode 100644 it/dockerfile-base-as-arg-buildconfig/src/main/docker/Dockerfile create mode 100644 src/test/resources/io/fabric8/maven/docker/util/Dockerfile_from_build_arg diff --git a/it/dockerfile-base-as-arg-buildconfig/pom.xml b/it/dockerfile-base-as-arg-buildconfig/pom.xml new file mode 100644 index 000000000..8e0c22cc2 --- /dev/null +++ b/it/dockerfile-base-as-arg-buildconfig/pom.xml @@ -0,0 +1,46 @@ + + + 4.0.0 + + + io.fabric8.dmp.itests + dmp-it-parent + 0.45-SNAPSHOT + ../pom.xml + + + dmp-it-dockerfile-base-as-arg-buildconfig + 0.45-SNAPSHOT + dmp-it-dockerfile-base-as-arg-buildconfig + + + + + io.fabric8 + docker-maven-plugin + + + + fabric8:dmp-it-dockerfile-base-as-arg-buildconfig + + Dockerfile + + openjdk:21-slim + + + + + + + + build + + build + + install + + + + + + diff --git a/it/dockerfile-base-as-arg-buildconfig/src/main/docker/Dockerfile b/it/dockerfile-base-as-arg-buildconfig/src/main/docker/Dockerfile new file mode 100644 index 000000000..047be8d70 --- /dev/null +++ b/it/dockerfile-base-as-arg-buildconfig/src/main/docker/Dockerfile @@ -0,0 +1,2 @@ +ARG FROM_IMAGE +FROM ${FROM_IMAGE} AS jlink diff --git a/it/pom.xml b/it/pom.xml index c2eba2b37..f2e186e97 100644 --- a/it/pom.xml +++ b/it/pom.xml @@ -31,6 +31,7 @@ docker-compose-dependon dockerfile dockerfile-base-as-arg + dockerfile-base-as-arg-buildconfig dockerignore healthcheck helloworld diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildService.java b/src/main/java/io/fabric8/maven/docker/service/BuildService.java index b694bdb8b..a099b69fb 100644 --- a/src/main/java/io/fabric8/maven/docker/service/BuildService.java +++ b/src/main/java/io/fabric8/maven/docker/service/BuildService.java @@ -66,7 +66,7 @@ public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePul Map buildArgs = addBuildArgs(buildContext); if (imagePullManager != null) { - autoPullBaseImage(imageConfig, imagePullManager, buildContext, buildArgs); + autoPullBaseImage(imageConfig, imagePullManager, buildContext, prepareBuildArgs(buildArgs, imageConfig.getBuildConfiguration())); autoPullCacheFromImage(imageConfig, imagePullManager, buildContext); } diff --git a/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java b/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java index 2c454fb56..8e1a5cf66 100644 --- a/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java +++ b/src/test/java/io/fabric8/maven/docker/service/BuildServiceTest.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.Properties; +import org.apache.maven.execution.MavenSession; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.project.MavenProject; import org.junit.jupiter.api.Assertions; @@ -62,6 +63,9 @@ class BuildServiceTest { @Mock MavenProject mavenProject; + @Mock + MavenSession mavenSession; + @Mock private QueryService queryService; @@ -191,6 +195,35 @@ void testBuildImageWithCacheFrom_ShouldPullImage() throws Exception { verifyImagePull(buildConfig, pullManager, buildContext, "fabric8/s1i-java"); } + @Test + void testDockerfileWithBuildArgsInBuildConfig_ShouldPullImage() throws Exception { + BuildImageConfiguration buildConfig = new BuildImageConfiguration.Builder() + .dockerFile(getClass().getResource("/io/fabric8/maven/docker/util/Dockerfile_from_build_arg").getPath()) + .args(Collections.singletonMap("FROM_IMAGE", "sample/base-image:latest")) + .build(); + + buildConfig.initAndValidate(logger); + + imageConfig = new ImageConfiguration.Builder() + .name("build-image") + .buildConfig(buildConfig) + .build(); + + final ImagePullManager pullManager = new ImagePullManager(null, null, null); + final BuildService.BuildContext buildContext = new BuildService.BuildContext.Builder() + .mojoParameters(mojoParameters) + .build(); + + Mockito.when(mojoParameters.getSession()).thenReturn(mavenSession); + mockMavenProject(); + + File buildArchive = buildService.buildArchive(imageConfig, buildContext, ""); + buildService.buildImage(imageConfig, pullManager, buildContext, buildArchive); + + //verify that tries to pull both images + verifyImagePull(buildConfig, pullManager, buildContext, "sample/base-image:latest"); + } + @Test void testBuildImagePullsDefaultImageWhenNoFromImage() throws Exception { BuildImageConfiguration buildConfig = new BuildImageConfiguration.Builder() diff --git a/src/test/resources/io/fabric8/maven/docker/util/Dockerfile_from_build_arg b/src/test/resources/io/fabric8/maven/docker/util/Dockerfile_from_build_arg new file mode 100644 index 000000000..047be8d70 --- /dev/null +++ b/src/test/resources/io/fabric8/maven/docker/util/Dockerfile_from_build_arg @@ -0,0 +1,2 @@ +ARG FROM_IMAGE +FROM ${FROM_IMAGE} AS jlink