fix : RegistryService should also consider buildArgs while resolving …

…base images Signed-off-by: Rohan Kumar <[email protected]>
fabric8io · Jun 22, 2024 · c745446 · c745446
1 parent ac5e09a
commit c745446
Show file tree

Hide file tree

Showing 17 changed files with 356 additions and 225 deletions.
diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml
@@ -65,6 +65,7 @@ jobs:
           # Workaround for https://github.com/docker/for-linux/issues/748
           DOCKER_INSTALL_PATH=`which docker`
           sudo cp $DOCKER_INSTALL_PATH-init /sbin/docker-init
+          docker run -d -p 5000:5000 --restart=always --name registry registry:2
           cd it/
           mvn clean install
   macos-build:
@@ -100,4 +101,5 @@ jobs:
         run: |
           export DOCKER_HOST=unix:///Users/runner/.colima/default/docker.sock
           cd it/
+          docker run -d -p 5000:5000 --restart=always --name registry registry:2
           mvn clean install
diff --git a/doc/changelog.md b/doc/changelog.md
@@ -2,6 +2,7 @@
 * **0.45-SNAPSHOT**:
   - Automatically create parent directories of portPropertyFile path
   - Added support for `platform` attribute of a container in the docker-compose configuration.
+  - `docker:push` failed with build `ARG` in `FROM` ([1778](https://github.com/fabric8io/docker-maven-plugin/issues/1778))
 
 * **0.44.0** (2024-02-17):
   - Add new option "useDefaultExclusion" for build configuration to handle exclusion of hidden files ([1708](https://github.com/fabric8io/docker-maven-plugin/issues/1708))

diff --git a/it/dockerfile-push-build-arg-from/pom.xml b/it/dockerfile-push-build-arg-from/pom.xml
@@ -0,0 +1,56 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>io.fabric8.dmp.itests</groupId>
+    <artifactId>dmp-it-parent</artifactId>
+    <version>0.45-SNAPSHOT</version>
+    <relativePath>../pom.xml</relativePath>
+  </parent>
+
+  <artifactId>dockerfile-push-build-arg-from</artifactId>
+  <version>0.45-SNAPSHOT</version>
+  <name>dmp-it-dockerfile-build-arg-from</name>
+
+  <properties>
+    <docker.buildArg.baseImage>busybox</docker.buildArg.baseImage>
+  </properties>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>io.fabric8</groupId>
+        <artifactId>docker-maven-plugin</artifactId>
+        <configuration>
+          <images>
+            <image>
+              <name>localhost:5000/dmp-it-dockerfile-push-build-arg-from:latest</name>
+              <alias>dockerfile</alias>
+              <build>
+                <contextDir>${project.basedir}/src/main/docker</contextDir>
+              </build>
+            </image>
+          </images>
+        </configuration>
+        <executions>
+          <execution>
+            <id>build</id>
+            <goals>
+              <goal>build</goal>
+            </goals>
+            <phase>install</phase>
+          </execution>
+          <execution>
+            <id>push</id>
+            <goals>
+              <goal>push</goal>
+            </goals>
+            <phase>install</phase>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/it/dockerfile-push-build-arg-from/src/main/docker/Dockerfile b/it/dockerfile-push-build-arg-from/src/main/docker/Dockerfile
@@ -0,0 +1,2 @@
+ARG baseImage
+FROM ${baseImage}
diff --git a/it/pom.xml b/it/pom.xml
@@ -32,6 +32,7 @@
     <module>dockerfile</module>
     <module>dockerfile-base-as-arg</module>
     <module>dockerfile-base-as-arg-buildconfig</module>
+    <module>dockerfile-push-build-arg-from</module>
     <module>dockerignore</module>
     <module>graceful-container-removal</module>
     <module>healthcheck</module>

diff --git a/src/main/java/io/fabric8/maven/docker/AbstractBuildSupportMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractBuildSupportMojo.java
diff --git a/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java b/src/main/java/io/fabric8/maven/docker/AbstractDockerMojo.java
@@ -25,6 +25,7 @@
 import io.fabric8.maven.docker.log.LogDispatcher;
 import io.fabric8.maven.docker.log.LogOutputSpecFactory;
 import io.fabric8.maven.docker.model.Container;
+import io.fabric8.maven.docker.service.BuildService;
 import io.fabric8.maven.docker.service.DockerAccessFactory;
 import io.fabric8.maven.docker.service.ImagePullManager;
 import io.fabric8.maven.docker.service.QueryService;
@@ -242,6 +243,12 @@ public abstract class AbstractDockerMojo extends AbstractMojo implements Context
 
     @Parameter
     protected Map<String, String> buildArgs;
+    // ==============================================================================================================
+    // Parameters required from Maven when building an assembly. They cannot be injected directly
+    // into DockerAssemblyCreator.
+    // See also here: http://maven.40175.n5.nabble.com/Mojo-Java-1-5-Component-MavenProject-returns-null-vs-JavaDoc-parameter-expression-quot-project-quot-s-td5733805.html
+    @Parameter(property = "docker.pull.registry")
+    String pullRegistry;
 
     // Images resolved with external image resolvers and hooks for subclass to
     // mangle the image configurations.
@@ -315,6 +322,14 @@ public void execute() throws MojoExecutionException, MojoFailureException {
         }
     }
 
+    protected BuildService.BuildContext getBuildContext() {
+        return new BuildService.BuildContext.Builder()
+          .buildArgs(buildArgs)
+          .mojoParameters(createMojoParameters())
+          .registryConfig(getRegistryConfig(pullRegistry))
+          .build();
+    }
+
     private void logException(Exception exp) {
         if (exp.getCause() != null) {
             log.error("%s [%s]", exp.getMessage(), exp.getCause().getMessage());

diff --git a/src/main/java/io/fabric8/maven/docker/BuildMojo.java b/src/main/java/io/fabric8/maven/docker/BuildMojo.java
@@ -7,6 +7,7 @@
 import io.fabric8.maven.docker.service.ImagePullManager;
 import io.fabric8.maven.docker.service.JibBuildService;
 import io.fabric8.maven.docker.service.ServiceHub;
+import io.fabric8.maven.docker.service.helper.BuildArgResolver;
 import io.fabric8.maven.docker.util.EnvUtil;
 import io.fabric8.maven.docker.util.Logger;
 import org.apache.maven.plugin.MojoExecutionException;
@@ -24,6 +25,7 @@
 import java.net.URL;
 import java.util.Date;
 import java.util.Enumeration;
+import java.util.Map;
 
 import static io.fabric8.maven.docker.service.RegistryService.createCompleteAuthConfigList;
 
@@ -34,7 +36,7 @@
  * @since 28.07.14
  */
 @Mojo(name = "build", defaultPhase = LifecyclePhase.INSTALL, requiresDependencyResolution = ResolutionScope.TEST)
-public class BuildMojo extends AbstractBuildSupportMojo {
+public class BuildMojo extends AbstractDockerMojo {
 
     public static final String DMP_PLUGIN_DESCRIPTOR = "META-INF/maven/io.fabric8/dmp-plugin";
     public static final String DOCKER_EXTRA_DIR = "docker-extra";
@@ -101,7 +103,9 @@ private void proceedWithDockerBuild(ServiceHub hub, BuildService.BuildContext bu
         File buildArchiveFile = buildService.buildArchive(imageConfig, buildContext, resolveBuildArchiveParameter());
         if (Boolean.FALSE.equals(shallBuildArchiveOnly())) {
             if (imageConfig.isBuildX()) {
-                hub.getBuildXService().build(createProjectPaths(), imageConfig, null, createCompleteAuthConfigList(false, imageConfig, getRegistryConfig(pullRegistry), createMojoParameters()), buildArchiveFile);
+                BuildArgResolver buildArgResolver = new BuildArgResolver(log);
+                Map<String, String> buildArgsFromExternalSources = buildArgResolver.resolveBuildArgs(buildContext);
+                hub.getBuildXService().build(createProjectPaths(), imageConfig, null, createCompleteAuthConfigList(false, imageConfig, getRegistryConfig(pullRegistry), createMojoParameters(), buildArgsFromExternalSources), buildArchiveFile);
             } else {
                 buildService.buildImage(imageConfig, pullManager, buildContext, buildArchiveFile);
                 if (!skipTag && !imageConfig.getBuildConfiguration().skipTag()) {

diff --git a/src/main/java/io/fabric8/maven/docker/PushMojo.java b/src/main/java/io/fabric8/maven/docker/PushMojo.java
@@ -51,7 +51,7 @@ public void executeInternal(ServiceHub hub) throws DockerAccessException, MojoEx
     }
 
     private void executeDockerPush(ServiceHub hub) throws MojoExecutionException, DockerAccessException {
-        hub.getRegistryService().pushImages(createProjectPaths(), getResolvedImages(), retries, getRegistryConfig(pushRegistry), skipTag, createMojoParameters());
+        hub.getRegistryService().pushImages(createProjectPaths(), getResolvedImages(), retries, getRegistryConfig(pushRegistry), skipTag, getBuildContext());
     }
 
     private void executeJibPush(ServiceHub hub) throws MojoExecutionException {

diff --git a/src/main/java/io/fabric8/maven/docker/SourceMojo.java b/src/main/java/io/fabric8/maven/docker/SourceMojo.java
@@ -45,7 +45,7 @@
  * @since 25/10/15
  */
 @Mojo(name = "source", defaultPhase = LifecyclePhase.PACKAGE)
-public class SourceMojo extends AbstractBuildSupportMojo {
+public class SourceMojo extends AbstractDockerMojo {
 
     @Component
     private MavenProjectHelper projectHelper;

diff --git a/src/main/java/io/fabric8/maven/docker/WatchMojo.java b/src/main/java/io/fabric8/maven/docker/WatchMojo.java
@@ -41,7 +41,7 @@
  * @since 16/06/15
  */
 @Mojo(name = "watch")
-public class WatchMojo extends AbstractBuildSupportMojo {
+public class WatchMojo extends AbstractDockerMojo {
 
     /**
      * Watching mode for rebuilding images

diff --git a/src/main/java/io/fabric8/maven/docker/service/BuildService.java b/src/main/java/io/fabric8/maven/docker/service/BuildService.java
@@ -1,7 +1,6 @@
 package io.fabric8.maven.docker.service;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
 import io.fabric8.maven.docker.access.BuildOptions;
 import io.fabric8.maven.docker.access.DockerAccess;
@@ -14,6 +13,7 @@
 import io.fabric8.maven.docker.config.ImageConfiguration;
 import io.fabric8.maven.docker.model.ImageArchiveManifest;
 import io.fabric8.maven.docker.model.ImageArchiveManifestEntry;
+import io.fabric8.maven.docker.service.helper.BuildArgResolver;
 import io.fabric8.maven.docker.util.DockerFileUtil;
 import io.fabric8.maven.docker.util.EnvUtil;
 import io.fabric8.maven.docker.util.ImageArchiveUtil;
@@ -28,17 +28,13 @@
 import java.io.Serializable;
 import java.nio.file.Files;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Properties;
+import java.util.Optional;
 import java.util.regex.PatternSyntaxException;
 
 public class BuildService {
-
-    private final String argPrefix = "docker.buildArg.";
-
     private final DockerAccess docker;
     private final QueryService queryService;
     private final ArchiveService archiveService;
@@ -64,13 +60,14 @@ public class BuildService {
     public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, File buildArchiveFile)
             throws DockerAccessException, MojoExecutionException {
 
-        Map<String, String> buildArgs = addBuildArgs(buildContext);
+        BuildArgResolver buildArgResolver = new BuildArgResolver(log);
+        Map<String, String> buildArgsFromExternalSources = buildArgResolver.resolveBuildArgs(buildContext);
         if (imagePullManager != null) {
-            autoPullBaseImage(imageConfig, imagePullManager, buildContext, prepareBuildArgs(buildArgs, imageConfig.getBuildConfiguration()));
+            autoPullBaseImage(imageConfig, imagePullManager, buildContext, prepareBuildArgs(buildArgsFromExternalSources, imageConfig.getBuildConfiguration()));
             autoPullCacheFromImage(imageConfig, imagePullManager, buildContext);
         }
 
-        buildImage(imageConfig, buildContext.getMojoParameters(), ConfigHelper.isNoCache(imageConfig), checkForSquash(imageConfig), buildArgs, buildArchiveFile);
+        buildImage(imageConfig, buildContext.getMojoParameters(), ConfigHelper.isNoCache(imageConfig), checkForSquash(imageConfig), buildArgsFromExternalSources, buildArchiveFile);
     }
 
     /**
@@ -206,8 +203,8 @@ public void tagImage(String imageName, String tag, String repo, CleanupMode clea
         }
     }
 
-    private Map<String, String> prepareBuildArgs(Map<String, String> buildArgs, BuildImageConfiguration buildConfig) {
-        ImmutableMap.Builder<String, String> builder = ImmutableMap.<String, String>builder().putAll(buildArgs);
+    static Map<String, String> prepareBuildArgs(Map<String, String> buildArgs, BuildImageConfiguration buildConfig) {
+        ImmutableMap.Builder<String, String> builder = ImmutableMap.<String, String>builder().putAll(Optional.ofNullable(buildArgs).orElse(Collections.emptyMap()));
         if (buildConfig.getArgs() != null) {
             builder.putAll(buildConfig.getArgs());
         }
@@ -296,67 +293,7 @@ private String doBuildImage(String imageName, File dockerArchive, BuildOptions o
         return queryService.getImageId(imageName);
     }
 
-    private Map<String, String> addBuildArgs(BuildContext buildContext) {
-        Map<String, String> buildArgsFromProject = addBuildArgsFromProperties(buildContext.getMojoParameters().getProject().getProperties());
-        Map<String, String> buildArgsFromSystem = addBuildArgsFromProperties(System.getProperties());
-        Map<String, String> buildArgsFromDockerConfig = addBuildArgsFromDockerConfig();
-
-        //merge build args from all the sources into one map. Different sources maps are allowed to contain duplicate keys between them
-        Map<String, String> mergedBuildArgs = new HashMap<>();
-        mergedBuildArgs.putAll(buildArgsFromDockerConfig);
-        mergedBuildArgs.putAll(buildContext.getBuildArgs() != null ? buildContext.getBuildArgs() : Collections.<String, String>emptyMap());
-        mergedBuildArgs.putAll(buildArgsFromProject);
-        mergedBuildArgs.putAll(buildArgsFromSystem);
-
-        return ImmutableMap.copyOf(mergedBuildArgs);
-    }
-
-    private Map<String, String> addBuildArgsFromProperties(Properties properties) {
-        Map<String, String> buildArgs = new HashMap<>();
-        for (Object keyObj : properties.keySet()) {
-            String key = (String) keyObj;
-            if (key.startsWith(argPrefix)) {
-                String argKey = key.replaceFirst(argPrefix, "");
-                String value = properties.getProperty(key);
-
-                if (!isEmpty(value)) {
-                    buildArgs.put(argKey, value);
-                }
-            }
-        }
-        log.debug("Build args set %s", buildArgs);
-        return buildArgs;
-    }
 
-    private Map<String, String> addBuildArgsFromDockerConfig() {
-        JsonObject dockerConfig = DockerFileUtil.readDockerConfig();
-        if (dockerConfig == null) {
-            return Collections.emptyMap();
-        }
-
-        // add proxies
-        Map<String, String> buildArgs = new HashMap<>();
-        if (dockerConfig.has("proxies")) {
-            JsonObject proxies = dockerConfig.getAsJsonObject("proxies");
-            if (proxies.has("default")) {
-                JsonObject defaultProxyObj = proxies.getAsJsonObject("default");
-                String[] proxyMapping = new String[]{
-                        "httpProxy", "http_proxy",
-                        "httpsProxy", "https_proxy",
-                        "noProxy", "no_proxy",
-                        "ftpProxy", "ftp_proxy"
-                };
-
-                for (int index = 0; index < proxyMapping.length; index += 2) {
-                    if (defaultProxyObj.has(proxyMapping[index])) {
-                        buildArgs.put(proxyMapping[index + 1], defaultProxyObj.get(proxyMapping[index]).getAsString());
-                    }
-                }
-            }
-        }
-        log.debug("Build args set %s", buildArgs);
-        return buildArgs;
-    }
 
     private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, BuildContext buildContext, Map<String, String> buildArgs)
             throws DockerAccessException, MojoExecutionException {
@@ -370,7 +307,7 @@ private void autoPullBaseImage(ImageConfiguration imageConfig, ImagePullManager
 
         List<String> fromImages;
         if (buildConfig.isDockerFileMode()) {
-            fromImages = extractBaseFromDockerfile(buildConfig, buildContext, buildArgs);
+            fromImages = extractBaseFromDockerfile(buildConfig, buildContext.getMojoParameters(), buildArgs);
         } else {
             fromImages = new LinkedList<>();
             String baseImage = extractBaseFromConfiguration(buildConfig);
@@ -432,13 +369,20 @@ private String extractBaseFromConfiguration(BuildImageConfiguration buildConfig)
         return fromImage;
     }
 
-    private List<String> extractBaseFromDockerfile(BuildImageConfiguration buildConfig, BuildContext buildContext, Map<String, String> buildArgs) {
+    static List<String> extractBaseFromDockerfile(BuildImageConfiguration buildConfig, MojoParameters mojoParameters, Map<String, String> buildArgs) {
+        if (buildConfig.getDockerFile() == null || !buildConfig.getDockerFile().exists()) {
+            if (buildConfig.getFrom() != null && !buildConfig.getFrom().isEmpty()) {
+                return Collections.singletonList(buildConfig.getFrom());
+            }
+            return Collections.emptyList();
+        }
+
         List<String> fromImage;
         try {
-            File fullDockerFilePath = buildConfig.getAbsoluteDockerFilePath(buildContext.getMojoParameters());
+            File fullDockerFilePath = buildConfig.getAbsoluteDockerFilePath(mojoParameters);
             fromImage = DockerFileUtil.extractBaseImages(
                     fullDockerFilePath,
-                    DockerFileUtil.createInterpolator(buildContext.getMojoParameters(), buildConfig.getFilter()),
+                    DockerFileUtil.createInterpolator(mojoParameters, buildConfig.getFilter()),
                     buildArgs);
         } catch (IOException e) {
             // Cant extract base image, so we wont try an auto pull. An error will occur later anyway when