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 <me@whitebox.io>

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":

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) {

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)

pkg/chartutil/testdata/coleridge.yaml

@@ -10,3 +10,4 @@ water:
   water:
     where: "everywhere"
     nor: "any drop to drink"
+    temperature: 1234567890

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{}
 	}

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) {

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, &currentMap); err != nil {
+		if err := yaml.Unmarshal(bytes, &currentMap, chartutil.JSONNumberOn); err != nil {
 			return nil, errors.Wrapf(err, "failed to parse %s", filePath)
 		}
 		// Merge with the previous map