feat: Helm supports placeholders/variables with dotted notation

Signed-off-by: Marc Nuri <marc@marcnuri.com>
eclipse-jkube · Feb 1, 2022 · 2502caf · 2502caf
1 parent 09f42ab
commit 2502caf
Show file tree

Hide file tree

Showing 17 changed files with 148 additions and 28 deletions.
diff --git 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
@@ -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<String, Object, Object> GET_OR_NEW = (nK, nV) ->
+        nV == null ?  new LinkedHashMap<>() : nV;
+
     private MapUtil() {}
 
     /**
@@ -75,11 +79,67 @@ public static void putAllIfNotNull(Map<String, String> ret, Map<String, String>
      * Build a flattened representation of provided Map.
      *
      * <p> <i>The conversion is compliant with the thorntail spring-boot rules.</i>
+     *
+     * <p> Given a Map of Maps:
+     * <pre>{@code
+     * Collections.singletonMap("key", Collections.singletonMap("nested-key", "value"));
+     * }</pre>
+     *
+     * <p> It will return a Map with the following structure
+     * <pre>{@code
+     * Collections.singletonMap("key.nested-key", "value");
+     * }</pre>
      */
     public static Map<String, Object> getFlattenedMap(Map<?, ?> source) {
         return buildFlattenedMap(source, null);
     }
 
+    /**
+     * Build a nested representation of the provided Map.
+     *
+     * <p> Given a Map with a flat structure, it returns a Map of nested Maps. The original keys are split by the dot
+     * (<code>.</code>) character. For each element, a new Map node is created.
+     *
+     * <p> Given the following YAML representation of a Map:
+     * <pre>{@code
+     * one.two.key: value
+     * one.two.three: other
+     * }</pre>
+     *
+     * <p> It will converted to:
+     * <pre>{@code
+     * one:
+     *   two:
+     *     key: value
+     *     three: other
+     * }</pre>
+     */
+    public static Map<String, Object> getNestedMap(Map<String, ?> flattenedMap) {
+        final Map<String, Object> result = new LinkedHashMap<>();
+        flattenedMap.forEach((k, v) -> {
+            final String[] nodes = k.split("\\.");
+            if (nodes.length == 1) {
+                result.put(k, v);
+            } else {
+                Map<String, Object> 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<String, Object>) 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<String, Object> buildFlattenedMap(Map<?, ?> source, String keyPrefix) {
         final Map<String, Object> 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
@@ -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<String, Object> fiveNested = new LinkedHashMap();
+        final Map<String, Object> 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<String, Object> 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<String, Object> 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<String, Object> 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 <second> overlaps with node)");
+    }
+
+    @Test
+    public void getNestedMap_withInvalidFlattenedMap_shouldThrowNodeOverlapsException() {
+        // Given
+        final Map<String, Object> 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 <second> overlaps with key)");
+    }
 
     private Map<String, String> createMap(String ... args) {
         Map<String, String> 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
@@ -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<HelmParameter> 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<HelmParameter> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -17,13 +17,13 @@ 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
   - name: unused_value
     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
@@ -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/...n/it/src/it/helm-and-fragments/expected/helm/templates/helm-and-fragments-deployment.yaml b/...n/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
@@ -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
@@ -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/.../src/it/helm-and-fragments/expected/resource/kubernetes/helm-and-fragments-deployment.yml b/.../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
@@ -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
@@ -14,5 +14,5 @@
 
 kind: Template
 parameters:
-  - name: requests_memory
-    value: "256Mi"
+  - 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
@@ -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