Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

review: build args helper #122

Merged
merged 1 commit into from
Apr 30, 2024
Merged

review: build args helper #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 27 additions & 17 deletions .../build/api/src/main/java/org/eclipse/jkube/kit/build/api/helper/BuildArgResolverUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,47 +15,57 @@

import org.apache.commons.lang3.StringUtils;
import org.eclipse.jkube.kit.common.JKubeConfiguration;
import org.eclipse.jkube.kit.common.JKubeException;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Objects;
import java.util.Properties;

import static org.eclipse.jkube.kit.common.util.MapUtil.mergeMapsImmutable;
import java.util.stream.Stream;

public class BuildArgResolverUtil {

private static final String ARG_PREFIX = "docker.buildArg.";

private BuildArgResolverUtil() { }

/**
* Merges Docker Build Args in the following order (in decreasing order of precedence):
* 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>
* <li>Docker Proxy Build Args detected from ~/.docker/config.json</li>
* </ul>
* @param imageConfig ImageConfiguration for which Build Args would be resolved
* @param configuration {@link JKubeConfiguration} JKubeConfiguration
* @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> mergeBuildArgs(ImageConfiguration imageConfig, JKubeConfiguration configuration) {
Map<String, String> buildArgsFromProjectProperties = addBuildArgsFromProperties(configuration.getProject().getProperties());
Map<String, String> buildArgsFromSystemProperties = addBuildArgsFromProperties(System.getProperties());
Map<String, String> buildArgsFromDockerConfig = addBuildArgsFromDockerConfig();

return mergeMapsImmutable(imageConfig.getBuild().getArgs(),
buildArgsFromSystemProperties,
buildArgsFromProjectProperties,
Optional.ofNullable(configuration.getBuildArgs()).orElse(Collections.emptyMap()),
buildArgsFromDockerConfig);
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());
});
return buildArgs;
}

private static Map<String, String> addBuildArgsFromProperties(Properties properties) {
private static Map<String, String> buildArgsFromProperties(Properties properties) {
Map<String, String> buildArgs = new HashMap<>();
for (Object keyObj : properties.keySet()) {
String key = (String) keyObj;
Expand All @@ -71,7 +81,7 @@ private static Map<String, String> addBuildArgsFromProperties(Properties propert
return buildArgs;
}

private static Map<String, String> addBuildArgsFromDockerConfig() {
private static Map<String, String> buildArgsFromDockerConfig() {
final Map<String, Object> dockerConfig = DockerFileUtil.readDockerConfig();
if (dockerConfig == null) {
return Collections.emptyMap();
Expand Down
15 changes: 8 additions & 7 deletions ...t/java/org/eclipse/jkube/kit/build/api/helper/BuildArgResolverUtilMergeBuildArgsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package org.eclipse.jkube.kit.build.api.helper;

import org.eclipse.jkube.kit.common.JKubeConfiguration;
import org.eclipse.jkube.kit.common.JKubeException;
import org.eclipse.jkube.kit.common.JavaProject;
import org.eclipse.jkube.kit.common.util.EnvUtil;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;
Expand All @@ -34,7 +35,7 @@
import java.util.Properties;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

class BuildArgResolverUtilMergeBuildArgsTest {
private ImageConfiguration imageConfiguration;
Expand Down Expand Up @@ -111,9 +112,9 @@ void fromBuildConfigurationAndSystemPropertiesWithSameKey_shouldNotMergeBuildArg
System.setProperty("docker.buildArg.VERSION", "1.0.0");

// When & Then
assertThatIllegalArgumentException()
assertThatExceptionOfType(JKubeException.class)
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
.withMessage("Multiple entries with same key: VERSION=latest and VERSION=1.0.0");
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
}

@Test
Expand All @@ -124,9 +125,9 @@ void fromBuildConfigurationAndProjectPropertiesWithSameKey_shouldNotMergeBuildAr
projectProperties.setProperty("docker.buildArg.VERSION", "1.0.0");

// When & Then
assertThatIllegalArgumentException()
assertThatExceptionOfType(JKubeException.class)
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
.withMessage("Multiple entries with same key: VERSION=latest and VERSION=1.0.0");
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
}

@Test
Expand All @@ -137,9 +138,9 @@ void fromBuildConfigurationAndJKubeConfigurationWithSameKey_shouldNotMergeBuildA
givenBuildArgsFromJKubeConfiguration("VERSION", "1.0.0");

// When & Then
assertThatIllegalArgumentException()
assertThatExceptionOfType(JKubeException.class)
.isThrownBy(() -> BuildArgResolverUtil.mergeBuildArgs(imageConfiguration, jKubeConfiguration))
.withMessage("Multiple entries with same key: VERSION=latest and VERSION=1.0.0");
.withMessage("Multiple Build Args with the same key: VERSION=latest and VERSION=1.0.0");
}

@Test
Expand Down
26 changes: 0 additions & 26 deletions jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/MapUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,32 +42,6 @@ public static void mergeIfAbsent(Map<String, String> map, Map<String, String> to
}
}

/**
* Returns a new map with all the entries of first map with rest map entries. It throws IllegalArgumentException
* when it encounters conflicting keys
*
* @param maps var arg parameter of maps to be merged
* @return merged map
* @param <K> key type
* @param <V> value type
* @throws IllegalArgumentException when multiple entries with same key are found
*/
@SafeVarargs
public static <K,V> Map<K,V> mergeMapsImmutable(Map<K, V>... maps) {
Map<K, V> answer = new HashMap<>();
for (int i = maps.length-1; i >= 0; i--) {
if (maps[i] != null) {
for (Map.Entry<K, V> e : maps[i].entrySet()) {
if (answer.containsKey(e.getKey())) {
throw new IllegalArgumentException(String.format("Multiple entries with same key: %s=%s and %s=%s", e.getKey(), e.getValue(), e.getKey(), answer.get(e.getKey())));
}
answer.put(e.getKey(), e.getValue());
}
}
}
return answer;
}

/**
* Returns a new map with all the entries of first map with rest map entries which don't override map1.
*
Expand Down
37 changes: 0 additions & 37 deletions jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/MapUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,12 @@
import java.util.Map;

import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.entry;
import static org.eclipse.jkube.kit.common.util.MapUtil.getFlattenedMap;
import static org.eclipse.jkube.kit.common.util.MapUtil.getNestedMap;
import static org.eclipse.jkube.kit.common.util.MapUtil.mergeMapsImmutable;
import static org.junit.jupiter.api.Assertions.assertThrows;

/**
Expand Down Expand Up @@ -140,39 +136,6 @@ void getNestedMap_withInvalidFlattenedMap_shouldThrowNodeOverlapsException() {
assertThat(result).hasMessage("The provided input Map is invalid (node <second> overlaps with key)");
}

@Nested
class MergeMapsImmutable {
@Test
@DisplayName("different keys present in maps, then merge maps")
void whenNoConflictingKeysProvided_thenMergeMaps() {
// Given
Map<String, String> m1 = createMap("foo", "bar");
Map<String, String> m2 = createMap("BAR", "FOO");

// When
Map<String, String> result = mergeMapsImmutable(m1, m2);

// Then
assertThat(result)
.containsEntry("foo", "bar")
.containsEntry("BAR", "FOO");
}

@Test
@DisplayName("same keys present in maps, then throw exception")
void whenConflictingKeysProvided_thenThrowException() {
// Given
Map<String, String> m1 = createMap("foo", "bar");
Map<String, String> m2 = createMap("foo", "FOO");

// When + Then
assertThatIllegalArgumentException()
.isThrownBy(() -> MapUtil.mergeMapsImmutable(m1, m2, null))
.withMessage("Multiple entries with same key: foo=bar and foo=FOO");
}
}


private Map<String, String> createMap(String ... args) {
Map<String, String> ret = new LinkedHashMap<>();
for (int i = 0; i < args.length; i+=2) {
Expand Down