From d53993f4f88a6ee5b3c96f7c70adccf0c763ba4b Mon Sep 17 00:00:00 2001 From: Oleg Sidorov Date: Sat, 2 Nov 2019 23:11:49 +0100 Subject: [PATCH] chartutil.ReadValues unmarshals numbers into json.Number refs #1707 This change is a second attempt to fix the aforementioned problem. The first one (#6032) caused a major regression in numeric values #6708. The new approach takes the experience of the failed attempt under consideration and introduces some deeper-level changes to the rendering engine. The proposed change overloads all defined template functions and converts json.Number arguments in runtime. The rest of the description is preserved from the original ticket: This change is an attempt to address the common problem of json number unmarshalling where any number is converted into a float64 and represented in a scientific notation on a marshall call. This behavior breaks things like: chart versions and image tags if not converted to yaml strings explicitly. An example of this behavior: k8s failure to fetch an image tagged with a big number like: `$IMAGE:20190612073634` after a few steps of yaml re-rendering turns into: `$IMAGE:2.0190612073634e+13` Example issue: #1707 This commit forces yaml parser to use JSON modifiers and explicitly enables interface{} unmarshalling instead of float64. The change introduced might be breaking so should be processed with an extra care. Due to the fact helm mostly dals with human-produced data (charts), we have a decent level of confidence this change looses no functionality helm users rely upon (the scientific notation). Relevant doc: https://golang.org/pkg/encoding/json/#Decoder.UseNumber Signed-off-by: Oleg Sidorov --- pkg/chart/loader/load.go | 6 +- pkg/chart/loader/load_test.go | 45 +++ pkg/chartutil/dependencies_test.go | 5 + pkg/chartutil/testdata/coleridge.yaml | 1 + pkg/chartutil/values.go | 10 +- pkg/chartutil/values_test.go | 14 +- pkg/cli/values/options.go | 3 +- pkg/engine/engine_test.go | 383 ++++++++++++++++++++++++++ pkg/engine/funcs.go | 16 ++ pkg/engine/overloads.go | 338 +++++++++++++++++++++++ 10 files changed, 817 insertions(+), 4 deletions(-) create mode 100644 pkg/engine/overloads.go diff --git a/pkg/chart/loader/load.go b/pkg/chart/loader/load.go index dd4fd2dff..8643e90db 100644 --- a/pkg/chart/loader/load.go +++ b/pkg/chart/loader/load.go @@ -18,6 +18,7 @@ package loader import ( "bytes" + "encoding/json" "log" "os" "path/filepath" @@ -96,7 +97,10 @@ func LoadFiles(files []*BufferedFile) (*chart.Chart, error) { } case f.Name == "values.yaml": c.Values = make(map[string]interface{}) - if err := yaml.Unmarshal(f.Data, &c.Values); err != nil { + if err := yaml.Unmarshal(f.Data, &c.Values, func(d *json.Decoder) *json.Decoder { + d.UseNumber() + return d + }); err != nil { return c, errors.Wrap(err, "cannot load values.yaml") } case f.Name == "values.schema.json": diff --git a/pkg/chart/loader/load_test.go b/pkg/chart/loader/load_test.go index 26513d359..5286907d5 100644 --- a/pkg/chart/loader/load_test.go +++ b/pkg/chart/loader/load_test.go @@ -20,6 +20,7 @@ import ( "archive/tar" "bytes" "compress/gzip" + "encoding/json" "io/ioutil" "os" "path/filepath" @@ -199,6 +200,50 @@ icon: https://example.com/64x64.png } } +// This test case covers some special numeric values +func TestLoadNumericValuesAsJsonNumber(t *testing.T) { + files := []*BufferedFile{ + { + Name: "Chart.yaml", + Data: []byte(`apiVersion: v1 +name: frobnitz +description: This is a frobnitz. +version: "1.2.3" +`), + }, + { + Name: "values.yaml", + Data: []byte(`varInt: 1234567890 +varIntNeg: -987654321 +varFloat: 3.141593 +varFloatSci: 5.e-6 +varString: "2.71828"`), + }, + } + expected := map[string]interface{}{ + "varInt": json.Number("1234567890"), + "varIntNeg": json.Number("-987654321"), + "varFloat": json.Number("3.141593"), + // varFloatSci case is quite unpleasant: with all the dancing we do + // around formatting numbers, we can't preserve the original scientific + // notation without deep-hacking into yaml parser. This case sounds like + // something a user should expect if they provide a float number in + // scientific notation that it would be interpreted and re-formatted. + "varFloatSci": json.Number("0.000005"), + "varString": "2.71828", + } + c, err := LoadFiles(files) + if err != nil { + t.Errorf("Expected files to be loaded, got %v", err) + } + for varName, expVal := range expected { + if c.Values[varName] != expVal { + t.Errorf("Unexpected loaded value %s: got (%T)%+[2]v, want: (%T)%+[3]v", + varName, c.Values[varName], expVal) + } + } +} + // Packaging the chart on a Windows machine will produce an // archive that has \\ as delimiters. Test that we support these archives func TestLoadFileBackslash(t *testing.T) { diff --git a/pkg/chartutil/dependencies_test.go b/pkg/chartutil/dependencies_test.go index 342d7fe87..6e82149ee 100644 --- a/pkg/chartutil/dependencies_test.go +++ b/pkg/chartutil/dependencies_test.go @@ -15,6 +15,7 @@ limitations under the License. package chartutil import ( + "encoding/json" "os" "path/filepath" "sort" @@ -231,6 +232,10 @@ func TestProcessDependencyImportValues(t *testing.T) { if b := strconv.FormatBool(pv); b != vv { t.Errorf("failed to match imported bool value %v with expected %v", b, vv) } + case json.Number: + if n := pv.String(); n != vv { + t.Errorf("failed to match imported json.Number value %v with expected %v", n, vv) + } default: if pv != vv { t.Errorf("failed to match imported string value %q with expected %q", pv, vv) diff --git a/pkg/chartutil/testdata/coleridge.yaml b/pkg/chartutil/testdata/coleridge.yaml index b6579628b..15535988b 100644 --- a/pkg/chartutil/testdata/coleridge.yaml +++ b/pkg/chartutil/testdata/coleridge.yaml @@ -10,3 +10,4 @@ water: water: where: "everywhere" nor: "any drop to drink" + temperature: 1234567890 diff --git a/pkg/chartutil/values.go b/pkg/chartutil/values.go index e1cdf4642..b191ac60f 100644 --- a/pkg/chartutil/values.go +++ b/pkg/chartutil/values.go @@ -17,6 +17,7 @@ limitations under the License. package chartutil import ( + "encoding/json" "fmt" "io" "io/ioutil" @@ -31,6 +32,13 @@ import ( // GlobalKey is the name of the Values key that is used for storing global vars. const GlobalKey = "global" +// JSONNumberOn is a JSON parser option enforcing parsing numeric values to +// json.JSONNumber instead of numeric golang primitives. +var JSONNumberOn yaml.JSONOpt = func(d *json.Decoder) *json.Decoder { + d.UseNumber() + return d +} + // Values represents a collection of chart values. type Values map[string]interface{} @@ -105,7 +113,7 @@ func tableLookup(v Values, simple string) (Values, error) { // ReadValues will parse YAML byte data into a Values. func ReadValues(data []byte) (vals Values, err error) { - err = yaml.Unmarshal(data, &vals) + err = yaml.Unmarshal(data, &vals, JSONNumberOn) if len(vals) == 0 { vals = Values{} } diff --git a/pkg/chartutil/values_test.go b/pkg/chartutil/values_test.go index c95fa503a..ffe5d854d 100644 --- a/pkg/chartutil/values_test.go +++ b/pkg/chartutil/values_test.go @@ -45,6 +45,7 @@ water: water: where: "everywhere" nor: "any drop to drink" + temperature: 1234567890 ` data, err := ReadValues([]byte(doc)) @@ -53,7 +54,12 @@ water: } matchValues(t, data) - tests := []string{`poet: "Coleridge"`, "# Just a comment", ""} + tests := []string{ + `poet: "Coleridge"`, + "# Just a comment", + "water.water.temperature: 1234567890", + "", + } for _, tt := range tests { data, err = ReadValues([]byte(tt)) @@ -245,6 +251,12 @@ func matchValues(t *testing.T, data map[string]interface{}) { } else if o != "everywhere" { t.Errorf("Expected water water everywhere") } + + if o, err := ttpl("{{.water.water.temperature}}", data); err != nil { + t.Errorf(".water.water.temperature: %s", err) + } else if o != "1234567890" { + t.Errorf("Expected water water temperature: 1234567890, got: %s", o) + } } func ttpl(tpl string, v map[string]interface{}) (string, error) { diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go index e6ad71767..8c2c9c4e9 100644 --- a/pkg/cli/values/options.go +++ b/pkg/cli/values/options.go @@ -25,6 +25,7 @@ import ( "github.com/pkg/errors" "sigs.k8s.io/yaml" + "helm.sh/helm/v3/pkg/chartutil" "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/strvals" ) @@ -50,7 +51,7 @@ func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, er return nil, err } - if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { + if err := yaml.Unmarshal(bytes, ¤tMap, chartutil.JSONNumberOn); err != nil { return nil, errors.Wrapf(err, "failed to parse %s", filePath) } // Merge with the previous map diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index d5f36aac8..33ac616fd 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -698,3 +698,386 @@ func TestRenderRecursionLimit(t *testing.T) { } } + +func TestTemplateFuncs(t *testing.T) { + tests := []struct { + Name string + Templates []*chart.File + Values string + ExpectTplStr string + }{ + { + Name: "TplIntFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ .Values.value | int}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplInt64Function", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ .Values.value | int64}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplFloat64Function", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ .Values.value | float64}}" .}}`)}, + }, + Values: `value: 3.14159265359`, + ExpectTplStr: "Evaluate tpl Value: 3.14159265359", + }, + { + Name: "TplAdd1Function", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ .Values.value | add1}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 43", + }, + { + Name: "TplAddFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ add .Values.value 1 2 3}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 48", + }, + { + Name: "TplSubFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ sub .Values.value 20}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 22", + }, + { + Name: "TplDivFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ div .Values.value 2}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 21", + }, + { + Name: "TplModFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ mod .Values.value 5}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 2", + }, + { + Name: "TplMulFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ mul .Values.value 1 2 3}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 252", + }, + { + Name: "TplMaxFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ max .Values.value 100 1 0 -1}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 100", + }, + { + Name: "TplMinFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ min .Values.value 100 1 0 -1}}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: -1", + }, + { + Name: "TplCeilFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ ceil .Values.value }}" .}}`)}, + }, + Values: `value: 3.14159265359`, + ExpectTplStr: "Evaluate tpl Value: 4", + }, + { + Name: "TplFloorFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ floor .Values.value }}" .}}`)}, + }, + Values: `value: 3.14159265359`, + ExpectTplStr: "Evaluate tpl Value: 3", + }, + { + Name: "TplRoundFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "Value: {{ round .Values.value 2 }}" .}}`)}, + }, + Values: `value: 3.14159265359`, + ExpectTplStr: "Evaluate tpl Value: 3.14", + }, + { + Name: "TplEqFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if eq .Values.value 42 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplEqFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if eq .Values.value 42.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplGtFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if gt .Values.value 41 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplGtFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if gt .Values.value 41.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplGeFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if ge .Values.value 42 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplGeFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if ge .Values.value 42.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplLtFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if lt .Values.value 43 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplLtFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if lt .Values.value 43.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplLeFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if le .Values.value 42 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplLeFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if le .Values.value 42.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplNeFunctionInt", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if ne .Values.value 43 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplNeFunctionFloat", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ if ne .Values.value 43.0 }}Value: {{ .Values.value }}{{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl Value: 42", + }, + { + Name: "TplUntilFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ range $ix := until .Values.value }}{{ $ix }}{{ end }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl 01234", + }, + { + Name: "TplUntilStepFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ range $ix := untilStep 0 .Values.value 7 }}{{ $ix }} {{ end }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl 0 7 14 21 28 35 ", + }, + { + Name: "TplSplitnFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ range $s := splitn \".\" .Values.value \"foo.bar.baz.boo\" }}{{ $s }}{{ end }}" .}}`)}, + }, + Values: `value: 3`, + ExpectTplStr: "Evaluate tpl foobarbaz.boo", + }, + { + Name: "TplAbbrevFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ abbrev .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl he...", + }, + { + Name: "TplAbbrevBothFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ abbrevboth .Values.value 10 \"1234 5678 9123\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl ...5678...", + }, + { + Name: "TplTruncFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ trunc .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl hello", + }, + { + Name: "TplSubstrFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ substr 0 .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl hello", + }, + { + Name: "TplRepeatFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ repeat .Values.value \"hello\" }}" .}}`)}, + }, + Values: `value: 3`, + ExpectTplStr: "Evaluate tpl hellohellohello", + }, + { + Name: "TplWrapFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ wrap .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl hello\nworld", + }, + { + Name: "TplWrapWithFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ wrapWith .Values.value \"\t\" \"hello world\" }}" .}}`)}, + }, + Values: `value: 5`, + ExpectTplStr: "Evaluate tpl hello\tworld", + }, + { + Name: "TplIndentFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ indent .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 4`, + ExpectTplStr: "Evaluate tpl hello world", + }, + { + Name: "TplNindentFunction", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ nindent .Values.value \"hello world\" }}" .}}`)}, + }, + Values: `value: 4`, + ExpectTplStr: "Evaluate tpl \n hello world", + }, + { + Name: "TplPluralFunctionSingular", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ plural \"one anchovy\" \"many anchovies\" .Values.value }}" .}}`)}, + }, + Values: `value: 1`, + ExpectTplStr: "Evaluate tpl one anchovy", + }, + { + Name: "TplPluralFunctionPlural", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ plural \"one anchovy\" \"many anchovies\" .Values.value }}" .}}`)}, + }, + Values: `value: 42`, + ExpectTplStr: "Evaluate tpl many anchovies", + }, + { + Name: "TplSliceFunctionNoOffset", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ slice .Values.slice .Values.value }}" .}}`)}, + }, + Values: ` +value: 2 +slice: [1,2,3,4,5]`, + ExpectTplStr: "Evaluate tpl [3 4 5]", + }, + { + Name: "TplSliceFunctionWithOffset", + Templates: []*chart.File{ + {Name: "templates/base", Data: []byte(`Evaluate tpl {{tpl "{{ slice .Values.slice .Values.start .Values.end }}" .}}`)}, + }, + Values: ` +slice: [1,2,3,4,5] +start: 2 +end: 4`, + ExpectTplStr: "Evaluate tpl [3 4]", + }, + } + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + tplChart := &chart.Chart{ + Metadata: &chart.Metadata{Name: tt.Name}, + Templates: tt.Templates, + } + + values, err := chartutil.ReadValues([]byte(tt.Values)) + if err != nil { + t.Fatalf(err.Error()) + } + + tplValues := chartutil.Values{ + "Values": values, + "Chart": tplChart.Metadata, + "Release": chartutil.Values{ + "Name": "TestRelease", + }, + } + + e := &Engine{} + outTpl, err := e.Render(tplChart, tplValues) + if err != nil { + t.Fatal(err) + } + + if gotTplStr := outTpl[tt.Name+"/templates/base"]; gotTplStr != tt.ExpectTplStr { + t.Errorf("Expected %q, got %q (%v)", tt.ExpectTplStr, gotTplStr, outTpl) + } + }) + } +} diff --git a/pkg/engine/funcs.go b/pkg/engine/funcs.go index e5769cbe0..832b1f66d 100644 --- a/pkg/engine/funcs.go +++ b/pkg/engine/funcs.go @@ -68,6 +68,22 @@ func funcMap() template.FuncMap { f[k] = v } + for k, v := range f { + f[k] = overloadFunc(v) + } + + stdTmplOverloads := map[string]interface{}{ + "eq": _templateBuiltinEq, + "ge": _templateBuiltinGe, + "gt": _templateBuiltinGt, + "le": _templateBuiltinLe, + "lt": _templateBuiltinLt, + "ne": _templateBuiltinNe, + } + for fn, fun := range stdTmplOverloads { + f[fn] = overloadFunc(fun) + } + return f } diff --git a/pkg/engine/overloads.go b/pkg/engine/overloads.go new file mode 100644 index 000000000..38d1ec993 --- /dev/null +++ b/pkg/engine/overloads.go @@ -0,0 +1,338 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package engine + +import ( + "C" + "reflect" + _ "unsafe" +) + +// jsonNumber is an interface that mocks json.Number behavior. The method set is +// completely identical to the original struct definition. Using an internal +// interface allows us to get rid of explicit encoding/json dependency in this +// package. +type jsonNumber interface { + Float64() (float64, error) + Int64() (int64, error) + String() string +} + +type argctx uint8 + +const ( + ctxInt argctx = 1 << (iota + 1) + ctxFloat + ctxAllref +) + +var ( + // A hack to get a type of an empty interface + intfType reflect.Type = reflect.ValueOf(func(interface{}) {}).Type().In(0) + intType = reflect.TypeOf(int(0)) + int64Type = reflect.TypeOf(int64(0)) + float64Type = reflect.TypeOf(float64(0)) +) + +var castNumericTo map[reflect.Kind]reflect.Kind +var typeConverters map[reflect.Kind]reflect.Type + +func init() { + castNumericTo = make(map[reflect.Kind]reflect.Kind) + castNumericTo[reflect.Interface] = 0 + for _, kind := range []reflect.Kind{reflect.Int, reflect.Uint} { + castNumericTo[kind] = reflect.Int + } + for _, kind := range []reflect.Kind{reflect.Int32, reflect.Int64, reflect.Uint32, reflect.Uint64} { + castNumericTo[kind] = reflect.Int64 + } + for _, kind := range []reflect.Kind{reflect.Float32, reflect.Float64} { + castNumericTo[kind] = reflect.Float64 + } + typeConverters = map[reflect.Kind]reflect.Type{ + reflect.Int: intType, + reflect.Int64: int64Type, + reflect.Float64: float64Type, + } +} + +func isIntKind(kind reflect.Kind) bool { + switch kind { + case reflect.Int, + reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return true + } + return false +} + +func isFloatKind(kind reflect.Kind) bool { + switch kind { + case reflect.Float32, reflect.Float64: + return true + } + return false +} + +// guessArgsCtx iterates over the input arguments and tries to guess a common +// value type numeric context. The return value is a bit mask holding bit flags +// for float and int types. If all arguments are of type reflect.Value, the +// result bitmask will contain a dedicated flag ctxAllref set to 1. For +// variadic functions the last argument is expected to be a slice and is handled +// the same way as the main list. +func guessArgsCtx(args []reflect.Value, isvariadic bool) argctx { + var ctx, msk argctx + ctx |= ctxAllref + for _, arg := range args { + msk = ^ctxAllref + kind := arg.Kind() + switch kind { + case reflect.Struct: + if v, ok := arg.Interface().(reflect.Value); ok { + kind = v.Kind() + } + msk |= ctxAllref + case reflect.Interface: + v := reflect.ValueOf(arg.Interface()) + kind = v.Kind() + } + if isFloatKind(kind) { + ctx |= ctxFloat + } else if isIntKind(kind) { + ctx |= ctxInt + } + ctx &= msk + } + // Variadic functions are handled in a slightly special way + if isvariadic && len(args) > 0 { + // The last argument in variadic functions is a slice and we should + // iterate all over the variadic arguments the same way we do it for the + // regular args. + varg := args[len(args)-1] + varargs := make([]reflect.Value, 0, varg.Len()) + for i := 0; i < varg.Len(); i++ { + varargs = append(varargs, varg.Index(i)) + } + // We call the same routine with an explicit flag that the argument list + // is not variadic + varctx := guessArgsCtx(varargs, false) + varmsk := ^ctxAllref + if ctx&ctxAllref > 0 { + varmsk |= ctxAllref + } + return (varctx | ctx) & varmsk + } + return ctx +} + +// convJSONNumber converts a jsonNumber argument to a particular primitive +// defined by a wanted kind or an argument context. +// The context would be used if the wanted kind is unknown (we use 0 as an +// undefined value. This can happen if the argument wanted kind is interface{} +// or reflect.Value, which defines no specific primitive to convert to. In this +// case a broader observaion is used to define the optimal conversion strategy. +func convJSONNumber(jsnum jsonNumber, wantkind reflect.Kind, ctx argctx) (interface{}, error) { + switch wantkind { + case reflect.Int: + int64val, err := jsnum.Int64() + if err != nil { + return nil, err + } + return int(int64val), nil + case reflect.Int64: + return jsnum.Int64() + case reflect.Float64: + return jsnum.Float64() + // The wanted kind is unknown yet, we should guess it from the context + case 0: + switch { + case ctx&ctxInt > 0: + if intval, err := convJSONNumber(jsnum, reflect.Int64, ctx); err == nil { + return intval, nil + } + case ctx&ctxFloat > 0: + if floatval, err := convJSONNumber(jsnum, reflect.Float64, ctx); err == nil { + return floatval, nil + } + } + } + return jsnum.String(), nil +} + +// convIntf converts a given value to a wanted kind within a provided argument +// context. If the value conforms to jsonNumber interface, the conversion is +// delegated to convJSONNumber. If the value is of a numeric type, conversion is +// performed according to the conversion table defined by typeConverters. +func convIntf(val reflect.Value, wantkind reflect.Kind, ctx argctx) reflect.Value { + intf := val.Interface() + if jsnum, ok := intf.(jsonNumber); ok { + if convval, err := convJSONNumber(jsnum, castNumericTo[wantkind], ctx); err == nil { + return reflect.ValueOf(convval) + } + } + if convtype, ok := typeConverters[wantkind]; ok { + if reflect.TypeOf(intf).ConvertibleTo(convtype) { + return reflect.ValueOf(intf).Convert(convtype) + } + } + // If no conversion was performed, we return the value as is + return val +} + +// convFuncArg converts an argument of 2 particular types: interface{} and reflect.Value +// to a primitive. Both types provide no certainty on the final type and kind and +// therefore the function converts the input value using convIntf. If the input +// value is of type reflect.Value (means: reflect.ValueOf(reflect.ValueOf(...))) +// and the context has both int and float bits set, convVal forces conversion +// type to float64. +func convFuncArg(val reflect.Value, wantkind reflect.Kind, ctx argctx) reflect.Value { + conv := val + switch val.Kind() { + case reflect.Interface: + conv = convIntf(val, wantkind, ctx) + case reflect.Struct: + if rv, ok := val.Interface().(reflect.Value); ok { + if ((ctx & ctxAllref) > 0) && ((ctx & ctxFloat) > 0) { + wantkind = reflect.Float64 + } + conv = reflect.ValueOf(convIntf(rv, wantkind, ctx)) + } + } + return conv +} + +// convFuncArgs accepts a list of factual arguments, corresponding expected types +// and returns a list of converted arguments. The last argument is a flag +// indicating the list of values is invoked on a variadic function (in this case +// the last argument in the returned list would be safely converted to a +// variadic-friendly slice. +func convFuncArgs(args []reflect.Value, wantkind []reflect.Kind, isvariadic bool) []reflect.Value { + ctx := guessArgsCtx(args, isvariadic) + newargs := make([]reflect.Value, 0, len(args)) + for i, arg := range args { + convarg := convFuncArg(arg, wantkind[i], ctx) + newargs = append(newargs, convarg) + } + if isvariadic && len(newargs) > 0 { + varargs := newargs[len(newargs)-1] + for i := 0; i < varargs.Len(); i++ { + vararg := varargs.Index(i) + convarg := convFuncArg(vararg, reflect.Interface, ctx) + vararg.Set(convarg) + } + } + return newargs +} + +// getArgTypes takes a function type as an argument and returns 2 lists: return +// argument types and return argument kinds. The returned type list will contain +// pre-casted types for all known types from the conversion table: e.g. uint8 +// would be pre-casted to int64. +func getArgTypes(functype reflect.Type) ([]reflect.Type, []reflect.Kind) { + newargs := make([]reflect.Type, 0, functype.NumIn()) + wantkind := make([]reflect.Kind, 0, functype.NumIn()) + for i := 0; i < functype.NumIn(); i++ { + newtype := functype.In(i) + argkind := functype.In(i).Kind() + wantkind = append(wantkind, argkind) + // This is a bit cryptic: if there is a converter for a provided + // function argument type, we substitute it with an interface type so we + // can do an ad-hoc conversion when the overloaded function would be + // invoked. + // + // For example, if a template function is defined as: + // ```func foo(bar int64)```, + // + // The argument type list would look like: + // `[]reflect.Type{reflect.Int64}` + // + // What it means in fact is: when the template rendering engine will + // invoke the function, the factual argument will be of any type + // convertible to int64 (from reflect's POV). When we allow external + // types (like: JSONNumber), we have to convert them to int64 explicitly + // and on top of that we have to relax the rendering function formal + // argument type check strictness. In other words, we let any value in + // by using interface{} type instead of int64 so the reflect-backed + // gotmpl invocation of rendering functions keeps working. + // + // An overloaded foo(1) will have the following signature: + // ```func foo(bar interface{})```. + if _, ok := castNumericTo[argkind]; ok { + newtype = intfType + } + newargs = append(newargs, newtype) + } + return newargs, wantkind +} + +// overloadFunc modifies the input function so it can handle JSONNumber +// arguments as regular numeric values. It relaxes formal argument type +// if needed. For example: if a function signature expects and argument of type +// int64, the overloaded function will expect an interface{} argument and +// perform the corresponding conversion and type checking in the runtime. +// It mainly searches for 3 categories of arguments: +// * numeric arguments +// * interface{} +// * reflect.Value +// If the input function signature expects a specific type, override will +// preserve it and convert during the runtime before propagating the invocation +// to the input func. If the type is vague (e.g. interface{}), the best cast +// option would be guessed from the argument list context. For example: if the +// input function expects 2 arguments: float64 and interface{}, and the 2nd one +// is of jsonNumber type, it would be casted to float64 as well. +func overloadFunc(fn interface{}) interface{} { + funcval := reflect.ValueOf(fn) + functype := funcval.Type() + + newargs, wantkind := getArgTypes(functype) + newreturn := make([]reflect.Type, 0, functype.NumOut()) + for i := 0; i < functype.NumOut(); i++ { + newreturn = append(newreturn, functype.Out(i)) + } + + newfunctype := reflect.FuncOf(newargs, newreturn, functype.IsVariadic()) + newfunc := func(args []reflect.Value) []reflect.Value { + convargs := convFuncArgs(args, wantkind, functype.IsVariadic()) + if functype.IsVariadic() { + return funcval.CallSlice(convargs) + } + return funcval.Call(convargs) + } + + return reflect.MakeFunc(newfunctype, newfunc).Interface() +} + +// These 6 functions are imported from text/template and are meant to be +// overloaded in funcMap call. + +//go:linkname _templateBuiltinEq text/template.eq +func _templateBuiltinEq(arg1 reflect.Value, arg2 ...reflect.Value) (bool, error) + +//go:linkname _templateBuiltinGe text/template.ge +func _templateBuiltinGe(arg1, arg2 reflect.Value) (bool, error) + +//go:linkname _templateBuiltinGt text/template.gt +func _templateBuiltinGt(arg1, arg2 reflect.Value) (bool, error) + +//go:linkname _templateBuiltinLe text/template.le +func _templateBuiltinLe(arg1, arg2 reflect.Value) (bool, error) + +//go:linkname _templateBuiltinLt text/template.lt +func _templateBuiltinLt(arg1, arg2 reflect.Value) (bool, error) + +//go:linkname _templateBuiltinNe text/template.ne +func _templateBuiltinNe(arg1, arg2 reflect.Value) (bool, error)