Merge pull request #3272 from monopole/moreTests

Add more tests around yaml parsing.

api/internal/wrappy/factory.go

@@ -4,7 +4,6 @@
 package wrappy
 
 import (
-	"bytes"
 	"fmt"
 
 	"sigs.k8s.io/kustomize/api/hasher"
@@ -36,9 +35,7 @@ type WNodeFactory struct {
 var _ ifc.KunstructuredFactory = (*WNodeFactory)(nil)
 
 func (k *WNodeFactory) SliceFromBytes(bs []byte) ([]ifc.Kunstructured, error) {
-	r := kio.ByteReader{OmitReaderAnnotations: true}
+	yamlRNodes, err := kio.FromBytes(bs)
-	r.Reader = bytes.NewBuffer(bs)
-	yamlRNodes, err := r.Read()
 	if err != nil {
 		return nil, err
 	}

api/internal/wrappy/factory_test.go

@@ -1,12 +1,15 @@
 // Copyright 2020 The Kubernetes Authors.
 // SPDX-License-Identifier: Apache-2.0
 
-package wrappy
+package wrappy_test
 
 import (
 	"fmt"
 	"reflect"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	. "sigs.k8s.io/kustomize/api/internal/wrappy"
 )
 
 func TestHasher(t *testing.T) {
@@ -57,6 +60,45 @@ func TestSliceFromBytes(t *testing.T) {
 				testConfigMap,
 			},
 		}
+	testDeploymentSpec := map[string]interface{}{
+		"template": map[string]interface{}{
+			"spec": map[string]interface{}{
+				"hostAliases": []interface{}{
+					map[string]interface{}{
+						"hostnames": []interface{}{
+							"a.example.com",
+						},
+						"ip": "8.8.8.8",
+					},
+				},
+			},
+		},
+	}
+	testDeploymentA := map[string]interface{}{
+		"apiVersion": "apps/v1",
+		"kind":       "Deployment",
+		"metadata": map[string]interface{}{
+			"name": "deployment-a",
+		},
+		"spec": testDeploymentSpec,
+	}
+	testDeploymentB := map[string]interface{}{
+		"apiVersion": "apps/v1",
+		"kind":       "Deployment",
+		"metadata": map[string]interface{}{
+			"name": "deployment-b",
+		},
+		"spec": testDeploymentSpec,
+	}
+	testDeploymentList :=
+		map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "DeploymentList",
+			"items": []interface{}{
+				testDeploymentA,
+				testDeploymentB,
+			},
+		}
 
 	type expected struct {
 		out   []map[string]interface{}
@@ -224,28 +266,54 @@ items:
 				out: []map[string]interface{}{testConfigMapList},
 			},
 		},
+		"listWithAnchors": {
+			input: []byte(`
+apiVersion: v1
+kind: DeploymentList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-a
+  spec: &hostAliases
+    template:
+      spec:
+        hostAliases:
+        - hostnames:
+          - a.example.com
+          ip: 8.8.8.8
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-b
+  spec:
+    <<: *hostAliases
+`),
+			exp: expected{
+				out: []map[string]interface{}{testDeploymentList},
+			},
+		},
 	}
 
 	for n := range testCases {
 		tc := testCases[n]
 		t.Run(n, func(t *testing.T) {
 			rs, err := factory.SliceFromBytes(tc.input)
-			if tc.exp.isErr && err == nil {
+			if err != nil {
-				t.Fatalf("%v: should return error", n)
+				assert.True(t, tc.exp.isErr)
-			}
+				return
-			if !tc.exp.isErr && err != nil {
-				t.Fatalf("%v: unexpected error: %s", n, err)
-			}
-			if len(tc.exp.out) != len(rs) {
-				fmt.Printf("%s: \nexpected:%v\nactual: %v\n",
-					n, tc.exp.out, rs)
-				t.Fatalf("%s: length mismatch; expected %d, actual %d",
-					n, len(tc.exp.out), len(rs))
 			}
+			assert.False(t, tc.exp.isErr)
+			assert.Equal(t, len(tc.exp.out), len(rs))
 			for i := range rs {
+				assert.Equal(
+					t, fmt.Sprintf("%v", tc.exp.out[i]), fmt.Sprintf("%v", rs[i].Map()))
+				if n != "listWithAnchors" {
+					// https://github.com/kubernetes-sigs/kustomize/issues/3271
 					if !reflect.DeepEqual(tc.exp.out[i], rs[i].Map()) {
-					t.Fatalf("%s: Got: %v\nexpected:%v",
+						t.Fatalf("%s:\nexpected: %v\n  actual: %v",
-						n, rs[i].Map(), tc.exp.out[i])
+							n, tc.exp.out[i], rs[i].Map())
+					}
 				}
 			}
 		})

api/resource/factory.go

@@ -94,7 +94,7 @@ func (rf *Factory) SliceFromPatches(
 	return result, nil
 }
 
-// FromBytes unmarshals bytes into one Resource.
+// FromBytes unmarshalls bytes into one Resource.
 func (rf *Factory) FromBytes(in []byte) (*Resource, error) {
 	result, err := rf.SliceFromBytes(in)
 	if err != nil {

api/resource/factory_test.go

@@ -350,7 +350,7 @@ kind: List
 			expectedErr: false,
 		},
 		{
-			name:        "listWithNo'items:'",
+			name:        "listWithNoItems",
 			input:       []types.PatchStrategicMerge{patchList4},
 			expectedOut: []*Resource{},
 			expectedErr: false,
@@ -364,10 +364,7 @@ kind: List
 			continue
 		}
 		assert.False(t, test.expectedErr, "expected no error")
-		if len(rs) != len(test.expectedOut) {
+		assert.Equal(t, len(test.expectedOut), len(rs))
-			t.Fatalf("%s: length mismatch %d != %d",
-				test.name, len(rs), len(test.expectedOut))
-		}
 		for i := range rs {
 			expYaml, err := test.expectedOut[i].AsYAML()
 			assert.NoError(t, err)

kyaml/kio/byteio_reader.go

@@ -85,6 +85,14 @@ func ParseAll(inputs ...string) ([]*yaml.RNode, error) {
 	}).Read()
 }
 
+// FromBytes reads from a byte slice.
+func FromBytes(bs []byte) ([]*yaml.RNode, error) {
+	return (&ByteReader{
+		OmitReaderAnnotations: true,
+		Reader:                bytes.NewBuffer(bs),
+	}).Read()
+}
+
 // StringAll writes all of the resources to a string
 func StringAll(resources []*yaml.RNode) (string, error) {
 	var b bytes.Buffer

kyaml/kio/byteio_reader_test.go

@@ -423,3 +423,306 @@ metadata:
 		})
 	}
 }
+
+func TestFromBytes(t *testing.T) {
+	type expected struct {
+		isErr bool
+		sOut  []string
+	}
+
+	testCases := map[string]struct {
+		input []byte
+		exp   expected
+	}{
+		"garbage": {
+			input: []byte("garbageIn: garbageOut"),
+			exp: expected{
+				sOut: []string{"garbageIn: garbageOut"},
+			},
+		},
+		"noBytes": {
+			input: []byte{},
+			exp: expected{
+				sOut: []string{},
+			},
+		},
+		"goodJson": {
+			input: []byte(`
+{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie"}}
+`),
+			exp: expected{
+				sOut: []string{`
+{"apiVersion": "v1", "kind": "ConfigMap", "metadata": {"name": "winnie"}}
+`},
+			},
+		},
+		"goodYaml1": {
+			input: []byte(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`},
+			},
+		},
+		"goodYaml2": {
+			input: []byte(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`, `
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`},
+			},
+		},
+		"localConfigYaml": {
+			input: []byte(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie-skip
+  annotations:
+    # this annotation causes the Resource to be ignored by kustomize
+    config.kubernetes.io/local-config: ""
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie-skip
+  annotations:
+    # this annotation causes the Resource to be ignored by kustomize
+    config.kubernetes.io/local-config: ""
+`,
+					`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`},
+			},
+		},
+		"garbageInOneOfTwoObjects": {
+			input: []byte(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+---
+WOOOOOOOOOOOOOOOOOOOOOOOOT:     woot
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`,
+					`
+WOOOOOOOOOOOOOOOOOOOOOOOOT: woot
+`},
+			},
+		},
+		"emptyObjects": {
+			input: []byte(`
+---
+#a comment
+
+---
+
+`),
+			exp: expected{
+				sOut: []string{},
+			},
+		},
+		"Missing .metadata.name in object": {
+			input: []byte(`
+apiVersion: v1
+kind: Namespace
+metadata:
+  annotations:
+    foo: bar
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: Namespace
+metadata:
+  annotations:
+    foo: bar
+`},
+			},
+		},
+		"nil value in list": {
+			input: []byte(`
+apiVersion: builtin
+kind: ConfigMapGenerator
+metadata:
+  name: kube100-site
+	labels:
+	  app: web
+testList:
+- testA
+-
+`),
+			exp: expected{
+				isErr: true,
+			},
+		},
+		"List": {
+			input: []byte(`
+apiVersion: v1
+kind: List
+items:
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`, `
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: winnie
+`},
+			},
+		},
+		"ConfigMapList": {
+			input: []byte(`
+apiVersion: v1
+kind: ConfigMapList
+items:
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: ConfigMapList
+items:
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+- apiVersion: v1
+  kind: ConfigMap
+  metadata:
+    name: winnie
+`},
+			},
+		},
+		"listWithAnchors": {
+			input: []byte(`
+apiVersion: v1
+kind: DeploymentList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-a
+  spec: &hostAliases
+    template:
+      spec:
+        hostAliases:
+        - hostnames:
+          - a.example.com
+          ip: 8.8.8.8
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-b
+  spec:
+    <<: *hostAliases
+`),
+			exp: expected{
+				sOut: []string{`
+apiVersion: v1
+kind: DeploymentList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-a
+  spec: &hostAliases
+    template:
+      spec:
+        hostAliases:
+        - hostnames:
+          - a.example.com
+          ip: 8.8.8.8
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-b
+  spec:
+    !!merge <<: *hostAliases
+`},
+			},
+		},
+	}
+
+	for n := range testCases {
+		tc := testCases[n]
+		t.Run(n, func(t *testing.T) {
+			rNodes, err := FromBytes(tc.input)
+			if err != nil {
+				assert.True(t, tc.exp.isErr)
+				return
+			}
+			assert.False(t, tc.exp.isErr)
+			assert.Equal(t, len(tc.exp.sOut), len(rNodes))
+			for i, n := range rNodes {
+				json, err := n.String()
+				assert.NoError(t, err)
+				assert.Equal(
+					t, strings.TrimSpace(tc.exp.sOut[i]),
+					strings.TrimSpace(json), n)
+			}
+		})
+	}
+}