Repository: grafana/tanka
Pull request #358 (2020-08-24 20:05:37)

fix(manifest): yaml unmarshalling
The `manifest` package operates using the assumption that only the JSON
types are present in the `map[string]interface{}`:

- float64 for numbers
- string for texts
- bool for booleans
- nil for null values

The `gopkg.in/yaml` package however also uses `int`, etc. This breaks
our package.

To work around this, the custom `yaml.Marshaller` has been modified to
not actually unmarshal, but call `json.Unmarshal` instead, making both
formats behave exactly the same.

Extensive tests have been added to ensure this.


Patch:
diff --git a/docs/docs/namespaces.md b/docs/docs/namespaces.md
index 18a88f9c..b5b58994 100644
--- a/docs/docs/namespaces.md
+++ b/docs/docs/namespaces.md
@@ -34,7 +34,7 @@ Some resources in Kubernetes are cluster-wide, meaning they don't belong to a si
 Tanka will make an attempt to not add namespaces to *known* cluster-wide types. 
 It does this with a short list of types in [the source code](https://github.com/grafana/tanka/blob/master/pkg/process/namespace.go).
 
-Tanka cannot feasably maintain this list for all known custom resource types. In those cases, resources will have namespaces added to their manifests,
+Tanka cannot feasibly maintain this list for all known custom resource types. In those cases, resources will have namespaces added to their manifests,
 and kubectl should happily apply them as non-namespaced resources.
 
 If this presents a problem for your workflow, you can **override this** behavior
diff --git a/go.mod b/go.mod
index 88b6b69b..9db35c4c 100644
--- a/go.mod
+++ b/go.mod
@@ -8,7 +8,7 @@ require (
 	github.com/fatih/structs v1.1.0
 	github.com/go-clix/cli v0.1.1
 	github.com/gobwas/glob v0.2.3
-	github.com/google/go-cmp v0.3.0
+	github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2
 	github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785
 	github.com/karrick/godirwalk v1.15.5
 	github.com/pkg/errors v0.8.1
diff --git a/go.sum b/go.sum
index 5391d986..ae1d2b21 100644
--- a/go.sum
+++ b/go.sum
@@ -33,6 +33,8 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2 h1:CZtx9gNen+kr3PuC/JQff3n1pJbgpy7Wr3hzjnupqdw=
+github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785 h1:+dlQ7fPoeAqO0U9V+94golo/rW1/V2Pn+v8aPp3ljRM=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785/go.mod h1:sOcuej3UW1vpPTZOr8L7RQimqai1a57bt5j22LzGZCw=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
@@ -121,6 +123,8 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181011042414-1f849cf54d09/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181030221726-6c7e314b6563/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
diff --git a/pkg/kubernetes/manifest/manifest.go b/pkg/kubernetes/manifest/manifest.go
index 688cb774..a62634c6 100644
--- a/pkg/kubernetes/manifest/manifest.go
+++ b/pkg/kubernetes/manifest/manifest.go
@@ -176,13 +176,17 @@ func (m *Manifest) UnmarshalJSON(data []byte) error {
 
 // UnmarshalYAML validates the Manifest during yaml parsing
 func (m *Manifest) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	type tmp Manifest
-	var t tmp
-	if err := unmarshal(&t); err != nil {
+	var tmp map[string]interface{}
+	if err := unmarshal(&tmp); err != nil {
 		return err
 	}
-	*m = Manifest(t)
-	return m.Verify()
+
+	data, err := json.Marshal(tmp)
+	if err != nil {
+		return err
+	}
+
+	return json.Unmarshal(data, m)
 }
 
 // Metadata is the metadata object from the Manifest


Test patch:
diff --git a/pkg/kubernetes/manifest/manifest_test.go b/pkg/kubernetes/manifest/manifest_test.go
new file mode 100644
index 00000000..fabd524e
--- /dev/null
+++ b/pkg/kubernetes/manifest/manifest_test.go
@@ -0,0 +1,92 @@
+package manifest
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
+)
+
+// UnmarshalExpect defines the expected Unmarshal result. Types are very
+// important here, only nil, float64, bool, string, map[string]interface{} and
+// []interface{} may exist.
+var UnmarshalExpect = Manifest{
+	"apiVersion": string("apps/v1"),
+	"kind":       string("Deployment"),
+	"metadata": map[string]interface{}{
+		"name": string("MyDeployment"),
+	},
+	"spec": map[string]interface{}{
+		"replicas": float64(3),
+		"template": map[string]interface{}{
+			"spec": map[string]interface{}{
+				"containers": []interface{}{
+					map[string]interface{}{
+						"name":  string("nginx"),
+						"image": string("nginx:1.14.2"),
+					},
+				},
+			},
+		},
+	},
+}
+
+func TestUnmarshalJSON(t *testing.T) {
+	const data = `
+{
+   "apiVersion":"apps/v1",
+   "kind":"Deployment",
+   "metadata":{
+	  "name":"MyDeployment"
+   },
+   "spec":{
+	  "replicas":3,
+	  "template":{
+		 "spec":{
+			"containers":[
+			   {
+				  "name":"nginx",
+				  "image":"nginx:1.14.2"
+			   }
+			]
+		 }
+	  }
+   }
+}
+`
+
+	var m Manifest
+	err := json.Unmarshal([]byte(data), &m)
+	require.NoError(t, err)
+
+	if s := cmp.Diff(UnmarshalExpect, m); s != "" {
+		t.Error(s)
+	}
+}
+
+func TestUnmarshalYAML(t *testing.T) {
+	const data = `
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: MyDeployment
+spec:
+  replicas: 3
+  template:
+    spec:
+      containers:
+      - name: nginx
+        image: nginx:1.14.2
+`
+
+	var m Manifest
+	err := yaml.Unmarshal([]byte(data), &m)
+	require.NoError(t, err)
+
+	if s := cmp.Diff(UnmarshalExpect, m); s != "" {
+		t.Error(s)
+	}
+}