Skip to content

Commit 00bf004

Browse files
rohanKanojiarohanKanojia
committed
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 <[email protected]>
1 parent 65db45e commit 00bf004

File tree

6 files changed

+190
-99
lines changed

6 files changed

+190
-99
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ Usage:
2929
* Fix #2860: Correctly pass Docker build-arg from the build configuration to the Openshift build strategy
3030
* Fix #2885: Provide a way to set labels on images defined by Generators
3131
* Fix #2901: Ensure Docker build arguments from properties are used during images pre-pulling
32+
* Fix #2904: `docker.buildArg.*` properties not taken into account in OpenShift plugins
3233

3334
### 1.16.2 (2024-03-27)
3435
* 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

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,46 @@ private BuildArgResolverUtil() { }
4444
* @param configuration {@link JKubeConfiguration}.
4545
* @return a Map containing merged Build Args from all sources.
4646
*/
47-
public static Map<String, String> mergeBuildArgs(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
47+
public static Map<String, String> mergeBuildArgsIncludingLocalDockerConfigProxySettings(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
48+
return mergeBuildArgsFrom(imageConfig.getBuild().getArgs(),
49+
buildArgsFromProperties(System.getProperties()),
50+
buildArgsFromProperties(configuration.getProject().getProperties()),
51+
configuration.getBuildArgs(),
52+
buildArgsFromDockerConfig());
53+
}
54+
55+
/**
56+
* Merges Docker Build Args from the following sources:
57+
* <ul>
58+
* <li>Build Args specified directly in ImageConfiguration</li>
59+
* <li>Build Args specified via System Properties</li>
60+
* <li>Build Args specified via Project Properties</li>
61+
* <li>Build Args specified via Plugin configuration</li>
62+
* </ul>
63+
* @param imageConfig ImageConfiguration where to get the Build Args from.
64+
* @param configuration {@link JKubeConfiguration}.
65+
* @return a Map containing merged Build Args from all sources.
66+
*/
67+
public static Map<String, String> mergeBuildArgsWithoutLocalDockerConfigProxySettings(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
68+
return mergeBuildArgsFrom(imageConfig.getBuild().getArgs(),
69+
buildArgsFromProperties(System.getProperties()),
70+
buildArgsFromProperties(configuration.getProject().getProperties()),
71+
configuration.getBuildArgs());
72+
}
73+
74+
@SafeVarargs
75+
private static Map<String, String> mergeBuildArgsFrom(Map<String, String>... buildArgSources) {
4876
final Map<String, String> buildArgs = new HashMap<>();
49-
Stream.of(
50-
imageConfig.getBuild().getArgs(),
51-
buildArgsFromProperties(System.getProperties()),
52-
buildArgsFromProperties(configuration.getProject().getProperties()),
53-
configuration.getBuildArgs(),
54-
buildArgsFromDockerConfig()
55-
)
56-
.filter(Objects::nonNull)
57-
.flatMap(map -> map.entrySet().stream())
58-
.forEach(entry -> {
59-
if (buildArgs.containsKey(entry.getKey())) {
60-
throw new JKubeException(String.format("Multiple Build Args with the same key: %s=%s and %s=%s",
61-
entry.getKey(), buildArgs.get(entry.getKey()), entry.getKey(), entry.getValue()));
62-
}
63-
buildArgs.put(entry.getKey(), entry.getValue());
64-
});
77+
Stream.of(buildArgSources)
78+
.filter(Objects::nonNull)
79+
.flatMap(map -> map.entrySet().stream())
80+
.forEach(entry -> {
81+
if (buildArgs.containsKey(entry.getKey())) {
82+
throw new JKubeException(String.format("Multiple Build Args with the same key: %s=%s and %s=%s",
83+
entry.getKey(), buildArgs.get(entry.getKey()), entry.getKey(), entry.getValue()));
84+
}
85+
buildArgs.put(entry.getKey(), entry.getValue());
86+
});
6587
return buildArgs;
6688
}
6789

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

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.junit.jupiter.api.AfterEach;
2323
import org.junit.jupiter.api.BeforeEach;
2424
import org.junit.jupiter.api.DisplayName;
25+
import org.junit.jupiter.api.Nested;
2526
import org.junit.jupiter.api.Test;
2627
import org.junit.jupiter.api.io.TempDir;
2728

@@ -62,7 +63,7 @@ void setUp() {
6263

6364
@Test
6465
@DisplayName("build args in image config and project properties")
65-
void whenBuildArgsFromImageConfigAndFromProjectProperties_shouldMergeBuildArgs() {
66+
void whenBuildArgsFromImageConfigAndFromProjectProperties_shouldMergeBuildArgsIncludingLocalDockerConfigProxySettings() {
6667
// Given
6768
projectProperties.setProperty("docker.buildArg.VERSION", "latest");
6869
projectProperties.setProperty("docker.buildArg.FULL_IMAGE", "busybox:latest");
@@ -74,7 +75,7 @@ void whenBuildArgsFromImageConfigAndFromProjectProperties_shouldMergeBuildArgs()
7475
.build();
7576

7677
// When
77-
Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
78+
Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
7879

7980
// Then
8081
assertThat(mergedBuildArgs)
@@ -86,15 +87,15 @@ void whenBuildArgsFromImageConfigAndFromProjectProperties_shouldMergeBuildArgs()
8687

8788
@Test
8889
@DisplayName("build args in image config, project properties, system properties, plugin configuration")
89-
void fromAllSourcesWithDifferentKeys_shouldMergeBuildArgs() {
90+
void fromAllSourcesWithDifferentKeys_shouldMergeBuildArgsIncludingLocalDockerConfigProxySettings() {
9091
// Given
9192
givenBuildArgsFromImageConfiguration("VERSION", "latest");
9293
System.setProperty("docker.buildArg.IMAGE-1", "openjdk");
9394
projectProperties.setProperty("docker.buildArg.REPO_1", "docker.io/library");
9495
givenBuildArgsFromJKubeConfiguration("FULL_IMAGE", "busybox:latest");
9596

9697
// When
97-
Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
98+
Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
9899

99100
// Then
100101
assertThat(mergedBuildArgs)
@@ -106,64 +107,88 @@ void fromAllSourcesWithDifferentKeys_shouldMergeBuildArgs() {
106107

107108
@Test
108109
@DisplayName("build args in image config and system properties with same key, should throw exception")
109-
void fromBuildConfigurationAndSystemPropertiesWithSameKey_shouldNotMergeBuildArgs() {
110+
void fromBuildConfigurationAndSystemPropertiesWithSameKey_shouldNotMergeBuildArgsIncludingLocalDockerConfigProxySettings() {
110111
// Given
111112
givenBuildArgsFromImageConfiguration("VERSION", "latest");
112113
System.setProperty("docker.buildArg.VERSION", "1.0.0");
113114

114115
// When & Then
115116
assertThatExceptionOfType(JKubeException.class)
116-
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
117+
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
117118
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
118119
}
119120

120121
@Test
121122
@DisplayName("build args in image config and project properties with same key, should throw exception")
122-
void fromBuildConfigurationAndProjectPropertiesWithSameKey_shouldNotMergeBuildArgs() {
123+
void fromBuildConfigurationAndProjectPropertiesWithSameKey_shouldNotMergeBuildArgsIncludingLocalDockerConfigProxySettings() {
123124
// Given
124125
givenBuildArgsFromImageConfiguration("VERSION", "latest");
125126
projectProperties.setProperty("docker.buildArg.VERSION", "1.0.0");
126127

127128
// When & Then
128129
assertThatExceptionOfType(JKubeException.class)
129-
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
130+
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
130131
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
131132
}
132133

133134
@Test
134135
@DisplayName("build args in image config and plugin config with same key, should throw exception")
135-
void fromBuildConfigurationAndJKubeConfigurationWithSameKey_shouldNotMergeBuildArgs() {
136+
void fromBuildConfigurationAndJKubeConfigurationWithSameKey_shouldNotMergeBuildArgsIncludingLocalDockerConfigProxySettings() {
136137
// Given
137138
givenBuildArgsFromImageConfiguration("VERSION", "latest");
138139
givenBuildArgsFromJKubeConfiguration("VERSION", "1.0.0");
139140

140141
// When & Then
141142
assertThatExceptionOfType(JKubeException.class)
142-
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
143+
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration))
143144
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
144145
}
145146

146-
@Test
147-
@DisplayName("should add proxy build args from ~/.docker/config.json")
148-
void shouldAddBuildArgsFromDockerConfig(@TempDir File temporaryFolder) throws IOException {
149-
try {
150-
// Given
147+
@Nested
148+
@DisplayName("local ~/.docker/config.json contains proxy settings")
149+
class LocalDockerConfigContainsProxySettings {
150+
@TempDir
151+
private File temporaryFolder;
152+
153+
@BeforeEach
154+
void setUp() throws IOException {
151155
Path dockerConfig = temporaryFolder.toPath();
152156
final Map<String, String> env = Collections.singletonMap("DOCKER_CONFIG", dockerConfig.toFile().getAbsolutePath());
153157
EnvUtil.overrideEnvGetter(env::get);
154158
Files.write(dockerConfig.resolve("config.json"), ("{\"proxies\": {\"default\": {\n" +
155-
" \"httpProxy\": \"http://proxy.example.com:3128\",\n" +
156-
" \"httpsProxy\": \"https://proxy.example.com:3129\",\n" +
157-
" \"noProxy\": \"*.test.example.com,.example.org,127.0.0.0/8\"\n" +
158-
" }}}").getBytes());
159+
" \"httpProxy\": \"http://proxy.example.com:3128\",\n" +
160+
" \"httpsProxy\": \"https://proxy.example.com:3129\",\n" +
161+
" \"noProxy\": \"*.test.example.com,.example.org,127.0.0.0/8\"\n" +
162+
" }}}").getBytes());
163+
164+
}
165+
166+
@Test
167+
@DisplayName("mergeBuildArgsIncludingLocalDockerConfigProxySettings, should add proxy build args for docker build strategy")
168+
void shouldAddBuildArgsFromDockerConfigInDockerBuild() {
159169
// When
160-
final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration);
170+
final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
161171
// Then
162172
assertThat(mergedBuildArgs)
163173
.containsEntry("docker.buildArg.http_proxy", "http://proxy.example.com:3128")
164174
.containsEntry("docker.buildArg.https_proxy", "https://proxy.example.com:3129")
165175
.containsEntry("docker.buildArg.no_proxy", "*.test.example.com,.example.org,127.0.0.0/8");
166-
} finally {
176+
}
177+
178+
@Test
179+
@DisplayName("mergeBuildArgsWithoutIncludingLocalDockerConfigProxySettings, should not add proxy build args for OpenShift build strategy")
180+
void shouldNotAddBuildArgsFromDockerConfig() {
181+
// When
182+
final Map<String, String> mergedBuildArgs = BuildArgResolverUtil.mergeBuildArgsWithoutLocalDockerConfigProxySettings(imageConfiguration, jKubeConfiguration);
183+
// Then
184+
assertThat(mergedBuildArgs)
185+
.doesNotContainEntry("docker.buildArg.http_proxy", "http://proxy.example.com:3128")
186+
.doesNotContainEntry("docker.buildArg.https_proxy", "https://proxy.example.com:3129")
187+
.doesNotContainEntry("docker.buildArg.no_proxy", "*.test.example.com,.example.org,127.0.0.0/8");
188+
}
189+
190+
@AfterEach
191+
void tearDown() {
167192
EnvUtil.overrideEnvGetter(System::getenv);
168193
}
169194
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
3535
import org.eclipse.jkube.kit.config.image.build.CleanupMode;
3636

37-
import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgs;
37+
import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgsIncludingLocalDockerConfigProxySettings;
3838
import static org.eclipse.jkube.kit.build.api.helper.BuildUtil.extractBaseFromConfiguration;
3939

4040
public class BuildService {
@@ -64,7 +64,7 @@ public class BuildService {
6464
public void buildImage(ImageConfiguration imageConfig, ImagePullManager imagePullManager, JKubeConfiguration configuration)
6565
throws IOException {
6666

67-
Map<String, String> mergedBuildArgs = mergeBuildArgs(imageConfig, configuration);
67+
Map<String, String> mergedBuildArgs = mergeBuildArgsIncludingLocalDockerConfigProxySettings(imageConfig, configuration);
6868

6969
if (imagePullManager != null) {
7070
autoPullBaseImage(imageConfig, imagePullManager, configuration, mergedBuildArgs);

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.util.Optional;
4848
import java.util.stream.Collectors;
4949

50+
import static org.eclipse.jkube.kit.build.api.helper.BuildArgResolverUtil.mergeBuildArgsWithoutLocalDockerConfigProxySettings;
5051
import static org.eclipse.jkube.kit.build.api.helper.BuildUtil.extractBaseFromDockerfile;
5152
import static org.eclipse.jkube.kit.config.service.openshift.ImageStreamService.resolveImageStreamName;
5253
import static org.eclipse.jkube.kit.config.service.openshift.OpenshiftBuildService.DEFAULT_BUILD_OUTPUT_KIND;
@@ -133,7 +134,9 @@ protected static BuildStrategy createBuildStrategy(
133134
.withNamespace(StringUtils.isEmpty(fromNamespace) ? null : fromNamespace)
134135
.endFrom()
135136
.withEnv(checkForEnv(imageConfig))
136-
.withBuildArgs(Optional.ofNullable(buildConfig.getArgs()).orElse(Collections.emptyMap()).entrySet().stream()
137+
.withBuildArgs(mergeBuildArgsWithoutLocalDockerConfigProxySettings(imageConfig, jKubeServiceHub.getConfiguration())
138+
.entrySet()
139+
.stream()
137140
.map(bcArg -> new EnvVarBuilder()
138141
.withName(bcArg.getKey())
139142
.withValue(bcArg.getValue()).build())

0 commit comments

Comments
 (0)