Skip to content

Commit

Permalink
chartutil.ReadValues unmarshals numbers into json.Number refs #1707
Browse files Browse the repository at this point in the history
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 <me@whitebox.io>
  • Loading branch information
Oleg Sidorov committed Mar 31, 2020
1 parent 97c68ad commit 5da63ca
Show file tree
Hide file tree
Showing 8 changed files with 770 additions and 2 deletions.
6 changes: 5 additions & 1 deletion pkg/chart/loader/load.go
Expand Up @@ -18,6 +18,7 @@ package loader

import (
"bytes"
"encoding/json"
"log"
"os"
"path/filepath"
Expand Down Expand Up @@ -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":
Expand Down
45 changes: 45 additions & 0 deletions pkg/chart/loader/load_test.go
Expand Up @@ -20,6 +20,7 @@ import (
"archive/tar"
"bytes"
"compress/gzip"
"encoding/json"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions pkg/chartutil/testdata/coleridge.yaml
Expand Up @@ -10,3 +10,4 @@ water:
water:
where: "everywhere"
nor: "any drop to drink"
temperature: 1234567890
6 changes: 5 additions & 1 deletion pkg/chartutil/values.go
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package chartutil

import (
"encoding/json"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -105,7 +106,10 @@ 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, func(d *json.Decoder) *json.Decoder {
d.UseNumber()
return d
})
if len(vals) == 0 {
vals = Values{}
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/chartutil/values_test.go
Expand Up @@ -45,6 +45,7 @@ water:
water:
where: "everywhere"
nor: "any drop to drink"
temperature: 1234567890
`

data, err := ReadValues([]byte(doc))
Expand Down Expand Up @@ -245,6 +246,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) {
Expand Down

0 comments on commit 5da63ca

Please sign in to comment.