From 1eb378254ab6309b8dfd38436b05a70b0514ba93 Mon Sep 17 00:00:00 2001 From: jregan Date: Tue, 24 Nov 2020 14:15:45 -0800 Subject: [PATCH] Add more tests around yaml parsing. --- api/internal/wrappy/factory.go | 5 +- api/internal/wrappy/factory_test.go | 98 +++++++-- api/resource/factory.go | 2 +- api/resource/factory_test.go | 7 +- kyaml/kio/byteio_reader.go | 8 + kyaml/kio/byteio_reader_test.go | 303 ++++++++++++++++++++++++++++ 6 files changed, 398 insertions(+), 25 deletions(-) diff --git a/api/internal/wrappy/factory.go b/api/internal/wrappy/factory.go index 4f3dd7b88..89008fd95 100644 --- a/api/internal/wrappy/factory.go +++ b/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} - r.Reader = bytes.NewBuffer(bs) - yamlRNodes, err := r.Read() + yamlRNodes, err := kio.FromBytes(bs) if err != nil { return nil, err } diff --git a/api/internal/wrappy/factory_test.go b/api/internal/wrappy/factory_test.go index fc2ff81e6..6d96aed1a 100644 --- a/api/internal/wrappy/factory_test.go +++ b/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 { - t.Fatalf("%v: should return error", n) - } - 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)) + if err != nil { + assert.True(t, tc.exp.isErr) + return } + assert.False(t, tc.exp.isErr) + assert.Equal(t, len(tc.exp.out), len(rs)) for i := range rs { - if !reflect.DeepEqual(tc.exp.out[i], rs[i].Map()) { - t.Fatalf("%s: Got: %v\nexpected:%v", - n, rs[i].Map(), tc.exp.out[i]) + 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:\nexpected: %v\n actual: %v", + n, tc.exp.out[i], rs[i].Map()) + } } } }) diff --git a/api/resource/factory.go b/api/resource/factory.go index 2064475ce..7b551356e 100644 --- a/api/resource/factory.go +++ b/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 { diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index 699323d16..0bd385e39 100644 --- a/api/resource/factory_test.go +++ b/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) { - t.Fatalf("%s: length mismatch %d != %d", - test.name, len(rs), len(test.expectedOut)) - } + assert.Equal(t, len(test.expectedOut), len(rs)) for i := range rs { expYaml, err := test.expectedOut[i].AsYAML() assert.NoError(t, err) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 97f3d2e48..8074b053d 100644 --- a/kyaml/kio/byteio_reader.go +++ b/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 diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 9b13aa29f..043eff101 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/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) + } + }) + } +}