diff --git a/api/internal/wrappy/factory.go b/api/internal/wrappy/factory.go index b03b9e8fc..4bce50602 100644 --- a/api/internal/wrappy/factory.go +++ b/api/internal/wrappy/factory.go @@ -4,26 +4,71 @@ package wrappy import ( + "bytes" + "fmt" + "sigs.k8s.io/kustomize/api/ifc" + "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/yaml" ) // WNodeFactory makes instances of WNode. +// // These instances in turn adapt // sigs.k8s.io/kustomize/kyaml/yaml.RNode // to implement ifc.Unstructured. // This factory is meant to implement ifc.KunstructuredFactory. +// +// This implementation should be thin, as both WNode and WNodeFactory must be +// factored away (deleted) along with ifc.Kunstructured in favor of direct use +// of RNode methods upon completion of +// https://github.com/kubernetes-sigs/kustomize/issues/2506. +// +// See also api/krusty/internal/provider/depprovider.go type WNodeFactory struct { } var _ ifc.KunstructuredFactory = (*WNodeFactory)(nil) func (k *WNodeFactory) SliceFromBytes(bs []byte) ([]ifc.Kunstructured, error) { - panic("TODO(#WNodeFactory): implement SliceFromBytes") + r := kio.ByteReader{OmitReaderAnnotations: true} + r.Reader = bytes.NewBuffer(bs) + yamlRNodes, err := r.Read() + if err != nil { + return nil, err + } + var result []ifc.Kunstructured + for i := range yamlRNodes { + rn := yamlRNodes[i] + meta, err := rn.GetValidatedMetadata() + if err != nil { + return nil, err + } + if !shouldDropObject(meta) { + if foundNil, path := rn.HasNilEntryInList(); foundNil { + return nil, fmt.Errorf("empty item at %v in object %v", path, rn) + } + result = append(result, FromRNode(rn)) + } + } + return result, nil +} + +// shouldDropObject returns true if the resource should not be accumulated. +func shouldDropObject(m yaml.ResourceMeta) bool { + _, y := m.ObjectMeta.Annotations[konfig.IgnoredByKustomizeResourceAnnotation] + return y } func (k *WNodeFactory) FromMap(m map[string]interface{}) ifc.Kunstructured { - panic("TODO(#WNodeFactory): implement FromMap") + rn, err := FromMap(m) + if err != nil { + // TODO(#WNodeFactory): handle or bubble error" + panic(err) + } + return rn } func (k *WNodeFactory) Hasher() ifc.KunstructuredHasher { diff --git a/api/internal/wrappy/factory_test.go b/api/internal/wrappy/factory_test.go index 4eea8268b..77dee3bae 100644 --- a/api/internal/wrappy/factory_test.go +++ b/api/internal/wrappy/factory_test.go @@ -2,3 +2,223 @@ // SPDX-License-Identifier: Apache-2.0 package wrappy + +import ( + "fmt" + "reflect" + "testing" +) + +func TestSliceFromBytes(t *testing.T) { + factory := &WNodeFactory{} + testConfigMap := + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "winnie", + }, + } + testConfigMapList := + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMapList", + "items": []interface{}{ + testConfigMap, + testConfigMap, + }, + } + + type expected struct { + out []map[string]interface{} + isErr bool + } + + testCases := map[string]struct { + input []byte + exp expected + }{ + "garbage": { + input: []byte("garbageIn: garbageOut"), + exp: expected{ + isErr: true, + }, + }, + "noBytes": { + input: []byte{}, + exp: expected{ + out: []map[string]interface{}{}, + }, + }, + "goodJson": { + input: []byte(` +{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie"}} +`), + exp: expected{ + out: []map[string]interface{}{testConfigMap}, + }, + }, + "goodYaml1": { + input: []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +`), + exp: expected{ + out: []map[string]interface{}{testConfigMap}, + }, + }, + "goodYaml2": { + input: []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +`), + exp: expected{ + out: []map[string]interface{}{testConfigMap, testConfigMap}, + }, + }, + "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{ + out: []map[string]interface{}{testConfigMap}, + }, + }, + "garbageInOneOfTwoObjects": { + input: []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: winnie +--- +WOOOOOOOOOOOOOOOOOOOOOOOOT: woot +`), + exp: expected{ + isErr: true, + }, + }, + "emptyObjects": { + input: []byte(` +--- +#a comment + +--- + +`), + exp: expected{ + out: []map[string]interface{}{}, + }, + }, + "Missing .metadata.name in object": { + input: []byte(` +apiVersion: v1 +kind: Namespace +metadata: + annotations: + foo: bar +`), + exp: expected{ + isErr: true, + }, + }, + "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{ + out: []map[string]interface{}{ + testConfigMap, + testConfigMap}, + }, + }, + "ConfigMapList": { + input: []byte(` +apiVersion: v1 +kind: ConfigMapList +items: +- apiVersion: v1 + kind: ConfigMap + metadata: + name: winnie +- apiVersion: v1 + kind: ConfigMap + metadata: + name: winnie +`), + exp: expected{ + out: []map[string]interface{}{testConfigMapList}, + }, + }, + } + + 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)) + } + 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]) + } + } + }) + } +} diff --git a/api/k8sdeps/kunstruct/factory.go b/api/k8sdeps/kunstruct/factory.go index cebfa1910..ec1333565 100644 --- a/api/k8sdeps/kunstruct/factory.go +++ b/api/k8sdeps/kunstruct/factory.go @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/k8sdeps/configmapandsecret" + "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/types" ) @@ -116,10 +117,6 @@ func (kf *KunstructuredFactoryImpl) validate(u unstructured.Unstructured) error return nil } -// nonKustomizableResourceAnnotation if set on a Resource will cause Kustomize to -// ignore the Resource rather than Kustomize it. -const ignoredByKustomizeResourceAnnotation = "config.kubernetes.io/local-config" - // skipResource returns true if the Resource should not be accumulated func (kf *KunstructuredFactoryImpl) skipResource(u unstructured.Unstructured) bool { an := u.GetAnnotations() @@ -128,7 +125,7 @@ func (kf *KunstructuredFactoryImpl) skipResource(u unstructured.Unstructured) bo return false } // check if the Resource has opt-ed out of kustomize - _, found := an[ignoredByKustomizeResourceAnnotation] + _, found := an[konfig.IgnoredByKustomizeResourceAnnotation] return found } diff --git a/api/konfig/general.go b/api/konfig/general.go index de0292415..02c4067ce 100644 --- a/api/konfig/general.go +++ b/api/konfig/general.go @@ -31,6 +31,9 @@ const ( // A program name, for use in help, finding the XDG_CONFIG_DIR, etc. ProgramName = "kustomize" + // If a resource has this annotation, kustomize will drop it. + IgnoredByKustomizeResourceAnnotation = "config.kubernetes.io/local-config" + // Label key that indicates the resources are built from Kustomize ManagedbyLabelKey = "app.kubernetes.io/managed-by" diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 5ff5bf765..2264dd3db 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -7,6 +7,8 @@ import ( "encoding/json" "fmt" "io/ioutil" + "strconv" + "strings" "gopkg.in/yaml.v3" "sigs.k8s.io/kustomize/kyaml/errors" @@ -458,8 +460,8 @@ func (rn *RNode) Elements() ([]*RNode, error) { return elements, nil } -// ElementValues returns a list of all observed values for a given field name in a -// list of elements. +// ElementValues returns a list of all observed values for a given field name +// in a list of elements. // Returns error for non-SequenceNodes. func (rn *RNode) ElementValues(key string) ([]string, error) { if err := ErrorIfInvalid(rn, yaml.SequenceNode); err != nil { @@ -602,6 +604,55 @@ func (rn *RNode) UnmarshalJSON(b []byte) error { return nil } +// GetValidatedMetadata returns metadata after subjecting it to some tests. +func (rn *RNode) GetValidatedMetadata() (ResourceMeta, error) { + m, err := rn.GetMeta() + if err != nil { + return m, err + } + if m.Kind == "" { + return m, fmt.Errorf("missing kind in object %v", m) + } + if strings.HasSuffix(m.Kind, "List") { + // A list doesn't require a name. + return m, nil + } + if m.NameMeta.Name == "" { + return m, fmt.Errorf("missing metadata.name in object %v", m) + } + return m, nil +} + +// HasNilEntryInList returns true if the RNode contains a list which has +// a nil item, along with the path to the missing item. +// TODO(broken): This was copied from +// api/k8sdeps/kunstruct/factory.go//checkListItemNil +// and doesn't do what it claims to do (see TODO in unit test and pr 1513). +func (rn *RNode) HasNilEntryInList() (bool, string) { + return hasNilEntryInList(rn.value) +} + +func hasNilEntryInList(in interface{}) (bool, string) { + switch v := in.(type) { + case map[string]interface{}: + for key, s := range v { + if result, path := hasNilEntryInList(s); result { + return result, key + "/" + path + } + } + case []interface{}: + for index, s := range v { + if s == nil { + return true, "" + } + if result, path := hasNilEntryInList(s); result { + return result, "[" + strconv.Itoa(index) + "]/" + path + } + } + } + return false, "" +} + func FromMap(m map[string]interface{}) (*RNode, error) { c, err := Marshal(m) if err != nil { diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go index 263e37975..ccb8aec72 100644 --- a/kyaml/yaml/rnode_test.go +++ b/kyaml/yaml/rnode_test.go @@ -11,6 +11,212 @@ import ( "github.com/stretchr/testify/assert" ) +func TestRNodeHasNilEntryInList(t *testing.T) { + testConfigMap := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "winnie", + }, + } + type resultExpected struct { + hasNil bool + path string + } + + testCases := map[string]struct { + theMap map[string]interface{} + rsExp resultExpected + }{ + "actuallyNil": { + theMap: nil, + rsExp: resultExpected{}, + }, + "empty": { + theMap: map[string]interface{}{}, + rsExp: resultExpected{}, + }, + "list": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "List", + "items": []interface{}{ + testConfigMap, + testConfigMap, + }, + }, + rsExp: resultExpected{}, + }, + "listWithNil": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "List", + "items": []interface{}{ + testConfigMap, + nil, + }, + }, + rsExp: resultExpected{ + hasNil: false, // TODO: This should be true. + path: "this/should/be/non-empty", + }, + }, + } + + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + rn, err := FromMap(tc.theMap) + if !assert.NoError(t, err) { + t.FailNow() + } + hasNil, path := rn.HasNilEntryInList() + if tc.rsExp.hasNil { + if !assert.True(t, hasNil) { + t.FailNow() + } + if !assert.Equal(t, tc.rsExp.path, path) { + t.FailNow() + } + } else { + if !assert.False(t, hasNil) { + t.FailNow() + } + if !assert.Empty(t, path) { + t.FailNow() + } + } + }) + } +} + +func TestRNodeGetValidatedMetadata(t *testing.T) { + testConfigMap := map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "winnie", + }, + } + type resultExpected struct { + out ResourceMeta + errMsg string + } + + testCases := map[string]struct { + theMap map[string]interface{} + rsExp resultExpected + }{ + "actuallyNil": { + theMap: nil, + rsExp: resultExpected{ + errMsg: "missing Resource metadata", + }, + }, + "empty": { + theMap: map[string]interface{}{}, + rsExp: resultExpected{ + errMsg: "missing Resource metadata", + }, + }, + "mostlyEmpty": { + theMap: map[string]interface{}{ + "hey": "there", + }, + rsExp: resultExpected{ + errMsg: "missing Resource metadata", + }, + }, + "noNameConfigMap": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + }, + rsExp: resultExpected{ + errMsg: "missing metadata.name", + }, + }, + "configmap": { + theMap: testConfigMap, + rsExp: resultExpected{ + out: ResourceMeta{ + TypeMeta: TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: ObjectMeta{ + NameMeta: NameMeta{ + Name: "winnie", + }, + }, + }, + }, + }, + "list": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "List", + "items": []interface{}{ + testConfigMap, + testConfigMap, + }, + }, + rsExp: resultExpected{ + out: ResourceMeta{ + TypeMeta: TypeMeta{ + APIVersion: "v1", + Kind: "List", + }, + }, + }, + }, + "configmaplist": { + theMap: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMapList", + "items": []interface{}{ + testConfigMap, + testConfigMap, + }, + }, + rsExp: resultExpected{ + out: ResourceMeta{ + TypeMeta: TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMapList", + }, + }, + }, + }, + } + + for n := range testCases { + tc := testCases[n] + t.Run(n, func(t *testing.T) { + rn, err := FromMap(tc.theMap) + if !assert.NoError(t, err) { + t.FailNow() + } + m, err := rn.GetValidatedMetadata() + if tc.rsExp.errMsg == "" { + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.Equal(t, tc.rsExp.out, m) { + t.FailNow() + } + } else { + if !assert.Error(t, err) { + t.FailNow() + } + if !assert.Contains(t, err.Error(), tc.rsExp.errMsg) { + t.FailNow() + } + } + }) + } +} + func TestRNodeFromMap(t *testing.T) { testConfigMap := map[string]interface{}{ "apiVersion": "v1", @@ -19,25 +225,25 @@ func TestRNodeFromMap(t *testing.T) { "name": "winnie", }, } - type thingExpected struct { + type resultExpected struct { out string err error } testCases := map[string]struct { - theMap map[string]interface{} - expected thingExpected + theMap map[string]interface{} + rsExp resultExpected }{ "actuallyNil": { theMap: nil, - expected: thingExpected{ + rsExp: resultExpected{ out: `{}`, err: nil, }, }, "empty": { theMap: map[string]interface{}{}, - expected: thingExpected{ + rsExp: resultExpected{ out: `{}`, err: nil, }, @@ -46,14 +252,14 @@ func TestRNodeFromMap(t *testing.T) { theMap: map[string]interface{}{ "hey": "there", }, - expected: thingExpected{ + rsExp: resultExpected{ out: `hey: there`, err: nil, }, }, "configmap": { theMap: testConfigMap, - expected: thingExpected{ + rsExp: resultExpected{ out: ` apiVersion: v1 kind: ConfigMap @@ -72,7 +278,7 @@ metadata: testConfigMap, }, }, - expected: thingExpected{ + rsExp: resultExpected{ out: ` apiVersion: v1 items: @@ -98,7 +304,7 @@ kind: List testConfigMap, }, }, - expected: thingExpected{ + rsExp: resultExpected{ out: ` apiVersion: v1 items: @@ -121,12 +327,12 @@ kind: ConfigMapList tc := testCases[n] t.Run(n, func(t *testing.T) { rn, err := FromMap(tc.theMap) - if tc.expected.err == nil { + if tc.rsExp.err == nil { if !assert.NoError(t, err) { t.FailNow() } if !assert.Equal(t, - strings.TrimSpace(tc.expected.out), + strings.TrimSpace(tc.rsExp.out), strings.TrimSpace(rn.MustString())) { t.FailNow() } @@ -134,7 +340,7 @@ kind: ConfigMapList if !assert.Error(t, err) { t.FailNow() } - if !assert.Equal(t, tc.expected.err, err) { + if !assert.Equal(t, tc.rsExp.err, err) { t.FailNow() } }