From 3ac57786f69c0ec86f401d2aa5a31a7939a9265c Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Sat, 24 Oct 2020 15:01:23 -0700 Subject: [PATCH] Update the ManifestReader interface and implementations to use Unstructured --- cmd/apply/cmdapply.go | 5 +- cmd/destroy/cmddestroy.go | 5 +- cmd/preview/cmdpreview.go | 7 +- cmd/status/cmdstatus.go | 5 +- pkg/apply/applier.go | 1 + pkg/apply/info/info_helper.go | 7 +- pkg/apply/task/apply_task_test.go | 2 +- pkg/manifestreader/common.go | 86 +++++++---- pkg/manifestreader/common_test.go | 218 ++++++++++++++++++--------- pkg/manifestreader/manifestreader.go | 8 +- pkg/manifestreader/path.go | 52 +++---- pkg/manifestreader/path_test.go | 31 +++- pkg/manifestreader/stream.go | 48 +++--- pkg/manifestreader/stream_test.go | 34 +++-- pkg/manifestreader/testdata.go | 24 +++ pkg/object/infos.go | 29 +++- pkg/object/infos_test.go | 77 ++++++++++ pkg/provider/fake-provider.go | 9 +- pkg/provider/provider.go | 7 +- 19 files changed, 448 insertions(+), 207 deletions(-) create mode 100644 pkg/manifestreader/testdata.go create mode 100644 pkg/object/infos_test.go diff --git a/cmd/apply/cmdapply.go b/cmd/apply/cmdapply.go index 1ce455e..3823079 100644 --- a/cmd/apply/cmdapply.go +++ b/cmd/apply/cmdapply.go @@ -17,7 +17,6 @@ import ( "sigs.k8s.io/cli-utils/cmd/printers" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/common" - "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/provider" "sigs.k8s.io/kustomize/kyaml/setters2" ) @@ -107,7 +106,7 @@ func (r *ApplyRunner) RunE(cmd *cobra.Command, args []string) error { if err != nil { return err } - infos, err := reader.Read() + objs, err := reader.Read() if err != nil { return err } @@ -117,7 +116,7 @@ func (r *ApplyRunner) RunE(cmd *cobra.Command, args []string) error { if err := r.Applier.Initialize(); err != nil { return err } - ch := r.Applier.Run(context.Background(), object.InfosToUnstructureds(infos), apply.Options{ + ch := r.Applier.Run(context.Background(), objs, apply.Options{ ServerSideOptions: r.serverSideOptions, PollInterval: r.period, ReconcileTimeout: r.reconcileTimeout, diff --git a/cmd/destroy/cmddestroy.go b/cmd/destroy/cmddestroy.go index c1d07ce..1aaa953 100644 --- a/cmd/destroy/cmddestroy.go +++ b/cmd/destroy/cmddestroy.go @@ -11,7 +11,6 @@ import ( "sigs.k8s.io/cli-utils/cmd/printers" "sigs.k8s.io/cli-utils/pkg/apply" "sigs.k8s.io/cli-utils/pkg/inventory" - "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/provider" ) @@ -58,11 +57,11 @@ func (r *DestroyRunner) RunE(cmd *cobra.Command, args []string) error { if err != nil { return err } - infos, err := reader.Read() + objs, err := reader.Read() if err != nil { return err } - inv, _, err := inventory.SplitUnstructureds(object.InfosToUnstructureds(infos)) + inv, _, err := inventory.SplitUnstructureds(objs) if err != nil { return err } diff --git a/cmd/preview/cmdpreview.go b/cmd/preview/cmdpreview.go index deefbb1..fa4fd58 100644 --- a/cmd/preview/cmdpreview.go +++ b/cmd/preview/cmdpreview.go @@ -15,7 +15,6 @@ import ( "sigs.k8s.io/cli-utils/pkg/apply/event" "sigs.k8s.io/cli-utils/pkg/common" "sigs.k8s.io/cli-utils/pkg/inventory" - "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/provider" "sigs.k8s.io/kustomize/kyaml/setters2" ) @@ -90,7 +89,7 @@ func (r *PreviewRunner) RunE(cmd *cobra.Command, args []string) error { if err != nil { return err } - infos, err := reader.Read() + objs, err := reader.Read() if err != nil { return err } @@ -118,14 +117,14 @@ func (r *PreviewRunner) RunE(cmd *cobra.Command, args []string) error { ForceConflicts: false, FieldManager: common.DefaultFieldManager, } - ch = r.Applier.Run(ctx, object.InfosToUnstructureds(infos), apply.Options{ + ch = r.Applier.Run(ctx, objs, apply.Options{ EmitStatusEvents: false, NoPrune: noPrune, DryRunStrategy: drs, ServerSideOptions: serverSideOptions, }) } else { - inv, _, err := inventory.SplitUnstructureds(object.InfosToUnstructureds(infos)) + inv, _, err := inventory.SplitUnstructureds(objs) if err != nil { return err } diff --git a/cmd/status/cmdstatus.go b/cmd/status/cmdstatus.go index 3024e53..d37dcbb 100644 --- a/cmd/status/cmdstatus.go +++ b/cmd/status/cmdstatus.go @@ -21,7 +21,6 @@ import ( "sigs.k8s.io/cli-utils/pkg/kstatus/polling/collector" "sigs.k8s.io/cli-utils/pkg/kstatus/polling/event" "sigs.k8s.io/cli-utils/pkg/kstatus/status" - "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/cli-utils/pkg/provider" "sigs.k8s.io/cli-utils/pkg/util/factory" ) @@ -79,13 +78,13 @@ func (r *StatusRunner) runE(cmd *cobra.Command, args []string) error { if err != nil { return err } - infos, err := reader.Read() + objs, err := reader.Read() if err != nil { return err } // Find the inventory template among the manifests. - inv, _, err := inventory.SplitUnstructureds(object.InfosToUnstructureds(infos)) + inv, _, err := inventory.SplitUnstructureds(objs) if err != nil { return err } diff --git a/pkg/apply/applier.go b/pkg/apply/applier.go index 6e7db0f..31bc078 100644 --- a/pkg/apply/applier.go +++ b/pkg/apply/applier.go @@ -105,6 +105,7 @@ func (a *Applier) prepareObjects(objs []*unstructured.Unstructured) (*ResourceOb // algorithm requires stopping if the merge is not successful. Otherwise, // the stored objects in inventory could become inconsistent. pruneIds, err := a.invClient.Merge(localInv, currentObjs) + if err != nil { return nil, err } diff --git a/pkg/apply/info/info_helper.go b/pkg/apply/info/info_helper.go index 809b879..873d6b7 100644 --- a/pkg/apply/info/info_helper.go +++ b/pkg/apply/info/info_helper.go @@ -56,8 +56,11 @@ func (ih *infoHelper) UpdateInfos(infos []*resource.Info) error { } func (ih *infoHelper) BuildInfos(objs []*unstructured.Unstructured) ([]*resource.Info, error) { - infos := object.UnstructuredsToInfos(objs) - err := ih.UpdateInfos(infos) + infos, err := object.UnstructuredsToInfos(objs) + if err != nil { + return nil, err + } + err = ih.UpdateInfos(infos) if err != nil { return nil, err } diff --git a/pkg/apply/task/apply_task_test.go b/pkg/apply/task/apply_task_test.go index 4cd9131..73cefe4 100644 --- a/pkg/apply/task/apply_task_test.go +++ b/pkg/apply/task/apply_task_test.go @@ -333,5 +333,5 @@ func (f *fakeInfoHelper) UpdateInfos([]*resource.Info) error { } func (f *fakeInfoHelper) BuildInfos(objs []*unstructured.Unstructured) ([]*resource.Info, error) { - return object.UnstructuredsToInfos(objs), nil + return object.UnstructuredsToInfos(objs) } diff --git a/pkg/manifestreader/common.go b/pkg/manifestreader/common.go index 1132743..b97d074 100644 --- a/pkg/manifestreader/common.go +++ b/pkg/manifestreader/common.go @@ -4,16 +4,16 @@ package manifestreader import ( + "encoding/json" "fmt" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/cli-runtime/pkg/resource" - "k8s.io/kubectl/pkg/cmd/util" "sigs.k8s.io/cli-utils/pkg/apply/solver" "sigs.k8s.io/cli-utils/pkg/inventory" - "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/kustomize/kyaml/kio/filters" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" + "sigs.k8s.io/kustomize/kyaml/yaml" ) // SetNamespaces verifies that every namespaced resource has the namespace @@ -22,29 +22,24 @@ import ( // This implementation will check each resource (that doesn't already have // the namespace set) on whether it is namespace or cluster scoped. It does // this by first checking the RESTMapper, and it there is not match there, -// it will look for CRDs in the provided Infos. -func SetNamespaces(factory util.Factory, infos []*resource.Info, +// it will look for CRDs in the provided Unstructureds. +func SetNamespaces(mapper meta.RESTMapper, objs []*unstructured.Unstructured, defaultNamespace string, enforceNamespace bool) error { - mapper, err := factory.ToRESTMapper() - if err != nil { - return err - } - - var crdInfos []*resource.Info + var crdObjs []*unstructured.Unstructured // find any crds in the set of resources. - for _, inf := range infos { - if solver.IsCRD(object.InfoToUnstructured(inf)) { - crdInfos = append(crdInfos, inf) + for _, obj := range objs { + if solver.IsCRD(obj) { + crdObjs = append(crdObjs, obj) } } - for _, inf := range infos { - accessor, _ := meta.Accessor(inf.Object) + for _, obj := range objs { + accessor, _ := meta.Accessor(obj) // Exclude any inventory objects here since we don't want to change // their namespace. - if inventory.IsInventoryObject(object.InfoToUnstructured(inf)) { + if inventory.IsInventoryObject(obj) { continue } @@ -59,7 +54,7 @@ func SetNamespaces(factory util.Factory, infos []*resource.Info, continue } - gk := inf.Object.GetObjectKind().GroupVersionKind().GroupKind() + gk := obj.GetObjectKind().GroupVersionKind().GroupKind() mapping, err := mapper.RESTMapping(gk) if err != nil && !meta.IsNoMatchError(err) { @@ -73,7 +68,6 @@ func SetNamespaces(factory util.Factory, infos []*resource.Info, // This means the resource does not have the namespace set, // but it is a namespaced resource. So we set the namespace // to the provided default value. - inf.Namespace = defaultNamespace accessor.SetNamespace(defaultNamespace) } continue @@ -87,12 +81,11 @@ func SetNamespaces(factory util.Factory, infos []*resource.Info, // namespace-scoped. If it is the latter, we set the namespace // to the provided default. var scope string - for _, crdInf := range crdInfos { - u, _ := crdInf.Object.(*unstructured.Unstructured) - group, _, _ := unstructured.NestedString(u.Object, "spec", "group") - kind, _, _ := unstructured.NestedString(u.Object, "spec", "names", "kind") + for _, crdObj := range crdObjs { + group, _, _ := unstructured.NestedString(crdObj.Object, "spec", "group") + kind, _, _ := unstructured.NestedString(crdObj.Object, "spec", "names", "kind") if gk.Kind == kind && gk.Group == group { - scope, _, _ = unstructured.NestedString(u.Object, "spec", "scope") + scope, _, _ = unstructured.NestedString(crdObj.Object, "spec", "scope") } } @@ -102,7 +95,6 @@ func SetNamespaces(factory util.Factory, infos []*resource.Info, case "Cluster": continue case "Namespaced": - inf.Namespace = defaultNamespace accessor.SetNamespace(defaultNamespace) } } @@ -110,18 +102,48 @@ func SetNamespaces(factory util.Factory, infos []*resource.Info, return nil } -// FilterLocalConfig returns a new slice of infos where all resources +// FilterLocalConfig returns a new slice of Unstructured where all resources // with the LocalConfig annotation is filtered out. -func FilterLocalConfig(infos []*resource.Info) []*resource.Info { - var filterInfos []*resource.Info - for _, inf := range infos { - acc, _ := meta.Accessor(inf.Object) +func FilterLocalConfig(objs []*unstructured.Unstructured) []*unstructured.Unstructured { + var filteredObjs []*unstructured.Unstructured + for _, obj := range objs { + acc, _ := meta.Accessor(obj) // Ignoring the value of the LocalConfigAnnotation here. This is to be // consistent with the behavior in the kyaml library: // https://github.com/kubernetes-sigs/kustomize/blob/30b58e90a39485bc5724b2278651c5d26b815cb2/kyaml/kio/filters/local.go#L29 if _, found := acc.GetAnnotations()[filters.LocalConfigAnnotation]; !found { - filterInfos = append(filterInfos, inf) + filteredObjs = append(filteredObjs, obj) } } - return filterInfos + return filteredObjs +} + +// removeAnnotations removes the specified kioutil annotations from the resource. +func removeAnnotations(n *yaml.RNode, annotations ...kioutil.AnnotationKey) error { + for _, a := range annotations { + err := n.PipeE(yaml.ClearAnnotation(a)) + if err != nil { + return err + } + } + return nil +} + +// kyamlNodeToUnstructured take a resource represented as a kyaml RNode and +// turns it into an Unstructured object. +func kyamlNodeToUnstructured(n *yaml.RNode) (*unstructured.Unstructured, error) { + b, err := n.MarshalJSON() + if err != nil { + return nil, err + } + + var m map[string]interface{} + err = json.Unmarshal(b, &m) + if err != nil { + return nil, err + } + + return &unstructured.Unstructured{ + Object: m, + }, nil } diff --git a/pkg/manifestreader/common_test.go b/pkg/manifestreader/common_test.go index 629588a..55e1d12 100644 --- a/pkg/manifestreader/common_test.go +++ b/pkg/manifestreader/common_test.go @@ -12,11 +12,12 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/cli-runtime/pkg/resource" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" "k8s.io/kubectl/pkg/scheme" "sigs.k8s.io/cli-utils/pkg/object" "sigs.k8s.io/kustomize/kyaml/kio/filters" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" + "sigs.k8s.io/kustomize/kyaml/yaml" ) func TestSetNamespaces(t *testing.T) { @@ -25,7 +26,7 @@ func TestSetNamespaces(t *testing.T) { _ = apiextv1.AddToScheme(scheme.Scheme) testCases := map[string]struct { - infos []*resource.Info + objs []*unstructured.Unstructured defaultNamspace string enforceNamespace bool @@ -33,13 +34,13 @@ func TestSetNamespaces(t *testing.T) { expectedErrText string }{ "resources already have namespace": { - infos: []*resource.Info{ - toInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toUnstructured(schema.GroupVersionKind{ Group: "apps", Version: "v1", Kind: "Deployment", }, "default"), - toInfo(schema.GroupVersionKind{ + toUnstructured(schema.GroupVersionKind{ Group: "policy", Version: "v1beta1", Kind: "PodDisruptionBudget", @@ -53,8 +54,8 @@ func TestSetNamespaces(t *testing.T) { }, }, "resources without namespace and mapping in RESTMapper": { - infos: []*resource.Info{ - toInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toUnstructured(schema.GroupVersionKind{ Group: "apps", Version: "v1", Kind: "Deployment", @@ -65,8 +66,8 @@ func TestSetNamespaces(t *testing.T) { expectedNamespaces: []string{"foo"}, }, "resource with namespace that does match enforced ns": { - infos: []*resource.Info{ - toInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toUnstructured(schema.GroupVersionKind{ Group: "apps", Version: "v1", Kind: "Deployment", @@ -77,8 +78,8 @@ func TestSetNamespaces(t *testing.T) { expectedNamespaces: []string{"bar"}, }, "resource with namespace that doesn't match enforced ns": { - infos: []*resource.Info{ - toInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toUnstructured(schema.GroupVersionKind{ Group: "apps", Version: "v1", Kind: "Deployment", @@ -89,13 +90,13 @@ func TestSetNamespaces(t *testing.T) { expectedErrText: "does not match the namespace", }, "cluster-scoped CR with CRD": { - infos: []*resource.Info{ - toInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toUnstructured(schema.GroupVersionKind{ Group: "custom.io", Version: "v1", Kind: "Custom", }, ""), - toCRDInfo(schema.GroupVersionKind{ + toCRDUnstructured(schema.GroupVersionKind{ Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", @@ -109,8 +110,8 @@ func TestSetNamespaces(t *testing.T) { expectedNamespaces: []string{"", ""}, }, "namespace-scoped CR with CRD": { - infos: []*resource.Info{ - toCRDInfo(schema.GroupVersionKind{ + objs: []*unstructured.Unstructured{ + toCRDUnstructured(schema.GroupVersionKind{ Group: "apiextensions.k8s.io", Version: "v1", Kind: "CustomResourceDefinition", @@ -118,7 +119,7 @@ func TestSetNamespaces(t *testing.T) { Group: "custom.io", Kind: "Custom", }, "Namespaced"), - toInfo(schema.GroupVersionKind{ + toUnstructured(schema.GroupVersionKind{ Group: "custom.io", Version: "v1", Kind: "Custom", @@ -135,7 +136,12 @@ func TestSetNamespaces(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("namespace") defer tf.Cleanup() - err := SetNamespaces(tf, tc.infos, tc.defaultNamspace, tc.enforceNamespace) + mapper, err := tf.ToRESTMapper() + if !assert.NoError(t, err) { + t.FailNow() + } + + err = SetNamespaces(mapper, tc.objs, tc.defaultNamspace, tc.enforceNamespace) if tc.expectedErrText != "" { if err == nil { @@ -147,10 +153,10 @@ func TestSetNamespaces(t *testing.T) { assert.NoError(t, err) - for i, inf := range tc.infos { + for i, obj := range tc.objs { expectedNs := tc.expectedNamespaces[i] - assert.Equal(t, expectedNs, inf.Namespace) - accessor, _ := meta.Accessor(inf.Object) + assert.Equal(t, expectedNs, obj.GetNamespace()) + accessor, _ := meta.Accessor(obj) assert.Equal(t, expectedNs, accessor.GetNamespace()) } }) @@ -178,13 +184,13 @@ var ( func TestFilterLocalConfigs(t *testing.T) { testCases := map[string]struct { - input []*resource.Info + input []*unstructured.Unstructured expected []string }{ "don't filter if no annotation": { - input: []*resource.Info{ - objMetaToInfo(depID), - objMetaToInfo(clusterRoleID), + input: []*unstructured.Unstructured{ + objMetaToUnstructured(depID), + objMetaToUnstructured(clusterRoleID), }, expected: []string{ depID.Name, @@ -192,17 +198,17 @@ func TestFilterLocalConfigs(t *testing.T) { }, }, "filter all if all have annotation": { - input: []*resource.Info{ - addAnnotation(t, objMetaToInfo(depID), filters.LocalConfigAnnotation, "true"), - addAnnotation(t, objMetaToInfo(clusterRoleID), filters.LocalConfigAnnotation, "false"), + input: []*unstructured.Unstructured{ + addAnnotation(t, objMetaToUnstructured(depID), filters.LocalConfigAnnotation, "true"), + addAnnotation(t, objMetaToUnstructured(clusterRoleID), filters.LocalConfigAnnotation, "false"), }, expected: []string{}, }, "filter even if resource have other annotations": { - input: []*resource.Info{ + input: []*unstructured.Unstructured{ addAnnotation(t, addAnnotation( - t, objMetaToInfo(depID), + t, objMetaToUnstructured(depID), filters.LocalConfigAnnotation, "true"), "my-annotation", "foo"), }, @@ -215,8 +221,8 @@ func TestFilterLocalConfigs(t *testing.T) { res := FilterLocalConfig(tc.input) var names []string - for _, inf := range res { - names = append(names, inf.Name) + for _, obj := range res { + names = append(names, obj.GetName()) } // Avoid test failures due to nil slice and empty slice @@ -229,59 +235,131 @@ func TestFilterLocalConfigs(t *testing.T) { } } -func toInfo(gvk schema.GroupVersionKind, namespace string) *resource.Info { - return &resource.Info{ - Namespace: namespace, - Object: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": gvk.GroupVersion().String(), - "kind": gvk.Kind, - "metadata": map[string]interface{}{ - "namespace": namespace, - }, +func TestRemoveAnnotations(t *testing.T) { + testCases := map[string]struct { + node *yaml.RNode + removeAnnotations []kioutil.AnnotationKey + expectedAnnotations []kioutil.AnnotationKey + }{ + "filter both kioutil annotations": { + node: yaml.MustParse(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + annotations: + config.kubernetes.io/path: deployment.yaml + config.kubernetes.io/index: 0 +`), + removeAnnotations: []kioutil.AnnotationKey{ + kioutil.PathAnnotation, + kioutil.IndexAnnotation, + }, + }, + "filter only a subset of the annotations": { + node: yaml.MustParse(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + annotations: + config.kubernetes.io/path: deployment.yaml + config.kubernetes.io/index: 0 +`), + removeAnnotations: []kioutil.AnnotationKey{ + kioutil.IndexAnnotation, + }, + expectedAnnotations: []kioutil.AnnotationKey{ + kioutil.PathAnnotation, + }, + }, + "filter none of the annotations": { + node: yaml.MustParse(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: foo + annotations: + config.kubernetes.io/path: deployment.yaml + config.kubernetes.io/index: 0 +`), + removeAnnotations: []kioutil.AnnotationKey{}, + expectedAnnotations: []kioutil.AnnotationKey{ + kioutil.PathAnnotation, + kioutil.IndexAnnotation, + }, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + node := tc.node + err := removeAnnotations(node, tc.removeAnnotations...) + if !assert.NoError(t, err) { + t.FailNow() + } + + for _, anno := range tc.removeAnnotations { + n, err := node.Pipe(yaml.GetAnnotation(anno)) + if !assert.NoError(t, err) { + t.FailNow() + } + assert.Nil(t, n) + } + for _, anno := range tc.expectedAnnotations { + n, err := node.Pipe(yaml.GetAnnotation(anno)) + if !assert.NoError(t, err) { + t.FailNow() + } + assert.NotNil(t, n) + } + }) + } +} + +func toUnstructured(gvk schema.GroupVersionKind, namespace string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "metadata": map[string]interface{}{ + "namespace": namespace, }, }, } } -func toCRDInfo(gvk schema.GroupVersionKind, gk schema.GroupKind, - scope string) *resource.Info { - return &resource.Info{ - Object: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": gvk.GroupVersion().String(), - "kind": gvk.Kind, - "spec": map[string]interface{}{ - "group": gk.Group, - "names": map[string]interface{}{ - "kind": gk.Kind, - }, - "scope": scope, +func toCRDUnstructured(gvk schema.GroupVersionKind, gk schema.GroupKind, + scope string) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": gvk.GroupVersion().String(), + "kind": gvk.Kind, + "spec": map[string]interface{}{ + "group": gk.Group, + "names": map[string]interface{}{ + "kind": gk.Kind, }, + "scope": scope, }, }, } } -func objMetaToInfo(id object.ObjMetadata) *resource.Info { - return &resource.Info{ - Namespace: id.Namespace, - Name: id.Name, - Object: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": fmt.Sprintf("%s/v1", id.GroupKind.Group), - "kind": id.GroupKind.Kind, - "metadata": map[string]interface{}{ - "namespace": id.Namespace, - "name": id.Name, - }, +func objMetaToUnstructured(id object.ObjMetadata) *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": fmt.Sprintf("%s/v1", id.GroupKind.Group), + "kind": id.GroupKind.Kind, + "metadata": map[string]interface{}{ + "namespace": id.Namespace, + "name": id.Name, }, }, } } -func addAnnotation(t *testing.T, info *resource.Info, name, val string) *resource.Info { - u := info.Object.(*unstructured.Unstructured) +func addAnnotation(t *testing.T, u *unstructured.Unstructured, name, val string) *unstructured.Unstructured { annos, found, err := unstructured.NestedStringMap(u.Object, "metadata", "annotations") if err != nil { t.Fatal(err) @@ -294,5 +372,5 @@ func addAnnotation(t *testing.T, info *resource.Info, name, val string) *resourc if err != nil { t.Fatal(err) } - return info + return u } diff --git a/pkg/manifestreader/manifestreader.go b/pkg/manifestreader/manifestreader.go index 43cb31e..171cc33 100644 --- a/pkg/manifestreader/manifestreader.go +++ b/pkg/manifestreader/manifestreader.go @@ -4,20 +4,20 @@ package manifestreader import ( - "k8s.io/cli-runtime/pkg/resource" - "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) // ManifestReader defines the interface for reading a set // of manifests into info objects. type ManifestReader interface { - Read() ([]*resource.Info, error) + Read() ([]*unstructured.Unstructured, error) } // ReaderOptions defines the shared inputs for the different // implementations of the ManifestReader interface. type ReaderOptions struct { - Factory util.Factory + Mapper meta.RESTMapper Validate bool Namespace string EnforceNamespace bool diff --git a/pkg/manifestreader/path.go b/pkg/manifestreader/path.go index 3dbea71..988287b 100644 --- a/pkg/manifestreader/path.go +++ b/pkg/manifestreader/path.go @@ -4,7 +4,9 @@ package manifestreader import ( - "k8s.io/cli-runtime/pkg/resource" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) // PathManifestReader reads manifests from the provided path @@ -17,39 +19,29 @@ type PathManifestReader struct { } // Read reads the manifests and returns them as Info objects. -func (p *PathManifestReader) Read() ([]*resource.Info, error) { - validator, err := p.Factory.Validator(p.Validate) +func (p *PathManifestReader) Read() ([]*unstructured.Unstructured, error) { + var objs []*unstructured.Unstructured + nodes, err := (&kio.LocalPackageReader{ + PackagePath: p.Path, + }).Read() if err != nil { - return nil, err + return objs, err } - fileNameOptions := &resource.FilenameOptions{ - Filenames: []string{p.Path}, - Recursive: true, + for _, n := range nodes { + err = removeAnnotations(n, kioutil.IndexAnnotation) + if err != nil { + return objs, err + } + u, err := kyamlNodeToUnstructured(n) + if err != nil { + return objs, err + } + objs = append(objs, u) } - enforceNamespace := false - result := p.Factory.NewBuilder(). - Local(). - Unstructured(). - Schema(validator). - ContinueOnError(). - FilenameParam(enforceNamespace, fileNameOptions). - Flatten(). - Do() + objs = FilterLocalConfig(objs) - if err := result.Err(); err != nil { - return nil, err - } - infos, err := result.Infos() - if err != nil { - return nil, err - } - infos = FilterLocalConfig(infos) - - err = SetNamespaces(p.Factory, infos, p.Namespace, p.EnforceNamespace) - if err != nil { - return nil, err - } - return infos, nil + err = SetNamespaces(p.Mapper, objs, p.Namespace, p.EnforceNamespace) + return objs, err } diff --git a/pkg/manifestreader/path_test.go b/pkg/manifestreader/path_test.go index b9092da..6400af5 100644 --- a/pkg/manifestreader/path_test.go +++ b/pkg/manifestreader/path_test.go @@ -9,7 +9,9 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/meta" cmdtesting "k8s.io/kubectl/pkg/cmd/testing" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) func TestPathManifestReader_Read(t *testing.T) { @@ -32,6 +34,17 @@ func TestPathManifestReader_Read(t *testing.T) { infosCount: 1, namespaces: []string{"foo"}, }, + "multiple manifests": { + manifests: map[string]string{ + "dep.yaml": depManifest, + "cm.yaml": cmManifest, + }, + namespace: "default", + enforceNamespace: true, + + infosCount: 2, + namespaces: []string{"default", "default"}, + }, } for tn, tc := range testCases { @@ -39,6 +52,11 @@ func TestPathManifestReader_Read(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") defer tf.Cleanup() + mapper, err := tf.ToRESTMapper() + if !assert.NoError(t, err) { + t.FailNow() + } + dir, err := ioutil.TempDir("", "path-reader-test") assert.NoError(t, err) for filename, content := range tc.manifests { @@ -47,10 +65,10 @@ func TestPathManifestReader_Read(t *testing.T) { assert.NoError(t, err) } - infos, err := (&PathManifestReader{ + objs, err := (&PathManifestReader{ Path: dir, ReaderOptions: ReaderOptions{ - Factory: tf, + Mapper: mapper, Namespace: tc.namespace, EnforceNamespace: tc.enforceNamespace, Validate: tc.validate, @@ -58,10 +76,13 @@ func TestPathManifestReader_Read(t *testing.T) { }).Read() assert.NoError(t, err) - assert.Equal(t, len(infos), tc.infosCount) + assert.Equal(t, len(objs), tc.infosCount) - for i, info := range infos { - assert.Equal(t, tc.namespaces[i], info.Namespace) + for i, obj := range objs { + assert.Equal(t, tc.namespaces[i], obj.GetNamespace()) + accessor, _ := meta.Accessor(obj) + _, ok := accessor.GetAnnotations()[kioutil.PathAnnotation] + assert.True(t, ok) } }) } diff --git a/pkg/manifestreader/stream.go b/pkg/manifestreader/stream.go index a961c9c..cde6e98 100644 --- a/pkg/manifestreader/stream.go +++ b/pkg/manifestreader/stream.go @@ -6,7 +6,9 @@ package manifestreader import ( "io" - "k8s.io/cli-runtime/pkg/resource" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/kustomize/kyaml/kio" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) // StreamManifestReader reads manifest from the provided io.Reader @@ -20,33 +22,29 @@ type StreamManifestReader struct { } // Read reads the manifests and returns them as Info objects. -func (r *StreamManifestReader) Read() ([]*resource.Info, error) { - validator, err := r.Factory.Validator(r.Validate) +func (r *StreamManifestReader) Read() ([]*unstructured.Unstructured, error) { + var objs []*unstructured.Unstructured + nodes, err := (&kio.ByteReader{ + Reader: r.Reader, + }).Read() if err != nil { - return nil, err + return objs, err } - result := r.Factory.NewBuilder(). - Local(). - Unstructured(). - Schema(validator). - ContinueOnError(). - Stream(r.Reader, r.ReaderName). - Flatten(). - Do() + for _, n := range nodes { + err = removeAnnotations(n, kioutil.IndexAnnotation) + if err != nil { + return objs, err + } + u, err := kyamlNodeToUnstructured(n) + if err != nil { + return objs, err + } + objs = append(objs, u) + } - if err := result.Err(); err != nil { - return nil, err - } - infos, err := result.Infos() - if err != nil { - return nil, err - } - infos = FilterLocalConfig(infos) + objs = FilterLocalConfig(objs) - err = SetNamespaces(r.Factory, infos, r.Namespace, r.EnforceNamespace) - if err != nil { - return nil, err - } - return infos, nil + err = SetNamespaces(r.Mapper, objs, r.Namespace, r.EnforceNamespace) + return objs, err } diff --git a/pkg/manifestreader/stream_test.go b/pkg/manifestreader/stream_test.go index 3484d84..41c1d29 100644 --- a/pkg/manifestreader/stream_test.go +++ b/pkg/manifestreader/stream_test.go @@ -11,17 +11,6 @@ import ( cmdtesting "k8s.io/kubectl/pkg/cmd/testing" ) -var ( - depManifest = ` -kind: Deployment -apiVersion: apps/v1 -metadata: - name: foo -spec: - replicas: 1 -` -) - func TestStreamManifestReader_Read(t *testing.T) { testCases := map[string]struct { manifests string @@ -40,6 +29,14 @@ func TestStreamManifestReader_Read(t *testing.T) { infosCount: 1, namespaces: []string{"foo"}, }, + "multiple resources": { + manifests: depManifest + "\n---\n" + cmManifest, + namespace: "bar", + enforceNamespace: false, + + infosCount: 2, + namespaces: []string{"bar", "bar"}, + }, } for tn, tc := range testCases { @@ -47,13 +44,18 @@ func TestStreamManifestReader_Read(t *testing.T) { tf := cmdtesting.NewTestFactory().WithNamespace("test-ns") defer tf.Cleanup() + mapper, err := tf.ToRESTMapper() + if !assert.NoError(t, err) { + t.FailNow() + } + stringReader := strings.NewReader(tc.manifests) - infos, err := (&StreamManifestReader{ + objs, err := (&StreamManifestReader{ ReaderName: "testReader", Reader: stringReader, ReaderOptions: ReaderOptions{ - Factory: tf, + Mapper: mapper, Namespace: tc.namespace, EnforceNamespace: tc.enforceNamespace, Validate: tc.validate, @@ -61,10 +63,10 @@ func TestStreamManifestReader_Read(t *testing.T) { }).Read() assert.NoError(t, err) - assert.Equal(t, len(infos), tc.infosCount) + assert.Equal(t, len(objs), tc.infosCount) - for i, info := range infos { - assert.Equal(t, tc.namespaces[i], info.Namespace) + for i, obj := range objs { + assert.Equal(t, tc.namespaces[i], obj.GetNamespace()) } }) } diff --git a/pkg/manifestreader/testdata.go b/pkg/manifestreader/testdata.go new file mode 100644 index 0000000..b68ab60 --- /dev/null +++ b/pkg/manifestreader/testdata.go @@ -0,0 +1,24 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package manifestreader + +var ( + depManifest = ` +kind: Deployment +apiVersion: apps/v1 +metadata: + name: dep +spec: + replicas: 1 +` + + cmManifest = ` +kind: ConfigMap +apiVersion: v1 +metadata: + name: cm +data: + foo: bar +` +) diff --git a/pkg/object/infos.go b/pkg/object/infos.go index dee3895..73b779a 100644 --- a/pkg/object/infos.go +++ b/pkg/object/infos.go @@ -4,21 +4,34 @@ package object import ( + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/cli-runtime/pkg/resource" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" ) func InfoToUnstructured(info *resource.Info) *unstructured.Unstructured { return info.Object.(*unstructured.Unstructured) } -func UnstructuredToInfo(obj *unstructured.Unstructured) *resource.Info { +func UnstructuredToInfo(obj *unstructured.Unstructured) (*resource.Info, error) { + accessor, _ := meta.Accessor(obj) + annos := accessor.GetAnnotations() + + source := "unstructured" + path, ok := annos[kioutil.PathAnnotation] + if ok { + source = path + delete(annos, kioutil.PathAnnotation) + accessor.SetAnnotations(annos) + } + return &resource.Info{ Name: obj.GetName(), Namespace: obj.GetNamespace(), - Source: "unstructured", + Source: source, Object: obj, - } + }, nil } func InfosToUnstructureds(infos []*resource.Info) []*unstructured.Unstructured { @@ -29,10 +42,14 @@ func InfosToUnstructureds(infos []*resource.Info) []*unstructured.Unstructured { return objs } -func UnstructuredsToInfos(objs []*unstructured.Unstructured) []*resource.Info { +func UnstructuredsToInfos(objs []*unstructured.Unstructured) ([]*resource.Info, error) { var infos []*resource.Info for _, obj := range objs { - infos = append(infos, UnstructuredToInfo(obj)) + inf, err := UnstructuredToInfo(obj) + if err != nil { + return infos, err + } + infos = append(infos, inf) } - return infos + return infos, nil } diff --git a/pkg/object/infos_test.go b/pkg/object/infos_test.go new file mode 100644 index 0000000..9782a2a --- /dev/null +++ b/pkg/object/infos_test.go @@ -0,0 +1,77 @@ +// Copyright 2020 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package object + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/kustomize/kyaml/kio/kioutil" +) + +func TestUnstructuredToInfo(t *testing.T) { + testCases := map[string]struct { + obj *unstructured.Unstructured + expectedSource string + expectedName string + expectedNamespace string + }{ + "with path annotation": { + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "annotations": map[string]interface{}{ + kioutil.PathAnnotation: "deployment.yaml", + }, + }, + }, + }, + expectedSource: "deployment.yaml", + expectedName: "foo", + expectedNamespace: "", + }, + "without path annotation": { + obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo", + "namespace": "bar", + }, + }, + }, + expectedSource: "unstructured", + expectedName: "foo", + expectedNamespace: "bar", + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + inf, err := UnstructuredToInfo(tc.obj) + if !assert.NoError(t, err) { + t.FailNow() + } + assert.Equal(t, tc.expectedSource, inf.Source) + assert.Equal(t, tc.expectedName, inf.Name) + assert.Equal(t, tc.expectedNamespace, inf.Namespace) + + u := inf.Object.(*unstructured.Unstructured) + annos, found, err := unstructured.NestedStringMap(u.Object, "metadata", "annotations") + if !assert.NoError(t, err) { + t.FailNow() + } + + if found { + _, hasAnnotation := annos[kioutil.PathAnnotation] + assert.False(t, hasAnnotation) + } + }) + } +} diff --git a/pkg/provider/fake-provider.go b/pkg/provider/fake-provider.go index fa5f069..9a5375f 100644 --- a/pkg/provider/fake-provider.go +++ b/pkg/provider/fake-provider.go @@ -40,9 +40,14 @@ func (f *FakeProvider) ToRESTMapper() (meta.RESTMapper, error) { return f.factory.ToRESTMapper() } -func (f *FakeProvider) ManifestReader(reader io.Reader, args []string) (manifestreader.ManifestReader, error) { +func (f *FakeProvider) ManifestReader(reader io.Reader, _ []string) (manifestreader.ManifestReader, error) { + mapper, err := f.factory.ToRESTMapper() + if err != nil { + return nil, err + } + readerOptions := manifestreader.ReaderOptions{ - Factory: f.factory, + Mapper: mapper, Namespace: metav1.NamespaceDefault, } return &manifestreader.StreamManifestReader{ diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 39af3bb..bfd1bbe 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -60,8 +60,13 @@ func (f *InventoryProvider) ManifestReader(reader io.Reader, args []string) (man return nil, err } + mapper, err := f.factory.ToRESTMapper() + if err != nil { + return nil, err + } + readerOptions := manifestreader.ReaderOptions{ - Factory: f.factory, + Mapper: mapper, Namespace: namespace, EnforceNamespace: enforceNamespace, }