From 2502caf563dbdf714bfbed555182344b5b3ec1ea Mon Sep 17 00:00:00 2001 From: Marc Nuri Date: Mon, 31 Jan 2022 16:03:13 +0100 Subject: [PATCH] feat: Helm supports placeholders/variables with dotted notation Signed-off-by: Marc Nuri --- CHANGELOG.md | 1 + .../jkube/kit/common/util/MapUtil.java | 60 +++++++++++++++++++ .../jkube/kit/common/util/MapUtilTest.java | 52 +++++++++++++++- .../jkube/kit/resource/helm/HelmService.java | 3 +- .../kit/resource/helm/HelmServiceIT.java | 5 +- .../kubernetes/templates/kubernetes.yaml | 5 +- .../it/expected/kubernetes/values.yaml | 6 +- .../it/expected/openshift/values.yaml | 6 +- .../resources/it/sources/global-template.yml | 4 +- .../it/sources/kubernetes/kubernetes.yml | 5 +- .../helm-and-fragments-deployment.yaml | 4 +- .../expected/helm/values.yaml | 7 ++- .../expected/resource/kubernetes.yml | 4 +- .../helm-and-fragments-deployment.yml | 4 +- .../src/main/jkube/deployment.yml | 4 +- .../src/main/jkube/first-template.yml | 4 +- .../src/main/jkube/template.yml | 2 +- 17 files changed, 148 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4300e8713b..261ade3a97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ Usage: * Fix #1197: Kubernetes Gradle Build tasks don't account for image model configuration skip field * Fix #1245: Deprecate `maven.jkube.io` annotation prefix used in enrichers in favor of `jkube.eclipse.org` * Fix #1244: Helm Parameters/Variables/Values.yaml can be configured via XML/DSL +* Fix #1244: Helm parameters can use dotted notation `${helm.parameter.with.dots}` ### 1.5.1 (2021-10-28) * Fix #1084: Gradle dependencies should be test or provided scope diff --git a/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/MapUtil.java b/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/MapUtil.java index f128c2e3eb..d3b0857033 100644 --- a/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/MapUtil.java +++ b/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/MapUtil.java @@ -21,9 +21,13 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.BiFunction; public class MapUtil { + private static final BiFunction GET_OR_NEW = (nK, nV) -> + nV == null ? new LinkedHashMap<>() : nV; + private MapUtil() {} /** @@ -75,11 +79,67 @@ public static void putAllIfNotNull(Map ret, Map * Build a flattened representation of provided Map. * *

The conversion is compliant with the thorntail spring-boot rules. + * + *

Given a Map of Maps: + *

{@code
+     * Collections.singletonMap("key", Collections.singletonMap("nested-key", "value"));
+     * }
+ * + *

It will return a Map with the following structure + *

{@code
+     * Collections.singletonMap("key.nested-key", "value");
+     * }
*/ public static Map getFlattenedMap(Map source) { return buildFlattenedMap(source, null); } + /** + * Build a nested representation of the provided Map. + * + *

Given a Map with a flat structure, it returns a Map of nested Maps. The original keys are split by the dot + * (.) character. For each element, a new Map node is created. + * + *

Given the following YAML representation of a Map: + *

{@code
+     * one.two.key: value
+     * one.two.three: other
+     * }
+ * + *

It will converted to: + *

{@code
+     * one:
+     *   two:
+     *     key: value
+     *     three: other
+     * }
+ */ + public static Map getNestedMap(Map flattenedMap) { + final Map result = new LinkedHashMap<>(); + flattenedMap.forEach((k, v) -> { + final String[] nodes = k.split("\\."); + if (nodes.length == 1) { + result.put(k, v); + } else { + Map currentNode = result; + for (int it = 0; it < nodes.length - 1; it++){ + final Object tempNode = currentNode.compute(nodes[it], GET_OR_NEW); + if (!(tempNode instanceof Map)) { + throw new IllegalArgumentException("The provided input Map is invalid (node <" + + nodes[it] + "> overlaps with key)"); + } + currentNode = (Map) tempNode; + } + final Object previousNodeValue = currentNode.put(nodes[nodes.length -1], v); + if (previousNodeValue != null) { + throw new IllegalArgumentException("The provided input Map is invalid (key <" + + nodes[nodes.length -1] + "> overlaps with node)"); + } + } + }); + return result; + } + private static Map buildFlattenedMap(Map source, String keyPrefix) { final Map result = new LinkedHashMap<>(); for (Map.Entry entry : source.entrySet()) { diff --git a/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/MapUtilTest.java b/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/MapUtilTest.java index 330c9b95bb..5b35a1bb54 100644 --- a/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/MapUtilTest.java +++ b/jkube-kit/common/src/test/java/org/eclipse/jkube/kit/common/util/MapUtilTest.java @@ -18,16 +18,18 @@ import java.util.LinkedHashMap; import java.util.Map; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.Test; import static org.assertj.core.api.Assertions.assertThat; 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.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; /** * @author roland - * @since 05/08/16 */ public class MapUtilTest { @@ -59,7 +61,7 @@ public void testGetFlattenedMap() { originalMap.put("two", Arrays.asList("1", "2", "3")); originalMap.put("three", Collections.singletonMap("three-nested", createMap("k1", "v1", "k2", "v2"))); originalMap.put("four", Collections.singletonMap("four-nested", Arrays.asList(1, 2, 3, 4))); - final Map fiveNested = new LinkedHashMap(); + final Map fiveNested = new LinkedHashMap<>(); originalMap.put("five", Collections.singletonMap("five-nested", fiveNested)); fiveNested.put("k1", createMap("nk1", "nv1", "nk2", "nv2")); fiveNested.put("k2", Arrays.asList(true, false)); @@ -88,6 +90,52 @@ public void testGetFlattenedMap() { ); } + @Test + public void getNestedMap_withFlattenedMap_shouldReturnNested() { + // Given + final Map flattenedMap = new LinkedHashMap<>(); + flattenedMap.put("key", "top-level-value"); + flattenedMap.put("first.second.key", "the-value-2"); + flattenedMap.put("first.other.key", "the-value-other"); + flattenedMap.put("first.second.third.key", "the-value-3"); + // When + final Map result = getNestedMap(flattenedMap); + // Then + assertThat(result) + .containsEntry("key", "top-level-value") + .extracting("first").asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("other", Collections.singletonMap("key", "the-value-other")) + .extracting("second").asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("key", "the-value-2") + .extracting("third").asInstanceOf(InstanceOfAssertFactories.map(String.class, Object.class)) + .containsEntry("key", "the-value-3"); + } + + @Test + public void getNestedMap_withInvalidFlattenedMap_shouldThrowKeyOverlapsException() { + // Given + final Map flattenedMap = new LinkedHashMap<>(); + flattenedMap.put("first.second.third.key", "the-value-3"); + flattenedMap.put("first.second", "the-value-2"); + // When + final IllegalArgumentException result = assertThrows(IllegalArgumentException.class, () -> + getNestedMap(flattenedMap)); + // Then + assertThat(result).hasMessage("The provided input Map is invalid (key overlaps with node)"); + } + + @Test + public void getNestedMap_withInvalidFlattenedMap_shouldThrowNodeOverlapsException() { + // Given + final Map flattenedMap = new LinkedHashMap<>(); + flattenedMap.put("first.second", "the-value-2"); + flattenedMap.put("first.second.third.key", "the-value-3"); + // When + final IllegalArgumentException result = assertThrows(IllegalArgumentException.class, () -> + getNestedMap(flattenedMap)); + // Then + assertThat(result).hasMessage("The provided input Map is invalid (node overlaps with key)"); + } private Map createMap(String ... args) { Map ret = new LinkedHashMap<>(); diff --git a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java index c694a48fc2..1545ab46d8 100644 --- a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java +++ b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmService.java @@ -48,6 +48,7 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; +import static org.eclipse.jkube.kit.common.util.MapUtil.getNestedMap; import static org.eclipse.jkube.kit.common.util.TemplateUtil.escapeYamlTemplate; import static org.eclipse.jkube.kit.resource.helm.HelmServiceUtil.isRepositoryValid; import static org.eclipse.jkube.kit.resource.helm.HelmServiceUtil.selectHelmRepository; @@ -306,7 +307,7 @@ private static void createValuesYaml(List helmParameters, File ou .collect(Collectors.toMap(HelmParameter::getHelmName, hp -> hp.getParameter().getValue())); File outputChartFile = new File(outputDir, VALUES_FILENAME); - ResourceUtil.save(outputChartFile, values, ResourceFileType.yaml); + ResourceUtil.save(outputChartFile, getNestedMap(values), ResourceFileType.yaml); } private static List collectParameters(HelmConfig helmConfig) { diff --git a/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmServiceIT.java b/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmServiceIT.java index 98894da86f..ee1e85a884 100644 --- a/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmServiceIT.java +++ b/jkube-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmServiceIT.java @@ -76,8 +76,9 @@ public void generateHelmChartsTest() throws Exception { helmConfig.setParameterTemplates(Collections.singletonList( ResourceUtil.load(new File(HelmServiceIT.class.getResource("/it/sources/global-template.yml").toURI()), Template.class) )); - helmConfig.setParameters(Collections.singletonList(new ParameterBuilder() - .withName("annotation_from_config").withValue("{{ .Chart.Name | upper }}").build())); + helmConfig.setParameters(Arrays.asList( + new ParameterBuilder().withName("annotation_from_config").withValue("{{ .Chart.Name | upper }}").build(), + new ParameterBuilder().withName("annotation.from.config.dotted").withValue("{{ .Chart.Name }}").build())); final AtomicInteger generatedChartCount = new AtomicInteger(0); helmConfig.setGeneratedChartListeners(Collections.singletonList( (helmConfig1, type, chartFile) -> generatedChartCount.incrementAndGet())); diff --git a/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/templates/kubernetes.yaml b/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/templates/kubernetes.yaml index 65893cfa46..60edcc6f21 100644 --- a/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/templates/kubernetes.yaml +++ b/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/templates/kubernetes.yaml @@ -56,13 +56,14 @@ items: helm-variable: {{ required "A valid .Values.global_template_env_var entry required!" .Values.global_template_env_var | default "This is a sample" }} escape-test: "{{"{{"}} {{"}}"}} should be escaped to prevent helm errors" annotation-from-config: {{ .Chart.Name | upper }} + annotation-from-config/dotted: {{ .Chart.Name }} labels: app: test provider: jkube version: 1337 name: test spec: - replicas: {{ .Values.deployment_replicas | default "1" }} + replicas: {{ .Values.deployment.replicas | default "1" }} revisionHistoryLimit: 2 selector: matchLabels: @@ -101,7 +102,7 @@ items: protocol: TCP resources: limits: - memory: {{ .Values.limits_memory | default "512Mi" }} + memory: {{ .Values.deployment.limits.memory | default "512Mi" }} requests: memory: {{ .Values.requests_memory | default "256Mi" }} securityContext: diff --git a/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/values.yaml b/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/values.yaml index a5f1df1314..3b728a60e0 100644 --- a/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/values.yaml +++ b/jkube-kit/resource/helm/src/test/resources/it/expected/kubernetes/values.yaml @@ -12,8 +12,10 @@ # Red Hat, Inc. - initial API and implementation # -deployment_replicas: "1" +deployment: + replicas: "1" + limits: + memory: 512Mi global_template_env_var: This is a sample -limits_memory: 512Mi unused_value: the-value requests_memory: 256Mi diff --git a/jkube-kit/resource/helm/src/test/resources/it/expected/openshift/values.yaml b/jkube-kit/resource/helm/src/test/resources/it/expected/openshift/values.yaml index a5f1df1314..3b728a60e0 100644 --- a/jkube-kit/resource/helm/src/test/resources/it/expected/openshift/values.yaml +++ b/jkube-kit/resource/helm/src/test/resources/it/expected/openshift/values.yaml @@ -12,8 +12,10 @@ # Red Hat, Inc. - initial API and implementation # -deployment_replicas: "1" +deployment: + replicas: "1" + limits: + memory: 512Mi global_template_env_var: This is a sample -limits_memory: 512Mi unused_value: the-value requests_memory: 256Mi diff --git a/jkube-kit/resource/helm/src/test/resources/it/sources/global-template.yml b/jkube-kit/resource/helm/src/test/resources/it/sources/global-template.yml index fccf3cb137..193b27b990 100644 --- a/jkube-kit/resource/helm/src/test/resources/it/sources/global-template.yml +++ b/jkube-kit/resource/helm/src/test/resources/it/sources/global-template.yml @@ -17,7 +17,7 @@ parameters: - name: GLOBAL_TEMPLATE_ENV_VAR required: true value: This is a sample - - name: limits_memory + - name: deployment.limits.memory value: 512Mi - name: requests_memory value: 256Mi @@ -25,5 +25,5 @@ parameters: value: the-value - name: golang_expression value: "{{ .Values.unused_value | upper | quote }}" - - name: deployment_replicas + - name: deployment.replicas value: 1 diff --git a/jkube-kit/resource/helm/src/test/resources/it/sources/kubernetes/kubernetes.yml b/jkube-kit/resource/helm/src/test/resources/it/sources/kubernetes/kubernetes.yml index 0904e4c237..54d8b1a139 100644 --- a/jkube-kit/resource/helm/src/test/resources/it/sources/kubernetes/kubernetes.yml +++ b/jkube-kit/resource/helm/src/test/resources/it/sources/kubernetes/kubernetes.yml @@ -56,13 +56,14 @@ items: helm-variable: ${GLOBAL_TEMPLATE_ENV_VAR} escape-test: "{{ }} should be escaped to prevent helm errors" annotation-from-config: ${annotation_from_config} + annotation-from-config/dotted: ${annotation.from.config.dotted} labels: app: test provider: jkube version: 1337 name: test spec: - replicas: ${deployment_replicas} + replicas: ${deployment.replicas} revisionHistoryLimit: 2 selector: matchLabels: @@ -101,7 +102,7 @@ items: protocol: TCP resources: limits: - memory: ${limits_memory} + memory: ${deployment.limits.memory} requests: memory: ${requests_memory} securityContext: diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/templates/helm-and-fragments-deployment.yaml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/templates/helm-and-fragments-deployment.yaml index 043c32a977..48d6652948 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/templates/helm-and-fragments-deployment.yaml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/templates/helm-and-fragments-deployment.yaml @@ -70,8 +70,8 @@ spec: protocol: TCP resources: limits: - memory: {{ .Values.limits_memory | default "512Mi" }} + memory: {{ .Values.deployment.limits.memory | default "512Mi" }} requests: - memory: {{ .Values.requests_memory | default "256Mi" }} + memory: {{ .Values.deployment.requests.memory | default "256Mi" }} securityContext: privileged: false diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/values.yaml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/values.yaml index 17ee8bc128..2a5a1c6f8a 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/values.yaml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/helm/values.yaml @@ -14,6 +14,9 @@ --- i_should_be_quoted: I'm a quoted uppercase value -limits_memory: 512Mi -requests_memory: 256Mi +deployment: + limits: + memory: 512Mi + requests: + memory: 256Mi a_value_with_a_default: The default diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes.yml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes.yml index e80c432657..c01975d077 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes.yml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes.yml @@ -92,7 +92,7 @@ items: group: org.eclipse.jkube name: first parameters: - - name: requests_memory + - name: deployment.requests.memory value: 256Mi - apiVersion: v1 kind: Template @@ -106,5 +106,5 @@ items: parameters: - name: replicas - name: paused - - name: limits_memory + - name: deployment.limits.memory value: 512Mi diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes/helm-and-fragments-deployment.yml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes/helm-and-fragments-deployment.yml index f89a89ba5d..75547af5a2 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes/helm-and-fragments-deployment.yml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/expected/resource/kubernetes/helm-and-fragments-deployment.yml @@ -64,8 +64,8 @@ spec: protocol: TCP resources: limits: - memory: ${limits_memory} + memory: ${deployment.limits.memory} requests: - memory: ${requests_memory} + memory: ${deployment.requests.memory} securityContext: privileged: false diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/deployment.yml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/deployment.yml index 35d7aea733..8d4da7281c 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/deployment.yml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/deployment.yml @@ -41,6 +41,6 @@ spec: privileged: false resources: limits: - memory: ${limits_memory} + memory: ${deployment.limits.memory} requests: - memory: ${requests_memory} + memory: ${deployment.requests.memory} diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/first-template.yml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/first-template.yml index 30244721ef..2402c097e7 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/first-template.yml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/first-template.yml @@ -14,5 +14,5 @@ kind: Template parameters: - - name: requests_memory - value: "256Mi" \ No newline at end of file + - name: deployment.requests.memory + value: "256Mi" diff --git a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/template.yml b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/template.yml index 47b215861b..aa417cba07 100644 --- a/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/template.yml +++ b/kubernetes-maven-plugin/it/src/it/helm-and-fragments/src/main/jkube/template.yml @@ -16,7 +16,7 @@ kind: Template parameters: - name: replicas - name: paused - - name: limits_memory + - name: deployment.limits.memory value: "512Mi" - name: parameter_with_no_value - name: unused_parameter