fix (jkube-kit/config) : docker.buildArg.* properties not taken into account in OpenShift plugins

CHANGELOG.md

@@ -29,6 +29,7 @@ Usage:
 * Fix #2860: Correctly pass Docker build-arg from the build configuration to the Openshift build strategy
 * Fix #2885: Provide a way to set labels on images defined by Generators
 * Fix #2901: Ensure Docker build arguments from properties are used during images pre-pulling
+* Fix #2904: `docker.buildArg.*` properties not taken into account in OpenShift plugins
 
 ### 1.16.2 (2024-03-27)
 * Fix #2461: `k8s:watch`/`k8sWatch` should throw error in `buildpacks` build strategy

jkube-kit/build/api/src/main/java/org/eclipse/jkube/kit/build/api/helper/BuildArgResolverUtil.java

@@ -44,24 +44,46 @@ private BuildArgResolverUtil() { }
    * @param configuration {@link JKubeConfiguration}.
    * @return a Map containing merged Build Args from all sources.
    */
-  public static Map<String, String> mergeBuildArgs(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
+  public static Map<String, String> mergeBuildArgsIncludingLocalDockerConfigProxySettings(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
+    return mergeBuildArgsFrom(imageConfig.getBuild().getArgs(),
+            buildArgsFromProperties(System.getProperties()),
+            buildArgsFromProperties(configuration.getProject().getProperties()),
+            configuration.getBuildArgs(),
+            buildArgsFromDockerConfig());
+  }
+
+  /**
+   * Merges Docker Build Args from the following sources:
+   * <ul>
+   *   <li>Build Args specified directly in ImageConfiguration</li>
+   *   <li>Build Args specified via System Properties</li>
+   *   <li>Build Args specified via Project Properties</li>
+   *   <li>Build Args specified via Plugin configuration</li>
+   * </ul>
+   * @param imageConfig ImageConfiguration where to get the Build Args from.
+   * @param configuration {@link JKubeConfiguration}.
+   * @return a Map containing merged Build Args from all sources.
+   */
+  public static Map<String, String> mergeBuildArgsWithoutLocalDockerConfigProxySettings(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
+    return mergeBuildArgsFrom(imageConfig.getBuild().getArgs(),
+        buildArgsFromProperties(System.getProperties()),
+        buildArgsFromProperties(configuration.getProject().getProperties()),
+        configuration.getBuildArgs());
+  }
+
+  @SafeVarargs
+  private static Map<String, String> mergeBuildArgsFrom(Map<String, String>... buildArgSources) {
     final Map<String, String> buildArgs = new HashMap<>();
-    Stream.of(
-      imageConfig.getBuild().getArgs(),
-      buildArgsFromProperties(System.getProperties()),
-      buildArgsFromProperties(configuration.getProject().getProperties()),
-      configuration.getBuildArgs(),
-      buildArgsFromDockerConfig()
-    )
-      .filter(Objects::nonNull)
-      .flatMap(map -> map.entrySet().stream())
-      .forEach(entry -> {
-        if (buildArgs.containsKey(entry.getKey())) {
-          throw new JKubeException(String.format("Multiple Build Args with the same key: %s=%s and %s=%s",
-            entry.getKey(), buildArgs.get(entry.getKey()), entry.getKey(), entry.getValue()));
-        }
-        buildArgs.put(entry.getKey(), entry.getValue());
-      });
+    Stream.of(buildArgSources)
+        .filter(Objects::nonNull)
+        .flatMap(map -> map.entrySet().stream())
+        .forEach(entry -> {
+          if (buildArgs.containsKey(entry.getKey())) {
+            throw new JKubeException(String.format("Multiple Build Args with the same key: %s=%s and %s=%s",
+                entry.getKey(), buildArgs.get(entry.getKey()), entry.getKey(), entry.getValue()));
+          }
+          buildArgs.put(entry.getKey(), entry.getValue());
+        });
     return buildArgs;
   }
 

jkube-kit/build/api/src/test/java/org/eclipse/jkube/kit/build/api/helper/BuildArgResolverUtilMergeBuildArgsTest.java

@@ -22,6 +22,7 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
@@ -74,7 +75,7 @@ void whenBuildArgsFromImageConfigAndFromProjectProperties_shouldMergeBuildArgs()
         .build();
 
     // When
-    Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
+    Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
 
     // Then
     assertThat(mergedBuildArgs)
@@ -94,7 +95,7 @@ void fromAllSourcesWithDifferentKeys_shouldMergeBuildArgs() {
     givenBuildArgsFromJKubeConfiguration("FULL_IMAGE", "busybox:latest");
 
     // When
-    Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
+    Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
 
     // Then
     assertThat(mergedBuildArgs)
@@ -113,7 +114,7 @@ void fromBuildConfigurationAndSystemPropertiesWithSameKey_shouldNotMergeBuildArg
 
     // When & Then
     assertThatExceptionOfType(JKubeException.class)
-        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
+        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
         .withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
   }
 
@@ -126,7 +127,7 @@ void fromBuildConfigurationAndProjectPropertiesWithSameKey_shouldNotMergeBuildAr
 
     // When & Then
     assertThatExceptionOfType(JKubeException.class)
-        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
+        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
         .withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
   }
 
@@ -139,31 +140,55 @@ void fromBuildConfigurationAndJKubeConfigurationWithSameKey_shouldNotMergeBuildA
 
     // When & Then
     assertThatExceptionOfType(JKubeException.class)
-        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
+        .isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
         .withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
   }
 
-  @Test
-  @DisplayName("should add proxy build args from ~/.docker/config.json")
-  void shouldAddBuildArgsFromDockerConfig(@TempDir File temporaryFolder) throws IOException {
-    try {
-      // Given
+  @Nested
+  @DisplayName("local ~/.docker/config.json contains proxy settings")
+  class LocalDockerConfigContainsProxySettings {
+    @TempDir
+    private File temporaryFolder;
+
+    @BeforeEach
+    void setUp() throws IOException {
       Path dockerConfig = temporaryFolder.toPath();
       final Map<String, String> env = Collections.singletonMap("DOCKER_CONFIG", dockerConfig.toFile().getAbsolutePath());
       EnvUtil.overrideEnvGetter(env::get);
       Files.write(dockerConfig.resolve("config.json"), ("{\"proxies\": {\"default\": {\n" +
-              "     \"httpProxy\": \"http://proxy.example.com:3128\",\n" +
-              "     \"httpsProxy\": \"https://proxy.example.com:3129\",\n" +
-              "     \"noProxy\": \"*.test.example.com,.example.org,127.0.0.0/8\"\n" +
-              "   }}}").getBytes());
+          "     \"httpProxy\": \"http://proxy.example.com:3128\",\n" +
+          "     \"httpsProxy\": \"https://proxy.example.com:3129\",\n" +
+          "     \"noProxy\": \"*.test.example.com,.example.org,127.0.0.0/8\"\n" +
+          "   }}}").getBytes());
+
+    }
+
+    @Test
+    @DisplayName("mergeBuildArgsIncludingLocalDockerConfigProxySettings, should add proxy build args for docker build strategy")
+    void shouldAddBuildArgsFromDockerConfigInDockerBuild() {
       // When
-      final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
+      final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
       // Then
       assertThat(mergedBuildArgs)
           .containsEntry("docker.buildArg.http_proxy", "http://proxy.example.com:3128")
           .containsEntry("docker.buildArg.https_proxy", "https://proxy.example.com:3129")
           .containsEntry("docker.buildArg.no_proxy", "*.test.example.com,.example.org,127.0.0.0/8");
-    } finally {
+    }
+
+    @Test
+    @DisplayName("mergeBuildArgsWithoutIncludingLocalDockerConfigProxySettings, should not add proxy build args for OpenShift build strategy")
+    void shouldNotAddBuildArgsFromDockerConfig() {
+      // When
+      final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsWithoutLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
+      // Then
+      assertThat(mergedBuildArgs)
+          .doesNotContainEntry("docker.buildArg.http_proxy", "http://proxy.example.com:3128")
+          .doesNotContainEntry("docker.buildArg.https_proxy", "https://proxy.example.com:3129")
+          .doesNotContainEntry("docker.buildArg.no_proxy", "*.test.example.com,.example.org,127.0.0.0/8");
+    }
+
+    @AfterEach
+    void tearDown() {
       EnvUtil.overrideEnvGetter(System::getenv);
     }
   }

jkube-kit/build/service/docker/src/main/java/org/eclipse/jkube/kit/build/service/docker/BuildService.java

@@ -34,7 +34,7 @@
 import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
 import org.eclipse.jkube.kit.config.image.build.CleanupMode;
 
-import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgs;
+import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings;
 import static org.eclipse.jkube.kit.build.api.helper.BuildUtil.extractBaseFromConfiguration;
 
 public class BuildService {
@@ -64,7 +64,7 @@ public class BuildService {
     public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, JKubeConfiguration configuration)
             throws IOException {
 
-        Map<String, String> mergedBuildArgs = mergeBuildArgs(imageConfig, configuration);
+        Map<String, String> mergedBuildArgs = mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfig, configuration);
 
         if (imagePullManager != null) {
             autoPullBaseImage(imageConfig, imagePullManager, configuration, mergedBuildArgs);

jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtils.java

@@ -47,6 +47,7 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgsWithoutLocalDockerConfigProxySettings;
 import static org.eclipse.jkube.kit.build.api.helper.BuildUtil.extractBaseFromDockerfile;
 import static org.eclipse.jkube.kit.config.service.openshift.ImageStreamService.resolveImageStreamName;
 import static org.eclipse.jkube.kit.config.service.openshift.OpenshiftBuildService.DEFAULT_BUILD_OUTPUT_KIND;
@@ -133,7 +134,9 @@ protected static BuildStrategy createBuildStrategy(
               .withNamespace(StringUtils.isEmpty(fromNamespace) ? null : fromNamespace)
             .endFrom()
             .withEnv(checkForEnv(imageConfig))
-            .withBuildArgs(Optional.ofNullable(buildConfig.getArgs()).orElse(Collections.emptyMap()).entrySet().stream()
+            .withBuildArgs(mergeBuildArgsWithoutLocalDockerConfigProxySettings(imageConfig, jKubeServiceHub.getConfiguration())
+                .entrySet()
+                .stream()
                 .map(bcArg -> new EnvVarBuilder()
                     .withName(bcArg.getKey())
                     .withValue(bcArg.getValue()).build())