fix (jkube-kit/config) : docker.buildArg.* properties not taken int…

…o account in OpenShift plugins Added call to resolve build args from all sources in OpenShiftBuildServiceUtils so that build args get resolved from all the available sources (compared to just ImageConfig) Signed-off-by: Rohan Kumar <[email protected]>
eclipse-jkube · Apr 29, 2024 · c284d14 · c284d14
1 parent dbd1067
commit c284d14
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ Usage:
 * Fix #2662: Sanitize VCS remote URL used in `jkube.eclipse.org/git-url` annotation
 * Fix #2860: Correctly pass Docker build-arg from the build configuration to the Openshift build strategy
 * 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

diff --git a/.../main/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtils.java b/.../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.mergeBuildArgs;
 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,7 @@ 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(Optional.of(mergeBuildArgs(imageConfig, jKubeServiceHub.getConfiguration())).orElse(Collections.emptyMap()).entrySet().stream()
                 .map(bcArg -> new EnvVarBuilder()
                     .withName(bcArg.getKey())
                     .withValue(bcArg.getValue()).build())

diff --git a/...t/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtilsTest.java b/...t/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtilsTest.java
@@ -18,12 +18,15 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Properties;
 
 import io.fabric8.openshift.api.model.ImageStreamTag;
 import io.fabric8.openshift.api.model.ImageStreamTagBuilder;
+import org.assertj.core.api.InstanceOfAssertFactories;
 import org.eclipse.jkube.kit.build.api.assembly.ArchiverCustomizer;
 import org.eclipse.jkube.kit.build.api.assembly.JKubeBuildTarArchiver;
 import org.eclipse.jkube.kit.common.JKubeConfiguration;
+import org.eclipse.jkube.kit.common.JavaProject;
 import org.eclipse.jkube.kit.config.image.ImageConfiguration;
 import org.eclipse.jkube.kit.config.image.ImageName;
 import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
@@ -40,6 +43,8 @@
 import io.fabric8.openshift.api.model.BuildStrategy;
 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;
 import org.mockito.ArgumentCaptor;
@@ -183,64 +188,95 @@ void createBuildStrategy_withS2iBuildStrategyAndPullSecret_shouldReturnValidBuil
         .hasFieldOrPropertyWithValue("pullSecret.name", "my-secret-for-pull");
   }
 
-  @Test
-  void createBuildStrategy_withDockerBuildStrategyAndNoPullSecret_shouldReturnValidBuildStrategy() {
-    // Given
-    when(jKubeServiceHub.getBuildServiceConfig().getJKubeBuildStrategy())
-        .thenReturn(JKubeBuildStrategy.docker);
-    // When
-    final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, null);
-    // Then
-    assertThat(result)
-        .hasFieldOrPropertyWithValue("type", "Docker")
-        .extracting(BuildStrategy::getDockerStrategy)
-        .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
-        .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base");
-  }
-
-  @Test
-  void createBuildStrategy_withDockerBuildArgs_shouldReturnValidBuildStrategyWithBuildArgs() {
-    // Given
-    when(jKubeServiceHub.getBuildServiceConfig().getJKubeBuildStrategy())
-        .thenReturn(JKubeBuildStrategy.docker);
-    ImageConfiguration imageConfig = ImageConfiguration.builder()
-        .name("myapp")
-        .build(BuildConfiguration.builder()
-            .from("ubi8/s2i-base")
-            .args(Collections.singletonMap("BUILD_ARGS_KEY", "build-args-value"))
-            .build())
-        .build();
-    // When
-    final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfig, "my-secret-for-pull");
-    // Then
-    assertThat(result)
-        .hasFieldOrPropertyWithValue("type", "Docker")
-        .extracting(BuildStrategy::getDockerStrategy)
-        .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
-        .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base")
-        .extracting("buildArgs").asList().hasSize(1).first().satisfies(
-            envVar -> {
-              assertThat(envVar)
-                  .hasFieldOrPropertyWithValue("name", "BUILD_ARGS_KEY")
-                  .hasFieldOrPropertyWithValue("value", "build-args-value");
-            });
-  }
-
-  @Test
-  void createBuildStrategy_withDockerBuildStrategyAndPullSecret_shouldReturnValidBuildStrategy() {
-    // Given
-    when(jKubeServiceHub.getBuildServiceConfig().getJKubeBuildStrategy())
-        .thenReturn(JKubeBuildStrategy.docker);
-    // When
-    final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, "my-secret-for-pull");
-    // Then
-    assertThat(result)
-        .hasFieldOrPropertyWithValue("type", "Docker")
-        .extracting(BuildStrategy::getDockerStrategy)
-        .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
-        .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base")
-        .hasFieldOrPropertyWithValue("noCache", false)
-        .hasFieldOrPropertyWithValue("pullSecret.name", "my-secret-for-pull");
+  @Nested
+  @DisplayName("docker BuildStrategy")
+  class DockerBuildStrategy {
+    @BeforeEach
+    void setUp() {
+      when(jKubeServiceHub.getBuildServiceConfig().getJKubeBuildStrategy())
+          .thenReturn(JKubeBuildStrategy.docker);
+    }
+
+    @Test
+    @DisplayName("no pull secret specified")
+    void withDockerBuildStrategyAndNoPullSecret_shouldReturnValidBuildStrategy() {
+      // When
+      final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, null);
+      // Then
+      assertThat(result)
+          .hasFieldOrPropertyWithValue("type", "Docker")
+          .extracting(BuildStrategy::getDockerStrategy)
+          .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
+          .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base");
+    }
+
+    @Test
+    @DisplayName("pull secret specified, should reflect in Build Strategy")
+    void withPullSecret_shouldReturnValidBuildStrategy() {
+      // When
+      final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, "my-secret-for-pull");
+      // Then
+      assertThat(result)
+          .hasFieldOrPropertyWithValue("type", "Docker")
+          .extracting(BuildStrategy::getDockerStrategy)
+          .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
+          .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base")
+          .hasFieldOrPropertyWithValue("noCache", false)
+          .hasFieldOrPropertyWithValue("pullSecret.name", "my-secret-for-pull");
+    }
+
+    @Nested
+    @DisplayName("with build args")
+    class WithDockerBuildArgs {
+      @Test
+      @DisplayName("when docker build args in ImageConfig, then add them to BuildStrategy")
+      void withDockerBuildArgsInImageConfiguration_shouldReturnValidBuildStrategyWithBuildArgs() {
+        // Given
+        imageConfiguration = imageConfiguration.toBuilder()
+            .build(imageConfiguration.getBuild().toBuilder()
+                .args(Collections.singletonMap("BUILD_ARGS_KEY", "build-args-value"))
+                .build())
+            .build();
+        // When
+        final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, "my-secret-for-pull");
+        // Then
+        assertBuildArgPresentInBuildStrategy(result);
+      }
+
+      @Test
+      @DisplayName("when docker build args in project properties, then add them to BuildStrategy")
+      void withDockerBuildArgsInProjectProperties_shouldReturnValidBuildStrategyWithBuildArgs() {
+        // Given
+        Properties properties = new Properties();
+        properties.put("docker.buildArg.BUILD_ARGS_KEY", "build-args-value");
+        when(jKubeServiceHub.getConfiguration()).thenReturn(JKubeConfiguration.builder()
+            .project(JavaProject.builder()
+                .properties(properties)
+                .build())
+            .build());
+        // When
+        final BuildStrategy result = createBuildStrategy(jKubeServiceHub, imageConfiguration, "my-secret-for-pull");
+        // Then
+        assertBuildArgPresentInBuildStrategy(result);
+      }
+
+      void assertBuildArgPresentInBuildStrategy(BuildStrategy result) {
+        assertThat(result)
+            .hasFieldOrPropertyWithValue("type", "Docker")
+            .extracting(BuildStrategy::getDockerStrategy)
+            .hasFieldOrPropertyWithValue("from.kind", "DockerImage")
+            .hasFieldOrPropertyWithValue("from.name", "ubi8/s2i-base")
+            .extracting("buildArgs")
+            .asInstanceOf(InstanceOfAssertFactories.LIST)
+            .singleElement()
+            .satisfies(
+                envVar -> {
+                  assertThat(envVar)
+                      .hasFieldOrPropertyWithValue("name", "BUILD_ARGS_KEY")
+                      .hasFieldOrPropertyWithValue("value", "build-args-value");
+                });
+      }
+    }
   }
 
   @Test