B: unmarshal using json.Number rather than float64

by default, "encoding/json" (and hence "sigs.k8s.io/yaml") decode any
number as float64 (if you try to decode into a generic type like
interface{})

vars/template.go does not allow interpolating float64 into a string,
presumably because you could end up interpolating the number 123.0 as
e.g. 122.99999999999999 due to limitations with float precision.

this means that if you ever load_var on a file containing a number
(even an integer), then you can never interpolate that number into a
string, which can be pretty annoying.

json.Number, on the other hand, stores the number as a string, so there
is no issue with precision - and it means ints will stay as "ints".
since we never use these values directly (they are just
serialized/deserialized), this *shouldn't* cause issues.

Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>

atc/creds/eval.go

@@ -22,5 +22,10 @@ func evaluate(variablesResolver vars.Variables, in, out interface{}) error {
 		return err
 	}
 
-	return yaml.Unmarshal(bytes, out)
+	return yaml.Unmarshal(bytes, out, useJSONNumber)
+}
+
+func useJSONNumber(decoder *json.Decoder) *json.Decoder {
+	decoder.UseNumber()
+	return decoder
 }

atc/exec/load_var_step.go

@@ -1,8 +1,10 @@
 package exec
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"path/filepath"
@@ -157,14 +159,17 @@ func (step *LoadVarStep) fetchVars(
 	var value interface{}
 	switch format {
 	case "json":
-		value = map[string]interface{}{}
-		err = json.Unmarshal(fileContent, &value)
+		decoder := json.NewDecoder(bytes.NewReader(fileContent))
+		decoder.UseNumber()
+		err = decoder.Decode(&value)
 		if err != nil {
 			return nil, InvalidLocalVarFile{file, "json", err}
 		}
+		if decoder.More() {
+			return nil, InvalidLocalVarFile{file, "json", errors.New("invalid json: characters found after top-level value")}
+		}
 	case "yml", "yaml":
-		value = map[string]interface{}{}
-		err = yaml.Unmarshal(fileContent, &value)
+		err = yaml.Unmarshal(fileContent, &value, useJSONNumber)
 		if err != nil {
 			return nil, InvalidLocalVarFile{file, "yaml", err}
 		}
@@ -179,6 +184,11 @@ func (step *LoadVarStep) fetchVars(
 	return value, nil
 }
 
+func useJSONNumber(decoder *json.Decoder) *json.Decoder {
+	decoder.UseNumber()
+	return decoder
+}
+
 func (step *LoadVarStep) fileFormat(file string) (string, error) {
 	if step.isValidFormat(step.plan.Format) {
 		return step.plan.Format, nil

atc/exec/load_var_step_test.go

@@ -2,6 +2,7 @@ package exec_test
 
 import (
 	"context"
+	"encoding/json"
 	"strings"
 
 	"code.cloudfoundry.org/lager/lagerctx"
@@ -25,11 +26,12 @@ const plainString = "  pv  \n\n"
 const yamlString = `
 k1: yv1
 k2: yv2
+k3: 123
 `
 
 const jsonString = `
 {
-  "k1": "jv1", "k2": "jv2"
+  "k1": "jv1", "k2": "jv2", "k3": 123
 }
 `
 
@@ -159,7 +161,7 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
+			It("var should be parsed correctly", func() {
 				expectLocalVarAdded("some-var", strings.TrimSpace(plainString), true)
 			})
 		})
@@ -180,7 +182,7 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
+			It("var should be parsed correctly", func() {
 				expectLocalVarAdded("some-var", plainString, true)
 			})
 		})
@@ -201,8 +203,8 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "jv1", "k2": "jv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "jv1", "k2": "jv2", "k3": json.Number("123")}, true)
 			})
 		})
 
@@ -222,8 +224,8 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2", "k3": json.Number("123")}, true)
 			})
 		})
 
@@ -243,8 +245,8 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2", "k3": json.Number("123")}, true)
 			})
 		})
 	})
@@ -265,7 +267,7 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly as trim", func() {
+			It("var should be parsed correctly as trim", func() {
 				expectLocalVarAdded("some-var", strings.TrimSpace(plainString), true)
 			})
 		})
@@ -285,8 +287,8 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "jv1", "k2": "jv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "jv1", "k2": "jv2", "k3": json.Number("123")}, true)
 			})
 		})
 
@@ -305,8 +307,8 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2", "k3": json.Number("123")}, true)
 			})
 		})
 
@@ -325,21 +327,21 @@ var _ = Describe("LoadVarStep", func() {
 				Expect(stepOk).To(BeTrue())
 			})
 
-			It("should var parsed correctly", func() {
-				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2"}, true)
+			It("var should be parsed correctly", func() {
+				expectLocalVarAdded("some-var", map[string]interface{}{"k1": "yv1", "k2": "yv2", "k3": json.Number("123")}, true)
 			})
 		})
 	})
 
 	Context("when file is bad", func() {
-		Context("when json file is bad", func() {
+		Context("when json file is invalid JSON", func() {
 			BeforeEach(func() {
 				loadVarPlan = &atc.LoadVarPlan{
 					Name: "some-var",
 					File: "some-resource/a.json",
 				}
 
-				fakeArtifactStreamer.StreamFileFromArtifactReturns(&fakeReadCloser{str: plainString}, nil)
+				fakeArtifactStreamer.StreamFileFromArtifactReturns(&fakeReadCloser{str: jsonString + "{}"}, nil)
 			})
 
 			It("step should fail", func() {

testflight/across_test.go

@@ -17,8 +17,10 @@ var _ = Describe("A pipeline containing an across step", func() {
 		Expect(watch).To(gbytes.Say("running across static1 dynamic1"))
 		Expect(watch).To(gbytes.Say("running across static1 dynamic2"))
 		Expect(watch).To(gbytes.Say("running across static1 dynamic3"))
+		Expect(watch).To(gbytes.Say("running across static1 4"))
 		Expect(watch).To(gbytes.Say("running across static2 dynamic1"))
 		Expect(watch).To(gbytes.Say("running across static2 dynamic2"))
 		Expect(watch).To(gbytes.Say("running across static2 dynamic3"))
+		Expect(watch).To(gbytes.Say("running across static2 4"))
 	})
 })

testflight/fixtures/across.yml

@@ -5,7 +5,7 @@ resources:
   source:
     create_files:
       data.yml: |
-        [dynamic1, dynamic2, dynamic3]
+        [dynamic1, dynamic2, dynamic3, 4]
 
 jobs:
 - name: job

vars/template.go

@@ -31,6 +31,10 @@ func (t Template) ExtraVarNames() []string {
 func (t Template) Evaluate(vars Variables, opts EvaluateOpts) ([]byte, error) {
 	var obj interface{}
 
+	// Note: if we do end up changing from "gopkg.in/yaml.v2" to
+	// "sigs.k8s.io/yaml" here, we'll want to ensure we call
+	// `json.Decoder.UseNumber()` so that we don't lose precision unmarshaling
+	// numbers to float64.
 	err := yaml.Unmarshal(t.bytes, &obj)
 	if err != nil {
 		return []byte{}, err