refactor: don't require an OpenShift Template:Parameter model type

Signed-off-by: Marc Nuri <[email protected]>
eclipse-jkube · Sep 27, 2023 · 5cb4251 · 5cb4251
1 parent 1005190
commit 5cb4251
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 54 deletions.
diff --git a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmConfig.java b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmConfig.java
@@ -62,7 +62,7 @@ public class HelmConfig {
    * the values.yaml file.
    */
   private List<Template> parameterTemplates;
-  private List<Parameter> parameters;
+  private List<HelmParameter> parameters;
   private List<HelmType> types;
   private String sourceDir;
   private String outputDir;

diff --git a/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmParameter.java b/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmParameter.java
@@ -13,21 +13,54 @@
  */
 package org.eclipse.jkube.kit.resource.helm;
 
-import com.fasterxml.jackson.annotation.JsonCreator;
-import io.fabric8.openshift.api.model.Parameter;
+import lombok.AllArgsConstructor;
+import lombok.Builder;
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
+import lombok.NoArgsConstructor;
+import lombok.ToString;
+import org.apache.commons.lang3.StringUtils;
 
+@Builder(toBuilder = true)
+@AllArgsConstructor
+@NoArgsConstructor
+@Getter
+@EqualsAndHashCode
+@ToString
 public class HelmParameter {
 
-  @Getter
-  private final Parameter parameter;
+  private static final String GOLANG_EXPRESSION_REGEX = "\\{\\{.+}}";
 
-  @JsonCreator
-  public HelmParameter(Parameter parameter) {
-    this.parameter = parameter;
+  private boolean required;
+  private String name;
+  private String value;
+
+  boolean isString() {
+    return value instanceof String;
+  }
+
+  boolean isGolangExpression() {
+    return isString() && ((String) value).trim().matches(GOLANG_EXPRESSION_REGEX);
   }
 
-  public String getHelmName() {
-    return parameter.getName();
+
+  String toExpression() {
+    if (/*!isString() ||*/ isGolangExpression()) {
+      return StringUtils.trimToEmpty(getValue());
+    }
+    final String defaultValue = StringUtils.trimToEmpty((String) getValue());
+    final String defaultExpression;
+    if (StringUtils.isNotBlank(defaultValue)) {
+      defaultExpression = " | default \"" + defaultValue + "\"";
+    } else {
+      defaultExpression = "";
+    }
+    final String requiredExpression;
+    if (isRequired()) {
+      requiredExpression = "required \"A valid .Values." + getName() + " entry required!\" ";
+    } else {
+      requiredExpression = "";
+    }
+    return "{{ " + requiredExpression + ".Values." + getName() + defaultExpression + " }}";
   }
 }
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
@@ -27,7 +27,6 @@
 import java.util.function.UnaryOperator;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import org.eclipse.jkube.kit.common.JKubeConfiguration;
 import org.eclipse.jkube.kit.common.KitLogger;
@@ -43,10 +42,8 @@
 import org.eclipse.jkube.kit.config.resource.ResourceServiceConfig;
 import org.eclipse.jkube.kit.enricher.api.util.KubernetesResourceFragments;
 
-import com.google.common.collect.Streams;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.KubernetesResource;
-import io.fabric8.openshift.api.model.Parameter;
 import io.fabric8.openshift.api.model.Template;
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.io.FileUtils;
@@ -71,7 +68,6 @@ public class HelmService {
 
   private static final String CHART_FRAGMENT_REGEX = "^chart\\.helm\\.(?<ext>yaml|yml|json)$";
   public static final Pattern CHART_FRAGMENT_PATTERN = Pattern.compile(CHART_FRAGMENT_REGEX, Pattern.CASE_INSENSITIVE);
-  private static final String GOLANG_EXPRESSION_REGEX = "\\{\\{.+}}";
 
   private final JKubeConfiguration jKubeConfiguration;
   private final ResourceServiceConfig resourceServiceConfig;
@@ -258,7 +254,7 @@ private static Chart chartFromHelmConfig(HelmConfig helmConfig) {
   }
 
   private static Chart createChartFromFragment(ResourceServiceConfig resourceServiceConfig, Properties properties) {
-    File helmChartFragment = resolveChartYamlFileFromFragmentsDir(resourceServiceConfig);
+    File helmChartFragment = resolveHelmFragment(CHART_FRAGMENT_PATTERN, resourceServiceConfig);
     if (helmChartFragment != null && helmChartFragment.exists()) {
       try {
         String interpolatedFragmentContent = interpolate(helmChartFragment, properties, DEFAULT_FILTER);
@@ -270,12 +266,12 @@ private static Chart createChartFromFragment(ResourceServiceConfig resourceServi
     return null;
   }
 
-  private static File resolveChartYamlFileFromFragmentsDir(ResourceServiceConfig resourceServiceConfig) {
+  private static File resolveHelmFragment(Pattern filePattern, ResourceServiceConfig resourceServiceConfig) {
     final List<File> fragmentDirs = resourceServiceConfig.getResourceDirs();
     if (fragmentDirs != null) {
       for (File fragmentDir : fragmentDirs) {
         if (fragmentDir.exists() && fragmentDir.isDirectory()) {
-          final File[] fragments = fragmentDir.listFiles((dir, name) -> CHART_FRAGMENT_PATTERN.matcher(name).matches());
+          final File[] fragments = fragmentDir.listFiles((dir, name) -> filePattern.matcher(name).matches());
           if (fragments != null && fragments.length > 0) {
             return fragments[0];
           }
@@ -292,33 +288,18 @@ private static void copyAdditionalFiles(HelmConfig helmConfig, File outputDir) t
   }
 
   private static String interpolateTemplateWithHelmParameter(String template, HelmParameter parameter) {
-    String name = parameter.getParameter().getName();
+    final String name = parameter.getName();
     final String from = "$" + name;
     final String braceEnclosedFrom = "${" + name + "}";
     final String quotedBraceEnclosedFrom = "\"" + braceEnclosedFrom + "\"";
     String answer = template;
-    final String to = expression(parameter);
+    final String to = parameter.toExpression();
     answer = answer.replace(quotedBraceEnclosedFrom, to);
     answer = answer.replace(braceEnclosedFrom, to);
     answer = answer.replace(from, to);
     return answer;
   }
 
-  private static String expression(HelmParameter parameter) {
-    final String value = Optional.ofNullable(parameter.getParameter().getValue()).map(StringUtils::trimToEmpty).orElse("");
-    if (value.matches(GOLANG_EXPRESSION_REGEX)) {
-      return value;
-    }
-    String defaultExpression = "";
-    String required = "";
-    if (StringUtils.isNotBlank(value)) {
-      defaultExpression = " | default \"" + value + "\"";
-    }
-    if (Boolean.TRUE.equals(parameter.getParameter().getRequired())) {
-      required = "required \"A valid .Values." + parameter.getHelmName() + " entry required!\" ";
-    }
-    return "{{ " + required + ".Values." + parameter.getHelmName() + defaultExpression + " }}";
-  }
 
   private static void interpolateTemplateParameterExpressionsWithHelmExpressions(File file, List<HelmParameter> helmParameters) throws IOException {
     final String originalTemplate = FileUtils.readFileToString(file, Charset.defaultCharset());
@@ -340,26 +321,30 @@ private static void interpolateChartTemplates(List<HelmParameter> helmParameters
   }
 
   private static void createValuesYaml(List<HelmParameter> helmParameters, File outputDir) throws IOException {
-    final Map<String, String> values = helmParameters.stream()
-        .filter(hp -> hp.getParameter().getValue() != null)
+    final Map<String, Object> values = helmParameters.stream()
+        .filter(hp -> hp.getValue() != null)
         // Placeholders replaced by Go expressions don't need to be persisted in the values.yaml file
-        .filter(hp -> !hp.getParameter().getValue().trim().matches(GOLANG_EXPRESSION_REGEX))
-        .collect(Collectors.toMap(HelmParameter::getHelmName, hp -> hp.getParameter().getValue()));
+        .filter(hp -> !hp.isGolangExpression())
+        .collect(Collectors.toMap(HelmParameter::getName, HelmParameter::getValue));
 
     final File outputValuesFile = new File(outputDir, VALUES_FILENAME);
     ResourceUtil.save(outputValuesFile, getNestedMap(values), ResourceFileType.yaml);
   }
 
   private static List<HelmParameter> collectParameters(HelmConfig helmConfig) {
     final List<HelmParameter> parameters = new ArrayList<>();
-    final Stream<Parameter> fromYaml = Optional.ofNullable(helmConfig.getParameterTemplates())
-        .orElse(Collections.emptyList()).stream()
-        .map(Template::getParameters).flatMap(List::stream);
-    final Stream<Parameter> fromConfig = Optional.ofNullable(helmConfig.getParameters())
-        .orElse(Collections.emptyList()).stream();
-    Streams.concat(fromYaml, fromConfig).map(HelmParameter::new).forEach(parameters::add);
-    parameters.stream().filter(p -> p.getParameter().getName() == null).findAny().ifPresent(p -> {
-      throw new IllegalArgumentException("Helm parameters must be declared with a valid name: " + p.getParameter().toString());
+    if (helmConfig.getParameterTemplates() != null) {
+      helmConfig.getParameterTemplates().stream()
+          .map(Template::getParameters).flatMap(List::stream)
+          .map(p -> HelmParameter.builder()
+            .name(p.getName()).required(Boolean.TRUE.equals(p.getRequired())).value(p.getValue()).build())
+          .forEach(parameters::add);
+    }
+    if (helmConfig.getParameters() != null && !helmConfig.getParameters().isEmpty()) {
+      parameters.addAll(helmConfig.getParameters());
+    }
+    parameters.stream().filter(p -> p.getName() == null).findAny().ifPresent(p -> {
+      throw new IllegalArgumentException("Helm parameters must be declared with a valid name: " + p);
     });
     return parameters;
   }

diff --git a/...e-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmConfigTest.java b/...e-kit/resource/helm/src/test/java/org/eclipse/jkube/kit/resource/helm/HelmConfigTest.java
@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.stream.Stream;
 
-import io.fabric8.openshift.api.model.ParameterBuilder;
 import org.eclipse.jkube.kit.common.Maintainer;
 import org.eclipse.jkube.kit.resource.helm.HelmConfig.HelmType;
 
@@ -128,7 +127,7 @@ void deserialize() throws Exception {
         .hasFieldOrPropertyWithValue("stableRepository", new HelmRepository())
         .hasFieldOrPropertyWithValue("tarballOutputDir", "./tar-output")
         .hasFieldOrPropertyWithValue("parameterTemplates", Collections.singletonList(new Template()))
-        .hasFieldOrPropertyWithValue("parameters", Collections.singletonList(new ParameterBuilder().withName("key").build()))
+        .hasFieldOrPropertyWithValue("parameters", Collections.singletonList(HelmParameter.builder().name("key").build()))
         .hasFieldOrPropertyWithValue("description", "The description")
         .hasFieldOrPropertyWithValue("home", "e.t.")
         .hasFieldOrPropertyWithValue("icon", "Warhol")

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
@@ -31,7 +31,6 @@
 import org.eclipse.jkube.kit.common.KitLogger;
 import org.eclipse.jkube.kit.common.assertj.ArchiveAssertions;
 
-import io.fabric8.openshift.api.model.ParameterBuilder;
 import io.fabric8.openshift.api.model.Template;
 import org.apache.commons.io.FileUtils;
 import org.eclipse.jkube.kit.common.util.Serialization;
@@ -77,8 +76,8 @@ void generateHelmChartsTest() throws Exception {
         Serialization.unmarshal(HelmServiceIT.class.getResource("/it/sources/global-template.yml"), Template.class)
     ));
     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()));
+        HelmParameter.builder().name("annotation_from_config").value("{{ .Chart.Name | upper }}").build(),
+        HelmParameter.builder().name("annotation.from.config.dotted").value("{{ .Chart.Name }}").build()));
     final AtomicInteger generatedChartCount = new AtomicInteger(0);
     helmConfig.setGeneratedChartListeners(Collections.singletonList(
         (helmConfig1, type, chartFile) -> generatedChartCount.incrementAndGet()));
@@ -117,8 +116,8 @@ void generateHelmChartsTest() throws Exception {
   void generateHelmChartsTest_withInvalidParameters_throwsException() {
     // Given
     helmConfig.setTypes(Collections.singletonList(HelmConfig.HelmType.KUBERNETES));
-    helmConfig.setParameters(Collections.singletonList(new ParameterBuilder()
-        .withValue("{{ .Chart.Name | upper }}").build()));
+    helmConfig.setParameters(Collections.singletonList(HelmParameter.builder()
+        .value("{{ .Chart.Name | upper }}").build()));
     // When
     final IllegalArgumentException result = assertThrows(IllegalArgumentException.class, () ->
         helmService.generateHelmCharts(helmConfig));
@@ -130,7 +129,7 @@ void generateHelmChartsTest_withInvalidParameters_throwsException() {
   void generateHelmChartsTest_preserveParameterCase() throws Exception {
     // Given
     helmConfig.setTypes(Collections.singletonList(HelmConfig.HelmType.KUBERNETES));
-    helmConfig.setParameters(Collections.singletonList(new ParameterBuilder().withName("testCamelCase").withValue("testValue").build()));
+    helmConfig.setParameters(Collections.singletonList(HelmParameter.builder().name("testCamelCase").value("testValue").build()));
     // When
     helmService.generateHelmCharts(helmConfig);
     // Then

diff --git a/jkube-kit/resource/helm/src/test/resources/valid-helm-fragments/values.helm.yaml b/jkube-kit/resource/helm/src/test/resources/valid-helm-fragments/values.helm.yaml
@@ -0,0 +1,34 @@
+replicaCount: 1
+
+image:
+  repository: nginx
+  pullPolicy: IfNotPresent
+  # Overrides the image tag whose default is the chart appVersion.
+  tag: ""
+
+serviceAccount:
+  # Specifies whether a service account should be created
+  create: true
+  # Annotations to add to the service account
+  annotations: {}
+  # The name of the service account to use.
+  # If not set and create is true, a name is generated using the fullname template
+  name: ""
+
+service:
+  type: ClusterIP
+  port: 80
+
+ingress:
+  enabled: false
+  className: ""
+  annotations:
+    kubernetes.io/ingress.class: nginx
+    kubernetes.io/tls-acme: "true"
+  hosts:
+    - host: chart-example.local
+      paths:
+        - path: /
+          pathType: ImplementationSpecific
+  tls: []
+