From c284d14d79865b8598f0ee748831b3d466449fec Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Mon, 29 Apr 2024 23:23:52 +0530 Subject: [PATCH] fix (jkube-kit/config) : `docker.buildArg.*` properties not taken into 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 --- CHANGELOG.md | 1 + .../openshift/OpenShiftBuildServiceUtils.java | 3 +- .../OpenShiftBuildServiceUtilsTest.java | 152 +++++++++++------- 3 files changed, 97 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22727fe760..349f6076aa 100644 --- 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/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtils.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtils.java index e2144dcfb1..c47eb4bff2 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtils.java +++ b/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.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/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtilsTest.java b/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtilsTest.java index 9b1fec0d2a..3abc5eb28c 100644 --- a/jkube-kit/config/service/src/test/java/org/eclipse/jkube/kit/config/service/openshift/OpenShiftBuildServiceUtilsTest.java +++ b/jkube-kit/config/service/src/test/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