Merge pull request #22 from jaypipes/same-name-kind

fix inability to reference same-named Kinds

Author: jaypipes

README.md

@@ -2,7 +2,7 @@
 
 [![Go Reference](https://pkg.go.dev/badge/github.com/gdt-dev/kube.svg)](https://pkg.go.dev/github.com/gdt-dev/kube)
 [![Go Report Card](https://goreportcard.com/badge/github.com/gdt-dev/kube)](https://goreportcard.com/report/github.com/gdt-dev/kube)
-[![Build Status](https://github.com/gdt-dev/kube/actions/workflows/test.yml/badge.svg?branch=main)](https://github.com/gdt-dev/kube/actions)
+[![Build Status](https://github.com/gdt-dev/kube/actions/workflows/gate-tests.yml/badge.svg?branch=main)](https://github.com/gdt-dev/kube/actions)
 [![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md)
 
 <div style="float: left">

action.go

@@ -121,14 +121,11 @@ func (a *Action) get(
 	ns string,
 	out *interface{},
 ) error {
-	kind, name := a.Get.KindName()
-	gvk := schema.GroupVersionKind{
-		Kind: kind,
-	}
-	res, err := c.gvrFromGVK(gvk)
+	res, err := c.gvrFromArg(a.Get.Arg)
 	if err != nil {
 		return err
 	}
+	name := a.Get.Name
 	if name == "" {
 		list, err := a.doList(ctx, c, res, ns)
 		if err == nil {
@@ -154,7 +151,7 @@ func (a *Action) doList(
 	resName := res.Resource
 	labelSelString := ""
 	opts := metav1.ListOptions{}
-	withlabels := a.Get.Labels()
+	withlabels := a.Get.Labels
 	if withlabels != nil {
 		// We already validated the label selector during parse-time
 		labelsStr := labels.Set(withlabels).String()
@@ -250,21 +247,30 @@ func (a *Action) create(
 	}
 	for _, obj := range objs {
 		gvk := obj.GetObjectKind().GroupVersionKind()
-		ons := obj.GetNamespace()
-		if ons == "" {
-			ons = ns
-		}
 		res, err := c.gvrFromGVK(gvk)
 		if err != nil {
 			return err
 		}
 		resName := res.Resource
-		debug.Println(ctx, "kube.create: %s (ns: %s)", resName, ons)
-		obj, err := c.client.Resource(res).Namespace(ons).Create(
-			ctx,
-			obj,
-			metav1.CreateOptions{},
-		)
+		if c.resourceNamespaced(res) {
+			ons := obj.GetNamespace()
+			if ons == "" {
+				ons = ns
+			}
+			debug.Println(ctx, "kube.create: %s (ns: %s)", resName, ons)
+			obj, err = c.client.Resource(res).Namespace(ons).Create(
+				ctx,
+				obj,
+				metav1.CreateOptions{},
+			)
+		} else {
+			debug.Println(ctx, "kube.create: %s (non-namespaced resource)", resName)
+			obj, err = c.client.Resource(res).Create(
+				ctx,
+				obj,
+				metav1.CreateOptions{},
+			)
+		}
 		if err != nil {
 			return err
 		}
@@ -314,28 +320,44 @@ func (a *Action) apply(
 	}
 	for _, obj := range objs {
 		gvk := obj.GetObjectKind().GroupVersionKind()
-		ons := obj.GetNamespace()
-		if ons == "" {
-			ons = ns
-		}
 		res, err := c.gvrFromGVK(gvk)
 		if err != nil {
 			return err
 		}
 		resName := res.Resource
-		debug.Println(ctx, "kube.apply: %s (ns: %s)", resName, ons)
-		obj, err := c.client.Resource(res).Namespace(ns).Apply(
-			ctx,
-			// NOTE(jaypipes): Not sure why a separate name argument is
-			// necessary considering `obj` is of type
-			// `*unstructured.Unstructured` and therefore has the `GetName()`
-			// method...
-			obj.GetName(),
-			obj,
-			// TODO(jaypipes): Not sure if this hard-coded options struct is
-			// always going to work. Maybe add ability to control it?
-			metav1.ApplyOptions{FieldManager: fieldManagerName, Force: true},
-		)
+		if c.resourceNamespaced(res) {
+			ons := obj.GetNamespace()
+			if ons == "" {
+				ons = ns
+			}
+			debug.Println(ctx, "kube.apply: %s (ns: %s)", resName, ons)
+			obj, err = c.client.Resource(res).Namespace(ns).Apply(
+				ctx,
+				// NOTE(jaypipes): Not sure why a separate name argument is
+				// necessary considering `obj` is of type
+				// `*unstructured.Unstructured` and therefore has the `GetName()`
+				// method...
+				obj.GetName(),
+				obj,
+				// TODO(jaypipes): Not sure if this hard-coded options struct is
+				// always going to work. Maybe add ability to control it?
+				metav1.ApplyOptions{FieldManager: fieldManagerName, Force: true},
+			)
+		} else {
+			debug.Println(ctx, "kube.apply: %s (non-namespaced resource)", resName)
+			obj, err = c.client.Resource(res).Apply(
+				ctx,
+				// NOTE(jaypipes): Not sure why a separate name argument is
+				// necessary considering `obj` is of type
+				// `*unstructured.Unstructured` and therefore has the `GetName()`
+				// method...
+				obj.GetName(),
+				obj,
+				// TODO(jaypipes): Not sure if this hard-coded options struct is
+				// always going to work. Maybe add ability to control it?
+				metav1.ApplyOptions{FieldManager: fieldManagerName, Force: true},
+			)
+		}
 		if err != nil {
 			return err
 		}
@@ -385,14 +407,11 @@ func (a *Action) delete(
 		return nil
 	}
 
-	kind, name := a.Delete.KindName()
-	gvk := schema.GroupVersionKind{
-		Kind: kind,
-	}
-	res, err := c.gvrFromGVK(gvk)
+	res, err := c.gvrFromArg(a.Delete.Arg)
 	if err != nil {
 		return err
 	}
+	name := a.Delete.Name
 	if name == "" {
 		return a.doDeleteCollection(ctx, c, res, ns)
 	}
@@ -408,11 +427,22 @@ func (a *Action) doDelete(
 	name string,
 ) error {
 	resName := res.Resource
+	if c.resourceNamespaced(res) {
+		debug.Println(
+			ctx, "kube.delete: %s/%s (ns: %s)",
+			resName, name, ns,
+		)
+		return c.client.Resource(res).Namespace(ns).Delete(
+			ctx,
+			name,
+			metav1.DeleteOptions{},
+		)
+	}
 	debug.Println(
-		ctx, "kube.delete: %s/%s (ns: %s)",
-		resName, name, ns,
+		ctx, "kube.delete: %s/%s (non-namespaced resource)",
+		resName, name,
 	)
-	return c.client.Resource(res).Namespace(ns).Delete(
+	return c.client.Resource(res).Delete(
 		ctx,
 		name,
 		metav1.DeleteOptions{},
@@ -428,7 +458,7 @@ func (a *Action) doDeleteCollection(
 	ns string,
 ) error {
 	opts := metav1.ListOptions{}
-	withlabels := a.Delete.Labels()
+	withlabels := a.Delete.Labels
 	labelSelString := ""
 	if withlabels != nil {
 		// We already validated the label selector during parse-time
@@ -437,11 +467,22 @@ func (a *Action) doDeleteCollection(
 		opts.LabelSelector = labelsStr
 	}
 	resName := res.Resource
+	if c.resourceNamespaced(res) {
+		debug.Println(
+			ctx, "kube.delete: %s%s (ns: %s)",
+			resName, labelSelString, ns,
+		)
+		return c.client.Resource(res).Namespace(ns).DeleteCollection(
+			ctx,
+			metav1.DeleteOptions{},
+			opts,
+		)
+	}
 	debug.Println(
-		ctx, "kube.delete: %s%s (ns: %s)",
-		resName, labelSelString, ns,
+		ctx, "kube.delete: %s%s (non-namespaced resource)",
+		resName, labelSelString,
 	)
-	return c.client.Resource(res).Namespace(ns).DeleteCollection(
+	return c.client.Resource(res).DeleteCollection(
 		ctx,
 		metav1.DeleteOptions{},
 		opts,

connect.go

@@ -97,9 +97,27 @@ type connection struct {
 	client dynamic.Interface
 }
 
-// mappingFor returns a RESTMapper for a given resource type or kind
-func (c *connection) mappingFor(typeOrKind string) (*meta.RESTMapping, error) {
-	fullySpecifiedGVR, groupResource := schema.ParseResourceArg(typeOrKind)
+// mappingForGVK returns a RESTMapper for a given GroupVersionKind
+func (c *connection) mappingForGVK(gvk schema.GroupVersionKind) (*meta.RESTMapping, error) {
+	mapping, err := c.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+	if err != nil {
+		// if we error out here, it is because we could not match a resource or a kind
+		// for the given argument. To maintain consistency with previous behavior,
+		// announce that a resource type could not be found.
+		// if the error is _not_ a *meta.NoKindMatchError, then we had trouble doing discovery,
+		// so we should return the original error since it may help a user diagnose what is actually wrong
+		if meta.IsNoMatchError(err) {
+			return nil, fmt.Errorf("the server doesn't have a resource type %q", gvk)
+		}
+		return nil, err
+	}
+
+	return mapping, nil
+}
+
+// mappingForArg returns a RESTMapper for a given GroupVersionKind
+func (c *connection) mappingForArg(arg string) (*meta.RESTMapping, error) {
+	fullySpecifiedGVR, groupResource := schema.ParseResourceArg(arg)
 	gvk := schema.GroupVersionKind{}
 
 	if fullySpecifiedGVR != nil {
@@ -112,7 +130,7 @@ func (c *connection) mappingFor(typeOrKind string) (*meta.RESTMapping, error) {
 		return c.mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
 	}
 
-	fullySpecifiedGVK, groupKind := schema.ParseKindArg(typeOrKind)
+	fullySpecifiedGVK, groupKind := schema.ParseKindArg(arg)
 	if fullySpecifiedGVK == nil {
 		gvk := groupKind.WithVersion("")
 		fullySpecifiedGVK = &gvk
@@ -140,18 +158,34 @@ func (c *connection) mappingFor(typeOrKind string) (*meta.RESTMapping, error) {
 	return mapping, nil
 }
 
+// gvrFromArg returns a GroupVersionResource from a resource or kind arg
+// string, using the discovery client to look up the resource name (the plural
+// of the kind). The returned GroupVersionResource will have the proper Group
+// and Version filled in (as opposed to an APIResource which has empty Group
+// and Version strings because it "inherits" its APIResourceList's GroupVersion
+// ... ugh.)
+func (c *connection) gvrFromArg(
+	arg string,
+) (schema.GroupVersionResource, error) {
+	empty := schema.GroupVersionResource{}
+	r, err := c.mappingForArg(arg)
+	if err != nil {
+		return empty, ResourceUnknown(arg)
+	}
+
+	return r.Resource, nil
+}
+
 // gvrFromGVK returns a GroupVersionResource from a GroupVersionKind, using the
 // discovery client to look up the resource name (the plural of the kind). The
 // returned GroupVersionResource will have the proper Group and Version filled
 // in (as opposed to an APIResource which has empty Group and Version strings
 // because it "inherits" its APIResourceList's GroupVersion ... ugh.)
-func (c *connection) gvrFromGVK(
-	gvk schema.GroupVersionKind,
-) (schema.GroupVersionResource, error) {
+func (c *connection) gvrFromGVK(gvk schema.GroupVersionKind) (schema.GroupVersionResource, error) {
 	empty := schema.GroupVersionResource{}
-	r, err := c.mappingFor(gvk.Kind)
+	r, err := c.mappingForGVK(gvk)
 	if err != nil {
-		return empty, ResourceUnknown(gvk)
+		return empty, ResourceUnknown(gvk.String())
 	}
 
 	return r.Resource, nil

errors.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/gdt-dev/gdt/api"
 	"gopkg.in/yaml.v3"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 var (
@@ -181,9 +180,10 @@ func InvalidWithLabels(err error, node *yaml.Node) error {
 	)
 }
 
-// ResourceUnknown returns ErrRuntimeResourceUnknown for a given kind
-func ResourceUnknown(gvk schema.GroupVersionKind) error {
-	return fmt.Errorf("%w: %s", ErrResourceUnknown, gvk)
+// ResourceUnknown returns ErrRuntimeResourceUnknown for a given resource or
+// kind arg string
+func ResourceUnknown(arg string) error {
+	return fmt.Errorf("%w: %s", ErrResourceUnknown, arg)
 }
 
 // ExpectedNotFound returns ErrExpectedNotFound for a given status code or

eval_test.go

@@ -67,6 +67,23 @@ func TestCreateUnknownResource(t *testing.T) {
 	require.Nil(err)
 }
 
+func TestSameNamedKind(t *testing.T) {
+	testutil.SkipIfNoKind(t)
+	require := require.New(t)
+
+	fp := filepath.Join("testdata", "same-named-kind.yaml")
+
+	s, err := gdt.From(fp)
+	require.Nil(err)
+	require.NotNil(s)
+
+	ctx := gdtcontext.New()
+	ctx = gdtcontext.RegisterFixture(ctx, "kind", kindfix.New())
+
+	err = s.Run(ctx, t)
+	require.Nil(err)
+}
+
 func TestDeleteResourceNotFound(t *testing.T) {
 	testutil.SkipIfNoKind(t)
 	require := require.New(t)