From 5153857066e841a6fda604a80c5198912b02c77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Fri, 22 Feb 2019 09:27:10 +0100 Subject: [PATCH] fix(helm): allow duplicate keys in template Prevents "duplicated mapping key" errors when loading remote charts with duplicate keys (e.g. official ambassador chart). --- .../src/plugins/kubernetes/helm/common.ts | 19 ++++++++++-- .../duplicate-keys-in-template/.helmignore | 22 ++++++++++++++ .../duplicate-keys-in-template/Chart.yaml | 5 ++++ .../duplicate-keys-in-template/garden.yml | 4 +++ .../templates/service.yaml | 12 ++++++++ .../duplicate-keys-in-template/values.yaml | 29 +++++++++++++++++++ .../src/plugins/kubernetes/helm/common.ts | 5 ++++ 7 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 garden-service/test/data/test-projects/helm/duplicate-keys-in-template/.helmignore create mode 100644 garden-service/test/data/test-projects/helm/duplicate-keys-in-template/Chart.yaml create mode 100644 garden-service/test/data/test-projects/helm/duplicate-keys-in-template/garden.yml create mode 100644 garden-service/test/data/test-projects/helm/duplicate-keys-in-template/templates/service.yaml create mode 100644 garden-service/test/data/test-projects/helm/duplicate-keys-in-template/values.yaml diff --git a/garden-service/src/plugins/kubernetes/helm/common.ts b/garden-service/src/plugins/kubernetes/helm/common.ts index c8cb80a9ef..d3470a0e40 100644 --- a/garden-service/src/plugins/kubernetes/helm/common.ts +++ b/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 = safeLoadAll(await helm(namespace, context, log, + const objects = 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 }) +} diff --git a/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/.helmignore b/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/.helmignore new file mode 100644 index 0000000000..50af031725 --- /dev/null +++ b/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/ diff --git a/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/Chart.yaml b/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/Chart.yaml new file mode 100644 index 0000000000..52605c237a --- /dev/null +++ b/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 diff --git a/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/garden.yml b/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/garden.yml new file mode 100644 index 0000000000..76b6950c7c --- /dev/null +++ b/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 diff --git a/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/templates/service.yaml b/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/templates/service.yaml new file mode 100644 index 0000000000..9ed18a60f6 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/values.yaml b/garden-service/test/data/test-projects/helm/duplicate-keys-in-template/values.yaml new file mode 100644 index 0000000000..d0481dabb2 --- /dev/null +++ b/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: {} diff --git a/garden-service/test/src/plugins/kubernetes/helm/common.ts b/garden-service/test/src/plugins/kubernetes/helm/common.ts index e7eef849b4..15954a667b 100644 --- a/garden-service/test/src/plugins/kubernetes/helm/common.ts +++ b/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", () => {