diff --git a/README.md b/README.md index 6ca43d5..554a472 100644 --- a/README.md +++ b/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)
diff --git a/action.go b/action.go index adb49f5..10b9282 100644 --- a/action.go +++ b/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, diff --git a/connect.go b/connect.go index 2bdd8b6..ed57e72 100644 --- a/connect.go +++ b/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 diff --git a/errors.go b/errors.go index f5c3c9a..a162551 100644 --- a/errors.go +++ b/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 diff --git a/eval_test.go b/eval_test.go index 3e95c65..6ba9175 100644 --- a/eval_test.go +++ b/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) diff --git a/identifier.go b/identifier.go index 47184e7..65578b3 100644 --- a/identifier.go +++ b/identifier.go @@ -19,6 +19,8 @@ type resourceIdentifierWithSelector struct { // Type is the resource type to select. This should *not* be a type/name // combination. Type string `yaml:"type"` + // Name is the optional name of the resource to get + Name string `yaml:"name,omitempty"` // Labels is a map, keyed by metadata Label, of Label values to select a // resource by Labels map[string]string `yaml:"labels,omitempty"` @@ -28,27 +30,17 @@ type resourceIdentifierWithSelector struct { // either a string or a struct containing a selector with things like a label // key/value map. type ResourceIdentifier struct { - kind string `yaml:"-"` - name string `yaml:"-"` - labels map[string]string `yaml:"-"` + Arg string `yaml:"-"` + Name string `yaml:"-"` + Labels map[string]string `yaml:"-"` } // Title returns the resource identifier's kind and name, if present func (r *ResourceIdentifier) Title() string { - if r.name == "" { - return r.kind + if r.Name == "" { + return r.Arg } - return r.kind + "/" + r.name -} - -// KindName returns the resource identifier's kind and name -func (r *ResourceIdentifier) KindName() (string, string) { - return r.kind, r.name -} - -// Labels returns the resource identifier's labels map, if present -func (r *ResourceIdentifier) Labels() map[string]string { - return r.labels + return r.Arg + "/" + r.Name } // UnmarshalYAML is a custom unmarshaler that understands that the value of the @@ -67,7 +59,7 @@ func (r *ResourceIdentifier) UnmarshalYAML(node *yaml.Node) error { if strings.Count(s, "/") > 1 { return InvalidResourceSpecifier(s, node) } - r.kind, r.name = splitKindName(s) + r.Arg, r.Name = splitArgName(s) return nil } // Otherwise the resource identifier should be specified broken out as a @@ -80,21 +72,21 @@ func (r *ResourceIdentifier) UnmarshalYAML(node *yaml.Node) error { if err != nil { return InvalidWithLabels(err, node) } - r.kind = ri.Type - r.name = "" - r.labels = ri.Labels + r.Arg = ri.Type + r.Name = ri.Name + r.Labels = ri.Labels return nil } func NewResourceIdentifier( - kind string, + arg string, name string, labels map[string]string, ) *ResourceIdentifier { return &ResourceIdentifier{ - kind: kind, - name: name, - labels: labels, + Arg: arg, + Name: name, + Labels: labels, } } @@ -103,9 +95,9 @@ func NewResourceIdentifier( // like a label key/value map. type ResourceIdentifierOrFile struct { fp string `yaml:"-"` - kind string `yaml:"-"` - name string `yaml:"-"` - labels map[string]string `yaml:"-"` + Arg string `yaml:"-"` + Name string `yaml:"-"` + Labels map[string]string `yaml:"-"` } // FilePath returns the resource identifier's file path, if present @@ -119,20 +111,10 @@ func (r *ResourceIdentifierOrFile) Title() string { if r.fp != "" { return filepath.Base(r.fp) } - if r.name == "" { - return r.kind + if r.Name == "" { + return r.Arg } - return r.kind + "/" + r.name -} - -// KindName returns the resource identifier's kind and name -func (r *ResourceIdentifierOrFile) KindName() (string, string) { - return r.kind, r.name -} - -// Labels returns the resource identifier's labels map, if present -func (r *ResourceIdentifierOrFile) Labels() map[string]string { - return r.labels + return r.Arg + "/" + r.Name } // UnmarshalYAML is a custom unmarshaler that understands that the value of the @@ -158,7 +140,7 @@ func (r *ResourceIdentifierOrFile) UnmarshalYAML(node *yaml.Node) error { if strings.Count(s, "/") > 1 { return InvalidResourceSpecifierOrFilepath(s, node) } - r.kind, r.name = splitKindName(s) + r.Arg, r.Name = splitArgName(s) return nil } // Otherwise the resource identifier should be specified broken out as a @@ -171,30 +153,43 @@ func (r *ResourceIdentifierOrFile) UnmarshalYAML(node *yaml.Node) error { if err != nil { return InvalidWithLabels(err, node) } - r.kind = ri.Type - r.name = "" - r.labels = ri.Labels + r.Arg = ri.Type + r.Name = ri.Name + r.Labels = ri.Labels return nil } func NewResourceIdentifierOrFile( fp string, - kind string, + arg string, name string, labels map[string]string, ) *ResourceIdentifierOrFile { return &ResourceIdentifierOrFile{ fp: fp, - kind: kind, - name: name, - labels: labels, + Arg: arg, + Name: name, + Labels: labels, } } -// splitKindName returns the Kind for a supplied `Get` or `Delete` command -// where the user can specify either a resource kind or alias, e.g. "pods" or -// "po", or the resource kind followed by a forward slash and a resource name. -func splitKindName(subject string) (string, string) { - kind, name, _ := strings.Cut(subject, "/") - return kind, name +// splitArgName returns the resource or kind arg string for a supplied `Get` or +// `Delete` command where the user can specify either a resource kind or alias, +// e.g. "pods" or "po", or the resource kind followed by a forward slash and a +// resource name. +// +// Valid resource/kind arg plus name strings: +// +// * "pods" +// * "pod" +// * "pods/name" +// * "pod/name" +// * "deployments.apps/name" +// * "deployments.v1.apps/name" +// * "Deployment/name" +// * "Deployment.apps/name" +// * "Deployment.v1.apps/name" +func splitArgName(subject string) (string, string) { + arg, name, _ := strings.Cut(subject, "/") + return arg, name } diff --git a/spec.go b/spec.go index 75e3214..cbefb2d 100644 --- a/spec.go +++ b/spec.go @@ -138,7 +138,7 @@ func probablyFilePath(subject string) bool { if strings.ContainsAny(subject, " :\n\r\t") { return false } - return strings.ContainsRune(subject, '.') + return strings.HasSuffix(subject, ".yaml") || strings.HasSuffix(subject, ".yml") } func (s *Spec) SetBase(b api.Spec) { diff --git a/testdata/same-named-kind.yaml b/testdata/same-named-kind.yaml new file mode 100644 index 0000000..f592e5b --- /dev/null +++ b/testdata/same-named-kind.yaml @@ -0,0 +1,94 @@ +name: same-named-kind +description: test multiple Kinds with different APIVersions +fixtures: + - kind +tests: + - kube: + create: | + apiVersion: apiextensions.k8s.io/v1 + kind: CustomResourceDefinition + metadata: + name: foos.example.com + spec: + group: example.com + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + a: + type: string + b: + type: integer + scope: Namespaced + names: + kind: Foo + plural: foos + singular: foo + - kube: + create: | + apiVersion: apiextensions.k8s.io/v1 + kind: CustomResourceDefinition + metadata: + name: foos.anotherexample.com + spec: + group: anotherexample.com + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + a: + type: string + b: + type: integer + scope: Namespaced + names: + kind: Foo + plural: foos + singular: foo + - kube.get: foos.example.com/nonexisting + assert: + notfound: true + - kube.get: foos.anotherexample.com/nonexisting + assert: + notfound: true + # Kube assumes the first Kind-matching resource type, so this should return a + # not found, not an unknown resource + - kube.get: foos/nonexisting + assert: + notfound: true + - kube: + create: | + apiVersion: anotherexample.com/v1 + kind: Foo + metadata: + name: fooexample + spec: + a: example + b: 42 + - kube.get: foos.anotherexample.com/fooexample + assert: + matches: + spec: + b: 42 + - kube.get: foos.v1.anotherexample.com/fooexample + assert: + matches: + spec: + b: 42 + - kube.delete: foos.anotherexample.com/fooexample + - kube.delete: crds/foos.example.com + - kube.delete: crds/foos.anotherexample.com