Merge pull request #559 from garden-io/helm-handle-dupe-keys

fix(helm): allow duplicate keys in template

garden-service/src/plugins/kubernetes/helm/common.ts

@@ -16,7 +16,7 @@ import { PluginContext } from "../../../plugin-context"
 import { LogEntry } from "../../../logger/log-entry"
 import { getNamespace } from "../namespace"
 import { KubernetesResource } from "../types"
-import { safeLoadAll } from "js-yaml"
+import { loadAll } from "js-yaml"
 import { helm } from "./helm-cli"
 import { HelmModule, HelmModuleConfig, HelmResourceSpec } from "./config"
 import { HotReloadableResource } from "../hot-reload"
@@ -43,7 +43,7 @@ export async function getChartResources(ctx: PluginContext, module: Module, log:
   const context = ctx.provider.config.context
   const releaseName = getReleaseName(module)
 
-  const objects = <KubernetesResource[]>safeLoadAll(await helm(namespace, context, log,
+  const objects = <KubernetesResource[]>loadTemplate(await helm(namespace, context, log,
     "template",
     "--name", releaseName,
     "--namespace", namespace,
@@ -269,7 +269,7 @@ async function renderHelmTemplateString(
   try {
     await writeFile(tempFilePath, value)
 
-    const objects = safeLoadAll(await helm(namespace, context, log,
+    const objects = loadTemplate(await helm(namespace, context, log,
       "template",
       "--name", releaseName,
       "--namespace", namespace,
@@ -284,3 +284,16 @@ async function renderHelmTemplateString(
     await remove(tempFilePath)
   }
 }
+
+/**
+ * Helm templates can include duplicate keys, e.g. because of a mistake in the remote chart repo.
+ * We therefore load the template with `{ json: true }`, so that duplicate keys in a mapping will override values rather
+ * than throwing an error.
+ *
+ * However, this behavior is broken for the `safeLoadAll` function, see: https://github.com/nodeca/js-yaml/issues/456.
+ * We therefore need to use the `loadAll` function. See the following link for a conversation on using
+ * `loadAll` in this context: https://github.com/kubeapps/kubeapps/issues/636.
+ */
+function loadTemplate(template: string) {
+  return loadAll(template, undefined, { json: true })
+}

garden-service/test/data/test-projects/helm/duplicate-keys-in-template/.helmignore

@@ -0,0 +1,22 @@
+# Patterns to ignore when building packages.
+# This supports shell glob matching, relative path matching, and
+# negation (prefixed with !). Only one pattern per line.
+.DS_Store
+# Common VCS dirs
+.git/
+.gitignore
+.bzr/
+.bzrignore
+.hg/
+.hgignore
+.svn/
+# Common backup files
+*.swp
+*.bak
+*.tmp
+*~
+# Various IDEs
+.project
+.idea/
+*.tmproj
+.vscode/

garden-service/test/data/test-projects/helm/duplicate-keys-in-template/Chart.yaml

@@ -0,0 +1,5 @@
+apiVersion: v1
+appVersion: "1.0"
+description: A Helm chart for Kubernetes
+name: duplicate-keys-in-template
+version: 0.1.0

garden-service/test/data/test-projects/helm/duplicate-keys-in-template/garden.yml

@@ -0,0 +1,4 @@
+module:
+  description: Helm chart for the duplicate-keys-in-template container
+  type: helm
+  name: duplicate-keys-in-template

garden-service/test/data/test-projects/helm/duplicate-keys-in-template/templates/service.yaml

@@ -0,0 +1,12 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: duplicate-keys-in-template
+spec:
+  type: {{ .Values.service.type }}
+  type: {{ .Values.service.type }}
+  selector:
+    app: duplicate-keys-in-template
+  ports:
+  - port: 80
+    name: http

garden-service/test/data/test-projects/helm/duplicate-keys-in-template/values.yaml

@@ -0,0 +1,29 @@
+# Default values for node-service.
+# This is a YAML-formatted file.
+# Declare variables to be passed into your templates.
+
+replicaCount: 1
+
+nameOverride: ""
+fullnameOverride: ""
+
+service:
+  type: ClusterIP
+
+resources: {}
+  # We usually recommend not to specify default resources and to leave this as a conscious
+  # choice for the user. This also increases chances charts run on environments with little
+  # resources, such as Minikube. If you do want to specify resources, uncomment the following
+  # lines, adjust them as necessary, and remove the curly braces after 'resources:'.
+  # limits:
+  #  cpu: 100m
+  #  memory: 128Mi
+  # requests:
+  #  cpu: 100m
+  #  memory: 128Mi
+
+nodeSelector: {}
+
+tolerations: []
+
+affinity: {}

garden-service/test/src/plugins/kubernetes/helm/common.ts

@@ -433,6 +433,11 @@ describe("Helm common functions", () => {
         },
       ])
     })
+
+    it("should handle duplicate keys in template", async () => {
+      const module = await graph.getModule("duplicate-keys-in-template")
+      expect(await getChartResources(ctx, module, log)).to.not.throw
+    })
   })
 
   describe("getBaseModule", () => {