Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix inability to reference same-named Kinds #22

Merged
merged 1 commit into from
Sep 3, 2024
Merged

fix inability to reference same-named Kinds #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
131 changes: 86 additions & 45 deletions action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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{},
Expand All @@ -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
Expand All @@ -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,
Expand Down
52 changes: 43 additions & 9 deletions connect.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/gdt-dev/gdt/api"
"gopkg.in/yaml.v3"
"k8s.io/apimachinery/pkg/runtime/schema"
)

var (
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions eval_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading