From d1e9a242a7bcd829bc4d4036c62839fcb9a9580c Mon Sep 17 00:00:00 2001 From: Graham Reed Date: Thu, 20 Oct 2022 16:25:09 +0100 Subject: [PATCH 1/5] Pick the test improvement out of PR#8371 This covers: - `tpl` text can `include` a `define` provided in a partial file - `tpl` text can `include` a `define` provided in its text - `tpl` text can be loaded via `.Files.Get` Signed-off-by: Graham Reed --- pkg/engine/engine_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 54cd21ae2..2dcc75fa0 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -830,3 +830,38 @@ func TestRenderRecursionLimit(t *testing.T) { } } + +func TestRenderLoadTemplateForTplFromFile(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplLoadFromFile"}, + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`{{ tpl (.Files.Get .Values.filename) . }}`)}, + {Name: "templates/_function", Data: []byte(`{{define "test-function"}}test-function{{end}}`)}, + }, + Files: []*chart.File{ + {Name: "test", Data: []byte(`{{ tpl (.Files.Get .Values.filename2) .}}`)}, + {Name: "test2", Data: []byte(`{{include "test-function" .}}{{define "nested-define"}}nested-define-content{{end}} {{include "nested-define" .}}`)}, + }, + } + + v := chartutil.Values{ + "Values": chartutil.Values{ + "filename": "test", + "filename2": "test2", + }, + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + out, err := Render(c, v) + if err != nil { + t.Fatal(err) + } + + expect := "test-function nested-define-content" + if got := out["TplLoadFromFile/templates/base"]; got != expect { + t.Fatalf("Expected %q, got %q", expect, got) + } +} From 0a6e7d95aba9a029bf265a080591d1aa828d2144 Mon Sep 17 00:00:00 2001 From: Graham Reed Date: Thu, 20 Oct 2022 16:24:05 +0100 Subject: [PATCH 2/5] Make sure empty `tpl` values render empty. Signed-off-by: Graham Reed --- pkg/engine/engine_test.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 2dcc75fa0..41eb8c591 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -865,3 +865,36 @@ func TestRenderLoadTemplateForTplFromFile(t *testing.T) { t.Fatalf("Expected %q, got %q", expect, got) } } + +func TestRenderTplEmpty(t *testing.T) { + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplEmpty"}, + Templates: []*chart.File{ + {Name: "templates/empty-string", Data: []byte(`{{tpl "" .}}`)}, + {Name: "templates/empty-action", Data: []byte(`{{tpl "{{ \"\"}}" .}}`)}, + {Name: "templates/only-defines", Data: []byte(`{{tpl "{{define \"not-invoked\"}}not-rendered{{end}}" .}}`)}, + }, + } + v := chartutil.Values{ + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + out, err := Render(c, v) + if err != nil { + t.Fatal(err) + } + + expects := map[string]string{ + "TplEmpty/templates/empty-string": "", + "TplEmpty/templates/empty-action": "", + "TplEmpty/templates/only-defines": "", + } + for file, expect := range expects { + if out[file] != expect { + t.Errorf("Expected %q, got %q", expect, out[file]) + } + } +} From ebf5e1e2aff56bb15c8edbe209c5ce9da2af9237 Mon Sep 17 00:00:00 2001 From: Graham Reed Date: Thu, 20 Oct 2022 16:24:05 +0100 Subject: [PATCH 3/5] Check that `.Template` is passed through `tpl` Signed-off-by: Graham Reed --- pkg/engine/engine_test.go | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 41eb8c591..79e0d0b82 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -898,3 +898,52 @@ func TestRenderTplEmpty(t *testing.T) { } } } + +func TestRenderTplTemplateNames(t *testing.T) { + // .Template.BasePath and .Name make it through + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplTemplateNames"}, + Templates: []*chart.File{ + {Name: "templates/default-basepath", Data: []byte(`{{tpl "{{ .Template.BasePath }}" .}}`)}, + {Name: "templates/default-name", Data: []byte(`{{tpl "{{ .Template.Name }}" .}}`)}, + {Name: "templates/modified-basepath", Data: []byte(`{{tpl "{{ .Template.BasePath }}" .Values.dot}}`)}, + {Name: "templates/modified-name", Data: []byte(`{{tpl "{{ .Template.Name }}" .Values.dot}}`)}, + // Current implementation injects the 'tpl' template as if it were a template file, and + // so only BasePath and Name make it through. + {Name: "templates/modified-field", Data: []byte(`{{tpl "{{ .Template.Field }}" .Values.dot}}`)}, + }, + } + v := chartutil.Values{ + "Values": chartutil.Values{ + "dot": chartutil.Values{ + "Template": chartutil.Values{ + "BasePath": "path/to/template", + "Name": "name-of-template", + "Field": "extra-field", + }, + }, + }, + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + out, err := Render(c, v) + if err != nil { + t.Fatal(err) + } + + expects := map[string]string{ + "TplTemplateNames/templates/default-basepath": "TplTemplateNames/templates", + "TplTemplateNames/templates/default-name": "TplTemplateNames/templates/default-name", + "TplTemplateNames/templates/modified-basepath": "path/to/template", + "TplTemplateNames/templates/modified-name": "name-of-template", + "TplTemplateNames/templates/modified-field": "", + } + for file, expect := range expects { + if out[file] != expect { + t.Errorf("Expected %q, got %q", expect, out[file]) + } + } +} From 9fe912f3c559492daa925a3ecb7b8d4915dcdec3 Mon Sep 17 00:00:00 2001 From: Graham Reed Date: Thu, 20 Oct 2022 16:24:05 +0100 Subject: [PATCH 4/5] Check redefinition of define and include in tpl Signed-off-by: Graham Reed --- pkg/engine/engine_test.go | 51 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 79e0d0b82..9f1554d4d 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -947,3 +947,54 @@ func TestRenderTplTemplateNames(t *testing.T) { } } } + +func TestRenderTplRedefines(t *testing.T) { + // Redefining a template inside 'tpl' does not affect the outer definition + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplRedefines"}, + Templates: []*chart.File{ + {Name: "templates/_partials", Data: []byte(`{{define "partial"}}original-in-partial{{end}}`)}, + {Name: "templates/partial", Data: []byte( + `before: {{include "partial" .}}\n{{tpl .Values.partialText .}}\nafter: {{include "partial" .}}`, + )}, + {Name: "templates/manifest", Data: []byte( + `{{define "manifest"}}original-in-manifest{{end}}` + + `before: {{include "manifest" .}}\n{{tpl .Values.manifestText .}}\nafter: {{include "manifest" .}}`, + )}, + // The current implementation replaces the manifest text and re-parses, so a + // partial template defined only in the manifest invoking tpl cannot be accessed + // by that tpl call. + //{Name: "templates/manifest-only", Data: []byte( + // `{{define "manifest-only"}}only-in-manifest{{end}}` + + // `before: {{include "manifest-only" .}}\n{{tpl .Values.manifestOnlyText .}}\nafter: {{include "manifest-only" .}}`, + //)}, + }, + } + v := chartutil.Values{ + "Values": chartutil.Values{ + "partialText": `{{define "partial"}}redefined-in-tpl{{end}}tpl: {{include "partial" .}}`, + "manifestText": `{{define "manifest"}}redefined-in-tpl{{end}}tpl: {{include "manifest" .}}`, + "manifestOnlyText": `tpl: {{include "manifest-only" .}}`, + }, + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + out, err := Render(c, v) + if err != nil { + t.Fatal(err) + } + + expects := map[string]string{ + "TplRedefines/templates/partial": `before: original-in-partial\ntpl: original-in-partial\nafter: original-in-partial`, + "TplRedefines/templates/manifest": `before: original-in-manifest\ntpl: redefined-in-tpl\nafter: original-in-manifest`, + //"TplRedefines/templates/manifest-only": `before: only-in-manifest\ntpl: only-in-manifest\nafter: only-in-manifest`, + } + for file, expect := range expects { + if out[file] != expect { + t.Errorf("Expected %q, got %q", expect, out[file]) + } + } +} From f235f0f28564b4391ef8b0b5f06b2d754bc13873 Mon Sep 17 00:00:00 2001 From: Graham Reed Date: Thu, 11 May 2023 22:09:35 +0100 Subject: [PATCH 5/5] Check that missing keys are still handled in tpl Signed-off-by: Graham Reed --- pkg/engine/engine_test.go | 72 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index 9f1554d4d..f9b67d03f 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -21,6 +21,7 @@ import ( "strings" "sync" "testing" + "text/template" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" @@ -998,3 +999,74 @@ func TestRenderTplRedefines(t *testing.T) { } } } + +func TestRenderTplMissingKey(t *testing.T) { + // Rendering a missing key results in empty/zero output. + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplMissingKey"}, + Templates: []*chart.File{ + {Name: "templates/manifest", Data: []byte( + `missingValue: {{tpl "{{.Values.noSuchKey}}" .}}`, + )}, + }, + } + v := chartutil.Values{ + "Values": chartutil.Values{}, + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + out, err := Render(c, v) + if err != nil { + t.Fatal(err) + } + + expects := map[string]string{ + "TplMissingKey/templates/manifest": `missingValue: `, + } + for file, expect := range expects { + if out[file] != expect { + t.Errorf("Expected %q, got %q", expect, out[file]) + } + } +} + +func TestRenderTplMissingKeyString(t *testing.T) { + // Rendering a missing key results in error + c := &chart.Chart{ + Metadata: &chart.Metadata{Name: "TplMissingKeyStrict"}, + Templates: []*chart.File{ + {Name: "templates/manifest", Data: []byte( + `missingValue: {{tpl "{{.Values.noSuchKey}}" .}}`, + )}, + }, + } + v := chartutil.Values{ + "Values": chartutil.Values{}, + "Chart": c.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + e := new(Engine) + e.Strict = true + + out, err := e.Render(c, v) + if err == nil { + t.Errorf("Expected error, got %v", out) + return + } + switch err.(type) { + case (template.ExecError): + errTxt := fmt.Sprint(err) + if !strings.Contains(errTxt, "noSuchKey") { + t.Errorf("Expected error to contain 'noSuchKey', got %s", errTxt) + } + default: + // Some unexpected error. + t.Fatal(err) + } +}